[Qemu-devel] [PATCH 0/3] ppc/xics: interface cleanups

[Qemu-devel] [PATCH 0/3] ppc/xics: interface cleanups

[Cédric Le Goater]

Hello,

Here are a couple of changes removing duplicated code and modifying
the remaining icp_ routines still using a 'XICSState *' as argument.

Thanks,

C. 

Cédric Le Goater (3):
  ppc/xics: add a xics_set_nr_servers common routine
  ppc/xics: add a XICSState backlink in ICPState
  ppc/xics: change the icp_ routines API to use an 'ICPState *' argument

 hw/intc/xics.c        | 50 ++++++++++++++++++++++++++++++++------------------
 hw/intc/xics_kvm.c    | 13 +------------
 hw/intc/xics_spapr.c  | 31 ++++++++++++-------------------
 include/hw/ppc/xics.h | 10 +++++++---
 4 files changed, 52 insertions(+), 52 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/3] ppc/xics: add a xics_set_nr_servers common routine

[Cédric Le Goater]

xics_spapr and xics_kvm nearly define the same 'set_nr_servers'
handler. Only the type of the ICP differs. So let's make a common one
to remove some duplicated code.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xics.c        | 24 +++++++++++++++++++++---
 hw/intc/xics_kvm.c    | 13 +------------
 hw/intc/xics_spapr.c  | 13 +------------
 include/hw/ppc/xics.h |  2 ++
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 7fac964fbd27..c051eebb446b 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -183,6 +183,24 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor *v, const char *name,
     info->set_nr_irqs(xics, value, errp);
 }
 
+void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
+                         const char *typename, Error **errp)
+{
+    int i;
+
+    xics->nr_servers = nr_servers;
+
+    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
+    for (i = 0; i < xics->nr_servers; i++) {
+        char name[32];
+        ICPState *icp = &xics->ss[i];
+
+        object_initialize(icp, sizeof(*icp), typename);
+        snprintf(name, sizeof(name), "icp[%d]", i);
+        object_property_add_child(OBJECT(xics), name, OBJECT(icp), errp);
+    }
+}
+
 static void xics_prop_get_nr_servers(Object *obj, Visitor *v,
                                      const char *name, void *opaque,
                                      Error **errp)
@@ -198,7 +216,7 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
                                      Error **errp)
 {
     XICSState *xics = XICS_COMMON(obj);
-    XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
+    XICSStateClass *xsc = XICS_COMMON_GET_CLASS(xics);
     Error *error = NULL;
     int64_t value;
 
@@ -213,8 +231,8 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor *v,
         return;
     }
 
-    assert(info->set_nr_servers);
-    info->set_nr_servers(xics, value, errp);
+    assert(xsc->set_nr_servers);
+    xsc->set_nr_servers(xics, value, errp);
 }
 
 static void xics_common_initfn(Object *obj)
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 9c2f198fd142..17694eaa8709 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -373,18 +373,7 @@ static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
 static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                     Error **errp)
 {
-    int i;
-
-    xics->nr_servers = nr_servers;
-
-    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
-    for (i = 0; i < xics->nr_servers; i++) {
-        char buffer[32];
-        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_KVM_ICP);
-        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
-                                  errp);
-    }
+    xics_set_nr_servers(xics, nr_servers, TYPE_KVM_ICP, errp);
 }
 
 static void rtas_dummy(PowerPCCPU *cpu, sPAPRMachineState *spapr,
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index e8d0623c2cb5..a09e1b033df7 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -249,18 +249,7 @@ static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
 static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
                                       Error **errp)
 {
-    int i;
-
-    xics->nr_servers = nr_servers;
-
-    xics->ss = g_malloc0(xics->nr_servers * sizeof(ICPState));
-    for (i = 0; i < xics->nr_servers; i++) {
-        char buffer[32];
-        object_initialize(&xics->ss[i], sizeof(xics->ss[i]), TYPE_ICP);
-        snprintf(buffer, sizeof(buffer), "icp[%d]", i);
-        object_property_add_child(OBJECT(xics), buffer, OBJECT(&xics->ss[i]),
-                                  errp);
-    }
+    xics_set_nr_servers(xics, nr_servers, TYPE_ICP, errp);
 }
 
 static void xics_spapr_realize(DeviceState *dev, Error **errp)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 66ae55ded387..573b1920536b 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -188,6 +188,8 @@ void xics_spapr_free(XICSState *icp, int irq, int num);
 
 void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
 void xics_cpu_destroy(XICSState *icp, PowerPCCPU *cpu);
