* [Qemu-devel] [PATCH v4 1/9] ppc/xics: introduce an 'intc' backlink under PowerPCCPU
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-30 6:46 ` David Gibson
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine Cédric Le Goater
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
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 a generic 'intc' backlink
under PowerPCCPU, and let the machine core init routine do the
ICPState lookup. The resulting object is passed on to xics_cpu_setup()
which does the store under PowerPCCPU. 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 <clg@kaod.org>
---
Changes since v3:
- renamed 'icp' backlink to a more generic name 'intc'
Changes since v2:
- changed the 'icp' backlink type to be an 'Object'
hw/intc/xics.c | 6 +++---
hw/intc/xics_spapr.c | 20 +++++---------------
hw/ppc/spapr_cpu_core.c | 4 +++-
include/hw/ppc/xics.h | 2 +-
target/ppc/cpu.h | 1 +
5 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index e740989a1162..56fe70cd10e9 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 = ICP(cpu->intc);
assert(icp);
assert(cs == icp->cs);
@@ -61,15 +61,15 @@ void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
icp->cs = NULL;
}
-void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu)
+void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp)
{
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
- ICPState *icp = xics_icp_get(xi, cs->cpu_index);
ICPStateClass *icpc;
assert(icp);
+ cpu->intc = OBJECT(icp);
icp->cs = cs;
icpc = ICP_GET_CLASS(icp);
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 84d24b2837a7..58f100d379cb 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(ICP(cpu->intc), 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(ICP(cpu->intc));
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(ICP(cpu->intc));
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(ICP(cpu->intc), 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(ICP(cpu->intc), &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..7db61bd72476 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -63,6 +63,8 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
Error **errp)
{
CPUPPCState *env = &cpu->env;
+ XICSFabric *xi = XICS_FABRIC(spapr);
+ ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
/* Set time-base frequency to 512 MHz */
cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
@@ -80,7 +82,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
}
}
- xics_cpu_setup(XICS_FABRIC(spapr), cpu);
+ xics_cpu_setup(xi, cpu, icp);
qemu_register_reset(spapr_cpu_reset, cpu);
spapr_cpu_reset(cpu);
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 9a5e715fe553..5e0244447fcd 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -168,7 +168,7 @@ void spapr_dt_xics(int nr_servers, void *fdt, uint32_t phandle);
qemu_irq xics_get_qirq(XICSFabric *xi, int irq);
ICPState *xics_icp_get(XICSFabric *xi, int server);
-void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu);
+void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
/* Internal XICS interfaces */
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5ee33b3fd315..b5f93272b839 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1196,6 +1196,7 @@ struct PowerPCCPU {
uint32_t max_compat;
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
+ Object *intc;
/* Fields related to migration compatibility hacks */
bool pre_2_8_migration;
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/9] ppc/xics: introduce an 'intc' backlink under PowerPCCPU
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 1/9] ppc/xics: introduce an 'intc' backlink under PowerPCCPU Cédric Le Goater
@ 2017-03-30 6:46 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-03-30 6:46 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 7311 bytes --]
On Wed, Mar 29, 2017 at 03:53:23PM +0200, 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 a generic 'intc' backlink
> under PowerPCCPU, and let the machine core init routine do the
> ICPState lookup. The resulting object is passed on to xics_cpu_setup()
> which does the store under PowerPCCPU. 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 <clg@kaod.org>
This one's ready to go, I've merged to ppc-for-2.10.
> ---
>
> Changes since v3:
> - renamed 'icp' backlink to a more generic name 'intc'
>
> Changes since v2:
> - changed the 'icp' backlink type to be an 'Object'
>
> hw/intc/xics.c | 6 +++---
> hw/intc/xics_spapr.c | 20 +++++---------------
> hw/ppc/spapr_cpu_core.c | 4 +++-
> include/hw/ppc/xics.h | 2 +-
> target/ppc/cpu.h | 1 +
> 5 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index e740989a1162..56fe70cd10e9 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 = ICP(cpu->intc);
>
> assert(icp);
> assert(cs == icp->cs);
> @@ -61,15 +61,15 @@ void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
> icp->cs = NULL;
> }
>
> -void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu)
> +void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> - ICPState *icp = xics_icp_get(xi, cs->cpu_index);
> ICPStateClass *icpc;
>
> assert(icp);
>
> + cpu->intc = OBJECT(icp);
> icp->cs = cs;
>
> icpc = ICP_GET_CLASS(icp);
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 84d24b2837a7..58f100d379cb 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(ICP(cpu->intc), 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(ICP(cpu->intc));
>
> 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(ICP(cpu->intc));
>
> 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(ICP(cpu->intc), 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(ICP(cpu->intc), &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..7db61bd72476 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -63,6 +63,8 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> Error **errp)
> {
> CPUPPCState *env = &cpu->env;
> + XICSFabric *xi = XICS_FABRIC(spapr);
> + ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
>
> /* Set time-base frequency to 512 MHz */
> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
> @@ -80,7 +82,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> }
> }
>
> - xics_cpu_setup(XICS_FABRIC(spapr), cpu);
> + xics_cpu_setup(xi, cpu, icp);
>
> qemu_register_reset(spapr_cpu_reset, cpu);
> spapr_cpu_reset(cpu);
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 9a5e715fe553..5e0244447fcd 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -168,7 +168,7 @@ void spapr_dt_xics(int nr_servers, void *fdt, uint32_t phandle);
>
> qemu_irq xics_get_qirq(XICSFabric *xi, int irq);
> ICPState *xics_icp_get(XICSFabric *xi, int server);
> -void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu);
> +void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
>
> /* Internal XICS interfaces */
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 5ee33b3fd315..b5f93272b839 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1196,6 +1196,7 @@ struct PowerPCCPU {
> uint32_t max_compat;
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> + Object *intc;
>
> /* 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 1/9] ppc/xics: introduce an 'intc' backlink under PowerPCCPU Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-30 6:46 ` David Gibson
2017-03-30 15:08 ` [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 3/9] ppc/xics: add a realize() handler to ICPStateClass Cédric Le Goater
` (6 subsequent siblings)
8 siblings, 2 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
This is the second step to abstract the IRQ 'server' number of the
XICS layer. Now that the prereq cleanups have been done in the
previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
mapping in the sPAPR machine handler.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/intc/xics_spapr.c | 5 ++---
hw/ppc/spapr.c | 3 ++-
hw/ppc/spapr_cpu_core.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index 58f100d379cb..f05308b897f2 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
- target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
target_ulong mfrr = args[1];
- ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
+ ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
if (!icp) {
return H_PARAMETER;
@@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
}
nr = rtas_ld(args, 0);
- server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
+ server = rtas_ld(args, 1);
priority = rtas_ld(args, 2);
if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8aecea3dd10c..b9f7f8607869 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
ics_resend(spapr->ics);
}
-static ICPState *spapr_icp_get(XICSFabric *xi, int server)
+static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
+ int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
}
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 7db61bd72476..4e1a99591d19 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -64,7 +64,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
{
CPUPPCState *env = &cpu->env;
XICSFabric *xi = XICS_FABRIC(spapr);
- ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
+ ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
/* Set time-base frequency to 512 MHz */
cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine Cédric Le Goater
@ 2017-03-30 6:46 ` David Gibson
2017-03-30 13:04 ` Cédric Le Goater
2017-03-30 15:08 ` [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore Cédric Le Goater
1 sibling, 1 reply; 22+ messages in thread
From: David Gibson @ 2017-03-30 6:46 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3366 bytes --]
On Wed, Mar 29, 2017 at 03:53:24PM +0200, Cédric Le Goater wrote:
> This is the second step to abstract the IRQ 'server' number of the
> XICS layer. Now that the prereq cleanups have been done in the
> previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
> mapping in the sPAPR machine handler.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/intc/xics_spapr.c | 5 ++---
> hw/ppc/spapr.c | 3 ++-
> hw/ppc/spapr_cpu_core.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 58f100d379cb..f05308b897f2 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> target_ulong opcode, target_ulong *args)
> {
> - target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
> target_ulong mfrr = args[1];
> - ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
> + ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
>
> if (!icp) {
> return H_PARAMETER;
> @@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> }
>
> nr = rtas_ld(args, 0);
> - server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
> + server = rtas_ld(args, 1);
> priority = rtas_ld(args, 2);
>
> if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8aecea3dd10c..b9f7f8607869 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
> ics_resend(spapr->ics);
> }
>
> -static ICPState *spapr_icp_get(XICSFabric *xi, int server)
> +static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> + int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
The idea is good, but this is a bad name (as it was in the original
version, too). The whole point here is that the XICS server number
(as it appears in the ICS registers) is the input to this function,
and we no longer assume that is equal to cpu_index.
Seems we could just get the cpu object by dt_id here, then go to ICP(cpu->intc).
> return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
> }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 7db61bd72476..4e1a99591d19 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -64,7 +64,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> {
> CPUPPCState *env = &cpu->env;
> XICSFabric *xi = XICS_FABRIC(spapr);
> - ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
> + ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>
> /* Set time-base frequency to 512 MHz */
> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine
2017-03-30 6:46 ` David Gibson
@ 2017-03-30 13:04 ` Cédric Le Goater
2017-03-30 15:04 ` Cedric Le Goater
2017-04-02 6:48 ` David Gibson
0 siblings, 2 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-30 13:04 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 03/30/2017 08:46 AM, David Gibson wrote:
> On Wed, Mar 29, 2017 at 03:53:24PM +0200, Cédric Le Goater wrote:
>> This is the second step to abstract the IRQ 'server' number of the
>> XICS layer. Now that the prereq cleanups have been done in the
>> previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
>> mapping in the sPAPR machine handler.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>> hw/intc/xics_spapr.c | 5 ++---
>> hw/ppc/spapr.c | 3 ++-
>> hw/ppc/spapr_cpu_core.c | 2 +-
>> 3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>> index 58f100d379cb..f05308b897f2 100644
>> --- a/hw/intc/xics_spapr.c
>> +++ b/hw/intc/xics_spapr.c
>> @@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>> static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>> target_ulong opcode, target_ulong *args)
>> {
>> - target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
>> target_ulong mfrr = args[1];
>> - ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
>> + ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
>>
>> if (!icp) {
>> return H_PARAMETER;
>> @@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>> }
>>
>> nr = rtas_ld(args, 0);
>> - server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
>> + server = rtas_ld(args, 1);
>> priority = rtas_ld(args, 2);
>>
>> if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 8aecea3dd10c..b9f7f8607869 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
>> ics_resend(spapr->ics);
>> }
>>
>> -static ICPState *spapr_icp_get(XICSFabric *xi, int server)
>> +static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>> + int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
>
> The idea is good, but this is a bad name (as it was in the original
> version, too). The whole point here is that the XICS server number
> (as it appears in the ICS registers) is the input to this function,
> and we no longer assume that is equal to cpu_index.
>
> Seems we could just get the cpu object by dt_id here, then go to ICP(cpu->intc).
yes that would be nice and this is exactly what pnv does now, but
this is only possible because the PnvICPState objects are allocated
from under PnvCore. This is not the case for sPAPR yet.
Currently, when the sPAPR core are realized, we call spapr_cpu_init()
which first gets its ICP with :
xics_icp_get(xi, cpu->cpu_dt_id);
so we cannot use ICP(cpu->intc) in this XICSFabric handler, it is not
assigned yet. It only will be at the end of spapr_cpu_init() when
xics_cpu_setup(xi, cpu, icp);
is called.
As suggested in an email this morning, I think we could allocate
the ICP from under the sPAPR core if we knew which ICP type to use
(KVM or not).
For that, we could use a new XICSFabric handler :
const char *icp_type(XICSFabric *xi)
or a machine attribute to get the type. The ICP type would be chosen
in xics_system_init() a bit like it is done today but we would not
create the ICP object.
or we could use a machine helper (probably better) :
ICPState *spapr_icp_create(MachineState *machine);
which would only do the ICP part of xics_system_init(). The ICS
object can be created later on, it is not a problem.
We have kind of a chicken and egg problem with the Core and the
ICP objects today that it would be nice to untangle.
Suggestions ?
C.
>
>> return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
>> }
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 7db61bd72476..4e1a99591d19 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -64,7 +64,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>> {
>> CPUPPCState *env = &cpu->env;
>> XICSFabric *xi = XICS_FABRIC(spapr);
>> - ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
>> + ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>>
>> /* Set time-base frequency to 512 MHz */
>> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine
2017-03-30 13:04 ` Cédric Le Goater
@ 2017-03-30 15:04 ` Cedric Le Goater
2017-04-02 6:48 ` David Gibson
1 sibling, 0 replies; 22+ messages in thread
From: Cedric Le Goater @ 2017-03-30 15:04 UTC (permalink / raw)
To: Cédric Le Goater, David Gibson; +Cc: qemu-ppc, qemu-devel
On 03/30/2017 03:04 PM, Cédric Le Goater wrote:
> On 03/30/2017 08:46 AM, David Gibson wrote:
>> On Wed, Mar 29, 2017 at 03:53:24PM +0200, Cédric Le Goater wrote:
>>> This is the second step to abstract the IRQ 'server' number of the
>>> XICS layer. Now that the prereq cleanups have been done in the
>>> previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
>>> mapping in the sPAPR machine handler.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>> hw/intc/xics_spapr.c | 5 ++---
>>> hw/ppc/spapr.c | 3 ++-
>>> hw/ppc/spapr_cpu_core.c | 2 +-
>>> 3 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>>> index 58f100d379cb..f05308b897f2 100644
>>> --- a/hw/intc/xics_spapr.c
>>> +++ b/hw/intc/xics_spapr.c
>>> @@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>> static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>> target_ulong opcode, target_ulong *args)
>>> {
>>> - target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
>>> target_ulong mfrr = args[1];
>>> - ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
>>> + ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
>>>
>>> if (!icp) {
>>> return H_PARAMETER;
>>> @@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>> }
>>>
>>> nr = rtas_ld(args, 0);
>>> - server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
>>> + server = rtas_ld(args, 1);
>>> priority = rtas_ld(args, 2);
>>>
>>> if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 8aecea3dd10c..b9f7f8607869 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
>>> ics_resend(spapr->ics);
>>> }
>>>
>>> -static ICPState *spapr_icp_get(XICSFabric *xi, int server)
>>> +static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
>>> {
>>> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>>> + int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
>>
>> The idea is good, but this is a bad name (as it was in the original
>> version, too). The whole point here is that the XICS server number
>> (as it appears in the ICS registers) is the input to this function,
>> and we no longer assume that is equal to cpu_index.
>>
>> Seems we could just get the cpu object by dt_id here, then go to ICP(cpu->intc).
>
> yes that would be nice and this is exactly what pnv does now, but
> this is only possible because the PnvICPState objects are allocated
> from under PnvCore. This is not the case for sPAPR yet.
>
> Currently, when the sPAPR core are realized, we call spapr_cpu_init()
> which first gets its ICP with :
>
> xics_icp_get(xi, cpu->cpu_dt_id);
>
> so we cannot use ICP(cpu->intc) in this XICSFabric handler, it is not
> assigned yet. It only will be at the end of spapr_cpu_init() when
>
> xics_cpu_setup(xi, cpu, icp);
>
> is called.
>
>
> As suggested in an email this morning, I think we could allocate
> the ICP from under the sPAPR core if we knew which ICP type to use
> (KVM or not).
>
> For that, we could use a new XICSFabric handler :
>
> const char *icp_type(XICSFabric *xi)
>
> or a machine attribute to get the type. The ICP type would be chosen
> in xics_system_init() a bit like it is done today but we would not
> create the ICP object.
>
> or we could use a machine helper (probably better) :
>
> ICPState *spapr_icp_create(MachineState *machine);
>
> which would only do the ICP part of xics_system_init(). The ICS
> object can be created later on, it is not a problem.
>
> We have kind of a chicken and egg problem with the Core and the
> ICP objects today that it would be nice to untangle.
>
> Suggestions ?
I have cooked a patch for this idea. It applies correctly in the
v4 patchset between 'PATCH v4 2/9' and 'PATCH v4 3/9'. I will
send as a follow up of 'PATCH v4 2/9'.
Thanks,
C.
>
> C.
>
>
>>
>>> return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
>>> }
>>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>>> index 7db61bd72476..4e1a99591d19 100644
>>> --- a/hw/ppc/spapr_cpu_core.c
>>> +++ b/hw/ppc/spapr_cpu_core.c
>>> @@ -64,7 +64,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>>> {
>>> CPUPPCState *env = &cpu->env;
>>> XICSFabric *xi = XICS_FABRIC(spapr);
>>> - ICPState *icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
>>> + ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>>>
>>> /* Set time-base frequency to 512 MHz */
>>> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine
2017-03-30 13:04 ` Cédric Le Goater
2017-03-30 15:04 ` Cedric Le Goater
@ 2017-04-02 6:48 ` David Gibson
1 sibling, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-04-02 6:48 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3744 bytes --]
On Thu, Mar 30, 2017 at 03:04:47PM +0200, Cédric Le Goater wrote:
> On 03/30/2017 08:46 AM, David Gibson wrote:
> > On Wed, Mar 29, 2017 at 03:53:24PM +0200, Cédric Le Goater wrote:
> >> This is the second step to abstract the IRQ 'server' number of the
> >> XICS layer. Now that the prereq cleanups have been done in the
> >> previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
> >> mapping in the sPAPR machine handler.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >> ---
> >> hw/intc/xics_spapr.c | 5 ++---
> >> hw/ppc/spapr.c | 3 ++-
> >> hw/ppc/spapr_cpu_core.c | 2 +-
> >> 3 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> >> index 58f100d379cb..f05308b897f2 100644
> >> --- a/hw/intc/xics_spapr.c
> >> +++ b/hw/intc/xics_spapr.c
> >> @@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >> static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >> target_ulong opcode, target_ulong *args)
> >> {
> >> - target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
> >> target_ulong mfrr = args[1];
> >> - ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
> >> + ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
> >>
> >> if (!icp) {
> >> return H_PARAMETER;
> >> @@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >> }
> >>
> >> nr = rtas_ld(args, 0);
> >> - server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
> >> + server = rtas_ld(args, 1);
> >> priority = rtas_ld(args, 2);
> >>
> >> if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index 8aecea3dd10c..b9f7f8607869 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
> >> ics_resend(spapr->ics);
> >> }
> >>
> >> -static ICPState *spapr_icp_get(XICSFabric *xi, int server)
> >> +static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> >> {
> >> sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> >> + int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
> >
> > The idea is good, but this is a bad name (as it was in the original
> > version, too). The whole point here is that the XICS server number
> > (as it appears in the ICS registers) is the input to this function,
> > and we no longer assume that is equal to cpu_index.
> >
> > Seems we could just get the cpu object by dt_id here, then go to ICP(cpu->intc).
>
> yes that would be nice and this is exactly what pnv does now, but
> this is only possible because the PnvICPState objects are allocated
> from under PnvCore. This is not the case for sPAPR yet.
>
> Currently, when the sPAPR core are realized, we call spapr_cpu_init()
> which first gets its ICP with :
>
> xics_icp_get(xi, cpu->cpu_dt_id);
>
> so we cannot use ICP(cpu->intc) in this XICSFabric handler, it is not
> assigned yet. It only will be at the end of spapr_cpu_init() when
>
> xics_cpu_setup(xi, cpu, icp);
>
> is called.
Uh.. but spapr_cpu_init() has the spapr pointer and can obviously get
the cpu_index. So it could look up the right ICP in the array easily
enough instead of using xics_icp_get().
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine Cédric Le Goater
2017-03-30 6:46 ` David Gibson
@ 2017-03-30 15:08 ` Cédric Le Goater
2017-03-30 15:56 ` Cédric Le Goater
2017-04-02 8:25 ` David Gibson
1 sibling, 2 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-30 15:08 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Today, all the ICPs are created before the CPUs, stored in an array
under the sPAPR machine and linked to the CPU when the core threads
are realized. This modeling brings some complexity when a lookup in
the array is required and it can be simplified by allocating the ICPs
when the CPUs are.
This is the purpose of this proposal which introduces a new 'icp_type'
field under the machine and creates the ICP objects of the right type
(KVM or not) before the PowerPCCPU object are.
This change allows more cleanups : the removal of the icps array under
the sPAPR machine and the removal xics_get_cpu_index_by_dt_id() helper.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Tested with KVM and TCG. migration is fine.
hw/intc/xics.c | 11 -----------
hw/ppc/spapr.c | 47 ++++++++++++++---------------------------------
hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
include/hw/ppc/spapr.h | 2 +-
include/hw/ppc/xics.h | 2 --
5 files changed, 29 insertions(+), 51 deletions(-)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 56fe70cd10e9..d4428b41b03a 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -38,17 +38,6 @@
#include "monitor/monitor.h"
#include "hw/intc/intc.h"
-int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
-{
- PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
-
- if (cpu) {
- return cpu->parent_obj.cpu_index;
- }
-
- return -1;
-}
-
void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
{
CPUState *cs = CPU(cpu);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b9f7f8607869..18d8fe7458c1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
XICSFabric *xi = XICS_FABRIC(spapr);
Error *err = NULL, *local_err = NULL;
ICSState *ics = NULL;
- int i;
ics = ICS_SIMPLE(object_new(type_ics));
object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
@@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
error_propagate(&err, local_err);
if (err) {
- goto error;
+ error_propagate(errp, err);
+ return -1;
}
- spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
spapr->nr_servers = nr_servers;
-
- for (i = 0; i < nr_servers; i++) {
- ICPState *icp = &spapr->icps[i];
-
- object_initialize(icp, sizeof(*icp), type_icp);
- object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
- object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
- object_property_set_bool(OBJECT(icp), true, "realized", &err);
- if (err) {
- goto error;
- }
- object_unref(OBJECT(icp));
- }
-
spapr->ics = ics;
+ spapr->icp_type = type_icp;
return 0;
-
-error:
- error_propagate(errp, err);
- if (ics) {
- object_unparent(OBJECT(ics));
- }
- return -1;
}
static int xics_system_init(MachineState *machine,
@@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int version_id)
int err = 0;
if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
- int i;
- for (i = 0; i < spapr->nr_servers; i++) {
- icp_resend(&spapr->icps[i]);
+ CPUState *cs;
+ CPU_FOREACH(cs) {
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ icp_resend(ICP(cpu->intc));
}
}
@@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
{
- sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
- int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
+ PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
- return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
+ return cpu ? ICP(cpu->intc) : NULL;
}
static void spapr_pic_print_info(InterruptStatsProvider *obj,
Monitor *mon)
{
sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
- int i;
+ CPUState *cs;
+
+ CPU_FOREACH(cs) {
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
- for (i = 0; i < spapr->nr_servers; i++) {
- icp_pic_print_info(&spapr->icps[i], mon);
+ icp_pic_print_info(ICP(cpu->intc), mon);
}
ics_pic_print_info(spapr->ics, mon);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 4e1a99591d19..c80eb7c34f9f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
Error **errp)
{
CPUPPCState *env = &cpu->env;
- XICSFabric *xi = XICS_FABRIC(spapr);
- ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
/* Set time-base frequency to 512 MHz */
cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
@@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
}
}
- xics_cpu_setup(xi, cpu, icp);
-
qemu_register_reset(spapr_cpu_reset, cpu);
spapr_cpu_reset(cpu);
}
@@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp)
sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
CPUState *cs = CPU(child);
PowerPCCPU *cpu = POWERPC_CPU(cs);
+ Object *obj;
+
+ obj = object_new(spapr->icp_type);
+ object_property_add_child(OBJECT(spapr), "icp[*]", obj, NULL);
+ object_property_add_const_link(obj, "xics", OBJECT(spapr), NULL);
+ object_property_set_bool(obj, true, "realized", &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
object_property_set_bool(child, true, "realized", &local_err);
if (local_err) {
+ object_unparent(obj);
error_propagate(errp, local_err);
return;
}
spapr_cpu_init(spapr, cpu, &local_err);
if (local_err) {
+ object_unparent(obj);
error_propagate(errp, local_err);
return;
}
+
+ xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
}
static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 808aac870359..db3d4acb18a6 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -96,7 +96,7 @@ struct sPAPRMachineState {
MemoryHotplugState hotplug_memory;
uint32_t nr_servers;
- ICPState *icps;
+ const char *icp_type;
};
#define H_SUCCESS 0
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 5e0244447fcd..88e0f021b168 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
/* Internal XICS interfaces */
-int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
-
void icp_set_cppr(ICPState *icp, uint8_t cppr);
void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
uint32_t icp_accept(ICPState *ss);
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore
2017-03-30 15:08 ` [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore Cédric Le Goater
@ 2017-03-30 15:56 ` Cédric Le Goater
2017-04-02 8:25 ` David Gibson
1 sibling, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-30 15:56 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 03/30/2017 05:08 PM, Cédric Le Goater wrote:
> Today, all the ICPs are created before the CPUs, stored in an array
> under the sPAPR machine and linked to the CPU when the core threads
> are realized. This modeling brings some complexity when a lookup in
> the array is required and it can be simplified by allocating the ICPs
> when the CPUs are.
>
> This is the purpose of this proposal which introduces a new 'icp_type'
> field under the machine and creates the ICP objects of the right type
> (KVM or not) before the PowerPCCPU object are.
>
> This change allows more cleanups : the removal of the icps array under
> the sPAPR machine and the removal xics_get_cpu_index_by_dt_id() helper.
I forgot : ^
of the
and some more below.
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Tested with KVM and TCG. migration is fine.
>
> hw/intc/xics.c | 11 -----------
> hw/ppc/spapr.c | 47 ++++++++++++++---------------------------------
> hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
> include/hw/ppc/spapr.h | 2 +-
> include/hw/ppc/xics.h | 2 --
> 5 files changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 56fe70cd10e9..d4428b41b03a 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -38,17 +38,6 @@
> #include "monitor/monitor.h"
> #include "hw/intc/intc.h"
>
> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
> -{
> - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> -
> - if (cpu) {
> - return cpu->parent_obj.cpu_index;
> - }
> -
> - return -1;
> -}
> -
> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b9f7f8607869..18d8fe7458c1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
> XICSFabric *xi = XICS_FABRIC(spapr);
> Error *err = NULL, *local_err = NULL;
> ICSState *ics = NULL;
> - int i;
>
> ics = ICS_SIMPLE(object_new(type_ics));
> object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
> @@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
> object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
> error_propagate(&err, local_err);
> if (err) {
> - goto error;
> + error_propagate(errp, err);
> + return -1;
> }
>
> - spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
> spapr->nr_servers = nr_servers;
> -
> - for (i = 0; i < nr_servers; i++) {
> - ICPState *icp = &spapr->icps[i];
> -
> - object_initialize(icp, sizeof(*icp), type_icp);
> - object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
> - object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
> - object_property_set_bool(OBJECT(icp), true, "realized", &err);
> - if (err) {
> - goto error;
> - }
> - object_unref(OBJECT(icp));
> - }
> -
> spapr->ics = ics;
> + spapr->icp_type = type_icp;
> return 0;
> -
> -error:
> - error_propagate(errp, err);
> - if (ics) {
> - object_unparent(OBJECT(ics));
> - }
> - return -1;
> }
>
> static int xics_system_init(MachineState *machine,
> @@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int version_id)
> int err = 0;
>
> if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
> - int i;
> - for (i = 0; i < spapr->nr_servers; i++) {
> - icp_resend(&spapr->icps[i]);
> + CPUState *cs;
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> + icp_resend(ICP(cpu->intc));
> }
> }
>
> @@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
>
> static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> {
> - sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> - int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
> + PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>
> - return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
> + return cpu ? ICP(cpu->intc) : NULL;
> }
>
> static void spapr_pic_print_info(InterruptStatsProvider *obj,
> Monitor *mon)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> - int i;
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> - for (i = 0; i < spapr->nr_servers; i++) {
> - icp_pic_print_info(&spapr->icps[i], mon);
> + icp_pic_print_info(ICP(cpu->intc), mon);
> }
>
> ics_pic_print_info(spapr->ics, mon);
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4e1a99591d19..c80eb7c34f9f 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> Error **errp)
> {
> CPUPPCState *env = &cpu->env;
> - XICSFabric *xi = XICS_FABRIC(spapr);
> - ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>
> /* Set time-base frequency to 512 MHz */
> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
> @@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> }
> }
>
> - xics_cpu_setup(xi, cpu, icp);
> -
> qemu_register_reset(spapr_cpu_reset, cpu);
> spapr_cpu_reset(cpu);
> }
> @@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> CPUState *cs = CPU(child);
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> + Object *obj;
> +
> + obj = object_new(spapr->icp_type);
> + object_property_add_child(OBJECT(spapr), "icp[*]", obj, NULL);
> + object_property_add_const_link(obj, "xics", OBJECT(spapr), NULL);
That's wrong. It should be :
object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
object_property_add_const_link(obj, "xics", OBJECT(spapr), &error_abort);
but you have the idea.
Thanks,
C.
> + object_property_set_bool(obj, true, "realized", &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
>
> object_property_set_bool(child, true, "realized", &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
>
> spapr_cpu_init(spapr, cpu, &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
> +
> + xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
> }
>
> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 808aac870359..db3d4acb18a6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -96,7 +96,7 @@ struct sPAPRMachineState {
> MemoryHotplugState hotplug_memory;
>
> uint32_t nr_servers;
> - ICPState *icps;
> + const char *icp_type;
> };
>
> #define H_SUCCESS 0
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 5e0244447fcd..88e0f021b168 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
>
> /* Internal XICS interfaces */
> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
> -
> void icp_set_cppr(ICPState *icp, uint8_t cppr);
> void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
> uint32_t icp_accept(ICPState *ss);
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore
2017-03-30 15:08 ` [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore Cédric Le Goater
2017-03-30 15:56 ` Cédric Le Goater
@ 2017-04-02 8:25 ` David Gibson
2017-04-02 17:16 ` Cédric Le Goater
1 sibling, 1 reply; 22+ messages in thread
From: David Gibson @ 2017-04-02 8:25 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 8203 bytes --]
On Thu, Mar 30, 2017 at 05:08:04PM +0200, Cédric Le Goater wrote:
> Today, all the ICPs are created before the CPUs, stored in an array
> under the sPAPR machine and linked to the CPU when the core threads
> are realized. This modeling brings some complexity when a lookup in
> the array is required and it can be simplified by allocating the ICPs
> when the CPUs are.
>
> This is the purpose of this proposal which introduces a new 'icp_type'
> field under the machine and creates the ICP objects of the right type
> (KVM or not) before the PowerPCCPU object are.
>
> This change allows more cleanups : the removal of the icps array under
> the sPAPR machine and the removal xics_get_cpu_index_by_dt_id() helper.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Tested with KVM and TCG. migration is fine.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
But please resend with the whole series, I've lost track of which
versions are which.
>
> hw/intc/xics.c | 11 -----------
> hw/ppc/spapr.c | 47 ++++++++++++++---------------------------------
> hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
> include/hw/ppc/spapr.h | 2 +-
> include/hw/ppc/xics.h | 2 --
> 5 files changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 56fe70cd10e9..d4428b41b03a 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -38,17 +38,6 @@
> #include "monitor/monitor.h"
> #include "hw/intc/intc.h"
>
> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
> -{
> - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> -
> - if (cpu) {
> - return cpu->parent_obj.cpu_index;
> - }
> -
> - return -1;
> -}
> -
> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b9f7f8607869..18d8fe7458c1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
> XICSFabric *xi = XICS_FABRIC(spapr);
> Error *err = NULL, *local_err = NULL;
> ICSState *ics = NULL;
> - int i;
>
> ics = ICS_SIMPLE(object_new(type_ics));
> object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
> @@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
> object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
> error_propagate(&err, local_err);
> if (err) {
> - goto error;
> + error_propagate(errp, err);
> + return -1;
> }
>
> - spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
> spapr->nr_servers = nr_servers;
> -
> - for (i = 0; i < nr_servers; i++) {
> - ICPState *icp = &spapr->icps[i];
> -
> - object_initialize(icp, sizeof(*icp), type_icp);
> - object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
> - object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
> - object_property_set_bool(OBJECT(icp), true, "realized", &err);
> - if (err) {
> - goto error;
> - }
> - object_unref(OBJECT(icp));
> - }
> -
> spapr->ics = ics;
> + spapr->icp_type = type_icp;
> return 0;
> -
> -error:
> - error_propagate(errp, err);
> - if (ics) {
> - object_unparent(OBJECT(ics));
> - }
> - return -1;
> }
>
> static int xics_system_init(MachineState *machine,
> @@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int version_id)
> int err = 0;
>
> if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
> - int i;
> - for (i = 0; i < spapr->nr_servers; i++) {
> - icp_resend(&spapr->icps[i]);
> + CPUState *cs;
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> + icp_resend(ICP(cpu->intc));
> }
> }
>
> @@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
>
> static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
> {
> - sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> - int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
> + PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>
> - return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
> + return cpu ? ICP(cpu->intc) : NULL;
> }
>
> static void spapr_pic_print_info(InterruptStatsProvider *obj,
> Monitor *mon)
> {
> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> - int i;
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> - for (i = 0; i < spapr->nr_servers; i++) {
> - icp_pic_print_info(&spapr->icps[i], mon);
> + icp_pic_print_info(ICP(cpu->intc), mon);
> }
>
> ics_pic_print_info(spapr->ics, mon);
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4e1a99591d19..c80eb7c34f9f 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> Error **errp)
> {
> CPUPPCState *env = &cpu->env;
> - XICSFabric *xi = XICS_FABRIC(spapr);
> - ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>
> /* Set time-base frequency to 512 MHz */
> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
> @@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> }
> }
>
> - xics_cpu_setup(xi, cpu, icp);
> -
> qemu_register_reset(spapr_cpu_reset, cpu);
> spapr_cpu_reset(cpu);
> }
> @@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> CPUState *cs = CPU(child);
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> + Object *obj;
> +
> + obj = object_new(spapr->icp_type);
> + object_property_add_child(OBJECT(spapr), "icp[*]", obj, NULL);
> + object_property_add_const_link(obj, "xics", OBJECT(spapr), NULL);
> + object_property_set_bool(obj, true, "realized", &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
>
> object_property_set_bool(child, true, "realized", &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
>
> spapr_cpu_init(spapr, cpu, &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
> +
> + xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
> }
>
> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 808aac870359..db3d4acb18a6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -96,7 +96,7 @@ struct sPAPRMachineState {
> MemoryHotplugState hotplug_memory;
>
> uint32_t nr_servers;
> - ICPState *icps;
> + const char *icp_type;
> };
>
> #define H_SUCCESS 0
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 5e0244447fcd..88e0f021b168 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
>
> /* Internal XICS interfaces */
> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
> -
> void icp_set_cppr(ICPState *icp, uint8_t cppr);
> void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
> uint32_t icp_accept(ICPState *ss);
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 2+/9] spapr: allocate the ICPState object from under sPAPRCPUCore
2017-04-02 8:25 ` David Gibson
@ 2017-04-02 17:16 ` Cédric Le Goater
0 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-04-02 17:16 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 04/02/2017 10:25 AM, David Gibson wrote:
> On Thu, Mar 30, 2017 at 05:08:04PM +0200, Cédric Le Goater wrote:
>> Today, all the ICPs are created before the CPUs, stored in an array
>> under the sPAPR machine and linked to the CPU when the core threads
>> are realized. This modeling brings some complexity when a lookup in
>> the array is required and it can be simplified by allocating the ICPs
>> when the CPUs are.
>>
>> This is the purpose of this proposal which introduces a new 'icp_type'
>> field under the machine and creates the ICP objects of the right type
>> (KVM or not) before the PowerPCCPU object are.
>>
>> This change allows more cleanups : the removal of the icps array under
>> the sPAPR machine and the removal xics_get_cpu_index_by_dt_id() helper.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> Tested with KVM and TCG. migration is fine.
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> But please resend with the whole series, I've lost track of which
> versions are which.
sure. I was sending material to clarify what I was saying. I will
resend tomorrow.
Thanks,
C.
>>
>> hw/intc/xics.c | 11 -----------
>> hw/ppc/spapr.c | 47 ++++++++++++++---------------------------------
>> hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
>> include/hw/ppc/spapr.h | 2 +-
>> include/hw/ppc/xics.h | 2 --
>> 5 files changed, 29 insertions(+), 51 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 56fe70cd10e9..d4428b41b03a 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -38,17 +38,6 @@
>> #include "monitor/monitor.h"
>> #include "hw/intc/intc.h"
>>
>> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
>> -{
>> - PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>> -
>> - if (cpu) {
>> - return cpu->parent_obj.cpu_index;
>> - }
>> -
>> - return -1;
>> -}
>> -
>> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu)
>> {
>> CPUState *cs = CPU(cpu);
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index b9f7f8607869..18d8fe7458c1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -103,7 +103,6 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>> XICSFabric *xi = XICS_FABRIC(spapr);
>> Error *err = NULL, *local_err = NULL;
>> ICSState *ics = NULL;
>> - int i;
>>
>> ics = ICS_SIMPLE(object_new(type_ics));
>> object_property_add_child(OBJECT(spapr), "ics", OBJECT(ics), NULL);
>> @@ -112,34 +111,14 @@ static int try_create_xics(sPAPRMachineState *spapr, const char *type_ics,
>> object_property_set_bool(OBJECT(ics), true, "realized", &local_err);
>> error_propagate(&err, local_err);
>> if (err) {
>> - goto error;
>> + error_propagate(errp, err);
>> + return -1;
>> }
>>
>> - spapr->icps = g_malloc0(nr_servers * sizeof(ICPState));
>> spapr->nr_servers = nr_servers;
>> -
>> - for (i = 0; i < nr_servers; i++) {
>> - ICPState *icp = &spapr->icps[i];
>> -
>> - object_initialize(icp, sizeof(*icp), type_icp);
>> - object_property_add_child(OBJECT(spapr), "icp[*]", OBJECT(icp), NULL);
>> - object_property_add_const_link(OBJECT(icp), "xics", OBJECT(xi), NULL);
>> - object_property_set_bool(OBJECT(icp), true, "realized", &err);
>> - if (err) {
>> - goto error;
>> - }
>> - object_unref(OBJECT(icp));
>> - }
>> -
>> spapr->ics = ics;
>> + spapr->icp_type = type_icp;
>> return 0;
>> -
>> -error:
>> - error_propagate(errp, err);
>> - if (ics) {
>> - object_unparent(OBJECT(ics));
>> - }
>> - return -1;
>> }
>>
>> static int xics_system_init(MachineState *machine,
>> @@ -1366,9 +1345,10 @@ static int spapr_post_load(void *opaque, int version_id)
>> int err = 0;
>>
>> if (!object_dynamic_cast(OBJECT(spapr->ics), TYPE_ICS_KVM)) {
>> - int i;
>> - for (i = 0; i < spapr->nr_servers; i++) {
>> - icp_resend(&spapr->icps[i]);
>> + CPUState *cs;
>> + CPU_FOREACH(cs) {
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + icp_resend(ICP(cpu->intc));
>> }
>> }
>>
>> @@ -3026,20 +3006,21 @@ static void spapr_ics_resend(XICSFabric *dev)
>>
>> static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
>> {
>> - sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
>> - int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
>> + PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
>>
>> - return (server < spapr->nr_servers) ? &spapr->icps[server] : NULL;
>> + return cpu ? ICP(cpu->intc) : NULL;
>> }
>>
>> static void spapr_pic_print_info(InterruptStatsProvider *obj,
>> Monitor *mon)
>> {
>> sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>> - int i;
>> + CPUState *cs;
>> +
>> + CPU_FOREACH(cs) {
>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>>
>> - for (i = 0; i < spapr->nr_servers; i++) {
>> - icp_pic_print_info(&spapr->icps[i], mon);
>> + icp_pic_print_info(ICP(cpu->intc), mon);
>> }
>>
>> ics_pic_print_info(spapr->ics, mon);
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 4e1a99591d19..c80eb7c34f9f 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -63,8 +63,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>> Error **errp)
>> {
>> CPUPPCState *env = &cpu->env;
>> - XICSFabric *xi = XICS_FABRIC(spapr);
>> - ICPState *icp = xics_icp_get(xi, cpu->cpu_dt_id);
>>
>> /* Set time-base frequency to 512 MHz */
>> cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>> @@ -82,8 +80,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>> }
>> }
>>
>> - xics_cpu_setup(xi, cpu, icp);
>> -
>> qemu_register_reset(spapr_cpu_reset, cpu);
>> spapr_cpu_reset(cpu);
>> }
>> @@ -143,18 +139,32 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp)
>> sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> CPUState *cs = CPU(child);
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> + Object *obj;
>> +
>> + obj = object_new(spapr->icp_type);
>> + object_property_add_child(OBJECT(spapr), "icp[*]", obj, NULL);
>> + object_property_add_const_link(obj, "xics", OBJECT(spapr), NULL);
>> + object_property_set_bool(obj, true, "realized", &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>>
>> object_property_set_bool(child, true, "realized", &local_err);
>> if (local_err) {
>> + object_unparent(obj);
>> error_propagate(errp, local_err);
>> return;
>> }
>>
>> spapr_cpu_init(spapr, cpu, &local_err);
>> if (local_err) {
>> + object_unparent(obj);
>> error_propagate(errp, local_err);
>> return;
>> }
>> +
>> + xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
>> }
>>
>> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 808aac870359..db3d4acb18a6 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -96,7 +96,7 @@ struct sPAPRMachineState {
>> MemoryHotplugState hotplug_memory;
>>
>> uint32_t nr_servers;
>> - ICPState *icps;
>> + const char *icp_type;
>> };
>>
>> #define H_SUCCESS 0
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 5e0244447fcd..88e0f021b168 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -172,8 +172,6 @@ void xics_cpu_setup(XICSFabric *xi, PowerPCCPU *cpu, ICPState *icp);
>> void xics_cpu_destroy(XICSFabric *xi, PowerPCCPU *cpu);
>>
>> /* Internal XICS interfaces */
>> -int xics_get_cpu_index_by_dt_id(int cpu_dt_id);
>> -
>> void icp_set_cppr(ICPState *icp, uint8_t cppr);
>> void icp_set_mfrr(ICPState *icp, uint8_t mfrr);
>> uint32_t icp_accept(ICPState *ss);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 3/9] ppc/xics: add a realize() handler to ICPStateClass
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 1/9] ppc/xics: introduce an 'intc' backlink under PowerPCCPU Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 2/9] spapr: move the IRQ server number mapping under the machine Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 4/9] ppc/pnv: add a PnvICPState object Cédric Le Goater
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
It will be used by derived classes in PowerNV for customization.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/intc/xics.c | 5 +++++
include/hw/ppc/xics.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 56fe70cd10e9..625d9f73c550 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -348,6 +348,7 @@ static void icp_reset(void *dev)
static void icp_realize(DeviceState *dev, Error **errp)
{
ICPState *icp = ICP(dev);
+ ICPStateClass *icpc = ICP_GET_CLASS(dev);
Object *obj;
Error *err = NULL;
@@ -360,6 +361,10 @@ static void icp_realize(DeviceState *dev, Error **errp)
icp->xics = XICS_FABRIC(obj);
+ if (icpc->realize) {
+ icpc->realize(dev, errp);
+ }
+
qemu_register_reset(icp_reset, dev);
}
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 5e0244447fcd..9fc91fd28f1e 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -60,6 +60,7 @@ typedef struct XICSFabric XICSFabric;
struct ICPStateClass {
DeviceClass parent_class;
+ void (*realize)(DeviceState *dev, Error **errp);
void (*pre_save)(ICPState *s);
int (*post_load)(ICPState *s, int version_id);
void (*cpu_setup)(ICPState *icp, PowerPCCPU *cpu);
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4 4/9] ppc/pnv: add a PnvICPState object
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (2 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 3/9] ppc/xics: add a realize() handler to ICPStateClass Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 5/9] ppc/pnv: create the ICP object under PnvCore Cédric Le Goater
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
This provides a new ICPState object for the PowerNV machine (POWER8).
Access to the Interrupt Management area is done though a memory
region. It contains the registers of the Interrupt Control Presenters
of each thread which are used to accept, return, forward interrupts in
the system.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
Changes since v1:
- moved the memory region from PnvCore to a specific PnvICPState object
hw/intc/Makefile.objs | 1 +
hw/intc/xics_pnv.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/xics.h | 12 ++++
3 files changed, 193 insertions(+)
create mode 100644 hw/intc/xics_pnv.c
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index adedd0da5fd8..78426a7dafcd 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -35,6 +35,7 @@ obj-$(CONFIG_SH4) += sh_intc.o
obj-$(CONFIG_XICS) += xics.o
obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
obj-$(CONFIG_XICS_KVM) += xics_kvm.o
+obj-$(CONFIG_POWERNV) += xics_pnv.o
obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
obj-$(CONFIG_S390_FLIC) += s390_flic.o
obj-$(CONFIG_S390_FLIC_KVM) += s390_flic_kvm.o
diff --git a/hw/intc/xics_pnv.c b/hw/intc/xics_pnv.c
new file mode 100644
index 000000000000..b1f300ca6d2e
--- /dev/null
+++ b/hw/intc/xics_pnv.c
@@ -0,0 +1,180 @@
+/*
+ * QEMU PowerPC PowerNV Interrupt Control Presenter (ICP) model
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "hw/ppc/xics.h"
+
+static uint64_t pnv_icp_read(void *opaque, hwaddr addr, unsigned width)
+{
+ ICPState *icp = ICP(opaque);
+ PnvICPState *picp = PNV_ICP(opaque);
+ bool byte0 = (width == 1 && (addr & 0x3) == 0);
+ uint64_t val = 0xffffffff;
+
+ switch (addr & 0xffc) {
+ case 0: /* poll */
+ val = icp_ipoll(icp, NULL);
+ if (byte0) {
+ val >>= 24;
+ } else if (width != 4) {
+ goto bad_access;
+ }
+ break;
+ case 4: /* xirr */
+ if (byte0) {
+ val = icp_ipoll(icp, NULL) >> 24;
+ } else if (width == 4) {
+ val = icp_accept(icp);
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 12:
+ if (byte0) {
+ val = icp->mfrr;
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 16:
+ if (width == 4) {
+ val = picp->links[0];
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 20:
+ if (width == 4) {
+ val = picp->links[1];
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 24:
+ if (width == 4) {
+ val = picp->links[2];
+ } else {
+ goto bad_access;
+ }
+ break;
+ default:
+bad_access:
+ qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
+ HWADDR_PRIx"/%d\n", addr, width);
+ }
+
+ return val;
+}
+
+static void pnv_icp_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned width)
+{
+ ICPState *icp = ICP(opaque);
+ PnvICPState *picp = PNV_ICP(opaque);
+ bool byte0 = (width == 1 && (addr & 0x3) == 0);
+
+ switch (addr & 0xffc) {
+ case 4: /* xirr */
+ if (byte0) {
+ icp_set_cppr(icp, val);
+ } else if (width == 4) {
+ icp_eoi(icp, val);
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 12:
+ if (byte0) {
+ icp_set_mfrr(icp, val);
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 16:
+ if (width == 4) {
+ picp->links[0] = val;
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 20:
+ if (width == 4) {
+ picp->links[1] = val;
+ } else {
+ goto bad_access;
+ }
+ break;
+ case 24:
+ if (width == 4) {
+ picp->links[2] = val;
+ } else {
+ goto bad_access;
+ }
+ break;
+ default:
+bad_access:
+ qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
+ HWADDR_PRIx"/%d\n", addr, width);
+ }
+}
+
+static const MemoryRegionOps pnv_icp_ops = {
+ .read = pnv_icp_read,
+ .write = pnv_icp_write,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 4,
+ .impl.min_access_size = 1,
+ .impl.max_access_size = 4,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void pnv_icp_realize(DeviceState *dev, Error **errp)
+{
+ PnvICPState *icp = PNV_ICP(dev);
+
+ memory_region_init_io(&icp->mmio, OBJECT(dev), &pnv_icp_ops,
+ icp, "icp-thread", 0x1000);
+}
+
+static void pnv_icp_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ ICPStateClass *icpc = ICP_CLASS(klass);
+
+ icpc->realize = pnv_icp_realize;
+ dc->desc = "PowerNV ICP";
+}
+
+static const TypeInfo pnv_icp_info = {
+ .name = TYPE_PNV_ICP,
+ .parent = TYPE_ICP,
+ .instance_size = sizeof(PnvICPState),
+ .class_init = pnv_icp_class_init,
+ .class_size = sizeof(ICPStateClass),
+};
+
+static void pnv_icp_register_types(void)
+{
+ type_register_static(&pnv_icp_info);
+}
+
+type_init(pnv_icp_register_types)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 9fc91fd28f1e..0e1782a3dcd6 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -41,10 +41,12 @@
*/
typedef struct ICPStateClass ICPStateClass;
typedef struct ICPState ICPState;
+typedef struct PnvICPState PnvICPState;
typedef struct ICSStateClass ICSStateClass;
typedef struct ICSState ICSState;
typedef struct ICSIRQState ICSIRQState;
typedef struct XICSFabric XICSFabric;
+typedef struct PowerPCCPU PowerPCCPU;
#define TYPE_ICP "icp"
#define ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_ICP)
@@ -52,6 +54,9 @@ typedef struct XICSFabric XICSFabric;
#define TYPE_KVM_ICP "icp-kvm"
#define KVM_ICP(obj) OBJECT_CHECK(ICPState, (obj), TYPE_KVM_ICP)
+#define TYPE_PNV_ICP "pnv-icp"
+#define PNV_ICP(obj) OBJECT_CHECK(PnvICPState, (obj), TYPE_PNV_ICP)
+
#define ICP_CLASS(klass) \
OBJECT_CLASS_CHECK(ICPStateClass, (klass), TYPE_ICP)
#define ICP_GET_CLASS(obj) \
@@ -81,6 +86,13 @@ struct ICPState {
XICSFabric *xics;
};
+struct PnvICPState {
+ ICPState parent_obj;
+
+ MemoryRegion mmio;
+ uint32_t links[3];
+};
+
#define TYPE_ICS_BASE "ics-base"
#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_BASE)
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4 5/9] ppc/pnv: create the ICP object under PnvCore
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (3 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 4/9] ppc/pnv: add a PnvICPState object Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-30 8:28 ` [Qemu-devel] [PATCH v4.1 " Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 6/9] ppc/pnv: add a helper to calculate MMIO addresses registers Cédric Le Goater
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Each thread of a core is linked to an ICP. This allocates a PnvICPState
object when the PowerPCCPU object is realized and lets the XICSFabric
do the store under the 'intc' backlink when xics_cpu_setup() is
called.
This modeling removes the need of maintaining an array of ICP objects
under the PowerNV machine and also simplifies the XICSFabric icp_get()
handler.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v3:
- removed the array of ICP objects from under the PowerNV machine and
handled the allocation of the PnvICPState object for each thread
when the PowerPCCPU object is realized.
hw/ppc/pnv.c | 2 ++
hw/ppc/pnv_core.c | 30 ++++++++++++++++++++++++++----
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3fa722af82e6..9505ca7dc09a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -691,6 +691,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(pnv_core),
pcc->core_pir(chip, core_hwid),
"pir", &error_fatal);
+ object_property_add_const_link(OBJECT(pnv_core), "xics",
+ qdev_get_machine(), &error_fatal);
object_property_set_bool(OBJECT(pnv_core), true, "realized",
&error_fatal);
object_unref(OBJECT(pnv_core));
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index d79d530b4881..87686a1b9e3b 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -25,6 +25,7 @@
#include "hw/ppc/pnv.h"
#include "hw/ppc/pnv_core.h"
#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/xics.h"
static void powernv_cpu_reset(void *opaque)
{
@@ -43,12 +44,14 @@ static void powernv_cpu_reset(void *opaque)
env->msr |= MSR_HVB; /* Hypervisor mode */
}
-static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp)
+static void powernv_cpu_init(PowerPCCPU *cpu, XICSFabric *xi, Error **errp)
{
CPUPPCState *env = &cpu->env;
int core_pir;
int thread_index = 0; /* TODO: TCG supports only one thread */
ppc_spr_t *pir = &env->spr_cb[SPR_PIR];
+ Object *obj;
+ Error *local_err = NULL;
core_pir = object_property_get_int(OBJECT(cpu), "core-pir", &error_abort);
@@ -63,6 +66,17 @@ static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp)
cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
qemu_register_reset(powernv_cpu_reset, cpu);
+
+ obj = object_new(TYPE_PNV_ICP);
+ object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
+ object_property_add_const_link(obj, "xics", OBJECT(xi), &error_abort);
+ object_property_set_bool(obj, true, "realized", &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ xics_cpu_setup(xi, cpu, ICP(obj));
}
/*
@@ -110,7 +124,7 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
-static void pnv_core_realize_child(Object *child, Error **errp)
+static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
{
Error *local_err = NULL;
CPUState *cs = CPU(child);
@@ -122,7 +136,7 @@ static void pnv_core_realize_child(Object *child, Error **errp)
return;
}
- powernv_cpu_init(cpu, &local_err);
+ powernv_cpu_init(cpu, xi, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -140,6 +154,14 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
void *obj;
int i, j;
char name[32];
+ Object *xi;
+
+ xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
+ if (!xi) {
+ error_setg(errp, "%s: required link 'xics' not found: %s",
+ __func__, error_get_pretty(local_err));
+ return;
+ }
pc->threads = g_malloc0(size * cc->nr_threads);
for (i = 0; i < cc->nr_threads; i++) {
@@ -160,7 +182,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
for (j = 0; j < cc->nr_threads; j++) {
obj = pc->threads + j * size;
- pnv_core_realize_child(obj, &local_err);
+ pnv_core_realize_child(obj, XICS_FABRIC(xi), &local_err);
if (local_err) {
goto err;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4.1 5/9] ppc/pnv: create the ICP object under PnvCore
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 5/9] ppc/pnv: create the ICP object under PnvCore Cédric Le Goater
@ 2017-03-30 8:28 ` Cédric Le Goater
2017-04-03 2:18 ` David Gibson
0 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-30 8:28 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Each thread of a core is linked to an ICP. This allocates a PnvICPState
object before the PowerPCCPU object is realized and lets the XICSFabric
do the store under the 'intc' backlink when xics_cpu_setup() is
called.
This modeling removes the need of maintaining an array of ICP objects
under the PowerNV machine and also simplifies the XICSFabric icp_get()
handler.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v4:
- moved the creation of PnvICPState object before the PowerPCCPU
object is realized to handle correctly errors.
Changes since v3:
- removed the array of ICP objects from under the PowerNV machine and
handled the allocation of the PnvICPState object for each thread
when the PowerPCCPU object is realized.
hw/ppc/pnv.c | 2 ++
hw/ppc/pnv_core.c | 27 +++++++++++++++++++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3fa722af82e6..9505ca7dc09a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -691,6 +691,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(pnv_core),
pcc->core_pir(chip, core_hwid),
"pir", &error_fatal);
+ object_property_add_const_link(OBJECT(pnv_core), "xics",
+ qdev_get_machine(), &error_fatal);
object_property_set_bool(OBJECT(pnv_core), true, "realized",
&error_fatal);
object_unref(OBJECT(pnv_core));
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index d79d530b4881..1b7ec70f033d 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -25,6 +25,7 @@
#include "hw/ppc/pnv.h"
#include "hw/ppc/pnv_core.h"
#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/xics.h"
static void powernv_cpu_reset(void *opaque)
{
@@ -110,23 +111,37 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
-static void pnv_core_realize_child(Object *child, Error **errp)
+static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
{
Error *local_err = NULL;
CPUState *cs = CPU(child);
PowerPCCPU *cpu = POWERPC_CPU(cs);
+ Object *obj;
+
+ obj = object_new(TYPE_PNV_ICP);
+ object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
+ object_property_add_const_link(obj, "xics", OBJECT(xi), &error_abort);
+ object_property_set_bool(obj, true, "realized", &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
object_property_set_bool(child, true, "realized", &local_err);
if (local_err) {
+ object_unparent(obj);
error_propagate(errp, local_err);
return;
}
powernv_cpu_init(cpu, &local_err);
if (local_err) {
+ object_unparent(obj);
error_propagate(errp, local_err);
return;
}
+
+ xics_cpu_setup(xi, cpu, ICP(obj));
}
static void pnv_core_realize(DeviceState *dev, Error **errp)
@@ -140,6 +155,14 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
void *obj;
int i, j;
char name[32];
+ Object *xi;
+
+ xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
+ if (!xi) {
+ error_setg(errp, "%s: required link 'xics' not found: %s",
+ __func__, error_get_pretty(local_err));
+ return;
+ }
pc->threads = g_malloc0(size * cc->nr_threads);
for (i = 0; i < cc->nr_threads; i++) {
@@ -160,7 +183,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
for (j = 0; j < cc->nr_threads; j++) {
obj = pc->threads + j * size;
- pnv_core_realize_child(obj, &local_err);
+ pnv_core_realize_child(obj, XICS_FABRIC(xi), &local_err);
if (local_err) {
goto err;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4.1 5/9] ppc/pnv: create the ICP object under PnvCore
2017-03-30 8:28 ` [Qemu-devel] [PATCH v4.1 " Cédric Le Goater
@ 2017-04-03 2:18 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-04-03 2:18 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4564 bytes --]
On Thu, Mar 30, 2017 at 10:28:20AM +0200, Cédric Le Goater wrote:
> Each thread of a core is linked to an ICP. This allocates a PnvICPState
> object before the PowerPCCPU object is realized and lets the XICSFabric
> do the store under the 'intc' backlink when xics_cpu_setup() is
> called.
>
> This modeling removes the need of maintaining an array of ICP objects
> under the PowerNV machine and also simplifies the XICSFabric icp_get()
> handler.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>
> Changes since v4:
>
> - moved the creation of PnvICPState object before the PowerPCCPU
> object is realized to handle correctly errors.
>
> Changes since v3:
>
> - removed the array of ICP objects from under the PowerNV machine and
> handled the allocation of the PnvICPState object for each thread
> when the PowerPCCPU object is realized.
>
> hw/ppc/pnv.c | 2 ++
> hw/ppc/pnv_core.c | 27 +++++++++++++++++++++++++--
> 2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 3fa722af82e6..9505ca7dc09a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -691,6 +691,8 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
> object_property_set_int(OBJECT(pnv_core),
> pcc->core_pir(chip, core_hwid),
> "pir", &error_fatal);
> + object_property_add_const_link(OBJECT(pnv_core), "xics",
> + qdev_get_machine(), &error_fatal);
> object_property_set_bool(OBJECT(pnv_core), true, "realized",
> &error_fatal);
> object_unref(OBJECT(pnv_core));
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index d79d530b4881..1b7ec70f033d 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -25,6 +25,7 @@
> #include "hw/ppc/pnv.h"
> #include "hw/ppc/pnv_core.h"
> #include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/xics.h"
>
> static void powernv_cpu_reset(void *opaque)
> {
> @@ -110,23 +111,37 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
> .endianness = DEVICE_BIG_ENDIAN,
> };
>
> -static void pnv_core_realize_child(Object *child, Error **errp)
> +static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error **errp)
> {
> Error *local_err = NULL;
> CPUState *cs = CPU(child);
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> + Object *obj;
> +
> + obj = object_new(TYPE_PNV_ICP);
> + object_property_add_child(OBJECT(cpu), "icp", obj, NULL);
> + object_property_add_const_link(obj, "xics", OBJECT(xi), &error_abort);
> + object_property_set_bool(obj, true, "realized", &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
>
> object_property_set_bool(child, true, "realized", &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
>
> powernv_cpu_init(cpu, &local_err);
> if (local_err) {
> + object_unparent(obj);
> error_propagate(errp, local_err);
> return;
> }
> +
> + xics_cpu_setup(xi, cpu, ICP(obj));
> }
>
> static void pnv_core_realize(DeviceState *dev, Error **errp)
> @@ -140,6 +155,14 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
> void *obj;
> int i, j;
> char name[32];
> + Object *xi;
> +
> + xi = object_property_get_link(OBJECT(dev), "xics", &local_err);
> + if (!xi) {
> + error_setg(errp, "%s: required link 'xics' not found: %s",
> + __func__, error_get_pretty(local_err));
> + return;
> + }
>
> pc->threads = g_malloc0(size * cc->nr_threads);
> for (i = 0; i < cc->nr_threads; i++) {
> @@ -160,7 +183,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
> for (j = 0; j < cc->nr_threads; j++) {
> obj = pc->threads + j * size;
>
> - pnv_core_realize_child(obj, &local_err);
> + pnv_core_realize_child(obj, XICS_FABRIC(xi), &local_err);
> if (local_err) {
> goto err;
> }
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 6/9] ppc/pnv: add a helper to calculate MMIO addresses registers
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (4 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 5/9] ppc/pnv: create the ICP object under PnvCore Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 7/9] ppc/pnv: extend the machine with a XICSFabric interface Cédric Le Goater
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Some controllers (ICP, PSI) have a base register address which is
calculated using the chip id.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
include/hw/ppc/pnv.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index df98a72006e4..5693ba181d24 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -91,14 +91,24 @@ typedef struct PnvChipClass {
OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
/*
- * This generates a HW chip id depending on an index:
+ * This generates a HW chip id depending on an index, as found on a
+ * two socket system with dual chip modules :
*
* 0x0, 0x1, 0x10, 0x11
*
* 4 chips should be the maximum
+ *
+ * TODO: use a machine property to define the chip ids
*/
#define PNV_CHIP_HWID(i) ((((i) & 0x3e) << 3) | ((i) & 0x1))
+/*
+ * Converts back a HW chip id to an index. This is useful to calculate
+ * the MMIO addresses of some controllers which depend on the chip id.
+ */
+#define PNV_CHIP_INDEX(chip) \
+ (((chip)->chip_id >> 2) * 2 + ((chip)->chip_id & 0x3))
+
#define TYPE_POWERNV_MACHINE MACHINE_TYPE_NAME("powernv")
#define POWERNV_MACHINE(obj) \
OBJECT_CHECK(PnvMachineState, (obj), TYPE_POWERNV_MACHINE)
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4 7/9] ppc/pnv: extend the machine with a XICSFabric interface
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (5 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 6/9] ppc/pnv: add a helper to calculate MMIO addresses registers Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-04-03 2:22 ` David Gibson
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 8/9] ppc/pnv: extend the machine with a InterruptStatsProvider interface Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 9/9] ppc/pnv: add memory regions for the ICP registers Cédric Le Goater
8 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
A XICSFabric QOM interface is used by the XICS layer to manipulate the
ICP and ICS objects. Let's define the associated handlers for the
PowerNV machine. All handlers should be defined even if there is no
ICS under the PowerNV machine yet.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/ppc/pnv.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 9505ca7dc09a..57560b09e04e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -34,6 +34,7 @@
#include "qemu/cutils.h"
#include "qapi/visitor.h"
+#include "hw/ppc/xics.h"
#include "hw/ppc/pnv_xscom.h"
#include "hw/isa/isa.h"
@@ -739,6 +740,39 @@ static const TypeInfo pnv_chip_info = {
.abstract = true,
};
+/* The XICS layer needs valid handlers for the ICS objects also */
+static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
+{
+ return NULL;
+}
+
+static void pnv_ics_resend(XICSFabric *xi)
+{
+}
+
+static PowerPCCPU *ppc_get_vcpu_by_pir(int pir)
+{
+ CPUState *cs;
+
+ CPU_FOREACH(cs) {
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+ CPUPPCState *env = &cpu->env;
+
+ if (env->spr_cb[SPR_PIR].default_value == pir) {
+ return cpu;
+ }
+ }
+
+ return NULL;
+}
+
+static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
+{
+ PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
+
+ return cpu ? ICP(cpu->intc) : NULL;
+}
+
static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -789,6 +823,7 @@ static void powernv_machine_class_props_init(ObjectClass *oc)
static void powernv_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+ XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
mc->desc = "IBM PowerNV (Non-Virtualized)";
mc->init = ppc_powernv_init;
@@ -799,6 +834,9 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
mc->no_parallel = 1;
mc->default_boot_order = NULL;
mc->default_ram_size = 1 * G_BYTE;
+ xic->icp_get = pnv_icp_get;
+ xic->ics_get = pnv_ics_get;
+ xic->ics_resend = pnv_ics_resend;
powernv_machine_class_props_init(oc);
}
@@ -809,6 +847,10 @@ static const TypeInfo powernv_machine_info = {
.instance_size = sizeof(PnvMachineState),
.instance_init = powernv_machine_initfn,
.class_init = powernv_machine_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_XICS_FABRIC },
+ { },
+ },
};
static void powernv_machine_register_types(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/9] ppc/pnv: extend the machine with a XICSFabric interface
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 7/9] ppc/pnv: extend the machine with a XICSFabric interface Cédric Le Goater
@ 2017-04-03 2:22 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2017-04-03 2:22 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]
On Wed, Mar 29, 2017 at 03:53:29PM +0200, Cédric Le Goater wrote:
> A XICSFabric QOM interface is used by the XICS layer to manipulate the
> ICP and ICS objects. Let's define the associated handlers for the
> PowerNV machine. All handlers should be defined even if there is no
> ICS under the PowerNV machine yet.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/pnv.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 9505ca7dc09a..57560b09e04e 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -34,6 +34,7 @@
> #include "qemu/cutils.h"
> #include "qapi/visitor.h"
>
> +#include "hw/ppc/xics.h"
> #include "hw/ppc/pnv_xscom.h"
>
> #include "hw/isa/isa.h"
> @@ -739,6 +740,39 @@ static const TypeInfo pnv_chip_info = {
> .abstract = true,
> };
>
> +/* The XICS layer needs valid handlers for the ICS objects also */
> +static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
> +{
> + return NULL;
> +}
> +
> +static void pnv_ics_resend(XICSFabric *xi)
> +{
> +}
Putting in stub implementations of the ics hooks doesn't make sense to
me. Especially since they don't get implemented in the remaining
patches. Sure, it might stop a SEGV, but the thing can't possibly
actually work without real ICS callbacks, so what's the point?
> +static PowerPCCPU *ppc_get_vcpu_by_pir(int pir)
> +{
> + CPUState *cs;
> +
> + CPU_FOREACH(cs) {
> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> + CPUPPCState *env = &cpu->env;
> +
> + if (env->spr_cb[SPR_PIR].default_value == pir) {
> + return cpu;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
> +{
> + PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
> +
> + return cpu ? ICP(cpu->intc) : NULL;
> +}
> +
> static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -789,6 +823,7 @@ static void powernv_machine_class_props_init(ObjectClass *oc)
> static void powernv_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> + XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>
> mc->desc = "IBM PowerNV (Non-Virtualized)";
> mc->init = ppc_powernv_init;
> @@ -799,6 +834,9 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
> mc->no_parallel = 1;
> mc->default_boot_order = NULL;
> mc->default_ram_size = 1 * G_BYTE;
> + xic->icp_get = pnv_icp_get;
> + xic->ics_get = pnv_ics_get;
> + xic->ics_resend = pnv_ics_resend;
>
> powernv_machine_class_props_init(oc);
> }
> @@ -809,6 +847,10 @@ static const TypeInfo powernv_machine_info = {
> .instance_size = sizeof(PnvMachineState),
> .instance_init = powernv_machine_initfn,
> .class_init = powernv_machine_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_XICS_FABRIC },
> + { },
> + },
> };
>
> static void powernv_machine_register_types(void)
--
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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v4 8/9] ppc/pnv: extend the machine with a InterruptStatsProvider interface
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (6 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 7/9] ppc/pnv: extend the machine with a XICSFabric interface Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 9/9] ppc/pnv: add memory regions for the ICP registers Cédric Le Goater
8 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/ppc/pnv.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 57560b09e04e..15a908f4306a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -33,6 +33,8 @@
#include "exec/address-spaces.h"
#include "qemu/cutils.h"
#include "qapi/visitor.h"
+#include "monitor/monitor.h"
+#include "hw/intc/intc.h"
#include "hw/ppc/xics.h"
#include "hw/ppc/pnv_xscom.h"
@@ -773,6 +775,18 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
return cpu ? ICP(cpu->intc) : NULL;
}
+static void pnv_pic_print_info(InterruptStatsProvider *obj,
+ Monitor *mon)
+{
+ CPUState *cs;
+
+ CPU_FOREACH(cs) {
+ PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+ icp_pic_print_info(ICP(cpu->intc), mon);
+ }
+}
+
static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -824,6 +838,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
+ InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
mc->desc = "IBM PowerNV (Non-Virtualized)";
mc->init = ppc_powernv_init;
@@ -837,6 +852,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
xic->icp_get = pnv_icp_get;
xic->ics_get = pnv_ics_get;
xic->ics_resend = pnv_ics_resend;
+ ispc->print_info = pnv_pic_print_info;
powernv_machine_class_props_init(oc);
}
@@ -849,6 +865,7 @@ static const TypeInfo powernv_machine_info = {
.class_init = powernv_machine_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_XICS_FABRIC },
+ { TYPE_INTERRUPT_STATS_PROVIDER },
{ },
},
};
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH v4 9/9] ppc/pnv: add memory regions for the ICP registers
2017-03-29 13:53 [Qemu-devel] [PATCH v4 0/9] ppc/pnv: interrupt controller (POWER8) Cédric Le Goater
` (7 preceding siblings ...)
2017-03-29 13:53 ` [Qemu-devel] [PATCH v4 8/9] ppc/pnv: extend the machine with a InterruptStatsProvider interface Cédric Le Goater
@ 2017-03-29 13:53 ` Cédric Le Goater
8 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2017-03-29 13:53 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
This provides to a PowerNV chip (POWER8) access to the Interrupt
Management area, which contains the registers of the Interrupt Control
Presenters of each thread. These are used to accept, return, forward
interrupts in the system.
This area is modeled with a per-chip container memory region holding
all the ICP registers. Each thread of a chip is then associated with
its ICP registers using a memory subregion indexed by its PIR number
in the overall region.
The device tree is populated accordingly.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
Changes since v1:
- added multichip support
- adapted to use PnvICPState object
hw/ppc/pnv.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/ppc/pnv.h | 19 ++++++++++++
2 files changed, 100 insertions(+)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 15a908f4306a..eb91d0187890 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -218,6 +218,43 @@ static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt)
servers_prop, sizeof(servers_prop))));
}
+static void powernv_populate_icp(PnvChip *chip, void *fdt, uint32_t pir,
+ uint32_t nr_threads)
+{
+ uint64_t addr = PNV_ICP_BASE(chip) | (pir << 12);
+ char *name;
+ const char compat[] = "IBM,power8-icp\0IBM,ppc-xicp";
+ uint32_t irange[2], i, rsize;
+ uint64_t *reg;
+ int offset;
+
+ irange[0] = cpu_to_be32(pir);
+ irange[1] = cpu_to_be32(nr_threads);
+
+ rsize = sizeof(uint64_t) * 2 * nr_threads;
+ reg = g_malloc(rsize);
+ for (i = 0; i < nr_threads; i++) {
+ reg[i * 2] = cpu_to_be64(addr | ((pir + i) * 0x1000));
+ reg[i * 2 + 1] = cpu_to_be64(0x1000);
+ }
+
+ name = g_strdup_printf("interrupt-controller@%"PRIX64, addr);
+ offset = fdt_add_subnode(fdt, 0, name);
+ _FDT(offset);
+ g_free(name);
+
+ _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
+ _FDT((fdt_setprop(fdt, offset, "reg", reg, rsize)));
+ _FDT((fdt_setprop_string(fdt, offset, "device_type",
+ "PowerPC-External-Interrupt-Presentation")));
+ _FDT((fdt_setprop(fdt, offset, "interrupt-controller", NULL, 0)));
+ _FDT((fdt_setprop(fdt, offset, "ibm,interrupt-server-ranges",
+ irange, sizeof(irange))));
+ _FDT((fdt_setprop_cell(fdt, offset, "#interrupt-cells", 1)));
+ _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0)));
+ g_free(reg);
+}
+
static void powernv_populate_chip(PnvChip *chip, void *fdt)
{
PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
@@ -231,6 +268,10 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
powernv_create_core_node(chip, pnv_core, fdt);
+
+ /* Interrupt Control Presenters (ICP). One per core. */
+ powernv_populate_icp(chip, fdt, pnv_core->pir,
+ CPU_CORE(pnv_core)->nr_threads);
}
if (chip->ram_size) {
@@ -643,6 +684,38 @@ static void pnv_chip_init(Object *obj)
object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL);
}
+static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
+{
+ PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+ char *typename = pnv_core_typename(pcc->cpu_model);
+ size_t typesize = object_type_get_instance_size(typename);
+ int i, j;
+ char *name;
+ XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
+
+ name = g_strdup_printf("icp-%x", chip->chip_id);
+ memory_region_init(&chip->icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
+ sysbus_init_mmio(SYS_BUS_DEVICE(chip), &chip->icp_mmio);
+ g_free(name);
+
+ sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
+
+ /* Map the ICP registers for each thread */
+ for (i = 0; i < chip->nr_cores; i++) {
+ PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
+ int core_hwid = CPU_CORE(pnv_core)->core_id;
+
+ for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
+ uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
+ PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
+
+ memory_region_add_subregion(&chip->icp_mmio, pir << 12, &icp->mmio);
+ }
+ }
+
+ g_free(typename);
+}
+
static void pnv_chip_realize(DeviceState *dev, Error **errp)
{
PnvChip *chip = PNV_CHIP(dev);
@@ -713,6 +786,14 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
&error_fatal);
pnv_xscom_add_subregion(chip, PNV_XSCOM_LPC_BASE, &chip->lpc.xscom_regs);
+
+ /* Interrupt Management Area. This is the memory region holding
+ * all the Interrupt Control Presenter (ICP) registers */
+ pnv_chip_icp_realize(chip, &error);
+ if (error) {
+ error_propagate(errp, error);
+ return;
+ }
}
static Property pnv_chip_properties[] = {
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 5693ba181d24..96231c1ff708 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -54,6 +54,7 @@ typedef struct PnvChip {
MemoryRegion xscom_mmio;
MemoryRegion xscom;
AddressSpace xscom_as;
+ MemoryRegion icp_mmio;
PnvLpcController lpc;
} PnvChip;
@@ -136,4 +137,22 @@ typedef struct PnvMachineState {
#define PNV_XSCOM_BASE(chip) \
(chip->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
+/*
+ * XSCOM 0x20109CA defines the ICP BAR:
+ *
+ * 0:29 : bits 14 to 43 of address to define 1 MB region.
+ * 30 : 1 to enable ICP to receive loads/stores against its BAR region
+ * 31:63 : Constant 0
+ *
+ * Usually defined as :
+ *
+ * 0xffffe00200000000 -> 0x0003ffff80000000
+ * 0xffffe00600000000 -> 0x0003ffff80100000
+ * 0xffffe02200000000 -> 0x0003ffff80800000
+ * 0xffffe02600000000 -> 0x0003ffff80900000
+ */
+#define PNV_ICP_SIZE 0x0000000000100000ull
+#define PNV_ICP_BASE(chip) \
+ (0x0003ffff80000000ull + (uint64_t) PNV_CHIP_INDEX(chip) * PNV_ICP_SIZE)
+
#endif /* _PPC_PNV_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread