Re: [Xen-devel] [PATCH] xen: fix interrupt routing

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 19 May 2011, Ian Campbell wrote:
> On Wed, 2011-05-18 at 18:53 +0100, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > 
> > Match the routing informations built by seabios:
> > 
> > - remove i440fx_write_config_xen
> > we don't need to intercept pci config writes to i440FX;
> > 
> > - introduce piix3_write_config_xen
> > we do need to intercept pci config write to the PCI-ISA bridge to update
> > the PCI link routing;
> > 
> > - remove xen_pci_slot_get_pirq
> > we are now using the same link routing as seabios and qemu so we don't
> > need a diffirent get_pirq function;
> > 
> > - fix xen_piix3_set_irq
> > we always inject one of the 4 pci intx, so we can use
> > xc_hvm_set_isa_irq_level to inject the interrupt. Use pci_irqs as
> > initialized by seabios to map a pirq into an ISA irq. This has the
> > benefit of removing all the calls to xc_hvm_set_pci_intx_level that
> > doesn't work correctly anymore because from the same device number and
> > intx Xen calculates a different PCI link compared to Qemu and Seabios.
> 
> Why does Xen calculate a different PCI link? What are the other
> downsides of this disparity?
>
> For example, in the hypervisor I see that hvm_pci_intx_link is used in
> the passthrough code as well as in the HVMOP_set_pci_intx_level, is that
> safe/sane in conjunction with this change?
> 
> IOW -- are we hiding other problems for later by doing an end run around
> the interface instead of addressing the disconnect directly? It seems
> like it would be a relatively contained change on the SeaBIOS end to
> allow it to use the routing which Xen expects.


If you give a look at __hvm_pci_intx_assert in xen, it uses
hvm_pci_intx_link to calculate the link from device number and intx:

#define hvm_pci_intx_link(dev, intx) \
    (((dev) + (intx)) & 3)

while qemu and seabios calculate the link as follow:

static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
{
    int slot_addend;
    slot_addend = (pci_dev->devfn >> 3) - 1;
    return (pci_intx + slot_addend) & 3;
}

they are similar but unfortunately not indentical (the "- 1" is the
critical difference there).
That means that the routing for ISA irqs is different but if we always
use xc_hvm_set_isa_irq_level we avoid the issue because it takes an ISA
irq as parameter so Xen won't notice.

Unfortunately like you wrote, we would still have a problem with PCI
passthrough because Xen calls hvm_pci_intx_link internally.
We would need to add an option in Xen to switch to the new interrupt
routing.

I think we need to figure out if we want to keep the old MP tables and
the old ACPI _PRT entries or we want to start using Seabios' MP tables
and ACPI _PRT entries.
In both cases the impact on Qemu is pretty limited: it is just a matter
of changing the implementation of piix3_set_irq and pci_slot_get_pirq
(both have a xen specific implementation already).

It is interesting that the old PIIX3 in qemu_xen had 128 interrupt
lines, the routing informations of which are present in the ACPI PRTA
routing table.
The PIIX3 in the new qemu has only 4 lines so there is no need for
xc_hvm_set_pci_intx_level.
Considering that the next update of this patch will add PIIX3-xen there
is scope for increasing the interrupt lines again if we want to.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel