* [Qemu-devel] [PATCH 01/10] spapr: modify the prototype of the cpu_intc_create() method
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU Cédric Le Goater
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Today, the interrupt presenter is linked to a CPU using the
cpu_intc_create() method of the sPAPR IRQ backend. The resulting
object is assigned to the PowerPCCPU 'intc' pointer whatever the
interrupt mode, XICS or XIVE.
To support the 'dual' interrupt mode, we will need to distinguish
between the two presenter objects and for that, we plan to introduce a
second interrupt presenter object pointer under the PowerPCCPU. The
modifications below move the assignment of the presenter object under
the cpu_intc_create() method to prepare ground for the future changes.
Both sPAPR and PowerNV machines are impacted.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/pnv.h | 2 +-
include/hw/ppc/spapr_irq.h | 4 ++--
hw/ppc/pnv.c | 23 ++++++++++++++++-------
hw/ppc/pnv_core.c | 2 +-
hw/ppc/spapr_cpu_core.c | 2 +-
hw/ppc/spapr_irq.c | 34 ++++++++++++++++++++++++++--------
6 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 86d5f54e5459..6b65397b7ebf 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -98,7 +98,7 @@ typedef struct PnvChipClass {
DeviceRealize parent_realize;
uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
- Object *(*intc_create)(PnvChip *chip, Object *child, Error **errp);
+ void (*intc_create)(PnvChip *chip, PowerPCCPU *cpu, Error **errp);
ISABus *(*isa_create)(PnvChip *chip, Error **errp);
} PnvChipClass;
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index b34d5a00381b..d03d4d7ce687 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -42,8 +42,8 @@ typedef struct sPAPRIrq {
void (*print_info)(sPAPRMachineState *spapr, Monitor *mon);
void (*dt_populate)(sPAPRMachineState *spapr, uint32_t nr_servers,
void *fdt, uint32_t phandle);
- Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
- Error **errp);
+ void (*cpu_intc_create)(sPAPRMachineState *spapr, PowerPCCPU *cpu,
+ Error **errp);
int (*post_load)(sPAPRMachineState *spapr, int version_id);
void (*reset)(sPAPRMachineState *spapr, Error **errp);
} sPAPRIrq;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 346f5e7aedb5..8e83be54fca1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -668,11 +668,20 @@ static uint32_t pnv_chip_core_pir_p8(PnvChip *chip, uint32_t core_id)
return (chip->chip_id << 7) | (core_id << 3);
}
-static Object *pnv_chip_power8_intc_create(PnvChip *chip, Object *child,
- Error **errp)
+static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu,
+ Error **errp)
{
- return icp_create(child, TYPE_PNV_ICP, XICS_FABRIC(qdev_get_machine()),
- errp);
+ Error *local_err = NULL;
+ Object *obj;
+
+ obj = icp_create(OBJECT(cpu), TYPE_PNV_ICP, XICS_FABRIC(qdev_get_machine()),
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ cpu->intc = obj;
}
/*
@@ -690,10 +699,10 @@ static uint32_t pnv_chip_core_pir_p9(PnvChip *chip, uint32_t core_id)
return (chip->chip_id << 8) | (core_id << 2);
}
-static Object *pnv_chip_power9_intc_create(PnvChip *chip, Object *child,
- Error **errp)
+static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
+ Error **errp)
{
- return NULL;
+ return;
}
/* Allowed core identifiers on a POWER8 Processor Chip :
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index ad1bcc79904f..120273774874 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -114,7 +114,7 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp)
return;
}
- cpu->intc = pcc->intc_create(chip, OBJECT(cpu), &local_err);
+ pcc->intc_create(chip, cpu, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 82666436e9b4..2739b2a4b818 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -232,7 +232,7 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
qemu_register_reset(spapr_cpu_reset, cpu);
spapr_cpu_reset(cpu);
- cpu->intc = spapr->irq->cpu_intc_create(spapr, OBJECT(cpu), &local_err);
+ spapr->irq->cpu_intc_create(spapr, cpu, &local_err);
if (local_err) {
goto error_unregister;
}
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 7b3b5afec212..2d2e17b66533 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -190,10 +190,20 @@ static void spapr_irq_print_info_xics(sPAPRMachineState *spapr, Monitor *mon)
ics_pic_print_info(spapr->ics, mon);
}
-static Object *spapr_irq_cpu_intc_create_xics(sPAPRMachineState *spapr,
- Object *cpu, Error **errp)
+static void spapr_irq_cpu_intc_create_xics(sPAPRMachineState *spapr,
+ PowerPCCPU *cpu, Error **errp)
{
- return icp_create(cpu, spapr->icp_type, XICS_FABRIC(spapr), errp);
+ Error *local_err = NULL;
+ Object *obj;
+
+ obj = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr),
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ cpu->intc = obj;
}
static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
@@ -301,17 +311,25 @@ static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
spapr_xive_pic_print_info(spapr->xive, mon);
}
-static Object *spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
- Object *cpu, Error **errp)
+static void spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
+ PowerPCCPU *cpu, Error **errp)
{
- Object *obj = xive_tctx_create(cpu, XIVE_ROUTER(spapr->xive), errp);
+ Error *local_err = NULL;
+ Object *obj;
+
+ obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ cpu->intc = obj;
/*
* (TCG) Early setting the OS CAM line for hotplugged CPUs as they
- * don't benificiate from the reset of the XIVE IRQ backend
+ * don't beneficiate from the reset of the XIVE IRQ backend
*/
spapr_xive_set_tctx_os_cam(XIVE_TCTX(obj));
- return obj;
}
static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 01/10] spapr: modify the prototype of the cpu_intc_create() method Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-03 3:57 ` David Gibson
2019-01-02 5:57 ` [Qemu-devel] [PATCH 03/10] ppc: replace the 'Object *intc' by a 'ICPState *icp' pointer under the CPU Cédric Le Goater
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
which will be used by the machine only when the XIVE interrupt mode is
in use.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
target/ppc/cpu.h | 2 ++
hw/intc/xive.c | 6 +++---
hw/ppc/spapr_cpu_core.c | 7 ++++++-
hw/ppc/spapr_irq.c | 8 ++++----
4 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d5f99f1fc7b9..c76036985623 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1177,6 +1177,7 @@ do { \
typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
+typedef struct XiveTCTX XiveTCTX;
/**
* PowerPCCPU:
@@ -1196,6 +1197,7 @@ struct PowerPCCPU {
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
Object *intc;
+ XiveTCTX *tctx;
void *machine_data;
int32_t node_id; /* NUMA node this CPU belongs to */
PPCHash64Options *hash64_opts;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index ea33494338dc..410c53278a11 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -321,7 +321,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
- XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
+ XiveTCTX *tctx = cpu->tctx;
const XiveTmOp *xto;
/*
@@ -360,7 +360,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
{
PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
- XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
+ XiveTCTX *tctx = cpu->tctx;
const XiveTmOp *xto;
/*
@@ -1186,7 +1186,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
+ XiveTCTX *tctx = cpu->tctx;
int ring;
/*
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2739b2a4b818..1473ef853336 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -194,7 +194,12 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, sPAPRCPUCore *sc)
vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
}
qemu_unregister_reset(spapr_cpu_reset, cpu);
- object_unparent(cpu->intc);
+ if (cpu->intc) {
+ object_unparent(cpu->intc);
+ }
+ if (cpu->tctx) {
+ object_unparent(OBJECT(cpu->tctx));
+ }
cpu_remove_sync(CPU(cpu));
object_unparent(OBJECT(cpu));
}
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 2d2e17b66533..8d028db44ff4 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -305,7 +305,7 @@ static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- xive_tctx_pic_print_info(XIVE_TCTX(cpu->intc), mon);
+ xive_tctx_pic_print_info(cpu->tctx, mon);
}
spapr_xive_pic_print_info(spapr->xive, mon);
@@ -323,13 +323,13 @@ static void spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
return;
}
- cpu->intc = obj;
+ cpu->tctx = XIVE_TCTX(obj);
/*
* (TCG) Early setting the OS CAM line for hotplugged CPUs as they
* don't beneficiate from the reset of the XIVE IRQ backend
*/
- spapr_xive_set_tctx_os_cam(XIVE_TCTX(obj));
+ spapr_xive_set_tctx_os_cam(cpu->tctx);
}
static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
@@ -345,7 +345,7 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
PowerPCCPU *cpu = POWERPC_CPU(cs);
/* (TCG) Set the OS CAM line of the thread interrupt context. */
- spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
+ spapr_xive_set_tctx_os_cam(cpu->tctx);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
2019-01-02 5:57 ` [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU Cédric Le Goater
@ 2019-01-03 3:57 ` David Gibson
2019-01-03 17:44 ` Cédric Le Goater
0 siblings, 1 reply; 22+ messages in thread
From: David Gibson @ 2019-01-03 3:57 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5228 bytes --]
On Wed, Jan 02, 2019 at 06:57:35AM +0100, Cédric Le Goater wrote:
> which will be used by the machine only when the XIVE interrupt mode is
> in use.
I don't love the idea of putting a hook this specific into the
PowerPCCPU structure, though it might be the easiest path in the short
term.
A couple of approaches: 1) revisit my changes to allow for a pointer
to machine-defined per-cpu data. or 2) do we actually need a cpu to
tctx pointer.
Expanding on (2) - here you use the pointer to find the right TIMA
state to access, but that could also be handled by having different
TIMA IO instances and mapping those individually to cpu_as. On the
interrupt delivery side I think a tctx to cpu link will suffice. For
sPAPR there might be complications with translating cpu numbers in
hcalls to the right tctx.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> target/ppc/cpu.h | 2 ++
> hw/intc/xive.c | 6 +++---
> hw/ppc/spapr_cpu_core.c | 7 ++++++-
> hw/ppc/spapr_irq.c | 8 ++++----
> 4 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index d5f99f1fc7b9..c76036985623 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1177,6 +1177,7 @@ do { \
>
> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
> +typedef struct XiveTCTX XiveTCTX;
>
> /**
> * PowerPCCPU:
> @@ -1196,6 +1197,7 @@ struct PowerPCCPU {
> uint32_t compat_pvr;
> PPCVirtualHypervisor *vhyp;
> Object *intc;
> + XiveTCTX *tctx;
> void *machine_data;
> int32_t node_id; /* NUMA node this CPU belongs to */
> PPCHash64Options *hash64_opts;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index ea33494338dc..410c53278a11 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -321,7 +321,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
> uint64_t value, unsigned size)
> {
> PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> + XiveTCTX *tctx = cpu->tctx;
> const XiveTmOp *xto;
>
> /*
> @@ -360,7 +360,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
> static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> {
> PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> + XiveTCTX *tctx = cpu->tctx;
> const XiveTmOp *xto;
>
> /*
> @@ -1186,7 +1186,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
> + XiveTCTX *tctx = cpu->tctx;
> int ring;
>
> /*
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 2739b2a4b818..1473ef853336 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -194,7 +194,12 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, sPAPRCPUCore *sc)
> vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
> }
> qemu_unregister_reset(spapr_cpu_reset, cpu);
> - object_unparent(cpu->intc);
> + if (cpu->intc) {
> + object_unparent(cpu->intc);
> + }
> + if (cpu->tctx) {
> + object_unparent(OBJECT(cpu->tctx));
> + }
> cpu_remove_sync(CPU(cpu));
> object_unparent(OBJECT(cpu));
> }
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 2d2e17b66533..8d028db44ff4 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -305,7 +305,7 @@ static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
> CPU_FOREACH(cs) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> - xive_tctx_pic_print_info(XIVE_TCTX(cpu->intc), mon);
> + xive_tctx_pic_print_info(cpu->tctx, mon);
> }
>
> spapr_xive_pic_print_info(spapr->xive, mon);
> @@ -323,13 +323,13 @@ static void spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
> return;
> }
>
> - cpu->intc = obj;
> + cpu->tctx = XIVE_TCTX(obj);
>
> /*
> * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
> * don't beneficiate from the reset of the XIVE IRQ backend
> */
> - spapr_xive_set_tctx_os_cam(XIVE_TCTX(obj));
> + spapr_xive_set_tctx_os_cam(cpu->tctx);
> }
>
> static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
> @@ -345,7 +345,7 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> PowerPCCPU *cpu = POWERPC_CPU(cs);
>
> /* (TCG) Set the OS CAM line of the thread interrupt context. */
> - spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
> + spapr_xive_set_tctx_os_cam(cpu->tctx);
> }
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
2019-01-03 3:57 ` David Gibson
@ 2019-01-03 17:44 ` Cédric Le Goater
2019-01-04 5:25 ` David Gibson
0 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-03 17:44 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 1/3/19 4:57 AM, David Gibson wrote:
> On Wed, Jan 02, 2019 at 06:57:35AM +0100, Cédric Le Goater wrote:
>> which will be used by the machine only when the XIVE interrupt mode is
>> in use.
>
> I don't love the idea of putting a hook this specific into the
> PowerPCCPU structure, though it might be the easiest path in the short
> term.
>
> A couple of approaches: 1) revisit my changes to allow for a pointer
> to machine-defined per-cpu data.
ok but we still need two different presenters objects, one for each
mode.
> or 2) do we actually need a cpu to tctx pointer.
>
> Expanding on (2) - here you use the pointer to find the right TIMA
> state to access,
yes.
> but that could also be handled by having different TIMA IO instances
> and mapping those individually to cpu_as.
It might work for QEMU but I am not sure how to implement the KVM
backend with such a design. We only have a single ram ptr mapping
for the machine in the KVM case.
There are a couple of places where we need to loop on the XiveTCTX
(presenter, KVM) and we use the QEMU CPU list and the cpu->tctx for
that. One of the reasons we introduced the cpu->intc pointer some
time ago was to get rid of the ICP array.
I don't think we want to introduce a XiveTCTX array again.
> On the interrupt delivery side I think a tctx to cpu link will suffice.
yes. that we already have. It is mostly used by KVM in fact.
> For sPAPR there might be complications with translating cpu numbers in
> hcalls to the right tctx.
The XiveTCTX is not used by the hcalls. We are fine on that side.
The double pointer solution is what I prefer today, but if you are
uncomfortable with it, we can come back to the previous where I was
allocating a XiveTCTX child under the CPU and switching the presenter
objects at reset. The only issue remaining was the child unparenting
in the unrealize() method.
Thanks,
C.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> target/ppc/cpu.h | 2 ++
>> hw/intc/xive.c | 6 +++---
>> hw/ppc/spapr_cpu_core.c | 7 ++++++-
>> hw/ppc/spapr_irq.c | 8 ++++----
>> 4 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index d5f99f1fc7b9..c76036985623 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -1177,6 +1177,7 @@ do { \
>>
>> typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
>> typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
>> +typedef struct XiveTCTX XiveTCTX;
>>
>> /**
>> * PowerPCCPU:
>> @@ -1196,6 +1197,7 @@ struct PowerPCCPU {
>> uint32_t compat_pvr;
>> PPCVirtualHypervisor *vhyp;
>> Object *intc;
>> + XiveTCTX *tctx;
>> void *machine_data;
>> int32_t node_id; /* NUMA node this CPU belongs to */
>> PPCHash64Options *hash64_opts;
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index ea33494338dc..410c53278a11 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -321,7 +321,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>> uint64_t value, unsigned size)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
>> + XiveTCTX *tctx = cpu->tctx;
>> const XiveTmOp *xto;
>>
>> /*
>> @@ -360,7 +360,7 @@ static void xive_tm_write(void *opaque, hwaddr offset,
>> static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
>> + XiveTCTX *tctx = cpu->tctx;
>> const XiveTmOp *xto;
>>
>> /*
>> @@ -1186,7 +1186,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
>>
>> CPU_FOREACH(cs) {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>> - XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
>> + XiveTCTX *tctx = cpu->tctx;
>> int ring;
>>
>> /*
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index 2739b2a4b818..1473ef853336 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -194,7 +194,12 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, sPAPRCPUCore *sc)
>> vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
>> }
>> qemu_unregister_reset(spapr_cpu_reset, cpu);
>> - object_unparent(cpu->intc);
>> + if (cpu->intc) {
>> + object_unparent(cpu->intc);
>> + }
>> + if (cpu->tctx) {
>> + object_unparent(OBJECT(cpu->tctx));
>> + }
>> cpu_remove_sync(CPU(cpu));
>> object_unparent(OBJECT(cpu));
>> }
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 2d2e17b66533..8d028db44ff4 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -305,7 +305,7 @@ static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
>> CPU_FOREACH(cs) {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>
>> - xive_tctx_pic_print_info(XIVE_TCTX(cpu->intc), mon);
>> + xive_tctx_pic_print_info(cpu->tctx, mon);
>> }
>>
>> spapr_xive_pic_print_info(spapr->xive, mon);
>> @@ -323,13 +323,13 @@ static void spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
>> return;
>> }
>>
>> - cpu->intc = obj;
>> + cpu->tctx = XIVE_TCTX(obj);
>>
>> /*
>> * (TCG) Early setting the OS CAM line for hotplugged CPUs as they
>> * don't beneficiate from the reset of the XIVE IRQ backend
>> */
>> - spapr_xive_set_tctx_os_cam(XIVE_TCTX(obj));
>> + spapr_xive_set_tctx_os_cam(cpu->tctx);
>> }
>>
>> static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int version_id)
>> @@ -345,7 +345,7 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>
>> /* (TCG) Set the OS CAM line of the thread interrupt context. */
>> - spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>> + spapr_xive_set_tctx_os_cam(cpu->tctx);
>> }
>> }
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
2019-01-03 17:44 ` Cédric Le Goater
@ 2019-01-04 5:25 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2019-01-04 5:25 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]
On Thu, Jan 03, 2019 at 06:44:30PM +0100, Cédric Le Goater wrote:
> On 1/3/19 4:57 AM, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 06:57:35AM +0100, Cédric Le Goater wrote:
> >> which will be used by the machine only when the XIVE interrupt mode is
> >> in use.
> >
> > I don't love the idea of putting a hook this specific into the
> > PowerPCCPU structure, though it might be the easiest path in the short
> > term.
> >
> > A couple of approaches: 1) revisit my changes to allow for a pointer
> > to machine-defined per-cpu data.
>
> ok but we still need two different presenters objects, one for each
> mode.
>
> > or 2) do we actually need a cpu to tctx pointer.
> >
> > Expanding on (2) - here you use the pointer to find the right TIMA
> > state to access,
>
> yes.
>
> > but that could also be handled by having different TIMA IO instances
> > and mapping those individually to cpu_as.
>
> It might work for QEMU but I am not sure how to implement the KVM
> backend with such a design. We only have a single ram ptr mapping
> for the machine in the KVM case.
>
> There are a couple of places where we need to loop on the XiveTCTX
> (presenter, KVM) and we use the QEMU CPU list and the cpu->tctx for
> that. One of the reasons we introduced the cpu->intc pointer some
> time ago was to get rid of the ICP array.
>
> I don't think we want to introduce a XiveTCTX array again.
>
> > On the interrupt delivery side I think a tctx to cpu link will suffice.
>
> yes. that we already have. It is mostly used by KVM in fact.
>
> > For sPAPR there might be complications with translating cpu numbers in
> > hcalls to the right tctx.
>
> The XiveTCTX is not used by the hcalls. We are fine on that side.
>
>
> The double pointer solution is what I prefer today, but if you are
> uncomfortable with it, we can come back to the previous where I was
> allocating a XiveTCTX child under the CPU and switching the presenter
> objects at reset. The only issue remaining was the child unparenting
> in the unrealize() method.
Hm, yes, I see your point. For now I'm applying patches 1-3, and hope
to clean it up later with a revised version of my machine specific
per-cpu patch when I get the chance.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 03/10] ppc: replace the 'Object *intc' by a 'ICPState *icp' pointer under the CPU
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 01/10] spapr: modify the prototype of the cpu_intc_create() method Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 02/10] ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE Cédric Le Goater
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Now that the 'intc' pointer is only used by the XICS interrupt mode,
let's make things clear and use a XICS type and name.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
target/ppc/cpu.h | 3 ++-
hw/intc/xics_spapr.c | 10 +++++-----
hw/ppc/pnv.c | 6 +++---
hw/ppc/pnv_core.c | 2 +-
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_cpu_core.c | 4 ++--
hw/ppc/spapr_irq.c | 6 +++---
7 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c76036985623..757606bcb2e2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1178,6 +1178,7 @@ do { \
typedef struct PPCVirtualHypervisor PPCVirtualHypervisor;
typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
typedef struct XiveTCTX XiveTCTX;
+typedef struct ICPState ICPState;
/**
* PowerPCCPU:
@@ -1196,7 +1197,7 @@ struct PowerPCCPU {
int vcpu_id;
uint32_t compat_pvr;
PPCVirtualHypervisor *vhyp;
- Object *intc;
+ ICPState *icp;
XiveTCTX *tctx;
void *machine_data;
int32_t node_id; /* NUMA node this CPU belongs to */
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
index f67d3c80bf3a..9c1a90d7094b 100644
--- a/hw/intc/xics_spapr.c
+++ b/hw/intc/xics_spapr.c
@@ -44,7 +44,7 @@ static target_ulong h_cppr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
target_ulong cppr = args[0];
- icp_set_cppr(ICP(cpu->intc), cppr);
+ icp_set_cppr(cpu->icp, cppr);
return H_SUCCESS;
}
@@ -65,7 +65,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)
{
- uint32_t xirr = icp_accept(ICP(cpu->intc));
+ uint32_t xirr = icp_accept(cpu->icp);
args[0] = xirr;
return H_SUCCESS;
@@ -74,7 +74,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)
{
- uint32_t xirr = icp_accept(ICP(cpu->intc));
+ uint32_t xirr = icp_accept(cpu->icp);
args[0] = xirr;
args[1] = cpu_get_host_ticks();
@@ -86,7 +86,7 @@ static target_ulong h_eoi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
target_ulong xirr = args[0];
- icp_eoi(ICP(cpu->intc), xirr);
+ icp_eoi(cpu->icp, xirr);
return H_SUCCESS;
}
@@ -94,7 +94,7 @@ static target_ulong h_ipoll(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
uint32_t mfrr;
- uint32_t xirr = icp_ipoll(ICP(cpu->intc), &mfrr);
+ uint32_t xirr = icp_ipoll(cpu->icp, &mfrr);
args[0] = xirr;
args[1] = mfrr;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 8e83be54fca1..d84acef55b69 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -681,7 +681,7 @@ static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu,
return;
}
- cpu->intc = obj;
+ cpu->icp = ICP(obj);
}
/*
@@ -1099,7 +1099,7 @@ static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
{
PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
- return cpu ? ICP(cpu->intc) : NULL;
+ return cpu ? cpu->icp : NULL;
}
static void pnv_pic_print_info(InterruptStatsProvider *obj,
@@ -1112,7 +1112,7 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- icp_pic_print_info(ICP(cpu->intc), mon);
+ icp_pic_print_info(cpu->icp, mon);
}
for (i = 0; i < pnv->num_chips; i++) {
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 120273774874..b98f277f1e02 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -190,7 +190,7 @@ err:
static void pnv_unrealize_vcpu(PowerPCCPU *cpu)
{
qemu_unregister_reset(pnv_cpu_reset, cpu);
- object_unparent(cpu->intc);
+ object_unparent(OBJECT(cpu->icp));
cpu_remove_sync(CPU(cpu));
object_unparent(OBJECT(cpu));
}
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 19a07c5c9dd8..5e8ffda47372 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3841,7 +3841,7 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
{
PowerPCCPU *cpu = spapr_find_cpu(vcpu_id);
- return cpu ? ICP(cpu->intc) : NULL;
+ return cpu ? cpu->icp : NULL;
}
static void spapr_pic_print_info(InterruptStatsProvider *obj,
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 1473ef853336..0405306d1e59 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -194,8 +194,8 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, sPAPRCPUCore *sc)
vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data);
}
qemu_unregister_reset(spapr_cpu_reset, cpu);
- if (cpu->intc) {
- object_unparent(cpu->intc);
+ if (cpu->icp) {
+ object_unparent(OBJECT(cpu->icp));
}
if (cpu->tctx) {
object_unparent(OBJECT(cpu->tctx));
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 8d028db44ff4..50e767120d21 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -184,7 +184,7 @@ static void spapr_irq_print_info_xics(sPAPRMachineState *spapr, Monitor *mon)
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- icp_pic_print_info(ICP(cpu->intc), mon);
+ icp_pic_print_info(cpu->icp, mon);
}
ics_pic_print_info(spapr->ics, mon);
@@ -203,7 +203,7 @@ static void spapr_irq_cpu_intc_create_xics(sPAPRMachineState *spapr,
return;
}
- cpu->intc = obj;
+ cpu->icp = ICP(obj);
}
static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
@@ -212,7 +212,7 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
CPUState *cs;
CPU_FOREACH(cs) {
PowerPCCPU *cpu = POWERPC_CPU(cs);
- icp_resend(ICP(cpu->intc));
+ icp_resend(cpu->icp);
}
}
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (2 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 03/10] ppc: replace the 'Object *intc' by a 'ICPState *icp' pointer under the CPU Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-03 3:58 ` David Gibson
2019-01-02 5:57 ` [Qemu-devel] [PATCH 05/10] ppc: export the XICS and XIVE set_irq handlers Cédric Le Goater
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
The qirq routines of the XiveSource and the sPAPRXive model are only
used under the sPAPR IRQ backend. Simplify the overall call stack and
gather all the code under spapr_qirq_xive(). It will ease future
changes.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/spapr_xive.h | 1 -
include/hw/ppc/xive.h | 6 ------
hw/intc/spapr_xive.c | 14 --------------
hw/ppc/spapr_irq.c | 12 +++++++++++-
4 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 728735dbcfbe..9ee524fdb218 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -40,7 +40,6 @@ typedef struct sPAPRXive {
bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
-qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn);
typedef struct sPAPRMachineState sPAPRMachineState;
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 18cd114eb244..b05fe88b5b82 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -278,12 +278,6 @@ uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
void xive_source_pic_print_info(XiveSource *xsrc, uint32_t offset,
Monitor *mon);
-static inline qemu_irq xive_source_qirq(XiveSource *xsrc, uint32_t srcno)
-{
- assert(srcno < xsrc->nr_irqs);
- return xsrc->qirqs[srcno];
-}
-
static inline bool xive_source_irq_is_lsi(XiveSource *xsrc, uint32_t srcno)
{
assert(srcno < xsrc->nr_irqs);
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 0e39c90cbd07..eea28337e807 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -488,20 +488,6 @@ bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn)
return true;
}
-qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn)
-{
- XiveSource *xsrc = &xive->source;
-
- if (lisn >= xive->nr_irqs) {
- return NULL;
- }
-
- /* The sPAPR machine/device should have claimed the IRQ before */
- assert(xive_eas_is_valid(&xive->eat[lisn]));
-
- return xive_source_qirq(xsrc, lisn);
-}
-
/*
* XIVE hcalls
*
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 50e767120d21..b875065ef86b 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -294,7 +294,17 @@ static void spapr_irq_free_xive(sPAPRMachineState *spapr, int irq, int num)
static qemu_irq spapr_qirq_xive(sPAPRMachineState *spapr, int irq)
{
- return spapr_xive_qirq(spapr->xive, irq);
+ sPAPRXive *xive = spapr->xive;
+ XiveSource *xsrc = &xive->source;
+
+ if (irq >= xive->nr_irqs) {
+ return NULL;
+ }
+
+ /* The sPAPR machine/device should have claimed the IRQ before */
+ assert(xive_eas_is_valid(&xive->eat[irq]));
+
+ return xsrc->qirqs[irq];
}
static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE
2019-01-02 5:57 ` [Qemu-devel] [PATCH 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE Cédric Le Goater
@ 2019-01-03 3:58 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2019-01-03 3:58 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]
On Wed, Jan 02, 2019 at 06:57:37AM +0100, Cédric Le Goater wrote:
> The qirq routines of the XiveSource and the sPAPRXive model are only
> used under the sPAPR IRQ backend. Simplify the overall call stack and
> gather all the code under spapr_qirq_xive(). It will ease future
> changes.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Seems a good clean up in its own right, so I've applied.
> ---
> include/hw/ppc/spapr_xive.h | 1 -
> include/hw/ppc/xive.h | 6 ------
> hw/intc/spapr_xive.c | 14 --------------
> hw/ppc/spapr_irq.c | 12 +++++++++++-
> 4 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 728735dbcfbe..9ee524fdb218 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -40,7 +40,6 @@ typedef struct sPAPRXive {
> bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
> bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
> void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
> -qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn);
>
> typedef struct sPAPRMachineState sPAPRMachineState;
>
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 18cd114eb244..b05fe88b5b82 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -278,12 +278,6 @@ uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
> void xive_source_pic_print_info(XiveSource *xsrc, uint32_t offset,
> Monitor *mon);
>
> -static inline qemu_irq xive_source_qirq(XiveSource *xsrc, uint32_t srcno)
> -{
> - assert(srcno < xsrc->nr_irqs);
> - return xsrc->qirqs[srcno];
> -}
> -
> static inline bool xive_source_irq_is_lsi(XiveSource *xsrc, uint32_t srcno)
> {
> assert(srcno < xsrc->nr_irqs);
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 0e39c90cbd07..eea28337e807 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -488,20 +488,6 @@ bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn)
> return true;
> }
>
> -qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn)
> -{
> - XiveSource *xsrc = &xive->source;
> -
> - if (lisn >= xive->nr_irqs) {
> - return NULL;
> - }
> -
> - /* The sPAPR machine/device should have claimed the IRQ before */
> - assert(xive_eas_is_valid(&xive->eat[lisn]));
> -
> - return xive_source_qirq(xsrc, lisn);
> -}
> -
> /*
> * XIVE hcalls
> *
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 50e767120d21..b875065ef86b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -294,7 +294,17 @@ static void spapr_irq_free_xive(sPAPRMachineState *spapr, int irq, int num)
>
> static qemu_irq spapr_qirq_xive(sPAPRMachineState *spapr, int irq)
> {
> - return spapr_xive_qirq(spapr->xive, irq);
> + sPAPRXive *xive = spapr->xive;
> + XiveSource *xsrc = &xive->source;
> +
> + if (irq >= xive->nr_irqs) {
> + return NULL;
> + }
> +
> + /* The sPAPR machine/device should have claimed the IRQ before */
> + assert(xive_eas_is_valid(&xive->eat[irq]));
> +
> + return xsrc->qirqs[irq];
> }
>
> static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 05/10] ppc: export the XICS and XIVE set_irq handlers
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (3 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 04/10] spapr/xive: simplify the sPAPR IRQ qirq method for XIVE Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 06/10] pnv/psi: move the ICSState qemu_irq array under the PSI device model Cédric Le Goater
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
To support the 'dual' interrupt mode, XICS and XIVE, we plan to move
the qemu_irq array of each interrupt controller under the machine and
do the allocation under the sPAPR IRQ init method.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/xics.h | 2 ++
include/hw/ppc/xive.h | 2 ++
hw/intc/xics.c | 2 +-
hw/intc/xics_kvm.c | 2 +-
hw/intc/xive.c | 2 +-
5 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 14afda198cdb..686db51149f3 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -192,6 +192,8 @@ void icp_eoi(ICPState *icp, uint32_t xirr);
void ics_simple_write_xive(ICSState *ics, int nr, int server,
uint8_t priority, uint8_t saved_priority);
+void ics_simple_set_irq(void *opaque, int srcno, int val);
+void ics_kvm_set_irq(void *opaque, int srcno, int val);
void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
void icp_pic_print_info(ICPState *icp, Monitor *mon);
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index b05fe88b5b82..c279dc73b9f6 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -293,6 +293,8 @@ static inline void xive_source_irq_set(XiveSource *xsrc, uint32_t srcno,
}
}
+void xive_source_set_irq(void *opaque, int srcno, int val);
+
/*
* XIVE Router
*/
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 406efee06448..0d65549e3d2e 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -461,7 +461,7 @@ static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val)
ics_simple_resend_lsi(ics, srcno);
}
-static void ics_simple_set_irq(void *opaque, int srcno, int val)
+void ics_simple_set_irq(void *opaque, int srcno, int val)
{
ICSState *ics = (ICSState *)opaque;
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index e8fa9a53aeba..c469c85d53dc 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -298,7 +298,7 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
return 0;
}
-static void ics_kvm_set_irq(void *opaque, int srcno, int val)
+void ics_kvm_set_irq(void *opaque, int srcno, int val)
{
ICSState *ics = opaque;
struct kvm_irq_level args;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 410c53278a11..c7599608959c 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -845,7 +845,7 @@ static const MemoryRegionOps xive_source_esb_ops = {
},
};
-static void xive_source_set_irq(void *opaque, int srcno, int val)
+void xive_source_set_irq(void *opaque, int srcno, int val)
{
XiveSource *xsrc = XIVE_SOURCE(opaque);
bool notify = false;
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH 06/10] pnv/psi: move the ICSState qemu_irq array under the PSI device model
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (4 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 05/10] ppc: export the XICS and XIVE set_irq handlers Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 07/10] spapr: move the qemu_irq array under the machine Cédric Le Goater
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Future changes of the ICSState object will remove the qemu_irq array
from under the interrupt controller model. Prepare ground for the PSI
interrupt sources and introduce a new one directly under the PSI
device model.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/pnv_psi.h | 1 +
hw/ppc/pnv_psi.c | 7 ++++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h
index f6af5eae1fa8..64ac73512e81 100644
--- a/include/hw/ppc/pnv_psi.h
+++ b/include/hw/ppc/pnv_psi.h
@@ -40,6 +40,7 @@ typedef struct PnvPsi {
/* Interrupt generation */
ICSState ics;
+ qemu_irq *qirqs;
/* Registers */
uint64_t regs[PSIHB_XSCOM_MAX];
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 5b969127c303..8ced09506321 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -207,7 +207,6 @@ static const uint64_t stat_bits[] = {
void pnv_psi_irq_set(PnvPsi *psi, PnvPsiIrq irq, bool state)
{
- ICSState *ics = &psi->ics;
uint32_t xivr_reg;
uint32_t stat_reg;
uint32_t src;
@@ -227,14 +226,14 @@ void pnv_psi_irq_set(PnvPsi *psi, PnvPsiIrq irq, bool state)
/* TODO: optimization, check mask here. That means
* re-evaluating when unmasking
*/
- qemu_irq_raise(ics->qirqs[src]);
+ qemu_irq_raise(psi->qirqs[src]);
} else {
psi->regs[stat_reg] &= ~stat_bits[irq];
/* FSP and PSI are muxed so don't lower if either is still set */
if (stat_reg != PSIHB_XSCOM_CR ||
!(psi->regs[stat_reg] & (PSIHB_CR_PSI_IRQ | PSIHB_CR_FSP_IRQ))) {
- qemu_irq_lower(ics->qirqs[src]);
+ qemu_irq_lower(psi->qirqs[src]);
} else {
state = true;
}
@@ -491,6 +490,8 @@ static void pnv_psi_realize(DeviceState *dev, Error **errp)
ics_set_irq_type(ics, i, true);
}
+ psi->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
+
/* XSCOM region for PSI registers */
pnv_xscom_region_init(&psi->xscom_regs, OBJECT(dev), &pnv_psi_xscom_ops,
psi, "xscom-psi", PNV_XSCOM_PSIHB_SIZE);
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH 07/10] spapr: move the qemu_irq array under the machine
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (5 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 06/10] pnv/psi: move the ICSState qemu_irq array under the PSI device model Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-02 5:57 ` [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0 Cédric Le Goater
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
The qemu_irq array is now allocated at the machine level using a sPAPR
IRQ set_irq handler depending on the chosen interrupt mode. The use of
this handler is slightly inefficient today but it will become necessary
when the 'dual' interrupt mode is introduced.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/spapr.h | 1 +
include/hw/ppc/spapr_irq.h | 1 +
include/hw/ppc/xics.h | 1 -
include/hw/ppc/xive.h | 1 -
hw/intc/xics.c | 2 --
hw/intc/xics_kvm.c | 1 -
hw/intc/xive.c | 3 ---
hw/ppc/spapr_irq.c | 30 +++++++++++++++++++++++++++---
8 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 2c77a8ba8810..eda545f60d3c 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -178,6 +178,7 @@ struct sPAPRMachineState {
unsigned long *irq_map;
sPAPRXive *xive;
sPAPRIrq *irq;
+ qemu_irq *qirqs;
bool cmd_line_caps[SPAPR_CAP_NUM];
sPAPRCapabilities def, eff, mig;
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index d03d4d7ce687..283bb5002c16 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -46,6 +46,7 @@ typedef struct sPAPRIrq {
Error **errp);
int (*post_load)(sPAPRMachineState *spapr, int version_id);
void (*reset)(sPAPRMachineState *spapr, Error **errp);
+ void (*set_irq)(void *opaque, int srcno, int val);
} sPAPRIrq;
extern sPAPRIrq spapr_irq_xics;
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 686db51149f3..7668c381a887 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -131,7 +131,6 @@ struct ICSState {
/*< public >*/
uint32_t nr_irqs;
uint32_t offset;
- qemu_irq *qirqs;
ICSIRQState *irqs;
XICSFabric *xics;
};
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index c279dc73b9f6..ec23253ba448 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -184,7 +184,6 @@ typedef struct XiveSource {
/* IRQs */
uint32_t nr_irqs;
- qemu_irq *qirqs;
unsigned long *lsi_map;
/* PQ bits and LSI assertion bit */
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 0d65549e3d2e..16e8ffa2aaf7 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -571,8 +571,6 @@ static void ics_simple_realize(DeviceState *dev, Error **errp)
return;
}
- ics->qirqs = qemu_allocate_irqs(ics_simple_set_irq, ics, ics->nr_irqs);
-
qemu_register_reset(ics_simple_reset_handler, ics);
}
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index c469c85d53dc..ac94594b1919 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -344,7 +344,6 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp)
error_propagate(errp, local_err);
return;
}
- ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
qemu_register_reset(ics_kvm_reset_handler, ics);
}
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index c7599608959c..a3cb0cf0e348 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -932,9 +932,6 @@ static void xive_source_realize(DeviceState *dev, Error **errp)
&xive_source_esb_ops, xsrc, "xive.esb",
(1ull << xsrc->esb_shift) * xsrc->nr_irqs);
- xsrc->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc,
- xsrc->nr_irqs);
-
qemu_register_reset(xive_source_reset, dev);
}
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index b875065ef86b..d23914887ac0 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -171,7 +171,7 @@ static qemu_irq spapr_qirq_xics(sPAPRMachineState *spapr, int irq)
uint32_t srcno = irq - ics->offset;
if (ics_valid_irq(ics, irq)) {
- return ics->qirqs[srcno];
+ return spapr->qirqs[srcno];
}
return NULL;
@@ -218,6 +218,18 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
return 0;
}
+static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val)
+{
+ sPAPRMachineState *spapr = opaque;
+ MachineState *machine = MACHINE(opaque);
+
+ if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
+ ics_kvm_set_irq(spapr->ics, srcno, val);
+ } else {
+ ics_simple_set_irq(spapr->ics, srcno, val);
+ }
+}
+
#define SPAPR_IRQ_XICS_NR_IRQS 0x1000
#define SPAPR_IRQ_XICS_NR_MSIS \
(XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
@@ -235,6 +247,7 @@ sPAPRIrq spapr_irq_xics = {
.dt_populate = spapr_dt_xics,
.cpu_intc_create = spapr_irq_cpu_intc_create_xics,
.post_load = spapr_irq_post_load_xics,
+ .set_irq = spapr_irq_set_irq_xics,
};
/*
@@ -295,7 +308,6 @@ static void spapr_irq_free_xive(sPAPRMachineState *spapr, int irq, int num)
static qemu_irq spapr_qirq_xive(sPAPRMachineState *spapr, int irq)
{
sPAPRXive *xive = spapr->xive;
- XiveSource *xsrc = &xive->source;
if (irq >= xive->nr_irqs) {
return NULL;
@@ -304,7 +316,7 @@ static qemu_irq spapr_qirq_xive(sPAPRMachineState *spapr, int irq)
/* The sPAPR machine/device should have claimed the IRQ before */
assert(xive_eas_is_valid(&xive->eat[irq]));
- return xsrc->qirqs[irq];
+ return spapr->qirqs[irq];
}
static void spapr_irq_print_info_xive(sPAPRMachineState *spapr,
@@ -359,6 +371,13 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
}
}
+static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ xive_source_set_irq(&spapr->xive->source, srcno, val);
+}
+
/*
* XIVE uses the full IRQ number space. Set it to 8K to be compatible
* with XICS.
@@ -381,6 +400,7 @@ sPAPRIrq spapr_irq_xive = {
.cpu_intc_create = spapr_irq_cpu_intc_create_xive,
.post_load = spapr_irq_post_load_xive,
.reset = spapr_irq_reset_xive,
+ .set_irq = spapr_irq_set_irq_xive,
};
/*
@@ -394,6 +414,9 @@ void spapr_irq_init(sPAPRMachineState *spapr, Error **errp)
}
spapr->irq->init(spapr, errp);
+
+ spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr,
+ spapr->irq->nr_irqs);
}
int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp)
@@ -493,4 +516,5 @@ sPAPRIrq spapr_irq_xics_legacy = {
.dt_populate = spapr_dt_xics,
.cpu_intc_create = spapr_irq_cpu_intc_create_xics,
.post_load = spapr_irq_post_load_xics,
+ .set_irq = spapr_irq_set_irq_xics,
};
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (6 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 07/10] spapr: move the qemu_irq array under the machine Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-03 4:33 ` David Gibson
2019-01-02 5:57 ` [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS Cédric Le Goater
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
uninitialized") introduced an extra check on the ICS offset which is
not strictly necessary.
Revert the change to be able to map the XICS IRQ number space on the
XIVE IRQ number space.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/xics.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
index 7668c381a887..07508cbd217e 100644
--- a/include/hw/ppc/xics.h
+++ b/include/hw/ppc/xics.h
@@ -139,8 +139,7 @@ struct ICSState {
static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
{
- return (ics->offset != 0) && (nr >= ics->offset)
- && (nr < (ics->offset + ics->nr_irqs));
+ return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
}
struct ICSIRQState {
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0
2019-01-02 5:57 ` [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0 Cédric Le Goater
@ 2019-01-03 4:33 ` David Gibson
2019-01-03 17:45 ` Cédric Le Goater
0 siblings, 1 reply; 22+ messages in thread
From: David Gibson @ 2019-01-03 4:33 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]
On Wed, Jan 02, 2019 at 06:57:41AM +0100, Cédric Le Goater wrote:
> commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
> uninitialized") introduced an extra check on the ICS offset which is
> not strictly necessary.
The commit message for that suggests it was added to make pnv easier.
I take it you no longer need this for the current or expected pnv
code?
> Revert the change to be able to map the XICS IRQ number space on the
> XIVE IRQ number space.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/ppc/xics.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 7668c381a887..07508cbd217e 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -139,8 +139,7 @@ struct ICSState {
>
> static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
> {
> - return (ics->offset != 0) && (nr >= ics->offset)
> - && (nr < (ics->offset + ics->nr_irqs));
> + return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
> }
>
> struct ICSIRQState {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0
2019-01-03 4:33 ` David Gibson
@ 2019-01-03 17:45 ` Cédric Le Goater
2019-01-07 4:29 ` David Gibson
0 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-03 17:45 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 1/3/19 5:33 AM, David Gibson wrote:
> On Wed, Jan 02, 2019 at 06:57:41AM +0100, Cédric Le Goater wrote:
>> commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
>> uninitialized") introduced an extra check on the ICS offset which is
>> not strictly necessary.
>
> The commit message for that suggests it was added to make pnv easier.
> I take it you no longer need this for the current or expected pnv
> code?
This is really just a runtime check which makes sure that the
ICSState offset is initialized by the FW before being used.
The problem is more global. It comes from the fact the ICSState
offset in QEMU and the XICS offset in KVM is hardcoded to 0x1000.
Thinking aloud :
May be we should default the offset to 0 and introduce a service
to set the value in QEMU and KVM.
The 'dual' interrupt mode (using the qirq array at the machine level)
needs the IRQ number space of the XIVE and XICS interrupt mode to
be in sync, that is to start a 0x0, even if the lower 4K are ignored
in XICS.
So the 'dual' mode tunes the offset to 0 once the ICSState is created
and ignores the lower 4K when initializing the KVM XICS device.
C.
>> Revert the change to be able to map the XICS IRQ number space on the
>> XIVE IRQ number space.
>
>
>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> include/hw/ppc/xics.h | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 7668c381a887..07508cbd217e 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -139,8 +139,7 @@ struct ICSState {
>>
>> static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
>> {
>> - return (ics->offset != 0) && (nr >= ics->offset)
>> - && (nr < (ics->offset + ics->nr_irqs));
>> + return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
>> }
>>
>> struct ICSIRQState {
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0
2019-01-03 17:45 ` Cédric Le Goater
@ 2019-01-07 4:29 ` David Gibson
0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2019-01-07 4:29 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2421 bytes --]
On Thu, Jan 03, 2019 at 06:45:25PM +0100, Cédric Le Goater wrote:
> On 1/3/19 5:33 AM, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 06:57:41AM +0100, Cédric Le Goater wrote:
> >> commit 15ed653fa49a ("ppc/xics: An ICS with offset 0 is assumed to be
> >> uninitialized") introduced an extra check on the ICS offset which is
> >> not strictly necessary.
> >
> > The commit message for that suggests it was added to make pnv easier.
> > I take it you no longer need this for the current or expected pnv
> > code?
>
> This is really just a runtime check which makes sure that the
> ICSState offset is initialized by the FW before being used.
>
> The problem is more global. It comes from the fact the ICSState
> offset in QEMU and the XICS offset in KVM is hardcoded to 0x1000.
>
> Thinking aloud :
>
> May be we should default the offset to 0 and introduce a service
> to set the value in QEMU and KVM.
>
> The 'dual' interrupt mode (using the qirq array at the machine level)
> needs the IRQ number space of the XIVE and XICS interrupt mode to
> be in sync, that is to start a 0x0, even if the lower 4K are ignored
> in XICS.
>
> So the 'dual' mode tunes the offset to 0 once the ICSState is created
> and ignores the lower 4K when initializing the KVM XICS device.
Hm, ok.
I've applied patches 5..8.
>
> C.
>
> >> Revert the change to be able to map the XICS IRQ number space on the
> >> XIVE IRQ number space.
> >
> >
> >
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >> include/hw/ppc/xics.h | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> >> index 7668c381a887..07508cbd217e 100644
> >> --- a/include/hw/ppc/xics.h
> >> +++ b/include/hw/ppc/xics.h
> >> @@ -139,8 +139,7 @@ struct ICSState {
> >>
> >> static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
> >> {
> >> - return (ics->offset != 0) && (nr >= ics->offset)
> >> - && (nr < (ics->offset + ics->nr_irqs));
> >> + return (nr >= ics->offset) && (nr < (ics->offset + ics->nr_irqs));
> >> }
> >>
> >> struct ICSIRQState {
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (7 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 08/10] ppc/xics: allow ICSState to have an offset 0 Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-03 4:35 ` David Gibson
2019-01-02 5:57 ` [Qemu-devel] [PATCH 10/10] spapr: enable XIVE MMIOs at reset Cédric Le Goater
2019-01-07 4:48 ` [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE David Gibson
10 siblings, 1 reply; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
The 'dual' sPAPR IRQ backend supports both interrupt mode, XIVE
exploitation mode and the legacy compatibility mode (XICS). both modes
are not supported at the same time.
The machine starts with the legacy mode and a new interrupt mode can
then be negotiated by the CAS process. In this case, the new mode is
activated after a reset to take into account the required changes in
the machine. These impact the device tree layout, the interrupt
presenter object and the exposed MMIO regions in the case of XIVE.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/spapr_irq.h | 1 +
hw/ppc/spapr.c | 10 ++-
hw/ppc/spapr_hcall.c | 11 +++
hw/ppc/spapr_irq.c | 179 +++++++++++++++++++++++++++++++++++++
4 files changed, 198 insertions(+), 3 deletions(-)
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index 283bb5002c16..14b02c3aca33 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -52,6 +52,7 @@ typedef struct sPAPRIrq {
extern sPAPRIrq spapr_irq_xics;
extern sPAPRIrq spapr_irq_xics_legacy;
extern sPAPRIrq spapr_irq_xive;
+extern sPAPRIrq spapr_irq_dual;
void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5e8ffda47372..eb8ef741d860 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2633,11 +2633,11 @@ static void spapr_machine_init(MachineState *machine)
spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
/* advertise XIVE on POWER9 machines */
- if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
+ if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
if (ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
0, spapr->max_compat_pvr)) {
spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
- } else {
+ } else if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
error_report("XIVE-only machines require a POWER9 CPU");
exit(1);
}
@@ -3063,6 +3063,8 @@ static char *spapr_get_ic_mode(Object *obj, Error **errp)
return g_strdup("xics");
} else if (spapr->irq == &spapr_irq_xive) {
return g_strdup("xive");
+ } else if (spapr->irq == &spapr_irq_dual) {
+ return g_strdup("dual");
}
g_assert_not_reached();
}
@@ -3076,6 +3078,8 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp)
spapr->irq = &spapr_irq_xics;
} else if (strcmp(value, "xive") == 0) {
spapr->irq = &spapr_irq_xive;
+ } else if (strcmp(value, "dual") == 0) {
+ spapr->irq = &spapr_irq_dual;
} else {
error_setg(errp, "Bad value for \"ic-mode\" property");
}
@@ -3124,7 +3128,7 @@ static void spapr_instance_init(Object *obj)
object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
spapr_set_ic_mode, NULL);
object_property_set_description(obj, "ic-mode",
- "Specifies the interrupt controller mode (xics, xive)",
+ "Specifies the interrupt controller mode (xics, xive, dual)",
NULL);
}
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ae913d070f50..45c35d41fac6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1654,6 +1654,17 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
(spapr_h_cas_compose_response(spapr, args[1], args[2],
ov5_updates) != 0);
}
+
+ /*
+ * Generate a machine reset when we have an update of the
+ * interrupt mode. Only required when the machine supports both
+ * modes.
+ */
+ if (!spapr->cas_reboot) {
+ spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
+ && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
+ }
+
spapr_ovec_cleanup(ov5_updates);
if (spapr->cas_reboot) {
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index d23914887ac0..d110b8cdeec7 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -230,6 +230,11 @@ static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val)
}
}
+static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
+{
+ /* TODO: create the KVM XICS device */
+}
+
#define SPAPR_IRQ_XICS_NR_IRQS 0x1000
#define SPAPR_IRQ_XICS_NR_MSIS \
(XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
@@ -247,6 +252,7 @@ sPAPRIrq spapr_irq_xics = {
.dt_populate = spapr_dt_xics,
.cpu_intc_create = spapr_irq_cpu_intc_create_xics,
.post_load = spapr_irq_post_load_xics,
+ .reset = spapr_irq_reset_xics,
.set_irq = spapr_irq_set_irq_xics,
};
@@ -403,6 +409,179 @@ sPAPRIrq spapr_irq_xive = {
.set_irq = spapr_irq_set_irq_xive,
};
+/*
+ * Dual XIVE and XICS IRQ backend.
+ *
+ * Both interrupt mode, XIVE and XICS, objects are created but the
+ * machine starts in legacy interrupt mode (XICS). It can be changed
+ * by the CAS negotiation process and, in that case, the new mode is
+ * activated after an extra machine reset.
+ */
+
+/*
+ * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
+ * default.
+ */
+static sPAPRIrq *spapr_irq_current(sPAPRMachineState *spapr)
+{
+ return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
+ &spapr_irq_xive : &spapr_irq_xics;
+}
+
+static void spapr_irq_init_dual(sPAPRMachineState *spapr, Error **errp)
+{
+ MachineState *machine = MACHINE(spapr);
+ Error *local_err = NULL;
+
+ if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
+ error_setg(errp, "No KVM support for the 'dual' machine");
+ return;
+ }
+
+ spapr_irq_xics.init(spapr, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ /*
+ * Align the XICS and the XIVE IRQ number space under QEMU.
+ *
+ * However, the XICS KVM device still considers that the IRQ
+ * numbers should start at XICS_IRQ_BASE (0x1000). Either we
+ * should introduce a KVM device ioctl to set the offset or ignore
+ * the lower 4K numbers when using the get/set ioctl of the XICS
+ * KVM device. The second option seems the least intrusive.
+ */
+ spapr->ics->offset = 0;
+
+ spapr_irq_xive.init(spapr, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+}
+
+static int spapr_irq_claim_dual(sPAPRMachineState *spapr, int irq, bool lsi,
+ Error **errp)
+{
+ Error *local_err = NULL;
+ int ret;
+
+ ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return ret;
+ }
+
+ ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return ret;
+ }
+
+ return ret;
+}
+
+static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
+{
+ spapr_irq_xics.free(spapr, irq, num);
+ spapr_irq_xive.free(spapr, irq, num);
+}
+
+static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
+{
+ sPAPRXive *xive = spapr->xive;
+ ICSState *ics = spapr->ics;
+
+ if (irq >= spapr->irq->nr_irqs) {
+ return NULL;
+ }
+
+ /*
+ * The IRQ number should have been claimed under both interrupt
+ * controllers.
+ */
+ assert(!ICS_IRQ_FREE(ics, irq - ics->offset));
+ assert(xive_eas_is_valid(&xive->eat[irq]));
+
+ return spapr->qirqs[irq];
+}
+
+static void spapr_irq_print_info_dual(sPAPRMachineState *spapr, Monitor *mon)
+{
+ spapr_irq_current(spapr)->print_info(spapr, mon);
+}
+
+static void spapr_irq_dt_populate_dual(sPAPRMachineState *spapr,
+ uint32_t nr_servers, void *fdt,
+ uint32_t phandle)
+{
+ spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle);
+}
+
+static void spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
+ PowerPCCPU *cpu, Error **errp)
+{
+ Error *local_err = NULL;
+
+ spapr_irq_xive.cpu_intc_create(spapr, cpu, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ spapr_irq_xics.cpu_intc_create(spapr, cpu, errp);
+}
+
+static int spapr_irq_post_load_dual(sPAPRMachineState *spapr, int version_id)
+{
+ /*
+ * Force a reset of the XIVE backend after migration. The machine
+ * defaults to XICS at startup.
+ */
+ if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
+ spapr_irq_xive.reset(spapr, &error_fatal);
+ }
+
+ return spapr_irq_current(spapr)->post_load(spapr, version_id);
+}
+
+static void spapr_irq_reset_dual(sPAPRMachineState *spapr, Error **errp)
+{
+ spapr_irq_current(spapr)->reset(spapr, errp);
+}
+
+static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
+{
+ sPAPRMachineState *spapr = opaque;
+
+ spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
+}
+
+/*
+ * Define values in sync with the XIVE and XICS backend
+ */
+#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000
+#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI)
+
+sPAPRIrq spapr_irq_dual = {
+ .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS,
+ .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS,
+ .ov5 = SPAPR_OV5_XIVE_BOTH,
+
+ .init = spapr_irq_init_dual,
+ .claim = spapr_irq_claim_dual,
+ .free = spapr_irq_free_dual,
+ .qirq = spapr_qirq_dual,
+ .print_info = spapr_irq_print_info_dual,
+ .dt_populate = spapr_irq_dt_populate_dual,
+ .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
+ .post_load = spapr_irq_post_load_dual,
+ .reset = spapr_irq_reset_dual,
+ .set_irq = spapr_irq_set_irq_dual
+};
+
/*
* sPAPR IRQ frontend routines for devices
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
2019-01-02 5:57 ` [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS Cédric Le Goater
@ 2019-01-03 4:35 ` David Gibson
2019-01-03 17:45 ` Cédric Le Goater
0 siblings, 1 reply; 22+ messages in thread
From: David Gibson @ 2019-01-03 4:35 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 11409 bytes --]
On Wed, Jan 02, 2019 at 06:57:42AM +0100, Cédric Le Goater wrote:
> The 'dual' sPAPR IRQ backend supports both interrupt mode, XIVE
> exploitation mode and the legacy compatibility mode (XICS). both modes
> are not supported at the same time.
>
> The machine starts with the legacy mode and a new interrupt mode can
> then be negotiated by the CAS process. In this case, the new mode is
> activated after a reset to take into account the required changes in
> the machine. These impact the device tree layout, the interrupt
> presenter object and the exposed MMIO regions in the case of XIVE.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/ppc/spapr_irq.h | 1 +
> hw/ppc/spapr.c | 10 ++-
> hw/ppc/spapr_hcall.c | 11 +++
> hw/ppc/spapr_irq.c | 179 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 198 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 283bb5002c16..14b02c3aca33 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -52,6 +52,7 @@ typedef struct sPAPRIrq {
> extern sPAPRIrq spapr_irq_xics;
> extern sPAPRIrq spapr_irq_xics_legacy;
> extern sPAPRIrq spapr_irq_xive;
> +extern sPAPRIrq spapr_irq_dual;
>
> void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
> int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5e8ffda47372..eb8ef741d860 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2633,11 +2633,11 @@ static void spapr_machine_init(MachineState *machine)
> spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>
> /* advertise XIVE on POWER9 machines */
> - if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> + if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
> if (ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
> 0, spapr->max_compat_pvr)) {
> spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> - } else {
> + } else if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> error_report("XIVE-only machines require a POWER9 CPU");
> exit(1);
> }
> @@ -3063,6 +3063,8 @@ static char *spapr_get_ic_mode(Object *obj, Error **errp)
> return g_strdup("xics");
> } else if (spapr->irq == &spapr_irq_xive) {
> return g_strdup("xive");
> + } else if (spapr->irq == &spapr_irq_dual) {
> + return g_strdup("dual");
> }
> g_assert_not_reached();
> }
> @@ -3076,6 +3078,8 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp)
> spapr->irq = &spapr_irq_xics;
> } else if (strcmp(value, "xive") == 0) {
> spapr->irq = &spapr_irq_xive;
> + } else if (strcmp(value, "dual") == 0) {
> + spapr->irq = &spapr_irq_dual;
> } else {
> error_setg(errp, "Bad value for \"ic-mode\" property");
> }
> @@ -3124,7 +3128,7 @@ static void spapr_instance_init(Object *obj)
> object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
> spapr_set_ic_mode, NULL);
> object_property_set_description(obj, "ic-mode",
> - "Specifies the interrupt controller mode (xics, xive)",
> + "Specifies the interrupt controller mode (xics, xive, dual)",
> NULL);
> }
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..45c35d41fac6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1654,6 +1654,17 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
> (spapr_h_cas_compose_response(spapr, args[1], args[2],
> ov5_updates) != 0);
> }
> +
> + /*
> + * Generate a machine reset when we have an update of the
> + * interrupt mode. Only required when the machine supports both
> + * modes.
> + */
> + if (!spapr->cas_reboot) {
> + spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> + && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
> + }
> +
> spapr_ovec_cleanup(ov5_updates);
>
> if (spapr->cas_reboot) {
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d23914887ac0..d110b8cdeec7 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -230,6 +230,11 @@ static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val)
> }
> }
>
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> + /* TODO: create the KVM XICS device */
> +}
> +
> #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> #define SPAPR_IRQ_XICS_NR_MSIS \
> (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -247,6 +252,7 @@ sPAPRIrq spapr_irq_xics = {
> .dt_populate = spapr_dt_xics,
> .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
> .post_load = spapr_irq_post_load_xics,
> + .reset = spapr_irq_reset_xics,
> .set_irq = spapr_irq_set_irq_xics,
> };
>
> @@ -403,6 +409,179 @@ sPAPRIrq spapr_irq_xive = {
> .set_irq = spapr_irq_set_irq_xive,
> };
>
> +/*
> + * Dual XIVE and XICS IRQ backend.
> + *
> + * Both interrupt mode, XIVE and XICS, objects are created but the
> + * machine starts in legacy interrupt mode (XICS). It can be changed
> + * by the CAS negotiation process and, in that case, the new mode is
> + * activated after an extra machine reset.
> + */
> +
> +/*
> + * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
> + * default.
> + */
> +static sPAPRIrq *spapr_irq_current(sPAPRMachineState *spapr)
> +{
> + return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
> + &spapr_irq_xive : &spapr_irq_xics;
> +}
> +
> +static void spapr_irq_init_dual(sPAPRMachineState *spapr, Error **errp)
> +{
> + MachineState *machine = MACHINE(spapr);
> + Error *local_err = NULL;
> +
> + if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> + error_setg(errp, "No KVM support for the 'dual' machine");
> + return;
> + }
> +
> + spapr_irq_xics.init(spapr, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + /*
> + * Align the XICS and the XIVE IRQ number space under QEMU.
> + *
> + * However, the XICS KVM device still considers that the IRQ
> + * numbers should start at XICS_IRQ_BASE (0x1000). Either we
> + * should introduce a KVM device ioctl to set the offset or ignore
> + * the lower 4K numbers when using the get/set ioctl of the XICS
> + * KVM device. The second option seems the least intrusive.
> + */
> + spapr->ics->offset = 0;
> +
> + spapr_irq_xive.init(spapr, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +}
> +
> +static int spapr_irq_claim_dual(sPAPRMachineState *spapr, int irq, bool lsi,
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + int ret;
> +
> + ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return ret;
> + }
> +
> + ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
> +{
> + spapr_irq_xics.free(spapr, irq, num);
> + spapr_irq_xive.free(spapr, irq, num);
> +}
> +
> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
If the qirq array is now at the machine level, you shouldn't need an
irq backend version of the qirq function should you? Just a backend
specific version of set_irq.
> +{
> + sPAPRXive *xive = spapr->xive;
> + ICSState *ics = spapr->ics;
> +
> + if (irq >= spapr->irq->nr_irqs) {
> + return NULL;
> + }
> +
> + /*
> + * The IRQ number should have been claimed under both interrupt
> + * controllers.
> + */
> + assert(!ICS_IRQ_FREE(ics, irq - ics->offset));
> + assert(xive_eas_is_valid(&xive->eat[irq]));
> +
> + return spapr->qirqs[irq];
> +}
> +
> +static void spapr_irq_print_info_dual(sPAPRMachineState *spapr, Monitor *mon)
> +{
> + spapr_irq_current(spapr)->print_info(spapr, mon);
> +}
> +
> +static void spapr_irq_dt_populate_dual(sPAPRMachineState *spapr,
> + uint32_t nr_servers, void *fdt,
> + uint32_t phandle)
> +{
> + spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle);
> +}
> +
> +static void spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
> + PowerPCCPU *cpu, Error **errp)
> +{
> + Error *local_err = NULL;
> +
> + spapr_irq_xive.cpu_intc_create(spapr, cpu, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + spapr_irq_xics.cpu_intc_create(spapr, cpu, errp);
> +}
> +
> +static int spapr_irq_post_load_dual(sPAPRMachineState *spapr, int version_id)
> +{
> + /*
> + * Force a reset of the XIVE backend after migration. The machine
> + * defaults to XICS at startup.
> + */
> + if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> + spapr_irq_xive.reset(spapr, &error_fatal);
> + }
> +
> + return spapr_irq_current(spapr)->post_load(spapr, version_id);
> +}
> +
> +static void spapr_irq_reset_dual(sPAPRMachineState *spapr, Error **errp)
> +{
> + spapr_irq_current(spapr)->reset(spapr, errp);
> +}
> +
> +static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
> +{
> + sPAPRMachineState *spapr = opaque;
> +
> + spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
> +}
> +
> +/*
> + * Define values in sync with the XIVE and XICS backend
> + */
> +#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000
> +#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI)
> +
> +sPAPRIrq spapr_irq_dual = {
> + .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS,
> + .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS,
> + .ov5 = SPAPR_OV5_XIVE_BOTH,
> +
> + .init = spapr_irq_init_dual,
> + .claim = spapr_irq_claim_dual,
> + .free = spapr_irq_free_dual,
> + .qirq = spapr_qirq_dual,
> + .print_info = spapr_irq_print_info_dual,
> + .dt_populate = spapr_irq_dt_populate_dual,
> + .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> + .post_load = spapr_irq_post_load_dual,
> + .reset = spapr_irq_reset_dual,
> + .set_irq = spapr_irq_set_irq_dual
> +};
> +
> /*
> * sPAPR IRQ frontend routines for devices
> */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
2019-01-03 4:35 ` David Gibson
@ 2019-01-03 17:45 ` Cédric Le Goater
0 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-03 17:45 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 1/3/19 5:35 AM, David Gibson wrote:
> On Wed, Jan 02, 2019 at 06:57:42AM +0100, Cédric Le Goater wrote:
>> The 'dual' sPAPR IRQ backend supports both interrupt mode, XIVE
>> exploitation mode and the legacy compatibility mode (XICS). both modes
>> are not supported at the same time.
>>
>> The machine starts with the legacy mode and a new interrupt mode can
>> then be negotiated by the CAS process. In this case, the new mode is
>> activated after a reset to take into account the required changes in
>> the machine. These impact the device tree layout, the interrupt
>> presenter object and the exposed MMIO regions in the case of XIVE.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> include/hw/ppc/spapr_irq.h | 1 +
>> hw/ppc/spapr.c | 10 ++-
>> hw/ppc/spapr_hcall.c | 11 +++
>> hw/ppc/spapr_irq.c | 179 +++++++++++++++++++++++++++++++++++++
>> 4 files changed, 198 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index 283bb5002c16..14b02c3aca33 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -52,6 +52,7 @@ typedef struct sPAPRIrq {
>> extern sPAPRIrq spapr_irq_xics;
>> extern sPAPRIrq spapr_irq_xics_legacy;
>> extern sPAPRIrq spapr_irq_xive;
>> +extern sPAPRIrq spapr_irq_dual;
>>
>> void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
>> int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 5e8ffda47372..eb8ef741d860 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2633,11 +2633,11 @@ static void spapr_machine_init(MachineState *machine)
>> spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>>
>> /* advertise XIVE on POWER9 machines */
>> - if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
>> + if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
>> if (ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00,
>> 0, spapr->max_compat_pvr)) {
>> spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
>> - } else {
>> + } else if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
>> error_report("XIVE-only machines require a POWER9 CPU");
>> exit(1);
>> }
>> @@ -3063,6 +3063,8 @@ static char *spapr_get_ic_mode(Object *obj, Error **errp)
>> return g_strdup("xics");
>> } else if (spapr->irq == &spapr_irq_xive) {
>> return g_strdup("xive");
>> + } else if (spapr->irq == &spapr_irq_dual) {
>> + return g_strdup("dual");
>> }
>> g_assert_not_reached();
>> }
>> @@ -3076,6 +3078,8 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp)
>> spapr->irq = &spapr_irq_xics;
>> } else if (strcmp(value, "xive") == 0) {
>> spapr->irq = &spapr_irq_xive;
>> + } else if (strcmp(value, "dual") == 0) {
>> + spapr->irq = &spapr_irq_dual;
>> } else {
>> error_setg(errp, "Bad value for \"ic-mode\" property");
>> }
>> @@ -3124,7 +3128,7 @@ static void spapr_instance_init(Object *obj)
>> object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
>> spapr_set_ic_mode, NULL);
>> object_property_set_description(obj, "ic-mode",
>> - "Specifies the interrupt controller mode (xics, xive)",
>> + "Specifies the interrupt controller mode (xics, xive, dual)",
>> NULL);
>> }
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index ae913d070f50..45c35d41fac6 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1654,6 +1654,17 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>> (spapr_h_cas_compose_response(spapr, args[1], args[2],
>> ov5_updates) != 0);
>> }
>> +
>> + /*
>> + * Generate a machine reset when we have an update of the
>> + * interrupt mode. Only required when the machine supports both
>> + * modes.
>> + */
>> + if (!spapr->cas_reboot) {
>> + spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
>> + && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
>> + }
>> +
>> spapr_ovec_cleanup(ov5_updates);
>>
>> if (spapr->cas_reboot) {
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index d23914887ac0..d110b8cdeec7 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -230,6 +230,11 @@ static void spapr_irq_set_irq_xics(void *opaque, int srcno, int val)
>> }
>> }
>>
>> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>> +{
>> + /* TODO: create the KVM XICS device */
>> +}
>> +
>> #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
>> #define SPAPR_IRQ_XICS_NR_MSIS \
>> (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
>> @@ -247,6 +252,7 @@ sPAPRIrq spapr_irq_xics = {
>> .dt_populate = spapr_dt_xics,
>> .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>> .post_load = spapr_irq_post_load_xics,
>> + .reset = spapr_irq_reset_xics,
>> .set_irq = spapr_irq_set_irq_xics,
>> };
>>
>> @@ -403,6 +409,179 @@ sPAPRIrq spapr_irq_xive = {
>> .set_irq = spapr_irq_set_irq_xive,
>> };
>>
>> +/*
>> + * Dual XIVE and XICS IRQ backend.
>> + *
>> + * Both interrupt mode, XIVE and XICS, objects are created but the
>> + * machine starts in legacy interrupt mode (XICS). It can be changed
>> + * by the CAS negotiation process and, in that case, the new mode is
>> + * activated after an extra machine reset.
>> + */
>> +
>> +/*
>> + * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
>> + * default.
>> + */
>> +static sPAPRIrq *spapr_irq_current(sPAPRMachineState *spapr)
>> +{
>> + return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
>> + &spapr_irq_xive : &spapr_irq_xics;
>> +}
>> +
>> +static void spapr_irq_init_dual(sPAPRMachineState *spapr, Error **errp)
>> +{
>> + MachineState *machine = MACHINE(spapr);
>> + Error *local_err = NULL;
>> +
>> + if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
>> + error_setg(errp, "No KVM support for the 'dual' machine");
>> + return;
>> + }
>> +
>> + spapr_irq_xics.init(spapr, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> +
>> + /*
>> + * Align the XICS and the XIVE IRQ number space under QEMU.
>> + *
>> + * However, the XICS KVM device still considers that the IRQ
>> + * numbers should start at XICS_IRQ_BASE (0x1000). Either we
>> + * should introduce a KVM device ioctl to set the offset or ignore
>> + * the lower 4K numbers when using the get/set ioctl of the XICS
>> + * KVM device. The second option seems the least intrusive.
>> + */
>> + spapr->ics->offset = 0;
>> +
>> + spapr_irq_xive.init(spapr, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> +}
>> +
>> +static int spapr_irq_claim_dual(sPAPRMachineState *spapr, int irq, bool lsi,
>> + Error **errp)
>> +{
>> + Error *local_err = NULL;
>> + int ret;
>> +
>> + ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return ret;
>> + }
>> +
>> + ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return ret;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
>> +{
>> + spapr_irq_xics.free(spapr, irq, num);
>> + spapr_irq_xive.free(spapr, irq, num);
>> +}
>> +
>> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
>
> If the qirq array is now at the machine level, you shouldn't need an
> irq backend version of the qirq function should you? Just a backend
> specific version of set_irq.
The qirq function of the 'xics' backend takes into account the IRQ number
offset which is different (0x1000 vs. 0x0) and the qirq function of the
'dual' backend adds some extra checks on the IRQ number which should have
been claimed by both XICS and XIVE interrupt modes. This check might be a
little over kill.
We could improve things if we could find a way to get rid of the ICSState
offset which is spread all over the ics_* routine. I haven't found an
obvious way to do so.
C.
>
>> +{
>> + sPAPRXive *xive = spapr->xive;
>> + ICSState *ics = spapr->ics;
>> +
>> + if (irq >= spapr->irq->nr_irqs) {
>> + return NULL;
>> + }
>> +
>> + /*
>> + * The IRQ number should have been claimed under both interrupt
>> + * controllers.
>> + */
>> + assert(!ICS_IRQ_FREE(ics, irq - ics->offset));
>> + assert(xive_eas_is_valid(&xive->eat[irq]));
>> +
>> + return spapr->qirqs[irq];
>> +}
>> +
>> +static void spapr_irq_print_info_dual(sPAPRMachineState *spapr, Monitor *mon)
>> +{
>> + spapr_irq_current(spapr)->print_info(spapr, mon);
>> +}
>> +
>> +static void spapr_irq_dt_populate_dual(sPAPRMachineState *spapr,
>> + uint32_t nr_servers, void *fdt,
>> + uint32_t phandle)
>> +{
>> + spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle);
>> +}
>> +
>> +static void spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
>> + PowerPCCPU *cpu, Error **errp)
>> +{
>> + Error *local_err = NULL;
>> +
>> + spapr_irq_xive.cpu_intc_create(spapr, cpu, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> +
>> + spapr_irq_xics.cpu_intc_create(spapr, cpu, errp);
>> +}
>> +
>> +static int spapr_irq_post_load_dual(sPAPRMachineState *spapr, int version_id)
>> +{
>> + /*
>> + * Force a reset of the XIVE backend after migration. The machine
>> + * defaults to XICS at startup.
>> + */
>> + if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
>> + spapr_irq_xive.reset(spapr, &error_fatal);
>> + }
>> +
>> + return spapr_irq_current(spapr)->post_load(spapr, version_id);
>> +}
>> +
>> +static void spapr_irq_reset_dual(sPAPRMachineState *spapr, Error **errp)
>> +{
>> + spapr_irq_current(spapr)->reset(spapr, errp);
>> +}
>> +
>> +static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
>> +{
>> + sPAPRMachineState *spapr = opaque;
>> +
>> + spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
>> +}
>> +
>> +/*
>> + * Define values in sync with the XIVE and XICS backend
>> + */
>> +#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000
>> +#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI)
>> +
>> +sPAPRIrq spapr_irq_dual = {
>> + .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS,
>> + .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS,
>> + .ov5 = SPAPR_OV5_XIVE_BOTH,
>> +
>> + .init = spapr_irq_init_dual,
>> + .claim = spapr_irq_claim_dual,
>> + .free = spapr_irq_free_dual,
>> + .qirq = spapr_qirq_dual,
>> + .print_info = spapr_irq_print_info_dual,
>> + .dt_populate = spapr_irq_dt_populate_dual,
>> + .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
>> + .post_load = spapr_irq_post_load_dual,
>> + .reset = spapr_irq_reset_dual,
>> + .set_irq = spapr_irq_set_irq_dual
>> +};
>> +
>> /*
>> * sPAPR IRQ frontend routines for devices
>> */
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 10/10] spapr: enable XIVE MMIOs at reset
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (8 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS Cédric Le Goater
@ 2019-01-02 5:57 ` Cédric Le Goater
2019-01-07 4:48 ` [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE David Gibson
10 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-02 5:57 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel, Cédric Le Goater
Depending on the interrupt mode of the machine, enable or disable the
XIVE MMIOs.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/spapr_xive.h | 1 +
hw/intc/spapr_xive.c | 9 +++++++++
hw/ppc/spapr_irq.c | 9 +++++++++
3 files changed, 19 insertions(+)
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 9ee524fdb218..7fdc25057420 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -47,5 +47,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle);
void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);
+void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable);
#endif /* PPC_SPAPR_XIVE_H */
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index eea28337e807..d391177ab81f 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
}
+void spapr_xive_mmio_set_enabled(sPAPRXive *xive, bool enable)
+{
+ memory_region_set_enabled(&xive->source.esb_mmio, enable);
+ memory_region_set_enabled(&xive->tm_mmio, enable);
+
+ /* Disable the END ESBs until a guest OS makes use of them */
+ memory_region_set_enabled(&xive->end_source.esb_mmio, false);
+}
+
/*
* When a Virtual Processor is scheduled to run on a HW thread, the
* hypervisor pushes its identifier in the OS CAM line. Emulate the
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index d110b8cdeec7..5fce72fe0f6c 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -375,6 +375,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
/* (TCG) Set the OS CAM line of the thread interrupt context. */
spapr_xive_set_tctx_os_cam(cpu->tctx);
}
+
+ /* Activate the XIVE MMIOs */
+ spapr_xive_mmio_set_enabled(spapr->xive, true);
}
static void spapr_irq_set_irq_xive(void *opaque, int srcno, int val)
@@ -549,6 +552,12 @@ static int spapr_irq_post_load_dual(sPAPRMachineState *spapr, int version_id)
static void spapr_irq_reset_dual(sPAPRMachineState *spapr, Error **errp)
{
+ /*
+ * Deactivate the XIVE MMIOs. The XIVE backend will reenable them
+ * if selected.
+ */
+ spapr_xive_mmio_set_enabled(spapr->xive, false);
+
spapr_irq_current(spapr)->reset(spapr, errp);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE
2019-01-02 5:57 [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE Cédric Le Goater
` (9 preceding siblings ...)
2019-01-02 5:57 ` [Qemu-devel] [PATCH 10/10] spapr: enable XIVE MMIOs at reset Cédric Le Goater
@ 2019-01-07 4:48 ` David Gibson
2019-01-07 6:54 ` Cédric Le Goater
10 siblings, 1 reply; 22+ messages in thread
From: David Gibson @ 2019-01-07 4:48 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3101 bytes --]
On Wed, Jan 02, 2019 at 06:57:33AM +0100, Cédric Le Goater wrote:
> Hello,
>
> This series adds a new sPAPR IRQ backend called 'dual' which supports
> both interrupt mode, the XIVE native exploitation mode and the legacy
> compatibility mode (XICS).
>
> The machine operates with the legacy mode by default and lets CAS
> negotiate a new interrupt mode. If a new mode is selected, it is
> activated after a machine reset to take into account the required
> changes. These impact the device tree layout, the interrupt presenter
> object and the exposed MMIO regions in the case of XIVE.
>
> The preliminary changes for this new IRQ backend are the introduction
> of a second interrupt presenter object under the PowerPCCPU to support
> XIVE. The qemu_irq array of each interrupt controller model is also
> made common and moved under the machine.
Ok, I've now applied all of this series to ppc-for-4.0.
>
>
> GitHub trees available here :
>
> QEMU sPAPR:
>
> https://github.com/legoater/qemu/commits/xive-next
>
> QEMU PowerNV:
>
> https://github.com/legoater/qemu/commits/powernv-3.1
>
> Linux/KVM:
>
> https://github.com/legoater/linux/commits/xive-4.20
>
> OPAL:
>
> https://github.com/legoater/skiboot/commits/xive
>
> Best wishes for 2019 !
>
> C.
>
>
>
> Cédric Le Goater (10):
> spapr: modify the prototype of the cpu_intc_create() method
> ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
> ppc: replace the 'Object *intc' by a 'ICPState *icp' pointer under the
> CPU
> spapr/xive: simplify the sPAPR IRQ qirq method for XIVE
> ppc: export the XICS and XIVE set_irq handlers
> pnv/psi: move the ICSState qemu_irq array under the PSI device model
> spapr: move the ICSState qemu_irq array under the machine
> ppc/xics: allow ICSState to have an offset 0
> spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
> spapr: enable XIVE MMIOs at reset
>
> include/hw/ppc/pnv.h | 2 +-
> include/hw/ppc/pnv_psi.h | 1 +
> include/hw/ppc/spapr.h | 1 +
> include/hw/ppc/spapr_irq.h | 6 +-
> include/hw/ppc/spapr_xive.h | 2 +-
> include/hw/ppc/xics.h | 6 +-
> include/hw/ppc/xive.h | 9 +-
> target/ppc/cpu.h | 5 +-
> hw/intc/spapr_xive.c | 23 ++-
> hw/intc/xics.c | 4 +-
> hw/intc/xics_kvm.c | 3 +-
> hw/intc/xics_spapr.c | 10 +-
> hw/intc/xive.c | 11 +-
> hw/ppc/pnv.c | 27 ++--
> hw/ppc/pnv_core.c | 4 +-
> hw/ppc/pnv_psi.c | 7 +-
> hw/ppc/spapr.c | 12 +-
> hw/ppc/spapr_cpu_core.c | 9 +-
> hw/ppc/spapr_hcall.c | 11 ++
> hw/ppc/spapr_irq.c | 270 ++++++++++++++++++++++++++++++++++--
> 20 files changed, 342 insertions(+), 81 deletions(-)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE
2019-01-07 4:48 ` [Qemu-devel] [PATCH 00/10] spapr: introduce the 'dual' interrupt mode XICS/XIVE David Gibson
@ 2019-01-07 6:54 ` Cédric Le Goater
0 siblings, 0 replies; 22+ messages in thread
From: Cédric Le Goater @ 2019-01-07 6:54 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 1/7/19 5:48 AM, David Gibson wrote:
> On Wed, Jan 02, 2019 at 06:57:33AM +0100, Cédric Le Goater wrote:
>> Hello,
>>
>> This series adds a new sPAPR IRQ backend called 'dual' which supports
>> both interrupt mode, the XIVE native exploitation mode and the legacy
>> compatibility mode (XICS).
>>
>> The machine operates with the legacy mode by default and lets CAS
>> negotiate a new interrupt mode. If a new mode is selected, it is
>> activated after a machine reset to take into account the required
>> changes. These impact the device tree layout, the interrupt presenter
>> object and the exposed MMIO regions in the case of XIVE.
>>
>> The preliminary changes for this new IRQ backend are the introduction
>> of a second interrupt presenter object under the PowerPCCPU to support
>> XIVE. The qemu_irq array of each interrupt controller model is also
>> made common and moved under the machine.
>
> Ok, I've now applied all of this series to ppc-for-4.0.
Thanks,
We should pursue with KVM XIVE support now. Shall I send the Linux KVM
and the QEMU KVM patchsets in parallel ?
I still have some work to be done on the QEMU PowerNV before resending.
Thanks,
C.
>
>>
>>
>> GitHub trees available here :
>>
>> QEMU sPAPR:
>>
>> https://github.com/legoater/qemu/commits/xive-next
>>
>> QEMU PowerNV:
>>
>> https://github.com/legoater/qemu/commits/powernv-3.1
>>
>> Linux/KVM:
>>
>> https://github.com/legoater/linux/commits/xive-4.20
>>
>> OPAL:
>>
>> https://github.com/legoater/skiboot/commits/xive
>>
>> Best wishes for 2019 !
>>
>> C.
>>
>>
>>
>> Cédric Le Goater (10):
>> spapr: modify the prototype of the cpu_intc_create() method
>> ppc/xive: introduce a XiveTCTX pointer under PowerPCCPU
>> ppc: replace the 'Object *intc' by a 'ICPState *icp' pointer under the
>> CPU
>> spapr/xive: simplify the sPAPR IRQ qirq method for XIVE
>> ppc: export the XICS and XIVE set_irq handlers
>> pnv/psi: move the ICSState qemu_irq array under the PSI device model
>> spapr: move the ICSState qemu_irq array under the machine
>> ppc/xics: allow ICSState to have an offset 0
>> spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS
>> spapr: enable XIVE MMIOs at reset
>>
>> include/hw/ppc/pnv.h | 2 +-
>> include/hw/ppc/pnv_psi.h | 1 +
>> include/hw/ppc/spapr.h | 1 +
>> include/hw/ppc/spapr_irq.h | 6 +-
>> include/hw/ppc/spapr_xive.h | 2 +-
>> include/hw/ppc/xics.h | 6 +-
>> include/hw/ppc/xive.h | 9 +-
>> target/ppc/cpu.h | 5 +-
>> hw/intc/spapr_xive.c | 23 ++-
>> hw/intc/xics.c | 4 +-
>> hw/intc/xics_kvm.c | 3 +-
>> hw/intc/xics_spapr.c | 10 +-
>> hw/intc/xive.c | 11 +-
>> hw/ppc/pnv.c | 27 ++--
>> hw/ppc/pnv_core.c | 4 +-
>> hw/ppc/pnv_psi.c | 7 +-
>> hw/ppc/spapr.c | 12 +-
>> hw/ppc/spapr_cpu_core.c | 9 +-
>> hw/ppc/spapr_hcall.c | 11 ++
>> hw/ppc/spapr_irq.c | 270 ++++++++++++++++++++++++++++++++++--
>> 20 files changed, 342 insertions(+), 81 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread