[Qemu-devel] [PATCH for-4.1 1/2] spapr_pci: Get rid of duplicate code for node name creation

[Qemu-devel] [PATCH for-4.1 1/2] spapr_pci: Get rid of duplicate code for node name creation

[Greg Kurz]

According to the changelog of 298a971024534, SpaprPhbState::dtbusname was
introduced to "make it easier to relate the guest and qemu views of memory
to each other", hence its name.

Use it when creating the PHB node to avoid code duplication.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index bba3a86dda6c..c70688a0dc23 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2153,7 +2153,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
                           uint32_t nr_msis, int *node_offset)
 {
     int bus_off, i, j, ret;
-    gchar *nodename;
     uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
     struct {
         uint32_t hi;
@@ -2204,9 +2203,7 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     Error *errp = NULL;
 
     /* Start populating the FDT */
-    nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
-    _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
-    g_free(nodename);
+    _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
     if (node_offset) {
         *node_offset = bus_off;
     }

[Qemu-devel] [PATCH for-4.1 1/2] spapr_pci: Get rid of duplicate code for node name creation

[Greg Kurz]

According to the changelog of 298a971024534, SpaprPhbState::dtbusname was
introduced to "make it easier to relate the guest and qemu views of memory
to each other", hence its name.

Use it when creating the PHB node to avoid code duplication.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index bba3a86dda6c..c70688a0dc23 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2153,7 +2153,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
                           uint32_t nr_msis, int *node_offset)
 {
     int bus_off, i, j, ret;
-    gchar *nodename;
     uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
     struct {
         uint32_t hi;
@@ -2204,9 +2203,7 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     Error *errp = NULL;
 
     /* Start populating the FDT */
-    nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
-    _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
-    g_free(nodename);
+    _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
     if (node_offset) {
         *node_offset = bus_off;
     }

[Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[Greg Kurz]

LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
duplicates the code of pci_swizzle_map_irq_fn().

Expose the swizzling formula so that it can be used with a slot number
when building the device tree. Simply drop pci_spapr_map_irq() and call
pci_swizzle_map_irq_fn() instead.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/pci/pci.c         |    2 +-
 hw/ppc/spapr_pci.c   |   24 ++++--------------------
 include/hw/pci/pci.h |    4 ++++
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 35451c1e9987..bb52bafd8d1c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
  */
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
 {
-    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
+    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
 }
 
 /***********************************************************/
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index c70688a0dc23..26850d0b94f4 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -719,26 +719,10 @@ param_error_exit:
     rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
 }
 
-static int pci_spapr_swizzle(int slot, int pin)
-{
-    return (slot + pin) % PCI_NUM_PINS;
-}
-
-static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
-{
-    /*
-     * Here we need to convert pci_dev + irq_num to some unique value
-     * which is less than number of IRQs on the specific bus (4).  We
-     * use standard PCI swizzling, that is (slot number + pin number)
-     * % 4.
-     */
-    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
-}
-
 static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
 {
     /*
-     * Here we use the number returned by pci_spapr_map_irq to find a
+     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
      * corresponding qemu_irq.
      */
     SpaprPhbState *phb = opaque;
@@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
                                 &sphb->iowindow);
 
     bus = pci_register_root_bus(dev, NULL,
-                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
+                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
                                 &sphb->memspace, &sphb->iospace,
                                 PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
     phb->bus = bus;
@@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     }
 
     /* Build the interrupt-map, this must matches what is done
-     * in pci_spapr_map_irq
+     * in pci_swizzle_map_irq_fn
      */
     _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
                      &interrupt_map_mask, sizeof(interrupt_map_mask)));
     for (i = 0; i < PCI_SLOT_MAX; i++) {
         for (j = 0; j < PCI_NUM_PINS; j++) {
             uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
-            int lsi_num = pci_spapr_swizzle(i, j);
+            int lsi_num = pci_swizzle(i, j);
 
             irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
             irqmap[1] = 0;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d87f5f93e9cd..033b2145d98c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
 void pci_bus_irqs_cleanup(PCIBus *bus);
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
+static inline int pci_swizzle(int slot, int pin)
+{
+    return (slot + pin) % PCI_NUM_PINS;
+}
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
 PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
                               pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,

[Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[Greg Kurz]

LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
duplicates the code of pci_swizzle_map_irq_fn().

Expose the swizzling formula so that it can be used with a slot number
when building the device tree. Simply drop pci_spapr_map_irq() and call
pci_swizzle_map_irq_fn() instead.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/pci/pci.c         |    2 +-
 hw/ppc/spapr_pci.c   |   24 ++++--------------------
 include/hw/pci/pci.h |    4 ++++
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 35451c1e9987..bb52bafd8d1c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
  */
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
 {
-    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
+    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
 }
 
 /***********************************************************/
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index c70688a0dc23..26850d0b94f4 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -719,26 +719,10 @@ param_error_exit:
     rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
 }
 
-static int pci_spapr_swizzle(int slot, int pin)
-{
-    return (slot + pin) % PCI_NUM_PINS;
-}
-
-static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
-{
-    /*
-     * Here we need to convert pci_dev + irq_num to some unique value
-     * which is less than number of IRQs on the specific bus (4).  We
-     * use standard PCI swizzling, that is (slot number + pin number)
-     * % 4.
-     */
-    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
-}
-
 static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
 {
     /*
-     * Here we use the number returned by pci_spapr_map_irq to find a
+     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
      * corresponding qemu_irq.
      */
     SpaprPhbState *phb = opaque;
@@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
                                 &sphb->iowindow);
 
     bus = pci_register_root_bus(dev, NULL,
-                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
+                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
                                 &sphb->memspace, &sphb->iospace,
                                 PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
     phb->bus = bus;
@@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
     }
 
     /* Build the interrupt-map, this must matches what is done
-     * in pci_spapr_map_irq
+     * in pci_swizzle_map_irq_fn
      */
     _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
                      &interrupt_map_mask, sizeof(interrupt_map_mask)));
     for (i = 0; i < PCI_SLOT_MAX; i++) {
         for (j = 0; j < PCI_NUM_PINS; j++) {
             uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
-            int lsi_num = pci_spapr_swizzle(i, j);
+            int lsi_num = pci_swizzle(i, j);
 
             irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
             irqmap[1] = 0;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d87f5f93e9cd..033b2145d98c 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
 void pci_bus_irqs_cleanup(PCIBus *bus);
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
+static inline int pci_swizzle(int slot, int pin)
+{
+    return (slot + pin) % PCI_NUM_PINS;
+}
 int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
 PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
                               pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,

Re: [Qemu-devel] [PATCH for-4.1 1/2] spapr_pci: Get rid of duplicate code for node name creation

[David Gibson]

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

On Fri, Apr 05, 2019 at 06:30:43PM +0200, Greg Kurz wrote:
> According to the changelog of 298a971024534, SpaprPhbState::dtbusname was
> introduced to "make it easier to relate the guest and qemu views of memory
> to each other", hence its name.
> 
> Use it when creating the PHB node to avoid code duplication.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  hw/ppc/spapr_pci.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index bba3a86dda6c..c70688a0dc23 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2153,7 +2153,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>                            uint32_t nr_msis, int *node_offset)
>  {
>      int bus_off, i, j, ret;
> -    gchar *nodename;
>      uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
>      struct {
>          uint32_t hi;
> @@ -2204,9 +2203,7 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>      Error *errp = NULL;
>  
>      /* Start populating the FDT */
> -    nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
> -    _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
> -    g_free(nodename);
> +    _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
>      if (node_offset) {
>          *node_offset = bus_off;
>      }
> 

-- 
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 --]

Re: [Qemu-devel] [PATCH for-4.1 1/2] spapr_pci: Get rid of duplicate code for node name creation

[David Gibson]

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

On Fri, Apr 05, 2019 at 06:30:43PM +0200, Greg Kurz wrote:
> According to the changelog of 298a971024534, SpaprPhbState::dtbusname was
> introduced to "make it easier to relate the guest and qemu views of memory
> to each other", hence its name.
> 
> Use it when creating the PHB node to avoid code duplication.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied, thanks.

> ---
>  hw/ppc/spapr_pci.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index bba3a86dda6c..c70688a0dc23 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2153,7 +2153,6 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>                            uint32_t nr_msis, int *node_offset)
>  {
>      int bus_off, i, j, ret;
> -    gchar *nodename;
>      uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
>      struct {
>          uint32_t hi;
> @@ -2204,9 +2203,7 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>      Error *errp = NULL;
>  
>      /* Start populating the FDT */
> -    nodename = g_strdup_printf("pci@%" PRIx64, phb->buid);
> -    _FDT(bus_off = fdt_add_subnode(fdt, 0, nodename));
> -    g_free(nodename);
> +    _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
>      if (node_offset) {
>          *node_offset = bus_off;
>      }
> 

-- 
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 --]

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[David Gibson]

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

On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> duplicates the code of pci_swizzle_map_irq_fn().
> 
> Expose the swizzling formula so that it can be used with a slot number
> when building the device tree. Simply drop pci_spapr_map_irq() and call
> pci_swizzle_map_irq_fn() instead.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-4.1, thanks.

> ---
>  hw/pci/pci.c         |    2 +-
>  hw/ppc/spapr_pci.c   |   24 ++++--------------------
>  include/hw/pci/pci.h |    4 ++++
>  3 files changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 35451c1e9987..bb52bafd8d1c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
>   */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
>  {
> -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
>  }
>  
>  /***********************************************************/
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index c70688a0dc23..26850d0b94f4 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -719,26 +719,10 @@ param_error_exit:
>      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>  }
>  
> -static int pci_spapr_swizzle(int slot, int pin)
> -{
> -    return (slot + pin) % PCI_NUM_PINS;
> -}
> -
> -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> -{
> -    /*
> -     * Here we need to convert pci_dev + irq_num to some unique value
> -     * which is less than number of IRQs on the specific bus (4).  We
> -     * use standard PCI swizzling, that is (slot number + pin number)
> -     * % 4.
> -     */
> -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> -}
> -
>  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
>  {
>      /*
> -     * Here we use the number returned by pci_spapr_map_irq to find a
> +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
>       * corresponding qemu_irq.
>       */
>      SpaprPhbState *phb = opaque;
> @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>                                  &sphb->iowindow);
>  
>      bus = pci_register_root_bus(dev, NULL,
> -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
>                                  &sphb->memspace, &sphb->iospace,
>                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
>      phb->bus = bus;
> @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>      }
>  
>      /* Build the interrupt-map, this must matches what is done
> -     * in pci_spapr_map_irq
> +     * in pci_swizzle_map_irq_fn
>       */
>      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
>                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
>      for (i = 0; i < PCI_SLOT_MAX; i++) {
>          for (j = 0; j < PCI_NUM_PINS; j++) {
>              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> -            int lsi_num = pci_spapr_swizzle(i, j);
> +            int lsi_num = pci_swizzle(i, j);
>  
>              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
>              irqmap[1] = 0;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d87f5f93e9cd..033b2145d98c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  void pci_bus_irqs_cleanup(PCIBus *bus);
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> +static inline int pci_swizzle(int slot, int pin)
> +{
> +    return (slot + pin) % PCI_NUM_PINS;
> +}
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
>  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
>                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 

-- 
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 --]

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[David Gibson]

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

On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> duplicates the code of pci_swizzle_map_irq_fn().
> 
> Expose the swizzling formula so that it can be used with a slot number
> when building the device tree. Simply drop pci_spapr_map_irq() and call
> pci_swizzle_map_irq_fn() instead.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-4.1, thanks.

> ---
>  hw/pci/pci.c         |    2 +-
>  hw/ppc/spapr_pci.c   |   24 ++++--------------------
>  include/hw/pci/pci.h |    4 ++++
>  3 files changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 35451c1e9987..bb52bafd8d1c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
>   */
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
>  {
> -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
>  }
>  
>  /***********************************************************/
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index c70688a0dc23..26850d0b94f4 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -719,26 +719,10 @@ param_error_exit:
>      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>  }
>  
> -static int pci_spapr_swizzle(int slot, int pin)
> -{
> -    return (slot + pin) % PCI_NUM_PINS;
> -}
> -
> -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> -{
> -    /*
> -     * Here we need to convert pci_dev + irq_num to some unique value
> -     * which is less than number of IRQs on the specific bus (4).  We
> -     * use standard PCI swizzling, that is (slot number + pin number)
> -     * % 4.
> -     */
> -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> -}
> -
>  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
>  {
>      /*
> -     * Here we use the number returned by pci_spapr_map_irq to find a
> +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
>       * corresponding qemu_irq.
>       */
>      SpaprPhbState *phb = opaque;
> @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>                                  &sphb->iowindow);
>  
>      bus = pci_register_root_bus(dev, NULL,
> -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
>                                  &sphb->memspace, &sphb->iospace,
>                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
>      phb->bus = bus;
> @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
>      }
>  
>      /* Build the interrupt-map, this must matches what is done
> -     * in pci_spapr_map_irq
> +     * in pci_swizzle_map_irq_fn
>       */
>      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
>                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
>      for (i = 0; i < PCI_SLOT_MAX; i++) {
>          for (j = 0; j < PCI_NUM_PINS; j++) {
>              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> -            int lsi_num = pci_spapr_swizzle(i, j);
> +            int lsi_num = pci_swizzle(i, j);
>  
>              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
>              irqmap[1] = 0;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d87f5f93e9cd..033b2145d98c 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>  void pci_bus_irqs_cleanup(PCIBus *bus);
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
>  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> +static inline int pci_swizzle(int slot, int pin)
> +{
> +    return (slot + pin) % PCI_NUM_PINS;
> +}
>  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
>  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
>                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> 

-- 
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 --]

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[Greg Kurz]

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

On Mon, 8 Apr 2019 13:40:54 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> > LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> > duplicates the code of pci_swizzle_map_irq_fn().
> > 
> > Expose the swizzling formula so that it can be used with a slot number
> > when building the device tree. Simply drop pci_spapr_map_irq() and call
> > pci_swizzle_map_irq_fn() instead.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>  
> 
> Applied to ppc-for-4.1, thanks.
> 

Thanks ! I just realize that I had come up with a better title, at least
from my POV, but I forgot to update before posting...

spapr: Drop duplicate PCI swizzle code

No big deal if you keep the current title of course :)


> > ---
> >  hw/pci/pci.c         |    2 +-
> >  hw/ppc/spapr_pci.c   |   24 ++++--------------------
> >  include/hw/pci/pci.h |    4 ++++
> >  3 files changed, 9 insertions(+), 21 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 35451c1e9987..bb52bafd8d1c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> >   */
> >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
> >  {
> > -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> > +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
> >  }
> >  
> >  /***********************************************************/
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index c70688a0dc23..26850d0b94f4 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -719,26 +719,10 @@ param_error_exit:
> >      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> >  }
> >  
> > -static int pci_spapr_swizzle(int slot, int pin)
> > -{
> > -    return (slot + pin) % PCI_NUM_PINS;
> > -}
> > -
> > -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> > -{
> > -    /*
> > -     * Here we need to convert pci_dev + irq_num to some unique value
> > -     * which is less than number of IRQs on the specific bus (4).  We
> > -     * use standard PCI swizzling, that is (slot number + pin number)
> > -     * % 4.
> > -     */
> > -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> > -}
> > -
> >  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
> >  {
> >      /*
> > -     * Here we use the number returned by pci_spapr_map_irq to find a
> > +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
> >       * corresponding qemu_irq.
> >       */
> >      SpaprPhbState *phb = opaque;
> > @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> >                                  &sphb->iowindow);
> >  
> >      bus = pci_register_root_bus(dev, NULL,
> > -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> > +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
> >                                  &sphb->memspace, &sphb->iospace,
> >                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> >      phb->bus = bus;
> > @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
> >      }
> >  
> >      /* Build the interrupt-map, this must matches what is done
> > -     * in pci_spapr_map_irq
> > +     * in pci_swizzle_map_irq_fn
> >       */
> >      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
> >                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
> >      for (i = 0; i < PCI_SLOT_MAX; i++) {
> >          for (j = 0; j < PCI_NUM_PINS; j++) {
> >              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> > -            int lsi_num = pci_spapr_swizzle(i, j);
> > +            int lsi_num = pci_swizzle(i, j);
> >  
> >              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
> >              irqmap[1] = 0;
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index d87f5f93e9cd..033b2145d98c 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> >  void pci_bus_irqs_cleanup(PCIBus *bus);
> >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> >  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > +static inline int pci_swizzle(int slot, int pin)
> > +{
> > +    return (slot + pin) % PCI_NUM_PINS;
> > +}
> >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> >                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> >   
> 


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

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[Greg Kurz]

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

On Mon, 8 Apr 2019 13:40:54 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> > LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> > duplicates the code of pci_swizzle_map_irq_fn().
> > 
> > Expose the swizzling formula so that it can be used with a slot number
> > when building the device tree. Simply drop pci_spapr_map_irq() and call
> > pci_swizzle_map_irq_fn() instead.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>  
> 
> Applied to ppc-for-4.1, thanks.
> 

Thanks ! I just realize that I had come up with a better title, at least
from my POV, but I forgot to update before posting...

spapr: Drop duplicate PCI swizzle code

No big deal if you keep the current title of course :)


> > ---
> >  hw/pci/pci.c         |    2 +-
> >  hw/ppc/spapr_pci.c   |   24 ++++--------------------
> >  include/hw/pci/pci.h |    4 ++++
> >  3 files changed, 9 insertions(+), 21 deletions(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 35451c1e9987..bb52bafd8d1c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> >   */
> >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
> >  {
> > -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> > +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
> >  }
> >  
> >  /***********************************************************/
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index c70688a0dc23..26850d0b94f4 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -719,26 +719,10 @@ param_error_exit:
> >      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> >  }
> >  
> > -static int pci_spapr_swizzle(int slot, int pin)
> > -{
> > -    return (slot + pin) % PCI_NUM_PINS;
> > -}
> > -
> > -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> > -{
> > -    /*
> > -     * Here we need to convert pci_dev + irq_num to some unique value
> > -     * which is less than number of IRQs on the specific bus (4).  We
> > -     * use standard PCI swizzling, that is (slot number + pin number)
> > -     * % 4.
> > -     */
> > -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> > -}
> > -
> >  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
> >  {
> >      /*
> > -     * Here we use the number returned by pci_spapr_map_irq to find a
> > +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
> >       * corresponding qemu_irq.
> >       */
> >      SpaprPhbState *phb = opaque;
> > @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> >                                  &sphb->iowindow);
> >  
> >      bus = pci_register_root_bus(dev, NULL,
> > -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> > +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
> >                                  &sphb->memspace, &sphb->iospace,
> >                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> >      phb->bus = bus;
> > @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
> >      }
> >  
> >      /* Build the interrupt-map, this must matches what is done
> > -     * in pci_spapr_map_irq
> > +     * in pci_swizzle_map_irq_fn
> >       */
> >      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
> >                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
> >      for (i = 0; i < PCI_SLOT_MAX; i++) {
> >          for (j = 0; j < PCI_NUM_PINS; j++) {
> >              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> > -            int lsi_num = pci_spapr_swizzle(i, j);
> > +            int lsi_num = pci_swizzle(i, j);
> >  
> >              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
> >              irqmap[1] = 0;
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index d87f5f93e9cd..033b2145d98c 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> >  void pci_bus_irqs_cleanup(PCIBus *bus);
> >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> >  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > +static inline int pci_swizzle(int slot, int pin)
> > +{
> > +    return (slot + pin) % PCI_NUM_PINS;
> > +}
> >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> >                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> >   
> 


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

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[David Gibson]

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

On Mon, Apr 08, 2019 at 05:01:37PM +0200, Greg Kurz wrote:
> On Mon, 8 Apr 2019 13:40:54 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> > > LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> > > duplicates the code of pci_swizzle_map_irq_fn().
> > > 
> > > Expose the swizzling formula so that it can be used with a slot number
> > > when building the device tree. Simply drop pci_spapr_map_irq() and call
> > > pci_swizzle_map_irq_fn() instead.
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>  
> > 
> > Applied to ppc-for-4.1, thanks.
> > 
> 
> Thanks ! I just realize that I had come up with a better title, at least
> from my POV, but I forgot to update before posting...
> 
> spapr: Drop duplicate PCI swizzle code

Sure, that is a better title and the 4.1 queue won't be sent off for a
bit, so I've updated that.

> No big deal if you keep the current title of course :)



> 
> 
> > > ---
> > >  hw/pci/pci.c         |    2 +-
> > >  hw/ppc/spapr_pci.c   |   24 ++++--------------------
> > >  include/hw/pci/pci.h |    4 ++++
> > >  3 files changed, 9 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 35451c1e9987..bb52bafd8d1c 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> > >   */
> > >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
> > >  {
> > > -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> > > +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
> > >  }
> > >  
> > >  /***********************************************************/
> > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > index c70688a0dc23..26850d0b94f4 100644
> > > --- a/hw/ppc/spapr_pci.c
> > > +++ b/hw/ppc/spapr_pci.c
> > > @@ -719,26 +719,10 @@ param_error_exit:
> > >      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > >  }
> > >  
> > > -static int pci_spapr_swizzle(int slot, int pin)
> > > -{
> > > -    return (slot + pin) % PCI_NUM_PINS;
> > > -}
> > > -
> > > -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> > > -{
> > > -    /*
> > > -     * Here we need to convert pci_dev + irq_num to some unique value
> > > -     * which is less than number of IRQs on the specific bus (4).  We
> > > -     * use standard PCI swizzling, that is (slot number + pin number)
> > > -     * % 4.
> > > -     */
> > > -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> > > -}
> > > -
> > >  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
> > >  {
> > >      /*
> > > -     * Here we use the number returned by pci_spapr_map_irq to find a
> > > +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
> > >       * corresponding qemu_irq.
> > >       */
> > >      SpaprPhbState *phb = opaque;
> > > @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > >                                  &sphb->iowindow);
> > >  
> > >      bus = pci_register_root_bus(dev, NULL,
> > > -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> > > +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
> > >                                  &sphb->memspace, &sphb->iospace,
> > >                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> > >      phb->bus = bus;
> > > @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
> > >      }
> > >  
> > >      /* Build the interrupt-map, this must matches what is done
> > > -     * in pci_spapr_map_irq
> > > +     * in pci_swizzle_map_irq_fn
> > >       */
> > >      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
> > >                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
> > >      for (i = 0; i < PCI_SLOT_MAX; i++) {
> > >          for (j = 0; j < PCI_NUM_PINS; j++) {
> > >              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> > > -            int lsi_num = pci_spapr_swizzle(i, j);
> > > +            int lsi_num = pci_swizzle(i, j);
> > >  
> > >              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
> > >              irqmap[1] = 0;
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index d87f5f93e9cd..033b2145d98c 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > >  void pci_bus_irqs_cleanup(PCIBus *bus);
> > >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > >  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > > +static inline int pci_swizzle(int slot, int pin)
> > > +{
> > > +    return (slot + pin) % PCI_NUM_PINS;
> > > +}
> > >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > >                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > >   
> > 
> 



-- 
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 --]

Re: [Qemu-devel] [PATCH for-4.1 2/2] spapr: Drop duplicate code in LSI mapping

[David Gibson]

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

On Mon, Apr 08, 2019 at 05:01:37PM +0200, Greg Kurz wrote:
> On Mon, 8 Apr 2019 13:40:54 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Apr 05, 2019 at 06:30:48PM +0200, Greg Kurz wrote:
> > > LSI mapping in spapr currently open-codes standard PCI swizzling. It thus
> > > duplicates the code of pci_swizzle_map_irq_fn().
> > > 
> > > Expose the swizzling formula so that it can be used with a slot number
> > > when building the device tree. Simply drop pci_spapr_map_irq() and call
> > > pci_swizzle_map_irq_fn() instead.
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>  
> > 
> > Applied to ppc-for-4.1, thanks.
> > 
> 
> Thanks ! I just realize that I had come up with a better title, at least
> from my POV, but I forgot to update before posting...
> 
> spapr: Drop duplicate PCI swizzle code

Sure, that is a better title and the 4.1 queue won't be sent off for a
bit, so I've updated that.

> No big deal if you keep the current title of course :)



> 
> 
> > > ---
> > >  hw/pci/pci.c         |    2 +-
> > >  hw/ppc/spapr_pci.c   |   24 ++++--------------------
> > >  include/hw/pci/pci.h |    4 ++++
> > >  3 files changed, 9 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 35451c1e9987..bb52bafd8d1c 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -1532,7 +1532,7 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> > >   */
> > >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin)
> > >  {
> > > -    return (pin + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> > > +    return pci_swizzle(PCI_SLOT(pci_dev->devfn), pin);
> > >  }
> > >  
> > >  /***********************************************************/
> > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > index c70688a0dc23..26850d0b94f4 100644
> > > --- a/hw/ppc/spapr_pci.c
> > > +++ b/hw/ppc/spapr_pci.c
> > > @@ -719,26 +719,10 @@ param_error_exit:
> > >      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > >  }
> > >  
> > > -static int pci_spapr_swizzle(int slot, int pin)
> > > -{
> > > -    return (slot + pin) % PCI_NUM_PINS;
> > > -}
> > > -
> > > -static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
> > > -{
> > > -    /*
> > > -     * Here we need to convert pci_dev + irq_num to some unique value
> > > -     * which is less than number of IRQs on the specific bus (4).  We
> > > -     * use standard PCI swizzling, that is (slot number + pin number)
> > > -     * % 4.
> > > -     */
> > > -    return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
> > > -}
> > > -
> > >  static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
> > >  {
> > >      /*
> > > -     * Here we use the number returned by pci_spapr_map_irq to find a
> > > +     * Here we use the number returned by pci_swizzle_map_irq_fn to find a
> > >       * corresponding qemu_irq.
> > >       */
> > >      SpaprPhbState *phb = opaque;
> > > @@ -1744,7 +1728,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > >                                  &sphb->iowindow);
> > >  
> > >      bus = pci_register_root_bus(dev, NULL,
> > > -                                pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> > > +                                pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb,
> > >                                  &sphb->memspace, &sphb->iospace,
> > >                                  PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> > >      phb->bus = bus;
> > > @@ -2236,14 +2220,14 @@ int spapr_populate_pci_dt(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt,
> > >      }
> > >  
> > >      /* Build the interrupt-map, this must matches what is done
> > > -     * in pci_spapr_map_irq
> > > +     * in pci_swizzle_map_irq_fn
> > >       */
> > >      _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
> > >                       &interrupt_map_mask, sizeof(interrupt_map_mask)));
> > >      for (i = 0; i < PCI_SLOT_MAX; i++) {
> > >          for (j = 0; j < PCI_NUM_PINS; j++) {
> > >              uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
> > > -            int lsi_num = pci_spapr_swizzle(i, j);
> > > +            int lsi_num = pci_swizzle(i, j);
> > >  
> > >              irqmap[0] = cpu_to_be32(b_ddddd(i)|b_fff(0));
> > >              irqmap[1] = 0;
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index d87f5f93e9cd..033b2145d98c 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -411,6 +411,10 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > >  void pci_bus_irqs_cleanup(PCIBus *bus);
> > >  int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > >  /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > > +static inline int pci_swizzle(int slot, int pin)
> > > +{
> > > +    return (slot + pin) % PCI_NUM_PINS;
> > > +}
> > >  int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > >  PCIBus *pci_register_root_bus(DeviceState *parent, const char *name,
> > >                                pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > >   
> > 
> 



-- 
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 --]