+void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
+                         const char *typename, Error **errp);
 
 /* Internal XICS interfaces */
 int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
-- 
2.7.4

[Qemu-devel] [PATCH 2/3] ppc/xics: add a XICSState backlink in ICPState

[Cédric Le Goater]

The link will be used to change the API of the icp_* routines which
are still using an XICSState as an argument.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xics.c        | 1 +
 include/hw/ppc/xics.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index c051eebb446b..9f2c81a7f140 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -198,6 +198,7 @@ void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
         object_initialize(icp, sizeof(*icp), typename);
         snprintf(name, sizeof(name), "icp[%d]", i);
         object_property_add_child(OBJECT(xics), name, OBJECT(icp), errp);
+        icp->xics = xics;
     }
 }
 
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 573b1920536b..1468d6a89088 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -117,6 +117,8 @@ struct ICPState {
     uint8_t mfrr;
     qemu_irq output;
     bool cap_irq_xics_enabled;
+
+    XICSState *xics;
 };
 
 #define TYPE_ICS_BASE "ics-base"
-- 
2.7.4

[Qemu-devel] [PATCH 3/3] ppc/xics: change the icp_ routines API to use an 'ICPState *' argument

[Cédric Le Goater]

The routines :

	void icp_set_cppr(ICPState *icp, uint8_t cppr);
	void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
	void icp_eoi(ICPState *icp, uint32_t xirr);

now use one 'ICPState *icp' argument instead of a 'XICSState *' and a
server arguments. The backlink on XICSState* is used whenever needed.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xics.c        | 25 ++++++++++---------------
 hw/intc/xics_spapr.c  | 18 +++++++++++-------
 include/hw/ppc/xics.h |  6 +++---
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 9f2c81a7f140..095c16a30082 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -326,22 +326,20 @@ static void icp_check_ipi(ICPState *ss)
     qemu_irq_raise(ss->output);
 }
 
-static void icp_resend(XICSState *xics, int server)
+static void icp_resend(ICPState *ss)
 {
-    ICPState *ss = xics->ss + server;
     ICSState *ics;
 
     if (ss->mfrr < CPPR(ss)) {
         icp_check_ipi(ss);
     }
-    QLIST_FOREACH(ics, &xics->ics, list) {
+    QLIST_FOREACH(ics, &ss->xics->ics, list) {
         ics_resend(ics);
     }
 }
 
-void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
+void icp_set_cppr(ICPState *ss, uint8_t cppr)
 {
-    ICPState *ss = xics->ss + server;
     uint8_t old_cppr;
     uint32_t old_xisr;
 
@@ -361,15 +359,13 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
         }
     } else {
         if (!XISR(ss)) {
-            icp_resend(xics, server);
+            icp_resend(ss);
         }
     }
 }
 
-void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
+void icp_set_mfrr(ICPState *ss, uint8_t mfrr)
 {
-    ICPState *ss = xics->ss + server;
-
     ss->mfrr = mfrr;
     if (mfrr < CPPR(ss)) {
         icp_check_ipi(ss);
@@ -398,23 +394,22 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
     return ss->xirr;
 }
 
-void icp_eoi(XICSState *xics, int server, uint32_t xirr)
+void icp_eoi(ICPState *ss, uint32_t xirr)
 {
-    ICPState *ss = xics->ss + server;
     ICSState *ics;
     uint32_t irq;
 
     /* Send EOI -> ICS */
     ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
-    trace_xics_icp_eoi(server, xirr, ss->xirr);
+    trace_xics_icp_eoi(ss->cs->cpu_index, xirr, ss->xirr);
     irq = xirr & XISR_MASK;
-    QLIST_FOREACH(ics, &xics->ics, list) {
+    QLIST_FOREACH(ics, &ss->xics->ics, list) {
         if (ics_valid_irq(ics, irq)) {
             ics_eoi(ics, irq);
         }
     }
     if (!XISR(ss)) {
-        icp_resend(xics, server);
+        icp_resend(ss);
     }
 }
 
@@ -673,7 +668,7 @@ static int ics_simple_post_load(ICSState *ics, int version_id)
     int i;
 
     for (i = 0; i < ics->xics->nr_servers; i++) {
-        icp_resend(ics->xics, i);
+        icp_resend(&ics->xics->ss[i]);
     }
 
     return 0;
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index a09e1b033df7..b4e55012e039 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -43,9 +43,10 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
+    ICPState *icp = &spapr->xics->ss[cs->cpu_index];
     target_ulong cppr = args[0];
 
-    icp_set_cppr(spapr->xics, cs->cpu_index, cppr);
+    icp_set_cppr(icp, cppr);
     return H_SUCCESS;
 }
 
@@ -59,7 +60,7 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
         return H_PARAMETER;
     }
 
