[Qemu-devel] [PATCH 0/2] sam460ex: Improve logicial or'ed PCI-X interrupts

[Qemu-devel] [PATCH 0/2] sam460ex: Improve logicial or'ed PCI-X interrupts

[Sebastian Bauer]

The previous change 70a8ff3fd0c27e69a598e7603112de9d0fec5380 fixed
the interrupt connection not properly as the IRQ levels would not
be logcical or'ed. While other PCI cards already have worked with
that change, it didn't model the expected hardware behaviour
close enough.

This patch series is an attempt to fix this remaining problem.
Firtsly, it introduces the num-irqs property to the pci-xbus class
and implements the one common IRQ as a special case. The PCI bus
implementation logic will handle the logical or for us then.

Secondly, it adjust the sam460ex code to take advantage of the
new property.

Tested on the SAM460ex machine (which admittely is the only user
so far).

Sebastian Bauer (2):
  ppc: Allow clients of the 440 pcix bus to specify the number of
    interrupts
  sam460ex: Create the PCI-X bus with only one interrupt

 hw/ppc/ppc440_pcix.c | 28 +++++++++++++++++++++++++---
 hw/ppc/sam460ex.c    | 14 ++++++++++----
 2 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.18.0

[Qemu-devel] [PATCH 1/2] ppc: Allow clients of the 440 pcix bus to specify the number of interrupts

[Sebastian Bauer]

This can be done by using the newly introduced num_irqs property. In
particular, this change introduces a special case if num_irqs is 1 in which
case any interrupt pin will be connected to the single irq. The default
case is untouched (but note that the only client is the Sam460ex board for
which the special case was actually created).

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
---
 hw/ppc/ppc440_pcix.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index d8af04b70f..cb7d7cfd2b 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -57,6 +57,7 @@ typedef struct PPC440PCIXState {
     struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
     struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
     uint32_t sts;
+    uint16_t num_irqs;
     qemu_irq irq[PCI_NUM_PINS];
     AddressSpace bm_as;
     MemoryRegion bm;
@@ -423,6 +424,12 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
     return slot - 1;
 }
 
+/* All pins from each slot are tied the same and only board IRQ. */
+static int ppc440_pcix_map_irq_single(PCIDevice *pci_dev, int irq_num)
+{
+    return 0;
+}
+
 static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
 {
     qemu_irq *pci_irqs = opaque;
@@ -469,6 +476,7 @@ const MemoryRegionOps ppc440_pcix_host_data_ops = {
 
 static int ppc440_pcix_initfn(SysBusDevice *dev)
 {
+    pci_map_irq_fn map_irq;
     PPC440PCIXState *s;
     PCIHostState *h;
     int i;
@@ -476,14 +484,22 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
     h = PCI_HOST_BRIDGE(dev);
     s = PPC440_PCIX_HOST_BRIDGE(dev);
 
-    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+    if (s->num_irqs > 4) {
+        fprintf(stderr, "%s: Number of irqs must not exceed 4\n", __func__);
+        return -1;
+    }
+
+    for (i = 0; i < s->num_irqs; i++) {
         sysbus_init_irq(dev, &s->irq[i]);
     }
 
     memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+
+    map_irq = s->num_irqs == 1 ?
+            ppc440_pcix_map_irq_single : ppc440_pcix_map_irq;
     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);
+                         map_irq, s->irq, &s->busmem, get_system_io(),
+                         PCI_DEVFN(0, 0), s->num_irqs, TYPE_PCI_BUS);
 
     s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
@@ -507,6 +523,11 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
     return 0;
 }
 
+static Property ppc440_pcix_properties[] = {
+    DEFINE_PROP_UINT16("num-irqs", PPC440PCIXState, num_irqs, 4),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
@@ -514,6 +535,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
 
     k->init = ppc440_pcix_initfn;
     dc->reset = ppc440_pcix_reset;
+    dc->props = ppc440_pcix_properties;
 }
 
 static const TypeInfo ppc440_pcix_info = {
-- 
2.18.0

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] ppc: Allow clients of the 440 pcix bus to specify the number of interrupts

[Cédric Le Goater]

