Re: [Qemu-devel] [RESEND PATCH v2 2/2] seabios q35: Add new PCI slot to irq routing function

[Kevin O'Connor]

On Fri, Feb 15, 2013 at 02:11:41PM -0700, Alex Williamson wrote:
> q35/ich9 doesn't use the same interrupt mapping function as
> i440fx/piix.  PIRQA:D and PIRQE:H are programmed identically, but we
> start at index 0, not index -1.  Slots 25 through 31 are also
> programmed independently.
> 
> When running qemu w/o this patch, a device at address 0:6.0 will have
> its PCI interrupt line register programmed with irq 10 (as seen by
> info pci), but it actually uses irq 11 (as reported the guest).  Half
> of the interrupt lines are misprogrammedi like this.  Functionally, a
> fully emulated qemu guest doesn't care much, but when we try to use
> device assignment, we really need to know the correct irqs.

Thanks.  Before committing this, I have a couple of questions.

[...]
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -91,8 +91,10 @@ const u8 pci_irqs[4] = {
>      10, 10, 11, 11
>  };
>  
> +static int (*pci_slot_get_irq)(struct pci_device *pci, int pin);

This looks like it will cause a null pointer dereference if (for what
ever odd reason) neither the piix nor mch chipset is detected.

[...]
> +static int mch_pci_slot_get_irq(struct pci_device *pci, int pin)
> +{
> +    int irq, 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);
> +
> +    switch (slot) {
> +    /* Slots 0-24 rotate slot:pin mapping similar to piix above, but
> +       with a different starting index - see q35-acpi-dsdt.dsl */ 
> +    case 0 ... 24:
> +        irq = pci_irqs[(pin - 1 + pin_addend + slot) & 3];
> +        break;
> +    /* Slots 25-31 all use LNKA mapping (or LNKE, but A:D = E:H) */
> +    case 25 ... 31:
> +        irq = pci_irqs[(pin - 1 + pin_addend) & 3];
> +        break;
> +    }

I'm not sure I understand this.  Since the pin to irq mapping for each
pci slot is motherboard dependent (and not chipset dependent), why
would the qemu "motherboard" use such a weird mapping?  Why wouldn't
we just have qemu do something more consistent?

-Kevin