-    icp_set_mfrr(spapr->xics, server, mfrr);
+    icp_set_mfrr(spapr->xics->ss + server, mfrr);
     return H_SUCCESS;
 }
 
@@ -67,7 +68,8 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    uint32_t xirr = icp_accept(spapr->xics->ss + cs->cpu_index);
+    ICPState *icp = &spapr->xics->ss[cs->cpu_index];
+    uint32_t xirr = icp_accept(icp);
 
     args[0] = xirr;
     return H_SUCCESS;
@@ -77,8 +79,8 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                              target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *ss = &spapr->xics->ss[cs->cpu_index];
-    uint32_t xirr = icp_accept(ss);
+    ICPState *icp = &spapr->xics->ss[cs->cpu_index];
+    uint32_t xirr = icp_accept(icp);
 
     args[0] = xirr;
     args[1] = cpu_get_host_ticks();
@@ -89,9 +91,10 @@ static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
+    ICPState *icp = &spapr->xics->ss[cs->cpu_index];
     target_ulong xirr = args[0];
 
-    icp_eoi(spapr->xics, cs->cpu_index, xirr);
+    icp_eoi(icp, xirr);
     return H_SUCCESS;
 }
 
@@ -99,8 +102,9 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                             target_ulong opcode, target_ulong *args)
 {
     CPUState *cs = CPU(cpu);
+    ICPState *icp = &spapr->xics->ss[cs->cpu_index];
     uint32_t mfrr;
-    uint32_t xirr = icp_ipoll(spapr->xics->ss + cs->cpu_index, &mfrr);
+    uint32_t xirr = icp_ipoll(icp, &mfrr);
 
     args[0] = xirr;
     args[1] = mfrr;
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 1468d6a89088..6e5a113332c6 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -196,11 +196,11 @@ void xics_set_nr_servers(XICSState *xics, uint32_t nr_servers,
 /* Internal XICS interfaces */
 int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
 
-void icp_set_cppr(XICSState *icp, int server, uint8_t cppr);
-void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr);
+void icp_set_cppr(ICPState *icp, uint8_t cppr);
+void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
 uint32_t icp_accept(ICPState *ss);
 uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr);
-void icp_eoi(XICSState *icp, int server, uint32_t xirr);
+void icp_eoi(ICPState *icp, uint32_t xirr);
 
 void ics_simple_write_xive(ICSState *ics, int nr, int server,
                            uint8_t priority, uint8_t saved_priority);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 0/3] ppc/xics: interface cleanups

[David Gibson]

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

On Tue, Oct 18, 2016 at 10:06:31AM +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here are a couple of changes removing duplicated code and modifying
> the remaining icp_ routines still using a 'XICSState *' as argument.

Applied to ppc-for-2.8, thanks.

> 
> Thanks,
> 
> C. 
> 
> Cédric Le Goater (3):
>   ppc/xics: add a xics_set_nr_servers common routine
>   ppc/xics: add a XICSState backlink in ICPState
>   ppc/xics: change the icp_ routines API to use an 'ICPState *' argument
> 
>  hw/intc/xics.c        | 50 ++++++++++++++++++++++++++++++++------------------
>  hw/intc/xics_kvm.c    | 13 +------------
>  hw/intc/xics_spapr.c  | 31 ++++++++++++-------------------
>  include/hw/ppc/xics.h | 10 +++++++---
>  4 files changed, 52 insertions(+), 52 deletions(-)
> 

-- 
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: 819 bytes --]