On 07/31/2018 06:36 AM, Sebastian Bauer wrote:
> This can be done by using the newly introduced num_irqs property. In
> particular, this change introduces a special case if num_irqs is 1 in which
> case any interrupt pin will be connected to the single irq. The default
> case is untouched (but note that the only client is the Sam460ex board for
> which the special case was actually created).
> 
> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
> ---
>  hw/ppc/ppc440_pcix.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index d8af04b70f..cb7d7cfd2b 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -57,6 +57,7 @@ typedef struct PPC440PCIXState {
>      struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
>      struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
>      uint32_t sts;
> +    uint16_t num_irqs;
>      qemu_irq irq[PCI_NUM_PINS];
>      AddressSpace bm_as;
>      MemoryRegion bm;
> @@ -423,6 +424,12 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
>      return slot - 1;
>  }
>  
> +/* All pins from each slot are tied the same and only board IRQ. */
> +static int ppc440_pcix_map_irq_single(PCIDevice *pci_dev, int irq_num)
> +{
> +    return 0;
> +}
> +
>  static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
>  {
>      qemu_irq *pci_irqs = opaque;
> @@ -469,6 +476,7 @@ const MemoryRegionOps ppc440_pcix_host_data_ops = {
>  
>  static int ppc440_pcix_initfn(SysBusDevice *dev)
>  {
> +    pci_map_irq_fn map_irq;
>      PPC440PCIXState *s;
>      PCIHostState *h;
>      int i;
> @@ -476,14 +484,22 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
>      h = PCI_HOST_BRIDGE(dev);
>      s = PPC440_PCIX_HOST_BRIDGE(dev);
>  
> -    for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> +    if (s->num_irqs > 4) {

may be use PCI_NUM_PINS

> +        fprintf(stderr, "%s: Number of irqs must not exceed 4\n", __func__);

error_report would be better.
 
> +        return -1;
> +    }

It might be the right time to QOM'ify a bit more "ppc440-pcix-host" and 
introduce a realize() operation to handle errors. 

> +    for (i = 0; i < s->num_irqs; i++) {
>          sysbus_init_irq(dev, &s->irq[i]);
>      }
>  
>      memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
> +
> +    map_irq = s->num_irqs == 1 ?
> +            ppc440_pcix_map_irq_single : ppc440_pcix_map_irq;
>      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);
> +                         map_irq, s->irq, &s->busmem, get_system_io(),
> +                         PCI_DEVFN(0, 0), s->num_irqs, TYPE_PCI_BUS);
>  
>      s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
>  
> @@ -507,6 +523,11 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
>      return 0;
>  }
>  
> +static Property ppc440_pcix_properties[] = {
> +    DEFINE_PROP_UINT16("num-irqs", PPC440PCIXState, num_irqs, 4),

PCI_NUM_PINS ?

> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
>  {
>      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> @@ -514,6 +535,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
>  
>      k->init = ppc440_pcix_initfn;
>      dc->reset = ppc440_pcix_reset;
> +    dc->props = ppc440_pcix_properties;
>  }
>  
>  static const TypeInfo ppc440_pcix_info = {
> 

[Qemu-devel] [PATCH 2/2] sam460ex: Create the PCI-X bus with only one interrupt

[Sebastian Bauer]

This is done by unfolding the sysbus_create_varargs() call and by
announcing that only a single irq is needed before connecting the irqs
before the bus is initialized.

This should model the design of the SAM board better, which is that all
PCI interrupts are connected to a single interrupt pin, in particular that
the logical level of the destination pin is an combined or of all the
individual interrupt levels.

Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
---
 hw/ppc/sam460ex.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index b2b22f280d..28265bcb4c 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,10 +515,16 @@ static void sam460ex_init(MachineState *machine)
 
     /* PCI bus */
     ppc460ex_pcie_init(env);
-    /* 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][0], uic[1][0], uic[1][0],
-                                NULL);
+
+    /* All PCI ints of the PCI-X bus are connected to the same UIC pin (cf.
+     * UBoot source) so only one connection is needed. */
+    dev = qdev_create(NULL, "ppc440-pcix-host");
+    qdev_prop_set_uint16(dev, "num-irqs", 1);
+    sbdev = SYS_BUS_DEVICE(dev);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(sbdev, 0, 0xc0ec00000);
+    sysbus_connect_irq(sbdev, 0, uic[1][0]);
+
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (!pci_bus) {
         error_report("couldn't create PCI controller!");
-- 
2.18.0