[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 1/8] ppc/xics: introduce an ICPState backlink u
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH v2 1/8] ppc/xics: introduce an ICPState backlink under PowerPCCPU |
Date: |
Wed, 22 Mar 2017 17:33:49 +1100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Thu, Mar 16, 2017 at 03:35:05PM +0100, Cédric Le Goater wrote:
> Today, the ICPState array of the sPAPR machine is indexed with
> 'cpu_index' of the CPUState. This numbering of CPUs is internal to
> QEMU and the guest only knows about what is exposed in the device
> tree, that is the 'cpu_dt_id'. This is why sPAPR uses the helper
> xics_get_cpu_index_by_dt_id() to do the mapping in a couple of places.
>
> To provide a more generic XICS layer, we need to abstract the IRQ
> 'server' number and remove any assumption made on its nature. It
> should not be used as a 'cpu_index' for lookups like xics_cpu_setup()
> and xics_cpu_destroy() do.
>
> To reach that goal, we choose to introduce an ICPState backlink under
> PowerPCCPU, and let the machine core init routine do the ICPState
> lookup. The resulting object is stored under PowerPCCPU which is
> passed on to xics_cpu_setup(). The IRQ 'server' number in XICS is now
> generic. sPAPR uses 'cpu_dt_id' and PowerNV will use 'PIR' number.
>
> This also has the benefit of simplifying the sPAPR hcall routines
> which do not need to do any ICPState lookups anymore.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
Having a direct link from the cpu to the interrupt state is a good
idea. However, I'm not so fond of having a field that's specific to a
particular platforms intc in the CPU. I'd suggest making it instead
an Object *. We can use it for ICP now, but other platforms can use
it for pointers to per-cpu interrupt state if they need to.
> ---
> 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;
--
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
signature.asc
Description: PGP signature
- [Qemu-ppc] [PATCH v2 0/8] ppc/pnv: interrupt controller (POWER8), Cédric Le Goater, 2017/03/16
- [Qemu-ppc] [PATCH v2 1/8] ppc/xics: introduce an ICPState backlink under PowerPCCPU, Cédric Le Goater, 2017/03/16
- Re: [Qemu-ppc] [PATCH v2 1/8] ppc/xics: introduce an ICPState backlink under PowerPCCPU,
David Gibson <=
- [Qemu-ppc] [PATCH v2 2/8] spapr: move the IRQ server number mapping under the machine, Cédric Le Goater, 2017/03/16
- [Qemu-ppc] [PATCH v2 3/8] ppc/xics: add a realize() handler to ICPStateClass, Cédric Le Goater, 2017/03/16
- [Qemu-ppc] [PATCH v2 4/8] ppc/pnv: add a PnvICPState object, Cédric Le Goater, 2017/03/16
- [Qemu-ppc] [PATCH v2 6/8] ppc/pnv: add a helper to calculate MMIO addresses registers, Cédric Le Goater, 2017/03/16
- [Qemu-ppc] [PATCH v2 5/8] ppc/pnv: create the ICP and ICS objects under the machine, Cédric Le Goater, 2017/03/16