All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices
@ 2018-07-31 11:08 BALATON Zoltan
  2018-07-31 21:55 ` Sebastian Bauer
  0 siblings, 1 reply; 3+ messages in thread
From: BALATON Zoltan @ 2018-07-31 11:08 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: Alexander Graf, David Gibson, Sebastian Bauer, Peter Maydell

The four interrupts of the PCI bus are connected to the same UIC pin
on the real Sam460ex. Evidence for this can be found in the UBoot
source for the Sam460ex in the Sam460ex.c file where
PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
more like this.

This fixes the problem that can be observed when adding further PCI
cards that got their interrupt rotated to other interrupts than PCI
INT A. In particular, the bug was observed with an additional OHCI PCI
card or an ES1370 sound device.

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_pcix.c | 21 ++++++++-------------
 hw/ppc/sam460ex.c    |  6 ++----
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index d8af04b..64ed07a 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -57,7 +57,7 @@ typedef struct PPC440PCIXState {
     struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
     struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
     uint32_t sts;
-    qemu_irq irq[PCI_NUM_PINS];
+    qemu_irq irq;
     AddressSpace bm_as;
     MemoryRegion bm;
 
@@ -418,21 +418,20 @@ static void ppc440_pcix_reset(DeviceState *dev)
  * This may need further refactoring for other boards. */
 static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-    int slot = pci_dev->devfn >> 3;
-    trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, slot);
-    return slot - 1;
+    trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0);
+    return 0;
 }
 
 static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
 {
-    qemu_irq *pci_irqs = opaque;
+    qemu_irq *pci_irq = opaque;
 
     trace_ppc440_pcix_set_irq(irq_num);
     if (irq_num < 0) {
         error_report("%s: PCI irq %d", __func__, irq_num);
         return;
     }
-    qemu_set_irq(pci_irqs[irq_num], level);
+    qemu_set_irq(*pci_irq, level);
 }
 
 static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
@@ -471,19 +470,15 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
 {
     PPC440PCIXState *s;
     PCIHostState *h;
-    int i;
 
     h = PCI_HOST_BRIDGE(dev);
     s = PPC440_PCIX_HOST_BRIDGE(dev);
 
-    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
-        sysbus_init_irq(dev, &s->irq[i]);
-    }
-
+    sysbus_init_irq(dev, &s->irq);
     memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
     h->bus = pci_register_root_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
-                         ppc440_pcix_map_irq, s->irq, &s->busmem,
-                         get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+                         ppc440_pcix_map_irq, &s->irq, &s->busmem,
+                         get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
     s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efc..9c77183 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)
 
     /* PCI bus */
     ppc460ex_pcie_init(env);
-    /* FIXME: is this correct? */
-    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
-                                uic[1][0], uic[1][20], uic[1][21], uic[1][22],
-                                NULL);
+    /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
+    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000, uic[1][0]);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (!pci_bus) {
         error_report("couldn't create PCI controller!");
-- 
2.7.6

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices
  2018-07-31 11:08 [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices BALATON Zoltan
@ 2018-07-31 21:55 ` Sebastian Bauer
  2018-08-01  1:02   ` David Gibson
  0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Bauer @ 2018-07-31 21:55 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: qemu-devel, qemu-ppc, Alexander Graf, David Gibson, Peter Maydell

Am 2018-07-31 13:08, schrieb BALATON Zoltan:
> The four interrupts of the PCI bus are connected to the same UIC pin
> on the real Sam460ex. Evidence for this can be found in the UBoot
> source for the Sam460ex in the Sam460ex.c file where
> PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
> more like this.
> 
> This fixes the problem that can be observed when adding further PCI
> cards that got their interrupt rotated to other interrupts than PCI
> INT A. In particular, the bug was observed with an additional OHCI PCI
> card or an ES1370 sound device.
[...]
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 0999efc..9c77183 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)
> 
>      /* PCI bus */
>      ppc460ex_pcie_init(env);
> -    /* FIXME: is this correct? */
> -    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> -                                uic[1][0], uic[1][20], uic[1][21], 
> uic[1][22],
> -                                NULL);
> +    /* All PCI irqs are connected to the same UIC pin (cf. UBoot 
> source) */
> +    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000, 
> uic[1][0]);
>      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>      if (!pci_bus) {
>          error_report("couldn't create PCI controller!");

I'm fine with that change. I tested it with an additional OHCI 
controller on an emulated SAM machine under guest AmigaOS and it works.

Tested-by: Sebastian Bauer <mail@sebastianbauer.info>

Better usage of QOM etc. can IMO be done at a later time point as this 
fixes an unpleasant bug and any side change will just increase the 
probability to introduce new bugs.

Bye
Sebastian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices
  2018-07-31 21:55 ` Sebastian Bauer
@ 2018-08-01  1:02   ` David Gibson
  0 siblings, 0 replies; 3+ messages in thread
From: David Gibson @ 2018-08-01  1:02 UTC (permalink / raw)
  To: Sebastian Bauer
  Cc: BALATON Zoltan, qemu-devel, qemu-ppc, Alexander Graf, Peter Maydell

[-- Attachment #1: Type: text/plain, Size: 2220 bytes --]

On Tue, Jul 31, 2018 at 11:55:02PM +0200, Sebastian Bauer wrote:
> Am 2018-07-31 13:08, schrieb BALATON Zoltan:
> > The four interrupts of the PCI bus are connected to the same UIC pin
> > on the real Sam460ex. Evidence for this can be found in the UBoot
> > source for the Sam460ex in the Sam460ex.c file where
> > PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
> > more like this.
> > 
> > This fixes the problem that can be observed when adding further PCI
> > cards that got their interrupt rotated to other interrupts than PCI
> > INT A. In particular, the bug was observed with an additional OHCI PCI
> > card or an ES1370 sound device.
> [...]
> > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> > index 0999efc..9c77183 100644
> > --- a/hw/ppc/sam460ex.c
> > +++ b/hw/ppc/sam460ex.c
> > @@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)
> > 
> >      /* PCI bus */
> >      ppc460ex_pcie_init(env);
> > -    /* FIXME: is this correct? */
> > -    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
> > -                                uic[1][0], uic[1][20], uic[1][21],
> > uic[1][22],
> > -                                NULL);
> > +    /* All PCI irqs are connected to the same UIC pin (cf. UBoot
> > source) */
> > +    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
> > uic[1][0]);
> >      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> >      if (!pci_bus) {
> >          error_report("couldn't create PCI controller!");
> 
> I'm fine with that change. I tested it with an additional OHCI controller on
> an emulated SAM machine under guest AmigaOS and it works.
> 
> Tested-by: Sebastian Bauer <mail@sebastianbauer.info>
> 
> Better usage of QOM etc. can IMO be done at a later time point as this fixes
> an unpleasant bug and any side change will just increase the probability to
> introduce new bugs.

Thanks, applied to ppc-for-3.0.  I hope to send a pull request for
that today.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-08-01  3:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-31 11:08 [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices BALATON Zoltan
2018-07-31 21:55 ` Sebastian Bauer
2018-08-01  1:02   ` David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.