On 11/07/14 17:40, Gabriel Somlo wrote:
> On Mon, Nov 03, 2014 at 08:59:34PM +0100, Laszlo Ersek wrote:
>>> Additionally, initialize PCI_INTERRUPT_LINE registers for the typical
>>> set of PCI devices included by QEMU with the Q35 machine type. The
>>> corresponding PIIX4 initialization of PCI_INTERRUPT_LINE registers is
>>> cleaned up and the list of PIIX4 PCI devices updated to the list
>>> typically included with QEMU.
>>>
>>> NOTE: The initialization of PCI_INTERRUPT_LINE registers *should*
>>> be accomplished programmatically by enumerating all PCI devices
>>> present in the system and computing PCI_INTERRUPT_LINE from
>>> PCI_INTERRUPT_PIN, the slot/position of the device, and the available
>>> host IRQs (for an example, see SeaBIOS pci_bios_init_devices() in
>>> src/fw/pciinit.c). At the time of this patch, the relevant bits of
>>> OVMF PCI initialization are shown in the following call tree:
>>>
>>
>> (2) It's not hard to do the enumeration that you want. You just need to
>> locate all handles with PciIo protocol instances on them (since we're
>> just after a BdsLibConnectAll() call), open the PciIo protocol interface
>> on each in turn, read the header from the PCI config space, and do what
>> you want.
>>
>> Luckily, this code already exists for you, precisely in this file --
>> "OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c". The function you want to
>> call is
>>
>> VisitAllPciInstances()
>
> Again, thanks for the awesome tutorial ! I have something that works
> now (see patch on top of v4 6/6 below). But I'm a bit confuse about
> whether (and if yes, how) to handle multiple-bus situations. The
> SeaBIOS code I'm porting (slightly paraphrased for clarity, but
> hopefully functionally identical to the real thing) goes like this:
>
> /* host irqs corresponding to PCI irqs A-D */
> const u8 pci_irqs[4] = {
> 10, 10, 11, 11
> };
>
> static int piix_pci_slot_get_irq(struct pci_device *pci, int pin)
> {
> int index, slot, pin_addend = 0;
>
> while (pci->parent != NULL) {
> pin_addend += pci_bdf_to_dev(pci->bdf);
> pci = pci->parent;
> }
> slot = pci_bdf_to_dev(pci->bdf);
>
> index = pin - 1 + pin_addend + slot - 1;
>
> return pci_irqs[index & 3];
> }
>
> static int mch_pci_slot_get_irq(struct pci_device *pci, int pin)
> {
> int index, slot, pin_addend = 0;
>
> while (pci->parent != NULL) {
> pin_addend += pci_bdf_to_dev(pci->bdf);
> pci = pci->parent;
> }
> slot = pci_bdf_to_dev(pci->bdf);
>
> if (slot <= 24) {
> /* Slots 0-24 rotate slot:pin mapping similar to piix above, but
> with a different starting index - see q35-acpi-dsdt.dsl */
> index = pin - 1 + pin_addend + slot;
> } else {
> /* Slots 25-31 all use LNKA mapping (or LNKE, but A:D = E:H) */
> index = pin - 1 + pin_addend;
> }
>
> return pci_irqs[index & 3];
> }
>
>
> From what I have been able to see, "pci->parent" is always NULL from
> the very beginning, so the IRQ is computed based only on the "pin" and
> the device number ("pci_bdf_to_dev(pci->bdf)"), and by that I mean
> the *original* pci->bdf, not the result of walking up the parent link.
>
>
> Now, here's my current code:
>
>
> diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
> b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
> index 98bc05f..3121dcf 100644
> --- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
> @@ -25,7 +25,15 @@ EFI_EVENT mEfiDevPathEvent;
> VOID *mEmuVariableEventReg;
> EFI_EVENT mEmuVariableEvent;
> BOOLEAN mDetectVgaOnly;
> +UINT16 mHostBridgeDevId;
>
> +//
> +// Table of host IRQs matching PCI IRQs A-D
> +// (for configuring PCI Interrupt Line register)
> +//
> +CONST UINT8 PciHostIrqs[] = {
> + 0x0a, 0x0a, 0x0b, 0x0b
> +};
>
> //
> // Type definitions
> @@ -716,18 +724,96 @@ Returns:
> }
>
>
> +/**
> + Configure PCI Interrupt Line register for applicable devices
> +
> + @param[in] Handle - Handle of PCI device instance
> + @param[in] PciIo - PCI IO protocol instance
> + @param[in] PciHdr - PCI Header register block
> +
> + @retval EFI_SUCCESS - PCI Interrupt Line register configured successfully.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +ConfigurePciInterruptLine (
> + IN EFI_HANDLE Handle,
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN PCI_TYPE00 *PciHdr
> + )
> +{
> + EFI_DEVICE_PATH_PROTOCOL *DevPathNode;
> + PCI_DEVICE_PATH *PciDevPathData;
> + UINTN Idx;
> +
> + if (PciHdr->Device.InterruptPin != 0) {
> + DevPathNode = DevicePathFromHandle (Handle);
> + ASSERT (DevPathNode != NULL);
> +
> + //
> + // Walk PCI hardware device path
> + //
> + while (!IsDevicePathEnd (DevPathNode)) {
> + if (DevicePathType (DevPathNode) == HARDWARE_DEVICE_PATH &&
> + DevicePathSubType (DevPathNode) == HW_PCI_DP) {
> +
> + PciDevPathData = (PCI_DEVICE_PATH*) DevPathNode;
> +
> + //
> + // Calculate index into PciHostIrqs[] table
> + // (assuming all devices connected directly to root PCI bus 0)
> + //
> + switch (mHostBridgeDevId) {
> + case INTEL_82441_DEVICE_ID:
> + Idx = PciHdr->Device.InterruptPin - 1 + PciDevPathData->Device -
> 1;
> + break;
> + case INTEL_Q35_MCH_DEVICE_ID:
> + Idx = PciHdr->Device.InterruptPin - 1;
> + if (PciDevPathData->Device <= 24) {
> + Idx += PciDevPathData->Device;
> + }
> + break;
> + default:
> + ASSERT (FALSE); // should never get here
> + }
> +
> + //
> + // Set PCI Interrupt Line register for this device
> + //
> + PciWrite8 (
> + PCI_LIB_ADDRESS (
> + 0,
> + PciDevPathData->Device,
> + PciDevPathData->Function,
> + PCI_INT_LINE_OFFSET
> + ),
> + PciHostIrqs[Idx % 4] //FIXME: array_size(PciHostIrqs) macro ?
> + );
> +
> + //
> + // Stop after first PCI hardware device path node found
> + //
> + break;
> + }
> + DevPathNode = NextDevicePathNode (DevPathNode);
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
>
>
> I don't know how I could bring a bus number into the mix (I just
> assume the bus number is 0 and stop after the first device path node,
> which happens to match the SeaBIOS (pci->parent == NULL) case.
>
> But how would I (and should I even try) to port the whole thing ?
>
> I'm thinking I could maybe walk the whole DevicePath (and maybe
> encounter more pci hardware device path nodes in the process in a way
> equivalent to climbing the seabios pci->parent links), but I still
> don't know how I would figure out the bus number of the original
> device for which I'm trying to compute the IRQ line (for the
> PCI_LIB_ADDRESS write to PCI_INT_LINE_OFFSET).
I only have capacity for some random thoughts, some of which others have
already mentioned:
(a) No clue if OVMF supports bridges / multiple buses. One way to find
out is to combine the idea from Gerd with another idea from Olivier --
configure qemu to set up some non-root bridge, plug some (otherwise
recognized, like virtio-scsi-pci) device in it, and then in OVMF BDS
list (via VisitAllPciInstances / DEBUG) the device paths for *all* PCI
devices. And see if anything rings a bell. If it does, then you should
parse that format. (Note that, theoretically, you might find an
arbitrary depth of bridges after bridges.)
(b) Regarding how to compute a bus number to pass to PCI_LIB_ADDRESS():
You don't need to use PciWrite8() + PCI_LIB_ADDRESS(). In fact, you
*should not*. The canonical way (in DXE / UEFI code) to massage your
friendly PCI device is the PCI IO protocol.
You already get that for the handle in question via the PciIo function
parameter in your ConfigurePciInterruptLine() callback function.
The function call you're looking for is:
UINT8 InterruptLine;
InterruptLine = ...;
Status = PciIo->Pci.Write (
PciIo, // PciIo protocol on the device
// handle in question
EfiPciIoWidthUint8, // width and copy pattern
PCI_INT_LINE_OFFSET, // offset
1, // count
&InterruptLine // buffer
);
if (EFI_ERROR (Status)) {
...
}
The Pci.Write() member function pokes the device's configuration space.
You don't need to care about the device path, the bridges etc. that lead
up to it; the PCI root bridge driver and the PCI bus driver (underneath
your PciIo protocol interface) take care of that for you -- *if* OVMF in
fact supports them. In any case this is how the code should access the
device.
Please read the function's documentation in the UEFI spec, under 13.4
EFI PCI I/O Protocol.
Also, you can see some examples in
- IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
- PcAtChipsetPkg/8259InterruptControllerDxe/8259.c
(Search them for PCI_INT_LINE_OFFSET.)
(c) In fact I gave you suboptimal advice before, with regard to
retrieving your input parameters (like BDF). My advice wasn't
necessarily wrong, but it's quite inflexible for bridges.
The PciIo->GetLocation() function gives you everything you need.
My only excuse is that I never had to deal with multiple buses or
bridges, and while writing drivers, never needed to call GetLocation().
Summary:
- You probably don't have to parse the device path for getting your
input parameters. Just use PciIo->GetLocation().
- Drop PciWrite8 / PCI_LIB_ADDRESS(). You're in a DXE_DRIVER module (the
BDS driver); you shouldn't poke PCI devices directly (with a library),
you should use the UEFI interface, EFI_PCI_IO_PROTOCOL. I realize this
comes from the original BDS code, but hey that doesn't make it the right
style :)
Thanks
Laszlo
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
