| Message ID | 1364124760-5794-8-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Mar 24, 2013 at 11:32:37AM +0000, Peter Maydell wrote: > Implement the correct IRQ mapping for the Versatile PCI controller; it > differs between realview and versatile boards, but the previous QEMU > implementation was correct only for the first PCI card on a versatile > board, since we weren't swizzling IRQs based on the slot number. > > Note that this change will break any uses of PCI on Linux kernels which > have an equivalent bug (since they have effectively only been tested > against QEMU, not real hardware). Unfortunately this currently means > "all Linux kernels" and "all uses of versatilepb with a hard disk" > since we default to a PCI SCSI controller. > > We therefore provide a property for enabling the old broken IRQ mapping; > this can be enabled with the command line option: > -global versatile_pci.broken-irq-mapping=1 > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/versatile_pci.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 59 insertions(+), 2 deletions(-) > > diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c > index 576e619..7739f4c 100644 > --- a/hw/versatile_pci.c > +++ b/hw/versatile_pci.c > @@ -26,6 +26,7 @@ typedef struct { > > /* Constant for life of device: */ > int realview; > + uint8_t broken_irq_mapping; > } PCIVPBState; > > #define TYPE_VERSATILE_PCI "versatile_pci" > @@ -61,11 +62,52 @@ static const MemoryRegionOps pci_vpb_config_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static int pci_vpb_map_irq(PCIDevice *d, int irq_num) > +static int pci_vpb_broken_map_irq(PCIDevice *d, int irq_num) > { > + /* Map IRQs as old and buggy versions of QEMU have done in the past; > + * this is not how hardware behaves, and it will not work with guests > + * which drive the hardware correctly, but it allows us to work with > + * buggy Linux kernels which were written against the buggy QEMU. > + */ > return irq_num; > } > > +static int pci_vpb_map_irq(PCIDevice *d, int irq_num) > +{ > + /* Slot to IRQ mapping for RealView Platform Baseboard 926 backplane > + * name slot IntA IntB IntC IntD > + * A 31 IRQ28 IRQ29 IRQ30 IRQ27 > + * B 30 IRQ27 IRQ28 IRQ29 IRQ30 > + * C 29 IRQ30 IRQ27 IRQ28 IRQ29 > + * Slot C is for the host bridge; A and B the peripherals. > + * Our output irqs 0..3 correspond to the baseboard's 27..30. > + * > + * This mapping function takes account of an oddity in the PB926 > + * board wiring, where the FPGA's P_nINTA input is connected to > + * the INTB connection on the board PCI edge connector, P_nINTB > + * is connected to INTC, and so on, so everything is one number > + * further round from where you might expect. > + */ > + return (PCI_SLOT(d->devfn) + irq_num - 2) % PCI_NUM_PINS; It seems this can be a bit shorter: pci_swizzle_map_irq_fn(d, irq_num - 2) and below irq_num - 1 ? > +} > + > +static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num) > +{ > + /* Slot to IRQ mapping for RealView EB and PB1176 backplane > + * name slot IntA IntB IntC IntD > + * A 31 IRQ50 IRQ51 IRQ48 IRQ49 > + * B 30 IRQ49 IRQ50 IRQ51 IRQ48 > + * C 29 IRQ48 IRQ49 IRQ50 IRQ51 > + * Slot C is for the host bridge; A and B the peripherals. > + * Our output irqs 0..3 correspond to the baseboard's 48..51. > + * > + * The PB1176 and EB boards don't have the PB926 wiring oddity > + * described above; P_nINTA connects to INTA, P_nINTB to INTB > + * and so on, which is why this mapping function is different. > + */ > + return (PCI_SLOT(d->devfn) + irq_num - 1) % PCI_NUM_PINS; > +} > + > static void pci_vpb_set_irq(void *opaque, int irq_num, int level) > { > qemu_irq *pic = opaque; > @@ -95,13 +137,22 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp) > { > PCIVPBState *s = PCI_VPB(dev); > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + pci_map_irq_fn mapfn; > int i; > > for (i = 0; i < 4; i++) { > sysbus_init_irq(sbd, &s->irq[i]); > } > > - pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, pci_vpb_map_irq, s->irq, 4); > + if (s->broken_irq_mapping) { > + mapfn = pci_vpb_broken_map_irq; > + } else if (s->realview) { > + mapfn = pci_vpb_rv_map_irq; > + } else { > + mapfn = pci_vpb_map_irq; > + } > + > + pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4); > > /* ??? Register memory space. */ > > @@ -154,11 +205,17 @@ static const TypeInfo versatile_pci_host_info = { > .class_init = versatile_pci_host_class_init, > }; > > +static Property pci_vpb_properties[] = { > + DEFINE_PROP_UINT8("broken-irq-mapping", PCIVPBState, broken_irq_mapping, 0), > + DEFINE_PROP_END_OF_LIST() > +}; > + > static void pci_vpb_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->realize = pci_vpb_realize; > + dc->props = pci_vpb_properties; > } > > static const TypeInfo pci_vpb_info = { > -- > 1.7.9.5
On 25 March 2013 12:12, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Mar 24, 2013 at 11:32:37AM +0000, Peter Maydell wrote: >> + return (PCI_SLOT(d->devfn) + irq_num - 2) % PCI_NUM_PINS; > > It seems this can be a bit shorter: > pci_swizzle_map_irq_fn(d, irq_num - 2) > and below irq_num - 1 ? Yes (though does pci_swizzle_map_irq_fn() accept negative pin values deliberately or by fluke? it might be better to use irq_num + 2 / + 3 , maybe.) -- PMM
On Mon, Mar 25, 2013 at 12:17:39PM +0000, Peter Maydell wrote: > On 25 March 2013 12:12, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sun, Mar 24, 2013 at 11:32:37AM +0000, Peter Maydell wrote: > >> + return (PCI_SLOT(d->devfn) + irq_num - 2) % PCI_NUM_PINS; > > > > It seems this can be a bit shorter: > > pci_swizzle_map_irq_fn(d, irq_num - 2) > > and below irq_num - 1 ? > > Yes (though does pci_swizzle_map_irq_fn() accept negative > pin values deliberately or by fluke? it might be better to > use irq_num + 2 / + 3 , maybe.) > > -- PMM Yes, I prefer + too. The use of - here gave me pause though I figured out it's all right in the end.
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c index 576e619..7739f4c 100644 --- a/hw/versatile_pci.c +++ b/hw/versatile_pci.c @@ -26,6 +26,7 @@ typedef struct { /* Constant for life of device: */ int realview; + uint8_t broken_irq_mapping; } PCIVPBState; #define TYPE_VERSATILE_PCI "versatile_pci" @@ -61,11 +62,52 @@ static const MemoryRegionOps pci_vpb_config_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int pci_vpb_map_irq(PCIDevice *d, int irq_num) +static int pci_vpb_broken_map_irq(PCIDevice *d, int irq_num) { + /* Map IRQs as old and buggy versions of QEMU have done in the past; + * this is not how hardware behaves, and it will not work with guests + * which drive the hardware correctly, but it allows us to work with + * buggy Linux kernels which were written against the buggy QEMU. + */ return irq_num; } +static int pci_vpb_map_irq(PCIDevice *d, int irq_num) +{ + /* Slot to IRQ mapping for RealView Platform Baseboard 926 backplane + * name slot IntA IntB IntC IntD + * A 31 IRQ28 IRQ29 IRQ30 IRQ27 + * B 30 IRQ27 IRQ28 IRQ29 IRQ30 + * C 29 IRQ30 IRQ27 IRQ28 IRQ29 + * Slot C is for the host bridge; A and B the peripherals. + * Our output irqs 0..3 correspond to the baseboard's 27..30. + * + * This mapping function takes account of an oddity in the PB926 + * board wiring, where the FPGA's P_nINTA input is connected to + * the INTB connection on the board PCI edge connector, P_nINTB + * is connected to INTC, and so on, so everything is one number + * further round from where you might expect. + */ + return (PCI_SLOT(d->devfn) + irq_num - 2) % PCI_NUM_PINS; +} + +static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num) +{ + /* Slot to IRQ mapping for RealView EB and PB1176 backplane + * name slot IntA IntB IntC IntD + * A 31 IRQ50 IRQ51 IRQ48 IRQ49 + * B 30 IRQ49 IRQ50 IRQ51 IRQ48 + * C 29 IRQ48 IRQ49 IRQ50 IRQ51 + * Slot C is for the host bridge; A and B the peripherals. + * Our output irqs 0..3 correspond to the baseboard's 48..51. + * + * The PB1176 and EB boards don't have the PB926 wiring oddity + * described above; P_nINTA connects to INTA, P_nINTB to INTB + * and so on, which is why this mapping function is different. + */ + return (PCI_SLOT(d->devfn) + irq_num - 1) % PCI_NUM_PINS; +} + static void pci_vpb_set_irq(void *opaque, int irq_num, int level) { qemu_irq *pic = opaque; @@ -95,13 +137,22 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp) { PCIVPBState *s = PCI_VPB(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + pci_map_irq_fn mapfn; int i; for (i = 0; i < 4; i++) { sysbus_init_irq(sbd, &s->irq[i]); } - pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, pci_vpb_map_irq, s->irq, 4); + if (s->broken_irq_mapping) { + mapfn = pci_vpb_broken_map_irq; + } else if (s->realview) { + mapfn = pci_vpb_rv_map_irq; + } else { + mapfn = pci_vpb_map_irq; + } + + pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4); /* ??? Register memory space. */ @@ -154,11 +205,17 @@ static const TypeInfo versatile_pci_host_info = { .class_init = versatile_pci_host_class_init, }; +static Property pci_vpb_properties[] = { + DEFINE_PROP_UINT8("broken-irq-mapping", PCIVPBState, broken_irq_mapping, 0), + DEFINE_PROP_END_OF_LIST() +}; + static void pci_vpb_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->realize = pci_vpb_realize; + dc->props = pci_vpb_properties; } static const TypeInfo pci_vpb_info = {
Implement the correct IRQ mapping for the Versatile PCI controller; it differs between realview and versatile boards, but the previous QEMU implementation was correct only for the first PCI card on a versatile board, since we weren't swizzling IRQs based on the slot number. Note that this change will break any uses of PCI on Linux kernels which have an equivalent bug (since they have effectively only been tested against QEMU, not real hardware). Unfortunately this currently means "all Linux kernels" and "all uses of versatilepb with a hard disk" since we default to a PCI SCSI controller. We therefore provide a property for enabling the old broken IRQ mapping; this can be enabled with the command line option: -global versatile_pci.broken-irq-mapping=1 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/versatile_pci.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)