[edk2-devel] [PATCH v2] OvmfPkg/PlatformBootManagerLib: fix PCI interrupt link (LNKx)
Laszlo Ersek
lersek at redhat.com
Thu Dec 17 17:33:46 UTC 2020
On 12/17/20 15:12, Borghorst, Hendrik via groups.io wrote:
> This patch fixes an issue with the current programming of the i440fx
> PCI Interrupt routing assignment.
>
> Explanation by Laszlo Ersek:
>
> (1) The rotating pattern is a map:
>
> (slot, function) --> (interrupt link) [LNKA..LNKD]
>
> (more precisely, it is a pattern from (slot, pin) to (interrupt link),
> but function<->pin is an identity mapping in the QEMU hardware, so we
> can just use (slot, function) rather than (slot, pin) on the left hand
> side. But I digress.)
>
> The ACPI _PRT object is generated by QEMU; it describes this map.
>
> (2) Another map is
>
> (interrupt link) --> { set of possible interrupt numbers,
> for this link }
>
> This map is given by the LNK[A..D] ACPI objects, also given by QEMU.
>
> (3) What the firmware is expected to do is:
>
> (3a) for each interrupt link, select an *actual* interrupt from the set
> that's possible for that link, yielding a deterministic map
>
> (interrupt link) --> (actual interrupt number)
>
> and
>
> (3b) for each PCI device/function with an interrupt pin, resolve the
>
> (slot, function) --> (interrupt link) --> (actual interrupt number)
>
> functional composition, and program the result into the Interrupt Line
> register of the device.
>
> In OVMF, we do not parse the rotating map described under (1) from
> QEMU's _PRT object. Instead, we duplicate the code. This is not a
> problem.
>
> In OVMF, we also do not parse the map described under (2) from QEMU's
> ACPI content. Instead, we pick a specific selection (3a) that we
> "apriori" know satisfies (2). This is also not a problem. OVMF's
> particular selection is the PciHostIrqs table.
>
> (
>
> Table (2) from QEMU is
>
> LNKA -> { 5, 10, 11 }
> LNKB -> { 5, 10, 11 }
> LNKC -> { 5, 10, 11 }
> LNKD -> { 5, 10, 11 }
>
> and our specific pick in OVMF, in the PciHostIrqs table, is
>
> LNKA -> 10
> LNKB -> 10
> LNKC -> 11
> LNKD -> 11
>
> )
>
> In OVMF, we also cover step (3b), in the SetPciIntLine() function.
>
> What's missing in OVMF -- and what this patch corrects -- is that we
> currently fail to program our selection for table (3) into the hardware.
> We pick a specific LNKx->IRQ# mapping for each interrupt link, and we
> correctly program the PCI Interrupt Line registers through those
> link-to-IRQ mappings -- but we don't tell the hardware about the
> link-to-IRQ mappings. More precisely, we program such a link-to-IRQ
> mapping table into the hardware that is then not matched by the mapping
> we use for programming the PCI device/function interrupt lines. As a
> result, some PCI Interrupt Line registers will have impossible values --
> a given (slot, function) may use a particular link, but also report an
> interrupt number that was never picked for that link.
>
> Output of Linux PCI Interrupt Links for i440fx before the patch:
>
> [ 0.327305] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 10 *11)
> [ 0.327944] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 10 *11)
> [ 0.328582] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 *10 11)
> [ 0.329208] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 *10 11)
> [ 0.329807] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
>
> after the patch:
>
> [ 0.327292] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
> [ 0.327934] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
> [ 0.328564] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
> [ 0.329195] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
> [ 0.329785] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
>
> Output of Linux PCI Interrupt Links for q35 before the patch:
>
> [ 0.307474] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
> [ 0.308027] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
> [ 0.308764] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
> [ 0.309310] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
> [ 0.309853] ACPI: PCI Interrupt Link [LNKE] (IRQs 5 *10 11)
> [ 0.310508] ACPI: PCI Interrupt Link [LNKF] (IRQs 5 *10 11)
> [ 0.311051] ACPI: PCI Interrupt Link [LNKG] (IRQs 5 10 *11)
> [ 0.311589] ACPI: PCI Interrupt Link [LNKH] (IRQs 5 10 *11)
>
> after the patch:
>
> [ 0.301991] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
> [ 0.302833] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
> [ 0.303354] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
> [ 0.303873] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
> [ 0.304399] ACPI: PCI Interrupt Link [LNKE] (IRQs 5 *10 11)
> [ 0.304918] ACPI: PCI Interrupt Link [LNKF] (IRQs 5 *10 11)
> [ 0.305436] ACPI: PCI Interrupt Link [LNKG] (IRQs 5 10 *11)
> [ 0.305954] ACPI: PCI Interrupt Link [LNKH] (IRQs 5 10 *11)
>
> Signed-off-by: Hendrik Borghorst <hborghor at amazon.de>
> Reviewed-by: David Woodhouse <dwmw at amazon.co.uk>
Yes, David's R-b is now on the list:
https://edk2.groups.io/g/devel/message/68797
> ---
> .../PlatformBootManagerLib/BdsPlatform.c | 29 ++++++++++---------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 3c55ec9bd9..b0e9742937 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -29,7 +29,10 @@ UINT16 mHostBridgeDevId;
> // (for configuring PCI Interrupt Line register)
> //
You missed the comment update I requested for this part, namely:
//
// Table of (interrupt link) --> (actual interrupt number) mappings; for (a)
// programming the
//
// (slot, function) --> (interrupt link) --> (actual interrupt number)
//
// values into PCI Interrupt Line registers, and for (b) programming the
//
// (interrupt link) --> (actual interrupt number)
//
// mappings themselves into the routing (= link configuration) registers.
//
at:
https://edk2.groups.io/g/devel/message/68805
Also the subject isn't exactly what I asked for; but I guess I don't want to obsess about it.
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
Thanks
Laszlo
> CONST UINT8 PciHostIrqs[] = {
> - 0x0a, 0x0a, 0x0b, 0x0b
> + 0x0a, // LNKA, LNKE
> + 0x0a, // LNKB, LNKF
> + 0x0b, // LNKC, LNKG
> + 0x0b // LNKD, LNKH
> };
>
> //
> @@ -1211,24 +1214,24 @@ PciAcpiInitialization (
> //
> // 00:01.0 ISA Bridge (PIIX4) LNK routing targets
> //
> - PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x60), 0x0b); // A
> - PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x61), 0x0b); // B
> - PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x62), 0x0a); // C
> - PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x63), 0x0a); // D
> + PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x60), PciHostIrqs[0]); // A
> + PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x61), PciHostIrqs[1]); // B
> + PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x62), PciHostIrqs[2]); // C
> + PciWrite8 (PCI_LIB_ADDRESS (0, 1, 0, 0x63), PciHostIrqs[3]); // D
> break;
> case INTEL_Q35_MCH_DEVICE_ID:
> Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
> //
> // 00:1f.0 LPC Bridge (Q35) LNK routing targets
> //
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x60), 0x0a); // A
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x61), 0x0a); // B
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x62), 0x0b); // C
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x63), 0x0b); // D
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x68), 0x0a); // E
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x69), 0x0a); // F
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6a), 0x0b); // G
> - PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6b), 0x0b); // H
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x60), PciHostIrqs[0]); // A
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x61), PciHostIrqs[1]); // B
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x62), PciHostIrqs[2]); // C
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x63), PciHostIrqs[3]); // D
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x68), PciHostIrqs[0]); // E
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x69), PciHostIrqs[1]); // F
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6a), PciHostIrqs[2]); // G
> + PciWrite8 (PCI_LIB_ADDRESS (0, 0x1f, 0, 0x6b), PciHostIrqs[3]); // H
> break;
> default:
> if (XenDetected ()) {
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69142): https://edk2.groups.io/g/devel/message/69142
Mute This Topic: https://groups.io/mt/79036745/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list