* [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections @ 2018-07-30 4:39 Sebastian Bauer 2018-07-30 4:49 ` David Gibson 2018-07-30 11:06 ` BALATON Zoltan 0 siblings, 2 replies; 17+ messages in thread From: Sebastian Bauer @ 2018-07-30 4:39 UTC (permalink / raw) To: mail; +Cc: qemu-devel, david, agraf, qemu-ppc, balaton 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 in written. This change brings the connection in line with this. This fixes the problem that can be observed when adding further PCI cards that get their interrupt rotated to other interrupts than PCI INT A. In particular, the bug was observed and verified to be fixed (after this change) with an additional OHCI PCI card. Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> --- hw/ppc/sam460ex.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index 0999efcc1e..b2b22f280d 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) /* PCI bus */ ppc460ex_pcie_init(env); - /* FIXME: is this correct? */ + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) */ dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, - uic[1][0], uic[1][20], uic[1][21], uic[1][22], + uic[1][0], uic[1][0], uic[1][0], uic[1][0], NULL); pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0"); if (!pci_bus) { -- 2.18.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 4:39 [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections Sebastian Bauer @ 2018-07-30 4:49 ` David Gibson 2018-07-30 11:06 ` BALATON Zoltan 1 sibling, 0 replies; 17+ messages in thread From: David Gibson @ 2018-07-30 4:49 UTC (permalink / raw) To: Sebastian Bauer; +Cc: qemu-devel, agraf, qemu-ppc, balaton [-- Attachment #1: Type: text/plain, Size: 1766 bytes --] On Mon, Jul 30, 2018 at 06:39:04AM +0200, Sebastian Bauer wrote: > 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 in written. This > change brings the connection in line with this. > > This fixes the problem that can be observed when adding further PCI cards > that get their interrupt rotated to other interrupts than PCI INT A. In > particular, the bug was observed and verified to be fixed (after this > change) with an additional OHCI PCI card. > > Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> Applied to ppc-for-3.0. > --- > hw/ppc/sam460ex.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c > index 0999efcc1e..b2b22f280d 100644 > --- a/hw/ppc/sam460ex.c > +++ b/hw/ppc/sam460ex.c > @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) > > /* PCI bus */ > ppc460ex_pcie_init(env); > - /* FIXME: is this correct? */ > + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) */ > dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, > - uic[1][0], uic[1][20], uic[1][21], uic[1][22], > + uic[1][0], uic[1][0], uic[1][0], uic[1][0], > NULL); > pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0"); > if (!pci_bus) { -- 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] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 4:39 [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections Sebastian Bauer 2018-07-30 4:49 ` David Gibson @ 2018-07-30 11:06 ` BALATON Zoltan 2018-07-30 12:04 ` Sebastian Bauer 2018-07-30 22:47 ` [Qemu-devel] " Peter Maydell 1 sibling, 2 replies; 17+ messages in thread From: BALATON Zoltan @ 2018-07-30 11:06 UTC (permalink / raw) To: Sebastian Bauer; +Cc: qemu-devel, david, agraf, qemu-ppc On Mon, 30 Jul 2018, Sebastian Bauer wrote: > 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 in written. This > change brings the connection in line with this. > > This fixes the problem that can be observed when adding further PCI cards > that get their interrupt rotated to other interrupts than PCI INT A. In > particular, the bug was observed and verified to be fixed (after this > change) with an additional OHCI PCI card. > > Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> > --- > hw/ppc/sam460ex.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c > index 0999efcc1e..b2b22f280d 100644 > --- a/hw/ppc/sam460ex.c > +++ b/hw/ppc/sam460ex.c > @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) > > /* PCI bus */ > ppc460ex_pcie_init(env); > - /* FIXME: is this correct? */ > + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) */ > dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, > - uic[1][0], uic[1][20], uic[1][21], uic[1][22], > + uic[1][0], uic[1][0], uic[1][0], uic[1][0], I don't understand QOM. Does this really work? It will ultimately do qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); which will call for each of these object_property_set_link(dev, uic[1][0], "some name", &errp); Would this correctly add all device interrupts to the uic[1][0] irq or will this replace it so only the last line will be connected instead of the first after this patch? Could someone with more understanding about QOM confirm this? If this does not work this way would mapping all PCI interrupts to first line in ppc440_pcix_map_irq at hw/ppc/ppc440_pcix.c:417 and assign only that to uic[1][0] be a better fix? Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 11:06 ` BALATON Zoltan @ 2018-07-30 12:04 ` Sebastian Bauer 2018-07-30 22:37 ` BALATON Zoltan 2018-07-30 22:47 ` [Qemu-devel] " Peter Maydell 1 sibling, 1 reply; 17+ messages in thread From: Sebastian Bauer @ 2018-07-30 12:04 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-devel, david, agraf, qemu-ppc Am 2018-07-30 13:06, schrieb BALATON Zoltan: > On Mon, 30 Jul 2018, Sebastian Bauer wrote: >> 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 in written. >> This >> change brings the connection in line with this. >> >> This fixes the problem that can be observed when adding further PCI >> cards >> that get their interrupt rotated to other interrupts than PCI INT A. >> In >> particular, the bug was observed and verified to be fixed (after this >> change) with an additional OHCI PCI card. >> >> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> >> --- >> hw/ppc/sam460ex.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c >> index 0999efcc1e..b2b22f280d 100644 >> --- a/hw/ppc/sam460ex.c >> +++ b/hw/ppc/sam460ex.c >> @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) >> >> /* PCI bus */ >> ppc460ex_pcie_init(env); >> - /* FIXME: is this correct? */ >> + /* All PCI ints are connected to the same UIC pin (cf. UBoot >> source) */ >> dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, >> - uic[1][0], uic[1][20], uic[1][21], >> uic[1][22], >> + uic[1][0], uic[1][0], uic[1][0], >> uic[1][0], > > I don't understand QOM. Does this really work? It will ultimately do > > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); > > which will call for each of these > > object_property_set_link(dev, uic[1][0], "some name", &errp); I don't know QOM myself, but my research was that "some name" is different on each invocation. So you associate the same value to the same object (dev) under a property different name. What was not obvious to me was who is the owner of the value. But according to the doc QOM has the notation of strong references, but it is not created like that in qdev_connect_gpio_out_named(). So I assume that it is weak reference and the parent is not the owner. > Would this correctly add all device interrupts to the uic[1][0] irq or > will this replace it so only the last line will be connected instead > of the first after this patch? Could someone with more understanding > about QOM confirm this? I don't know how this affects QOM, but as I see it the PCI bus has four interrupts and so you need to specify all of them (and specifying wrong ones seems to be not ideal either). The code assumes this throughout, e.g., see ppc440_pcix_initfn(). As I have written in the commit log, I tested this change. I used two cards, the (default) SATA and the OHCI controller and everything was working nicely (contrary to the previous state where only the SATA card worked because this was put into slot 1). Did you have a chance to test it yourself? > If this does not work this way would mapping all PCI interrupts to > first line in ppc440_pcix_map_irq at hw/ppc/ppc440_pcix.c:417 and > assign only that to uic[1][0] be a better fix? I thought about this, but then it needs to passed that we are dealing with a SAM board, which seems to be more complicated than this solution. You also would need to adjust that ppc440_pcix_initfn() and I'm not sure if it is possible to register a root PCI bus with only one interrupt and how the remaining code deals with that. Overall, this solution seem to be much simpler. Of course, if it doesn't work the way I proposed it must be changed again. But thinking more about it, what it is indeed not so clear to me is how the interrupt states are logical combined. The hardware most likely would do an "or" (but it is difficult to verify this without schematics), not sure what QEMU does. It may just forward the level change which would be not the right thing of course. I'll investigate it. The behaviour would still be better than the previous state as usually both interrupt handlers are invoked, so no interrupt will be missed. Bye Sebastian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 12:04 ` Sebastian Bauer @ 2018-07-30 22:37 ` BALATON Zoltan 2018-07-30 23:00 ` Peter Maydell 0 siblings, 1 reply; 17+ messages in thread From: BALATON Zoltan @ 2018-07-30 22:37 UTC (permalink / raw) To: Sebastian Bauer; +Cc: qemu-devel, david, agraf, qemu-ppc On Mon, 30 Jul 2018, Sebastian Bauer wrote: > Am 2018-07-30 13:06, schrieb BALATON Zoltan: >> On Mon, 30 Jul 2018, Sebastian Bauer wrote: >>> 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 in written. This >>> change brings the connection in line with this. >>> >>> This fixes the problem that can be observed when adding further PCI cards >>> that get their interrupt rotated to other interrupts than PCI INT A. In >>> particular, the bug was observed and verified to be fixed (after this >>> change) with an additional OHCI PCI card. >>> >>> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> >>> --- >>> hw/ppc/sam460ex.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c >>> index 0999efcc1e..b2b22f280d 100644 >>> --- a/hw/ppc/sam460ex.c >>> +++ b/hw/ppc/sam460ex.c >>> @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) >>> >>> /* PCI bus */ >>> ppc460ex_pcie_init(env); >>> - /* FIXME: is this correct? */ >>> + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) >>> */ >>> dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, >>> - uic[1][0], uic[1][20], uic[1][21], >>> uic[1][22], >>> + uic[1][0], uic[1][0], uic[1][0], >>> uic[1][0], >> >> I don't understand QOM. Does this really work? It will ultimately do >> >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); >> >> which will call for each of these >> >> object_property_set_link(dev, uic[1][0], "some name", &errp); > > I don't know QOM myself, but my research was that "some name" is different on > each invocation. So you associate the same value to the same object (dev) > under a property different name. What was not obvious to me was who is the > owner of the value. But according to the doc QOM has the notation of strong > references, but it is not created like that in qdev_connect_gpio_out_named(). > So I assume that it is weak reference and the parent is not the owner. Yes, you are right, different name is generated for each irq line. But all this does not seem to matter because at the end irqs are raised/lowered directly in the irq array through functions passed when calling pci_register_root_bus so I'm not sure what these properties are used for if they are used at all so their value probably don't matter. >> Would this correctly add all device interrupts to the uic[1][0] irq or >> will this replace it so only the last line will be connected instead >> of the first after this patch? Could someone with more understanding >> about QOM confirm this? > > I don't know how this affects QOM, but as I see it the PCI bus has four > interrupts and so you need to specify all of them (and specifying wrong ones > seems to be not ideal either). The code assumes this throughout, e.g., see > ppc440_pcix_initfn(). I think the number of irq lines could be set, the functions have an nirq or num_irq parameters so the 4 lines is only assumed because PCI defines that and this is what's modelled but we could use different value here. Sam460ex seems to connect all four PCI irq lines to a single irq but I'm not sure what's the best way to model this (implementing 1 line in ppc440_pcix model or trying to merge these at some higher level). Using wrong irq lines is definitely wrong and was only left there because I had no clue about this when I've written that so your patch is definitely closer to what the board does. > As I have written in the commit log, I tested this change. I used two cards, > the (default) SATA and the OHCI controller and everything was working nicely > (contrary to the previous state where only the SATA card worked because this > was put into slot 1). Did you have a chance to test it yourself? I could not test it yet, I was trying to understand the code instead to make sure it works than just verifying it fixes one particular problem which I believe you've done. >> If this does not work this way would mapping all PCI interrupts to >> first line in ppc440_pcix_map_irq at hw/ppc/ppc440_pcix.c:417 and >> assign only that to uic[1][0] be a better fix? > > I thought about this, but then it needs to passed that we are dealing with a > SAM board, which seems to be more complicated than this solution. You also > would need to adjust that ppc440_pcix_initfn() and I'm not sure if it is > possible to register a root PCI bus with only one interrupt and how the > remaining code deals with that. Overall, this solution seem to be much > simpler. Of course, if it doesn't work the way I proposed it must be changed > again. The ppc440_pcix model is only used on the sam460ex so I think we can change it to model that and care about other cases when/if another SoC is added that has this part with different irqs just to keep things simple. > But thinking more about it, what it is indeed not so clear to me is how the > interrupt states are logical combined. The hardware most likely would do an > "or" (but it is difficult to verify this without schematics), not sure what > QEMU does. It may just forward the level change which would be not the right > thing of course. I'll investigate it. The behaviour would still be better > than the previous state as usually both interrupt handlers are invoked, so no > interrupt will be missed. I was trying to read hw/pci/pci.c to find out what would specifying the same irq multiple times do but I'm still not sure I could undestand it completely. AFAIU it will call the map_irq and set_irq functions specified in pci_register_root_bus. When we have the same irq in multiple lines theoretically it may happen that one slot is raising the interrupt and another is lowering another line which would change the same line here but I'm not sure how this is synchronised and if this could cause any problem. Probably we can just change the map function in ppc440_pcix.c to always return the first line then what's specified for other lines should not matter and the above problem is avoided. We could even get rid of those additional irqs by changing ppc440_pcix.c to only model a single line but I'd need someone with better understanding of this to confirm that I got this right. David, who should we ask to get advice on this? Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 22:37 ` BALATON Zoltan @ 2018-07-30 23:00 ` Peter Maydell 2018-07-30 23:31 ` BALATON Zoltan 0 siblings, 1 reply; 17+ messages in thread From: Peter Maydell @ 2018-07-30 23:00 UTC (permalink / raw) To: BALATON Zoltan Cc: Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson On 30 July 2018 at 23:37, BALATON Zoltan <balaton@eik.bme.hu> wrote: > I think the number of irq lines could be set, the functions have an nirq or > num_irq parameters so the 4 lines is only assumed because PCI defines that > and this is what's modelled but we could use different value here. Sam460ex > seems to connect all four PCI irq lines to a single irq but I'm not sure > what's the best way to model this (implementing 1 line in ppc440_pcix model > or trying to merge these at some higher level). Using wrong irq lines is > definitely wrong and was only left there because I had no clue about this > when I've written that so your patch is definitely closer to what the board > does. > >> As I have written in the commit log, I tested this change. I used two >> cards, the (default) SATA and the OHCI controller and everything was working >> nicely (contrary to the previous state where only the SATA card worked >> because this was put into slot 1). Did you have a chance to test it >> yourself? > > > I could not test it yet, I was trying to understand the code instead to make > sure it works than just verifying it fixes one particular problem which I > believe you've done. QEMU's implementation of qemu_irq signal lines is that the destination end provides the qemu_irq, which under the hood is a pointer to a struct containing a pointer to the function in the destination device which gets called when the source end says "the line level has changed". This means that there won't be a compile time or runtime error if you pass that qemu_irq to multiple sources (ie device outputs) simultaneously. But the behaviour will be wrong, because the destination will see all the "level is 0", "level is 1" calls from all the sources intermingled, eg device A output: ____|^^^^^^^^^^^^^|______ device B output: _______|^^^^^|___________ destination sees: ____|^^^^^^^^|___________ (because the destination gets the "level now 0" call when B's output goes to zero). To get the desired behaviour: logical OR: ____|^^^^^^^^^^^^^|_____ you need an OR gate. (I'm assuming wired-OR because that's the usual thing when several devices share an interrupt.) If your testing involves a setup which doesn't happen to assert several of the interrupt lines simultaneously you won't notice this problem. > Probably we can just change the map function in ppc440_pcix.c to always > return the first line then what's specified for other lines should not > matter and the above problem is avoided. We could even get rid of those > additional irqs by changing ppc440_pcix.c to only model a single line but > I'd need someone with better understanding of this to confirm that I got > this right. I think that would also fix the bug. The logically preferable approach would depend rather on what the actual hardware does: is there a PCI controller chip with 4 interrupt outputs which the board wires together to a single interrupt controller line, or does the PCI controller chip itself have just one output and do the merging of the different PCI interrupts itself? thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 23:00 ` Peter Maydell @ 2018-07-30 23:31 ` BALATON Zoltan 2018-07-31 0:18 ` David Gibson 0 siblings, 1 reply; 17+ messages in thread From: BALATON Zoltan @ 2018-07-30 23:31 UTC (permalink / raw) To: Peter Maydell Cc: Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson On Tue, 31 Jul 2018, Peter Maydell wrote: > On 30 July 2018 at 23:37, BALATON Zoltan <balaton@eik.bme.hu> wrote: > QEMU's implementation of qemu_irq signal lines is that the destination > end provides the qemu_irq, which under the hood is a pointer to a > struct containing a pointer to the function in the destination device > which gets called when the source end says "the line level has changed". > This means that there won't be a compile time or runtime error if you > pass that qemu_irq to multiple sources (ie device outputs) simultaneously. > But the behaviour will be wrong, because the destination will see all > the "level is 0", "level is 1" calls from all the sources intermingled, eg > > device A output: ____|^^^^^^^^^^^^^|______ > > device B output: _______|^^^^^|___________ > > destination sees: ____|^^^^^^^^|___________ > > (because the destination gets the "level now 0" call when B's output > goes to zero). To get the desired behaviour: > > logical OR: ____|^^^^^^^^^^^^^|_____ > > you need an OR gate. (I'm assuming wired-OR because that's the > usual thing when several devices share an interrupt.) > > If your testing involves a setup which doesn't happen to assert > several of the interrupt lines simultaneously you won't notice this > problem. I think the testing with SATA and USB mouse on a PCI card is likely to assert several interrupts simultanelously (eg. when moving the mouse while loading stuff from disk) but the OS might have some ways to still recover from this so maybe we won't notice it anyway. As this is now confirmed that using the same irq multiple times is wrong I think we need a better fix. David, can you please drop this patch, we'll come up with a different fix. >> Probably we can just change the map function in ppc440_pcix.c to always >> return the first line then what's specified for other lines should not >> matter and the above problem is avoided. We could even get rid of those >> additional irqs by changing ppc440_pcix.c to only model a single line but >> I'd need someone with better understanding of this to confirm that I got >> this right. > > I think that would also fix the bug. The logically preferable > approach would depend rather on what the actual hardware does: > is there a PCI controller chip with 4 interrupt outputs which > the board wires together to a single interrupt controller line, > or does the PCI controller chip itself have just one output > and do the merging of the different PCI interrupts itself? Hmm, don't really know. The PCI controller is part of the SoC but we don't have the manual of this SoC. The physical board itself also has a single PCI slot so however it's wired internally it's only using one irq for that. Not sure what other internal PCI devices are there. A comment in U-Boot sources says this (it lists PCIB twice but maybe that's meant to be PCID?): // IRQ2 = PCI_INT (PCIA, PCIB, PCIC, PCIB) So that suggests probably there are 4 PCI irqs that are wired together but probably this is inside the SoC. It could also be read as there's only a single PCI_INT that delivers all four PCI interrupts. So if we go from that either adding an OR gate to sam460ex.c that ORs together the PCI lines and connects it to uic[1][0] would work, or changing ppc440_pcix.c to provide a single PCI interrupt? We don't really have definitive info other than this comment so whatever Sebastian prefers and you approve is fine with me. Thank you, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 23:31 ` BALATON Zoltan @ 2018-07-31 0:18 ` David Gibson 2018-07-31 4:57 ` Sebastian Bauer 0 siblings, 1 reply; 17+ messages in thread From: David Gibson @ 2018-07-31 0:18 UTC (permalink / raw) To: BALATON Zoltan Cc: Peter Maydell, Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 4017 bytes --] On Tue, Jul 31, 2018 at 01:31:46AM +0200, BALATON Zoltan wrote: > On Tue, 31 Jul 2018, Peter Maydell wrote: > > On 30 July 2018 at 23:37, BALATON Zoltan <balaton@eik.bme.hu> wrote: > > QEMU's implementation of qemu_irq signal lines is that the destination > > end provides the qemu_irq, which under the hood is a pointer to a > > struct containing a pointer to the function in the destination device > > which gets called when the source end says "the line level has changed". > > This means that there won't be a compile time or runtime error if you > > pass that qemu_irq to multiple sources (ie device outputs) simultaneously. > > But the behaviour will be wrong, because the destination will see all > > the "level is 0", "level is 1" calls from all the sources intermingled, eg > > > > device A output: ____|^^^^^^^^^^^^^|______ > > > > device B output: _______|^^^^^|___________ > > > > destination sees: ____|^^^^^^^^|___________ > > > > (because the destination gets the "level now 0" call when B's output > > goes to zero). To get the desired behaviour: > > > > logical OR: ____|^^^^^^^^^^^^^|_____ > > > > you need an OR gate. (I'm assuming wired-OR because that's the > > usual thing when several devices share an interrupt.) > > > > If your testing involves a setup which doesn't happen to assert > > several of the interrupt lines simultaneously you won't notice this > > problem. > > I think the testing with SATA and USB mouse on a PCI card is likely to > assert several interrupts simultanelously (eg. when moving the mouse while > loading stuff from disk) but the OS might have some ways to still recover > from this so maybe we won't notice it anyway. As this is now confirmed that > using the same irq multiple times is wrong I think we need a better fix. > > David, can you please drop this patch, we'll come up with a different fix. Done. Should have looked at that patch a bit closer. > > > Probably we can just change the map function in ppc440_pcix.c to always > > > return the first line then what's specified for other lines should not > > > matter and the above problem is avoided. We could even get rid of those > > > additional irqs by changing ppc440_pcix.c to only model a single line but > > > I'd need someone with better understanding of this to confirm that I got > > > this right. > > > > I think that would also fix the bug. The logically preferable > > approach would depend rather on what the actual hardware does: > > is there a PCI controller chip with 4 interrupt outputs which > > the board wires together to a single interrupt controller line, > > or does the PCI controller chip itself have just one output > > and do the merging of the different PCI interrupts itself? > > Hmm, don't really know. The PCI controller is part of the SoC but we don't > have the manual of this SoC. The physical board itself also has a single PCI > slot so however it's wired internally it's only using one irq for that. Not > sure what other internal PCI devices are there. A comment in U-Boot sources > says this (it lists PCIB twice but maybe that's meant to be PCID?): > > // IRQ2 = PCI_INT (PCIA, PCIB, PCIC, PCIB) > > So that suggests probably there are 4 PCI irqs that are wired together but > probably this is inside the SoC. It could also be read as there's only a > single PCI_INT that delivers all four PCI interrupts. So if we go from that > either adding an OR gate to sam460ex.c that ORs together the PCI lines and > connects it to uic[1][0] would work, or changing ppc440_pcix.c to provide a > single PCI interrupt? We don't really have definitive info other than this > comment so whatever Sebastian prefers and you approve is fine with me. > > Thank you, > BALATON Zoltan > -- 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] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-31 0:18 ` David Gibson @ 2018-07-31 4:57 ` Sebastian Bauer 2018-07-31 6:06 ` David Gibson 2018-07-31 9:50 ` BALATON Zoltan 0 siblings, 2 replies; 17+ messages in thread From: Sebastian Bauer @ 2018-07-31 4:57 UTC (permalink / raw) To: David Gibson Cc: BALATON Zoltan, Peter Maydell, Alexander Graf, qemu-ppc, QEMU Developers Am 2018-07-31 02:18, schrieb David Gibson: >> David, can you please drop this patch, we'll come up with a different >> fix. > Done. Should have looked at that patch a bit closer. I created a follow up patch, unfortunately, based on the previous patch, as I did not spot your mail earlier than now. Note that the previous patch was an improvement over the old behaviour and under normal conditions the difference was not visible (as the OS serves both interrupts in the same run and at least on AmigaOS the interrupt handling is deferred but IRQs are acked quickly; it is still not correct though). The old implementation was not correct at all, i.e., all interrupts were missed. So it was technical an improvement (and the easiest to come up without deep understanding QEMU) ;) Anyway, I don't know if that follow up patch is the right approach. But building the logical or gate seems also be very much code, especially as in pci.c the case of multiple irqs sources are seems to be already handled (if I understand that code correctly). The follow up patch most likely does not model the hardware correctly, but the behaviour should be the same. The logical or gate would model the hardware more closely. Let me know if I should rebase to the state before my initial patch (I just looked and the previous patch was still not dropped) if you think that the change is fine. There is also the possibility to make the special (num-irqs == 1) the common case, as the Sam460ex platform is the only user of this bus so far (and probably stays the only one). I'm not sure if it is worth all the hassle. Also note that the entire ppc440_pcix.c source file seems to be created for the sam board. I have no idea why that mapping function based on slots was chosen in the first place so I kept it. I would also be fine to remove that, which would simplify things a lot. Let me know how to proceed. Bye Sebastian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-31 4:57 ` Sebastian Bauer @ 2018-07-31 6:06 ` David Gibson 2018-07-31 9:50 ` BALATON Zoltan 1 sibling, 0 replies; 17+ messages in thread From: David Gibson @ 2018-07-31 6:06 UTC (permalink / raw) To: Sebastian Bauer Cc: BALATON Zoltan, Peter Maydell, Alexander Graf, qemu-ppc, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 2692 bytes --] On Tue, Jul 31, 2018 at 06:57:31AM +0200, Sebastian Bauer wrote: > Am 2018-07-31 02:18, schrieb David Gibson: > > > David, can you please drop this patch, we'll come up with a > > > different fix. > > Done. Should have looked at that patch a bit closer. > > I created a follow up patch, unfortunately, based on the previous patch, as > I did not spot your mail earlier than now. Note that the previous patch was > an improvement over the old behaviour and under normal conditions the > difference was not visible (as the OS serves both interrupts in the same run > and at least on AmigaOS the interrupt handling is deferred but IRQs are > acked quickly; it is still not correct though). The old implementation was > not correct at all, i.e., all interrupts were missed. So it was technical an > improvement (and the easiest to come up without deep understanding QEMU) ;) > > Anyway, I don't know if that follow up patch is the right approach. But > building the logical or gate seems also be very much code, especially as in > pci.c the case of multiple irqs sources are seems to be already handled (if > I understand that code correctly). The follow up patch most likely does not > model the hardware correctly, but the behaviour should be the same. The > logical or gate would model the hardware more closely. Yeah, I think routing all the irqs to pin A should be ok for now. AIUI it's not user or guest visible so we can fix it up later if we get more information on the hardware. I believe there are a number of other embedded boards that have a similar hack too, since they only have a single PCI LSI line on the board-level PIC. > Let me know if I should rebase to the state before my initial patch (I just > looked and the previous patch was still not dropped) if you think that the > change is fine. I'd prefer a rebase onto the current state of my ppc-for-3.0 tree, please. > There is also the possibility to make the special (num-irqs == 1) the common > case, as the Sam460ex platform is the only user of this bus so far (and > probably stays the only one). I'm not sure if it is worth all the hassle. > Also note that the entire ppc440_pcix.c source file seems to be created for > the sam board. I have no idea why that mapping function based on slots was > chosen in the first place so I kept it. I would also be fine to remove that, > which would simplify things a lot. > > Let me know how to proceed. > > Bye > Sebastian > -- 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] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-31 4:57 ` Sebastian Bauer 2018-07-31 6:06 ` David Gibson @ 2018-07-31 9:50 ` BALATON Zoltan 2018-07-31 10:32 ` Sebastian Bauer 1 sibling, 1 reply; 17+ messages in thread From: BALATON Zoltan @ 2018-07-31 9:50 UTC (permalink / raw) To: Sebastian Bauer Cc: David Gibson, Peter Maydell, Alexander Graf, qemu-ppc, QEMU Developers On Tue, 31 Jul 2018, Sebastian Bauer wrote: > There is also the possibility to make the special (num-irqs == 1) the common > case, as the Sam460ex platform is the only user of this bus so far (and > probably stays the only one). I'm not sure if it is worth all the hassle. I'd vote for this as I've said before. This seems to be the simplest solution so just change ppc440_pcix to have one irq instead of 4. > Also note that the entire ppc440_pcix.c source file seems to be created for > the sam board. I have no idea why that mapping function based on slots was > chosen in the first place so I kept it. I would also be fine to remove that, > which would simplify things a lot. It may come from other similar devices I've based the implementation on (ppc4xx_pci and ppce500) so no particular reason as I had no info on the real hardware and since I haven't tried multiple PCI cards it was just left there. You can remove it, I don't have anything that says it should be there, definitely not needed for sam460ex. > Let me know how to proceed. Will you send an updated patch or should I take and modify your latest one to help with this? Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-31 9:50 ` BALATON Zoltan @ 2018-07-31 10:32 ` Sebastian Bauer 2018-07-31 11:24 ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan 0 siblings, 1 reply; 17+ messages in thread From: Sebastian Bauer @ 2018-07-31 10:32 UTC (permalink / raw) To: BALATON Zoltan Cc: David Gibson, Peter Maydell, Alexander Graf, qemu-ppc, QEMU Developers Hi, Am 2018-07-31 11:50, schrieb BALATON Zoltan: > On Tue, 31 Jul 2018, Sebastian Bauer wrote: >> There is also the possibility to make the special (num-irqs == 1) the >> common case, as the Sam460ex platform is the only user of this bus so >> far (and probably stays the only one). I'm not sure if it is worth all >> the hassle. > > I'd vote for this as I've said before. This seems to be the simplest > solution so just change ppc440_pcix to have one irq instead of 4. I now think so too, as, when devising my initial patch, I was not aware of the fact that this bus emulation was only created for the SAM. >> Let me know how to proceed. > Will you send an updated patch or should I take and modify your latest > one to help with this? I don't care so much about the how, but I would appreciate it, if the bug can be fixed, so that further PCI cards can be used without too much hassle (e.g., network cards). David seemed to be OK with my 2nd patch after I rebase it if I understood it correctly. My plan is to do this. But I'll also submit another alternative (shorter) patch that tailors the bus to the SAM only. Bye Sebastian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-31 10:32 ` Sebastian Bauer @ 2018-07-31 11:24 ` BALATON Zoltan 0 siblings, 0 replies; 17+ messages in thread From: BALATON Zoltan @ 2018-07-31 11:24 UTC (permalink / raw) To: Sebastian Bauer; +Cc: QEMU Developers, Peter Maydell, qemu-ppc, David Gibson On Tue, 31 Jul 2018, Sebastian Bauer wrote: > Am 2018-07-31 11:50, schrieb BALATON Zoltan: >> On Tue, 31 Jul 2018, Sebastian Bauer wrote: >>> There is also the possibility to make the special (num-irqs == 1) the >>> common case, as the Sam460ex platform is the only user of this bus so far >>> (and probably stays the only one). I'm not sure if it is worth all the >>> hassle. >> >> I'd vote for this as I've said before. This seems to be the simplest >> solution so just change ppc440_pcix to have one irq instead of 4. > > I now think so too, as, when devising my initial patch, I was not aware of > the fact that this bus emulation was only created for the SAM. > >>> Let me know how to proceed. >> Will you send an updated patch or should I take and modify your latest >> one to help with this? > > I don't care so much about the how, but I would appreciate it, if the bug can > be fixed, so that further PCI cards can be used without too much hassle > (e.g., network cards). David seemed to be OK with my 2nd patch after I rebase > it if I understood it correctly. My plan is to do this. But I'll also submit > another alternative (shorter) patch that tailors the bus to the SAM only. I've sent the shorter version as I think would be the easiest fix, please check if this is acceptable which may save you some work on updating your more complex patch which I think might not be needed at this point until something is found to need more interrupts in the ppc440_pcix model. I've checked that this improves sound with ES1370, haven't checked with pci-ohci so you could verify that. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 11:06 ` BALATON Zoltan 2018-07-30 12:04 ` Sebastian Bauer @ 2018-07-30 22:47 ` Peter Maydell 2018-07-30 23:00 ` BALATON Zoltan 1 sibling, 1 reply; 17+ messages in thread From: Peter Maydell @ 2018-07-30 22:47 UTC (permalink / raw) To: BALATON Zoltan Cc: Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson On 30 July 2018 at 12:06, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Mon, 30 Jul 2018, Sebastian Bauer wrote: >> >> 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 in written. This >> change brings the connection in line with this. >> >> This fixes the problem that can be observed when adding further PCI cards >> that get their interrupt rotated to other interrupts than PCI INT A. In >> particular, the bug was observed and verified to be fixed (after this >> change) with an additional OHCI PCI card. >> >> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> >> --- >> hw/ppc/sam460ex.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c >> index 0999efcc1e..b2b22f280d 100644 >> --- a/hw/ppc/sam460ex.c >> +++ b/hw/ppc/sam460ex.c >> @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) >> >> /* PCI bus */ >> ppc460ex_pcie_init(env); >> - /* FIXME: is this correct? */ >> + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) >> */ >> dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, >> - uic[1][0], uic[1][20], uic[1][21], >> uic[1][22], >> + uic[1][0], uic[1][0], uic[1][0], >> uic[1][0], > > > I don't understand QOM. Does this really work? It will ultimately do > > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); > qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); You are correct; this will not do the intended thing. If you want to wire up multiple outputs which are logically ORed together into a single input, you need to instantiate an OR gate for that (we have the TYPE_OR_IRQ for this). thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 22:47 ` [Qemu-devel] " Peter Maydell @ 2018-07-30 23:00 ` BALATON Zoltan 2018-07-30 23:15 ` Peter Maydell 0 siblings, 1 reply; 17+ messages in thread From: BALATON Zoltan @ 2018-07-30 23:00 UTC (permalink / raw) To: Peter Maydell Cc: Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson On Mon, 30 Jul 2018, Peter Maydell wrote: > On 30 July 2018 at 12:06, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> On Mon, 30 Jul 2018, Sebastian Bauer wrote: >>> >>> 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 in written. This >>> change brings the connection in line with this. >>> >>> This fixes the problem that can be observed when adding further PCI cards >>> that get their interrupt rotated to other interrupts than PCI INT A. In >>> particular, the bug was observed and verified to be fixed (after this >>> change) with an additional OHCI PCI card. >>> >>> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> >>> --- >>> hw/ppc/sam460ex.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c >>> index 0999efcc1e..b2b22f280d 100644 >>> --- a/hw/ppc/sam460ex.c >>> +++ b/hw/ppc/sam460ex.c >>> @@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine) >>> >>> /* PCI bus */ >>> ppc460ex_pcie_init(env); >>> - /* FIXME: is this correct? */ >>> + /* All PCI ints are connected to the same UIC pin (cf. UBoot source) >>> */ >>> dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, >>> - uic[1][0], uic[1][20], uic[1][21], >>> uic[1][22], >>> + uic[1][0], uic[1][0], uic[1][0], >>> uic[1][0], >> >> >> I don't understand QOM. Does this really work? It will ultimately do >> >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); >> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); > > You are correct; this will not do the intended thing. If you > want to wire up multiple outputs which are logically ORed > together into a single input, you need to instantiate an > OR gate for that (we have the TYPE_OR_IRQ for this). Thanks for confirming it. However after reading hw/pci/pci.c I still don't see where these properties are used at all. It looks like it will just call the map_irq and set_irq functions specified in pci_register_root_bus passing them the irq array given in opaque and these will change irq lines directly. So I think we could just change the map function to map everything to the first line and then the other lines don't matter at all. We could even get rid of them and change ppc440_pcix to have a single line as used in the sam460ex (which is the only board using this model currently so we can add more complexity when/if anything else needs it to be different). Is there anything why this simple solution would not work that justifies adding more complexity now? Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 23:00 ` BALATON Zoltan @ 2018-07-30 23:15 ` Peter Maydell 2018-07-31 5:09 ` Sebastian Bauer 0 siblings, 1 reply; 17+ messages in thread From: Peter Maydell @ 2018-07-30 23:15 UTC (permalink / raw) To: BALATON Zoltan Cc: Sebastian Bauer, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson On 31 July 2018 at 00:00, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Mon, 30 Jul 2018, Peter Maydell wrote: >> >> On 30 July 2018 at 12:06, BALATON Zoltan <balaton@eik.bme.hu> wrote: >>> I don't understand QOM. Does this really work? It will ultimately do >>> >>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]); >>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]); >>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]); >>> qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]); >> >> >> You are correct; this will not do the intended thing. If you >> want to wire up multiple outputs which are logically ORed >> together into a single input, you need to instantiate an >> OR gate for that (we have the TYPE_OR_IRQ for this). > > > Thanks for confirming it. However after reading hw/pci/pci.c I still don't > see where these properties are used at all. It looks like it will just call > the map_irq and set_irq functions specified in pci_register_root_bus passing > them the irq array given in opaque and these will change irq lines directly. When ppc4xx_pci_set_irq() calls qemu_set_irq() it passes it a qemu_irq which is one of the s->irq[] which was initialized via sysbus_init_irq(). The qdev_connect_gpio_out_named() calls above are what connect those to the interrupt controller at the other end of the line. Without them qemu_set_irq() does nothing (because an initialized-but-never-connected qemu_irq is actually a NULL pointer, and qemu_set_irq() exits doing nothing for NULL). On the output end of a device, a qemu_irq connection is a QOM property which is a link (pointer) property (a qemu_irq is a pointer to an opaque struct). On the input end of a device, initializing an input GPIO creates those opaque structs and sets the callback function which will be invoked for a level change. The various functions for wiring up gpio connections get a pointer to the input device's struct and pass it to the output device by setting its QOM property. > So I think we could just change the map function to map everything to the > first line and then the other lines don't matter at all. We could even get > rid of them and change ppc440_pcix to have a single line as used in the > sam460ex (which is the only board using this model currently so we can add > more complexity when/if anything else needs it to be different). Is there > anything why this simple solution would not work that justifies adding more > complexity now? It would work. But creating an OR gate is half a dozen lines or so of code, so it's not much more work than changing the PCI controller. So we should prefer whichever is closest to what the real hardware does, assuming we can determine that. thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections 2018-07-30 23:15 ` Peter Maydell @ 2018-07-31 5:09 ` Sebastian Bauer 0 siblings, 0 replies; 17+ messages in thread From: Sebastian Bauer @ 2018-07-31 5:09 UTC (permalink / raw) To: Peter Maydell Cc: BALATON Zoltan, Alexander Graf, qemu-ppc, QEMU Developers, David Gibson Am 2018-07-31 01:15, schrieb Peter Maydell: > It would work. But creating an OR gate is half a dozen lines or so of > code, so it's not much more work than changing the PCI controller. > So we should prefer whichever is closest to what the real hardware > does, assuming we can determine that. The real way is not known. The only source of information is UBoot which sets the interrupt line to 32 for all PCI devices. At least AmigaOS doesn't overwrite this setting. But UBoot could be wrong of course, as the hardware provides only one PCI slot (I think the sm502 is also connected to that bus in a direct fashion, but it doesn't have an PCI IRQ). Technically, we could also modify UBoot to chose other lines, at least AmigaOS would continue to work (if this maps the QEMU wiring) but this UBoot would be incompatible to the original hardware. Bye Sebastian ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-07-31 11:24 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-30 4:39 [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections Sebastian Bauer 2018-07-30 4:49 ` David Gibson 2018-07-30 11:06 ` BALATON Zoltan 2018-07-30 12:04 ` Sebastian Bauer 2018-07-30 22:37 ` BALATON Zoltan 2018-07-30 23:00 ` Peter Maydell 2018-07-30 23:31 ` BALATON Zoltan 2018-07-31 0:18 ` David Gibson 2018-07-31 4:57 ` Sebastian Bauer 2018-07-31 6:06 ` David Gibson 2018-07-31 9:50 ` BALATON Zoltan 2018-07-31 10:32 ` Sebastian Bauer 2018-07-31 11:24 ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan 2018-07-30 22:47 ` [Qemu-devel] " Peter Maydell 2018-07-30 23:00 ` BALATON Zoltan 2018-07-30 23:15 ` Peter Maydell 2018-07-31 5:09 ` Sebastian Bauer
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.