qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.10 1/8] ppc/xics: add a xics_get_cpu_index


From: Cédric Le Goater
Subject: Re: [Qemu-devel] [PATCH for-2.10 1/8] ppc/xics: add a xics_get_cpu_index_by_pir() helper
Date: Wed, 15 Mar 2017 11:04:07 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0

On 03/15/2017 05:53 AM, David Gibson wrote:
> On Tue, Mar 14, 2017 at 06:00:43PM +0100, Cédric Le Goater wrote:
>> On 03/14/2017 06:38 AM, David Gibson wrote:
>>> On Wed, Mar 08, 2017 at 11:52:44AM +0100, Cédric Le Goater wrote:
>>>> This helper will be used to translate the server number of the XIVE
>>>> (which is a PIR) into an ICPState index number (which is a cpu index).
>>>>
>>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>>
>>> This seems a slightly roundabout way of doing things.  Why not just
>>> have the vcpu_by_pir() interface, then have the XICSFabric implementor
>>> go directly from PIR to xics server state.
>>
>> So what you are saying is that we should try to move the "nature"
>> of the 'server' parameter of the xics framework in the icp_get() 
>> handler of the XICSFabric. Correct ? Because at the end, it all 
>> boils down to use a 'server' to look for an ICPState.
>>
>> Each machine would do its conversion : 
>>
>>      xics_get_cpu_index_by_dt_id() for spapr
>>      xics_get_cpu_index_by_pir() for powernv
> 
> Yes, that's exactly right.  I think it makes sense for the XICSFabric
> to be the thing that defines the mapping from XICS server numbers to
> whatever other ID is relevant.
> 

OK. So we need some more cleanups in the current code because 
'cpu_index' is still being used in a couple of places. Below
is a patch adding an ICPState backlink to ease the transition. 
After this one, moving the XICS server mapping under the 
icp_get() handler is trivial. Tell me if you like the idea and 
I will send both patches. 

If not, we can still do a few thing but we will need to move 
the lookup out of xics_cpu_setup() *and* xics_cpu_destoy().


Thanks,

C. 











>From dad67b9fdcafa7f9e84ce27e6ae5fd2c2bbd6b3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <address@hidden>
Date: Wed, 15 Mar 2017 09:17:11 +0100
Subject: [PATCH] ppc/xics: introduce a ICPState backlink under PowerPCCPU
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, 'cpu_index' is used to lookup for ICPState objects. But,
this can be a problem for platforms having a different index, the
PowerNV uses a core PIR number for instance.

One way to abstract the IRQ 'server' number of the XICS layer is to
let the core init routine of the platform do the lookup of the
ICPState and store the resulting ICPState object under PowerPCCPU.
This enables us to limit the use of 'cpu_index' in XICS making it more
generic for other platforms.

It also has the benefit of simplifying the sPAPR hcalls which do not
need to do any ICPState lookup anymore.

Signed-off-by: Cédric Le Goater <address@hidden>
---
 hw/intc/xics.c          |  4 ++--
 hw/intc/xics_spapr.c    | 20 +++++---------------
 hw/ppc/spapr_cpu_core.c |  4 +++-
 target/ppc/cpu.h        |  2 ++
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index e740989a1162..5cde86ceb3bc 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -52,7 +52,7 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
 void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(xi, cs->cpu_index);
+    ICPState *icp = cpu->icp;
 
     assert(icp);
     assert(cs == icp->cs);
@@ -65,7 +65,7 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
-    ICPState *icp = xics_icp_get(xi, cs->cpu_index);
+    ICPState *icp = cpu->icp;
     ICPStateClass *icpc;
 
     assert(icp);
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 84d24b2837a7..178b3adc8af7 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -43,11 +43,9 @@
 static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
-    CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), cs->cpu_index);
     target_ulong cppr = args[0];
 
-    icp_set_cppr(icp, cppr);
+    icp_set_cppr(cpu->icp, cppr);
     return H_SUCCESS;
 }
 
@@ -69,9 +67,7 @@ static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState 
*spapr,
 static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                            target_ulong opcode, target_ulong *args)
 {
-    CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), cs->cpu_index);
-    uint32_t xirr = icp_accept(icp);
+    uint32_t xirr = icp_accept(cpu->icp);
 
     args[0] = xirr;
     return H_SUCCESS;
@@ -80,9 +76,7 @@ static target_ulong h_xirr(PowerPCCPU *cpu, sPAPRMachineState 
*spapr,
 static target_ulong h_xirr_x(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                              target_ulong opcode, target_ulong *args)
 {
-    CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), cs->cpu_index);
-    uint32_t xirr = icp_accept(icp);
+    uint32_t xirr = icp_accept(cpu->icp);
 
     args[0] = xirr;
     args[1] = cpu_get_host_ticks();
@@ -92,21 +86,17 @@ static target_ulong h_xirr_x(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                           target_ulong opcode, target_ulong *args)
 {
-    CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), cs->cpu_index);
     target_ulong xirr = args[0];
 
-    icp_eoi(icp, xirr);
+    icp_eoi(cpu->icp, xirr);
     return H_SUCCESS;
 }
 
 static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                             target_ulong opcode, target_ulong *args)
 {
-    CPUState *cs = CPU(cpu);
-    ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), cs->cpu_index);
     uint32_t mfrr;
-    uint32_t xirr = icp_ipoll(icp, &mfrr);
+    uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
 
     args[0] = xirr;
     args[1] = mfrr;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 6883f0991ae9..59f1cba6fba5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -63,6 +63,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu,
                            Error **errp)
 {
     CPUPPCState *env = &cpu->env;
+    XICSFabric *xi = XICS_FABRIC(spapr);
 
     /* Set time-base frequency to 512 MHz */
     cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
@@ -80,7 +81,8 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu,
         }
     }
 
-    xics_cpu_setup(XICS_FABRIC(spapr), cpu);
+    cpu->icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
+    xics_cpu_setup(xi, cpu);
 
     qemu_register_reset(spapr_cpu_reset, cpu);
     spapr_cpu_reset(cpu);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5ee33b3fd315..b1626d0a6607 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1176,6 +1176,7 @@ do {                                            \
 
 typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
 typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
+typedef struct ICPState ICPState;
 
 /**
  * PowerPCCPU:
@@ -1196,6 +1197,7 @@ struct PowerPCCPU {
     uint32_t max_compat;
     uint32_t compat_pvr;
     PPCVirtualHypervisor *vhyp;
+    ICPState *icp;
 
     /* Fields related to migration compatibility hacks */
     bool pre_2_8_migration;
-- 
2.7.4






reply via email to

[Prev in Thread] Current Thread [Next in Thread]