* [PATCH v2 0/2] ppc/xive: Add support for PQ state bits offload
@ 2020-04-01 16:46 Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 1/2] ppc/xive: export PQ routines Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
0 siblings, 2 replies; 5+ messages in thread
From: Cédric Le Goater @ 2020-04-01 16:46 UTC (permalink / raw)
To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel
Hello,
When the XIVE router unit receives a trigger message coming from a HW
source, it contains a special bit informing the XIVE interrupt
controller that the PQ bits have been checked at the source or
not. Depending on the value, the IC can perform the check and the
state transition locally using its own PQ state bits.
The following changes add new accessors to the XiveRouter required to
query and update the PQ state bits. This is only applies to the
PowerNV machine, sPAPR is not concerned by such complex configuration.
We will use it for upcoming features offloading event coalescing on
the interrupt controller.
Thanks,
C.
Cédric Le Goater (2):
ppc/xive: export PQ routines
ppc/xive: Add support for PQ state bits offload
include/hw/ppc/xive.h | 12 +++++++--
hw/intc/pnv_xive.c | 37 ++++++++++++++++++++++++---
hw/intc/spapr_xive_kvm.c | 8 +++---
hw/intc/xive.c | 54 ++++++++++++++++++++++++++++++++--------
hw/pci-host/pnv_phb4.c | 9 +++++--
hw/ppc/pnv_psi.c | 8 ++++--
6 files changed, 105 insertions(+), 23 deletions(-)
--
2.21.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] ppc/xive: export PQ routines
2020-04-01 16:46 [PATCH v2 0/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
@ 2020-04-01 16:46 ` Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
1 sibling, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2020-04-01 16:46 UTC (permalink / raw)
To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
include/hw/ppc/xive.h | 4 ++++
hw/intc/spapr_xive_kvm.c | 8 ++++----
hw/intc/xive.c | 6 +++---
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 705cf48176fc..112fb6fb6dbe 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -255,6 +255,10 @@ static inline hwaddr xive_source_esb_mgmt(XiveSource *xsrc, int srcno)
#define XIVE_ESB_QUEUED (XIVE_ESB_VAL_P | XIVE_ESB_VAL_Q)
#define XIVE_ESB_OFF XIVE_ESB_VAL_Q
+bool xive_esb_trigger(uint8_t *pq);
+bool xive_esb_eoi(uint8_t *pq);
+uint8_t xive_esb_set(uint8_t *pq, uint8_t value);
+
/*
* "magic" Event State Buffer (ESB) MMIO offsets.
*
diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index edb7ee0e74f1..43f4d56b958c 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -308,7 +308,7 @@ static uint8_t xive_esb_read(XiveSource *xsrc, int srcno, uint32_t offset)
return xive_esb_rw(xsrc, srcno, offset, 0, 0) & 0x3;
}
-static void xive_esb_trigger(XiveSource *xsrc, int srcno)
+static void kvmppc_xive_esb_trigger(XiveSource *xsrc, int srcno)
{
uint64_t *addr = xsrc->esb_mmap + xive_source_esb_page(xsrc, srcno);
@@ -331,7 +331,7 @@ uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
offset == XIVE_ESB_LOAD_EOI) {
xive_esb_read(xsrc, srcno, XIVE_ESB_SET_PQ_00);
if (xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
- xive_esb_trigger(xsrc, srcno);
+ kvmppc_xive_esb_trigger(xsrc, srcno);
}
return 0;
} else {
@@ -375,7 +375,7 @@ void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val)
}
}
- xive_esb_trigger(xsrc, srcno);
+ kvmppc_xive_esb_trigger(xsrc, srcno);
}
/*
@@ -544,7 +544,7 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running,
* generate a trigger.
*/
if (pq == XIVE_ESB_RESET && old_pq == XIVE_ESB_QUEUED) {
- xive_esb_trigger(xsrc, i);
+ kvmppc_xive_esb_trigger(xsrc, i);
}
}
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index d6183f8ae40a..b8825577f719 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -789,7 +789,7 @@ void xive_tctx_destroy(XiveTCTX *tctx)
* XIVE ESB helpers
*/
-static uint8_t xive_esb_set(uint8_t *pq, uint8_t value)
+uint8_t xive_esb_set(uint8_t *pq, uint8_t value)
{
uint8_t old_pq = *pq & 0x3;
@@ -799,7 +799,7 @@ static uint8_t xive_esb_set(uint8_t *pq, uint8_t value)
return old_pq;
}
-static bool xive_esb_trigger(uint8_t *pq)
+bool xive_esb_trigger(uint8_t *pq)
{
uint8_t old_pq = *pq & 0x3;
@@ -819,7 +819,7 @@ static bool xive_esb_trigger(uint8_t *pq)
}
}
-static bool xive_esb_eoi(uint8_t *pq)
+bool xive_esb_eoi(uint8_t *pq)
{
uint8_t old_pq = *pq & 0x3;
--
2.21.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload
2020-04-01 16:46 [PATCH v2 0/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 1/2] ppc/xive: export PQ routines Cédric Le Goater
@ 2020-04-01 16:46 ` Cédric Le Goater
2020-04-02 10:51 ` Greg Kurz
1 sibling, 1 reply; 5+ messages in thread
From: Cédric Le Goater @ 2020-04-01 16:46 UTC (permalink / raw)
To: David Gibson; +Cc: Cédric Le Goater, qemu-ppc, Greg Kurz, qemu-devel
The trigger message coming from a HW source contains a special bit
informing the XIVE interrupt controller that the PQ bits have been
checked at the source or not. Depending on the value, the IC can
perform the check and the state transition locally using its own PQ
state bits.
The following changes add new accessors to the XiveRouter required to
query and update the PQ state bits. This is only applies to the
PowerNV machine, sPAPR is not concerned by such complex configuration.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/ppc/xive.h | 8 +++++--
hw/intc/pnv_xive.c | 37 +++++++++++++++++++++++++++++---
hw/intc/xive.c | 48 ++++++++++++++++++++++++++++++++++++------
hw/pci-host/pnv_phb4.c | 9 ++++++--
hw/ppc/pnv_psi.c | 8 +++++--
5 files changed, 94 insertions(+), 16 deletions(-)
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 112fb6fb6dbe..050e49c14fd9 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -160,7 +160,7 @@ typedef struct XiveNotifier XiveNotifier;
typedef struct XiveNotifierClass {
InterfaceClass parent;
- void (*notify)(XiveNotifier *xn, uint32_t lisn);
+ void (*notify)(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
} XiveNotifierClass;
/*
@@ -354,6 +354,10 @@ typedef struct XiveRouterClass {
/* XIVE table accessors */
int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
XiveEAS *eas);
+ int (*get_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
+ uint8_t *pq);
+ int (*set_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
+ uint8_t *pq);
int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
XiveEND *end);
int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
@@ -375,7 +379,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
XiveNVT *nvt);
int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
XiveNVT *nvt, uint8_t word_number);
-void xive_router_notify(XiveNotifier *xn, uint32_t lisn);
+void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
/*
* XIVE Presenter
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 77cacdd6c623..bcfe9dc54e3b 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -373,6 +373,34 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
}
+static int pnv_xive_get_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
+ uint8_t *pq)
+{
+ PnvXive *xive = PNV_XIVE(xrtr);
+
+ if (pnv_xive_block_id(xive) != blk) {
+ xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
+ return -1;
+ }
+
+ *pq = xive_source_esb_get(&xive->ipi_source, idx);
+ return 0;
+}
+
+static int pnv_xive_set_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
+ uint8_t *pq)
+{
+ PnvXive *xive = PNV_XIVE(xrtr);
+
+ if (pnv_xive_block_id(xive) != blk) {
+ xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
+ return -1;
+ }
+
+ *pq = xive_source_esb_set(&xive->ipi_source, idx, *pq);
+ return 0;
+}
+
/*
* One bit per thread id. The first register PC_THREAD_EN_REG0 covers
* the first cores 0-15 (normal) of the chip or 0-7 (fused). The
@@ -469,12 +497,12 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
* event notification to the Router. This is required on a multichip
* system.
*/
-static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
+static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno, bool pq_checked)
{
PnvXive *xive = PNV_XIVE(xn);
uint8_t blk = pnv_xive_block_id(xive);
- xive_router_notify(xn, XIVE_EAS(blk, srcno));
+ xive_router_notify(xn, XIVE_EAS(blk, srcno), pq_checked);
}
/*
@@ -1316,7 +1344,8 @@ static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
blk = XIVE_EAS_BLOCK(val);
idx = XIVE_EAS_INDEX(val);
- xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
+ xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx),
+ !!(val & XIVE_TRIGGER_PQ));
}
static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
@@ -1944,6 +1973,8 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
device_class_set_props(dc, pnv_xive_properties);
xrc->get_eas = pnv_xive_get_eas;
+ xrc->get_pq = pnv_xive_get_pq;
+ xrc->set_pq = pnv_xive_set_pq;
xrc->get_end = pnv_xive_get_end;
xrc->write_end = pnv_xive_write_end;
xrc->get_nvt = pnv_xive_get_nvt;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index b8825577f719..894be4b49ba4 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -927,7 +927,7 @@ static void xive_source_notify(XiveSource *xsrc, int srcno)
XiveNotifierClass *xnc = XIVE_NOTIFIER_GET_CLASS(xsrc->xive);
if (xnc->notify) {
- xnc->notify(xsrc->xive, srcno);
+ xnc->notify(xsrc->xive, srcno, true);
}
}
@@ -1343,6 +1343,24 @@ int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
return xrc->get_eas(xrtr, eas_blk, eas_idx, eas);
}
+static
+int xive_router_get_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
+ uint8_t *pq)
+{
+ XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
+
+ return xrc->get_pq(xrtr, eas_blk, eas_idx, pq);
+}
+
+static
+int xive_router_set_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
+ uint8_t *pq)
+{
+ XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
+
+ return xrc->set_pq(xrtr, eas_blk, eas_idx, pq);
+}
+
int xive_router_get_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
XiveEND *end)
{
@@ -1686,7 +1704,7 @@ do_escalation:
xive_get_field32(END_W5_ESC_END_DATA, end.w5));
}
-void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
+void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
{
XiveRouter *xrtr = XIVE_ROUTER(xn);
uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
@@ -1699,11 +1717,27 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
return;
}
- /*
- * The IVRE checks the State Bit Cache at this point. We skip the
- * SBC lookup because the state bits of the sources are modeled
- * internally in QEMU.
- */
+ if (!pq_checked) {
+ bool notify;
+ uint8_t pq;
+
+ /* PQ cache lookup */
+ if (xive_router_get_pq(xrtr, eas_blk, eas_idx, &pq)) {
+ /* Set FIR */
+ g_assert_not_reached();
+ }
+
+ notify = xive_esb_trigger(&pq);
+
+ if (xive_router_set_pq(xrtr, eas_blk, eas_idx, &pq)) {
+ /* Set FIR */
+ g_assert_not_reached();
+ }
+
+ if (!notify) {
+ return;
+ }
+ }
if (!xive_eas_is_valid(&eas)) {
qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %x\n", lisn);
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index ac824f877cf1..c451819bfd52 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1241,14 +1241,19 @@ static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
return phb->bus_path;
}
-static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno)
+static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
+ bool pq_checked)
{
PnvPHB4 *phb = PNV_PHB4(xf);
uint64_t notif_port = phb->regs[PHB_INT_NOTIFY_ADDR >> 3];
uint32_t offset = phb->regs[PHB_INT_NOTIFY_INDEX >> 3];
- uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
+ uint64_t data = offset | srcno;
MemTxResult result;
+ if (pq_checked) {
+ data |= XIVE_TRIGGER_PQ;
+ }
+
address_space_stq_be(&address_space_memory, notif_port, data,
MEMTXATTRS_UNSPECIFIED, &result);
if (result != MEMTX_OK) {
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index c34a49b000f2..63804f28f45e 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -652,7 +652,7 @@ static const TypeInfo pnv_psi_power8_info = {
#define PSIHB9_IRQ_STAT_DIO PPC_BIT(12)
#define PSIHB9_IRQ_STAT_PSU PPC_BIT(13)
-static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
+static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno, bool pq_checked)
{
PnvPsi *psi = PNV_PSI(xf);
uint64_t notif_port = psi->regs[PSIHB_REG(PSIHB9_ESB_NOTIF_ADDR)];
@@ -661,9 +661,13 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
uint32_t offset =
(psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT);
- uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
+ uint64_t data = offset | srcno;
MemTxResult result;
+ if (pq_checked) {
+ data |= XIVE_TRIGGER_PQ;
+ }
+
if (!valid) {
return;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload
2020-04-01 16:46 ` [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
@ 2020-04-02 10:51 ` Greg Kurz
2020-05-25 10:04 ` Cédric Le Goater
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2020-04-02 10:51 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson
On Wed, 1 Apr 2020 18:46:53 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> The trigger message coming from a HW source contains a special bit
> informing the XIVE interrupt controller that the PQ bits have been
> checked at the source or not. Depending on the value, the IC can
> perform the check and the state transition locally using its own PQ
> state bits.
>
> The following changes add new accessors to the XiveRouter required to
> query and update the PQ state bits. This is only applies to the
> PowerNV machine, sPAPR is not concerned by such complex configuration.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/ppc/xive.h | 8 +++++--
> hw/intc/pnv_xive.c | 37 +++++++++++++++++++++++++++++---
> hw/intc/xive.c | 48 ++++++++++++++++++++++++++++++++++++------
> hw/pci-host/pnv_phb4.c | 9 ++++++--
> hw/ppc/pnv_psi.c | 8 +++++--
> 5 files changed, 94 insertions(+), 16 deletions(-)
>
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 112fb6fb6dbe..050e49c14fd9 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -160,7 +160,7 @@ typedef struct XiveNotifier XiveNotifier;
>
> typedef struct XiveNotifierClass {
> InterfaceClass parent;
> - void (*notify)(XiveNotifier *xn, uint32_t lisn);
> + void (*notify)(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
> } XiveNotifierClass;
>
> /*
> @@ -354,6 +354,10 @@ typedef struct XiveRouterClass {
> /* XIVE table accessors */
> int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> XiveEAS *eas);
> + int (*get_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> + uint8_t *pq);
> + int (*set_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> + uint8_t *pq);
> int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
> XiveEND *end);
> int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
> @@ -375,7 +379,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> XiveNVT *nvt);
> int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
> XiveNVT *nvt, uint8_t word_number);
> -void xive_router_notify(XiveNotifier *xn, uint32_t lisn);
> +void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
>
> /*
> * XIVE Presenter
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 77cacdd6c623..bcfe9dc54e3b 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -373,6 +373,34 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
> return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
> }
>
> +static int pnv_xive_get_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
> + uint8_t *pq)
> +{
> + PnvXive *xive = PNV_XIVE(xrtr);
> +
> + if (pnv_xive_block_id(xive) != blk) {
> + xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
> + return -1;
> + }
> +
> + *pq = xive_source_esb_get(&xive->ipi_source, idx);
> + return 0;
> +}
> +
> +static int pnv_xive_set_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
> + uint8_t *pq)
> +{
> + PnvXive *xive = PNV_XIVE(xrtr);
> +
> + if (pnv_xive_block_id(xive) != blk) {
> + xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
> + return -1;
> + }
> +
> + *pq = xive_source_esb_set(&xive->ipi_source, idx, *pq);
> + return 0;
> +}
> +
> /*
> * One bit per thread id. The first register PC_THREAD_EN_REG0 covers
> * the first cores 0-15 (normal) of the chip or 0-7 (fused). The
> @@ -469,12 +497,12 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
> * event notification to the Router. This is required on a multichip
> * system.
> */
> -static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
> +static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno, bool pq_checked)
> {
> PnvXive *xive = PNV_XIVE(xn);
> uint8_t blk = pnv_xive_block_id(xive);
>
> - xive_router_notify(xn, XIVE_EAS(blk, srcno));
> + xive_router_notify(xn, XIVE_EAS(blk, srcno), pq_checked);
> }
>
> /*
> @@ -1316,7 +1344,8 @@ static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
> blk = XIVE_EAS_BLOCK(val);
> idx = XIVE_EAS_INDEX(val);
>
> - xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
> + xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx),
> + !!(val & XIVE_TRIGGER_PQ));
> }
>
> static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
> @@ -1944,6 +1973,8 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
> device_class_set_props(dc, pnv_xive_properties);
>
> xrc->get_eas = pnv_xive_get_eas;
> + xrc->get_pq = pnv_xive_get_pq;
> + xrc->set_pq = pnv_xive_set_pq;
> xrc->get_end = pnv_xive_get_end;
> xrc->write_end = pnv_xive_write_end;
> xrc->get_nvt = pnv_xive_get_nvt;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index b8825577f719..894be4b49ba4 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -927,7 +927,7 @@ static void xive_source_notify(XiveSource *xsrc, int srcno)
> XiveNotifierClass *xnc = XIVE_NOTIFIER_GET_CLASS(xsrc->xive);
>
> if (xnc->notify) {
> - xnc->notify(xsrc->xive, srcno);
> + xnc->notify(xsrc->xive, srcno, true);
> }
> }
>
> @@ -1343,6 +1343,24 @@ int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> return xrc->get_eas(xrtr, eas_blk, eas_idx, eas);
> }
>
> +static
> +int xive_router_get_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> + uint8_t *pq)
> +{
> + XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
> +
> + return xrc->get_pq(xrtr, eas_blk, eas_idx, pq);
> +}
> +
> +static
> +int xive_router_set_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
> + uint8_t *pq)
> +{
> + XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
> +
> + return xrc->set_pq(xrtr, eas_blk, eas_idx, pq);
> +}
> +
Only the powernv machine implements the PQ bits hooks. I understand
that pseries doesn't need them because it cannot execute the helpers
above in practice, since xive_router_notify() is always called with
pq_checked == true :
spapr_xive_set_irq()
xive_source_set_irq()
xive_source_notify()
{
xnc->notify(xsrc->xive, srcno, true);
}
IMHO the XIVE code is complex enough that we should maybe make
this really explicit with a comment and assert(xrc->[sg]et_pq)
in the PQ bits helpers.
> int xive_router_get_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
> XiveEND *end)
> {
> @@ -1686,7 +1704,7 @@ do_escalation:
> xive_get_field32(END_W5_ESC_END_DATA, end.w5));
> }
>
> -void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
> +void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
> {
> XiveRouter *xrtr = XIVE_ROUTER(xn);
> uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
> @@ -1699,11 +1717,27 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
> return;
> }
>
> - /*
> - * The IVRE checks the State Bit Cache at this point. We skip the
> - * SBC lookup because the state bits of the sources are modeled
> - * internally in QEMU.
> - */
> + if (!pq_checked) {
> + bool notify;
> + uint8_t pq;
> +
> + /* PQ cache lookup */
> + if (xive_router_get_pq(xrtr, eas_blk, eas_idx, &pq)) {
> + /* Set FIR */
> + g_assert_not_reached();
I'm not sure about the intent of this assert... Actual QEMU
bug that should never happen ever or guest misuse of XIVE
that we might try to handle one day ?
In the former case, I'd rather assert() in the PQ bits
machine hooks when the error condition is first detected.
Apart from that, rest looks good.
> + }
> +
> + notify = xive_esb_trigger(&pq);
> +
> + if (xive_router_set_pq(xrtr, eas_blk, eas_idx, &pq)) {
> + /* Set FIR */
> + g_assert_not_reached();
> + }
> +
> + if (!notify) {
> + return;
> + }
> + }
>
> if (!xive_eas_is_valid(&eas)) {
> qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %x\n", lisn);
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index ac824f877cf1..c451819bfd52 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1241,14 +1241,19 @@ static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
> return phb->bus_path;
> }
>
> -static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno)
> +static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
> + bool pq_checked)
> {
> PnvPHB4 *phb = PNV_PHB4(xf);
> uint64_t notif_port = phb->regs[PHB_INT_NOTIFY_ADDR >> 3];
> uint32_t offset = phb->regs[PHB_INT_NOTIFY_INDEX >> 3];
> - uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
> + uint64_t data = offset | srcno;
> MemTxResult result;
>
> + if (pq_checked) {
> + data |= XIVE_TRIGGER_PQ;
> + }
> +
> address_space_stq_be(&address_space_memory, notif_port, data,
> MEMTXATTRS_UNSPECIFIED, &result);
> if (result != MEMTX_OK) {
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index c34a49b000f2..63804f28f45e 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -652,7 +652,7 @@ static const TypeInfo pnv_psi_power8_info = {
> #define PSIHB9_IRQ_STAT_DIO PPC_BIT(12)
> #define PSIHB9_IRQ_STAT_PSU PPC_BIT(13)
>
> -static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
> +static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno, bool pq_checked)
> {
> PnvPsi *psi = PNV_PSI(xf);
> uint64_t notif_port = psi->regs[PSIHB_REG(PSIHB9_ESB_NOTIF_ADDR)];
> @@ -661,9 +661,13 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
>
> uint32_t offset =
> (psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT);
> - uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
> + uint64_t data = offset | srcno;
> MemTxResult result;
>
> + if (pq_checked) {
> + data |= XIVE_TRIGGER_PQ;
> + }
> +
> if (!valid) {
> return;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload
2020-04-02 10:51 ` Greg Kurz
@ 2020-05-25 10:04 ` Cédric Le Goater
0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2020-05-25 10:04 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-ppc, qemu-devel, David Gibson
On 4/2/20 12:51 PM, Greg Kurz wrote:
> On Wed, 1 Apr 2020 18:46:53 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
>> The trigger message coming from a HW source contains a special bit
>> informing the XIVE interrupt controller that the PQ bits have been
>> checked at the source or not. Depending on the value, the IC can
>> perform the check and the state transition locally using its own PQ
>> state bits.
>>
>> The following changes add new accessors to the XiveRouter required to
>> query and update the PQ state bits. This is only applies to the
>> PowerNV machine, sPAPR is not concerned by such complex configuration.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> include/hw/ppc/xive.h | 8 +++++--
>> hw/intc/pnv_xive.c | 37 +++++++++++++++++++++++++++++---
>> hw/intc/xive.c | 48 ++++++++++++++++++++++++++++++++++++------
>> hw/pci-host/pnv_phb4.c | 9 ++++++--
>> hw/ppc/pnv_psi.c | 8 +++++--
>> 5 files changed, 94 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 112fb6fb6dbe..050e49c14fd9 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -160,7 +160,7 @@ typedef struct XiveNotifier XiveNotifier;
>>
>> typedef struct XiveNotifierClass {
>> InterfaceClass parent;
>> - void (*notify)(XiveNotifier *xn, uint32_t lisn);
>> + void (*notify)(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
>> } XiveNotifierClass;
>>
>> /*
>> @@ -354,6 +354,10 @@ typedef struct XiveRouterClass {
>> /* XIVE table accessors */
>> int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> XiveEAS *eas);
>> + int (*get_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> + uint8_t *pq);
>> + int (*set_pq)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> + uint8_t *pq);
>> int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>> XiveEND *end);
>> int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>> @@ -375,7 +379,7 @@ int xive_router_get_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> XiveNVT *nvt);
>> int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>> XiveNVT *nvt, uint8_t word_number);
>> -void xive_router_notify(XiveNotifier *xn, uint32_t lisn);
>> +void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked);
>>
>> /*
>> * XIVE Presenter
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 77cacdd6c623..bcfe9dc54e3b 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -373,6 +373,34 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>> return pnv_xive_vst_read(xive, VST_TSEL_IVT, blk, idx, eas);
>> }
>>
>> +static int pnv_xive_get_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>> + uint8_t *pq)
>> +{
>> + PnvXive *xive = PNV_XIVE(xrtr);
>> +
>> + if (pnv_xive_block_id(xive) != blk) {
>> + xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
>> + return -1;
>> + }
>> +
>> + *pq = xive_source_esb_get(&xive->ipi_source, idx);
>> + return 0;
>> +}
>> +
>> +static int pnv_xive_set_pq(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>> + uint8_t *pq)
>> +{
>> + PnvXive *xive = PNV_XIVE(xrtr);
>> +
>> + if (pnv_xive_block_id(xive) != blk) {
>> + xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
>> + return -1;
>> + }
>> +
>> + *pq = xive_source_esb_set(&xive->ipi_source, idx, *pq);
>> + return 0;
>> +}
>> +
>> /*
>> * One bit per thread id. The first register PC_THREAD_EN_REG0 covers
>> * the first cores 0-15 (normal) of the chip or 0-7 (fused). The
>> @@ -469,12 +497,12 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
>> * event notification to the Router. This is required on a multichip
>> * system.
>> */
>> -static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
>> +static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno, bool pq_checked)
>> {
>> PnvXive *xive = PNV_XIVE(xn);
>> uint8_t blk = pnv_xive_block_id(xive);
>>
>> - xive_router_notify(xn, XIVE_EAS(blk, srcno));
>> + xive_router_notify(xn, XIVE_EAS(blk, srcno), pq_checked);
>> }
>>
>> /*
>> @@ -1316,7 +1344,8 @@ static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
>> blk = XIVE_EAS_BLOCK(val);
>> idx = XIVE_EAS_INDEX(val);
>>
>> - xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
>> + xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx),
>> + !!(val & XIVE_TRIGGER_PQ));
>> }
>>
>> static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
>> @@ -1944,6 +1973,8 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
>> device_class_set_props(dc, pnv_xive_properties);
>>
>> xrc->get_eas = pnv_xive_get_eas;
>> + xrc->get_pq = pnv_xive_get_pq;
>> + xrc->set_pq = pnv_xive_set_pq;
>> xrc->get_end = pnv_xive_get_end;
>> xrc->write_end = pnv_xive_write_end;
>> xrc->get_nvt = pnv_xive_get_nvt;
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index b8825577f719..894be4b49ba4 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -927,7 +927,7 @@ static void xive_source_notify(XiveSource *xsrc, int srcno)
>> XiveNotifierClass *xnc = XIVE_NOTIFIER_GET_CLASS(xsrc->xive);
>>
>> if (xnc->notify) {
>> - xnc->notify(xsrc->xive, srcno);
>> + xnc->notify(xsrc->xive, srcno, true);
>> }
>> }
>>
>> @@ -1343,6 +1343,24 @@ int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> return xrc->get_eas(xrtr, eas_blk, eas_idx, eas);
>> }
>>
>> +static
>> +int xive_router_get_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> + uint8_t *pq)
>> +{
>> + XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>> +
>> + return xrc->get_pq(xrtr, eas_blk, eas_idx, pq);
>> +}
>> +
>> +static
>> +int xive_router_set_pq(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>> + uint8_t *pq)
>> +{
>> + XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
>> +
>> + return xrc->set_pq(xrtr, eas_blk, eas_idx, pq);
>> +}
>> +
>
> Only the powernv machine implements the PQ bits hooks. I understand
> that pseries doesn't need them because it cannot execute the helpers
> above in practice, since xive_router_notify() is always called with
> pq_checked == true :
>
> spapr_xive_set_irq()
> xive_source_set_irq()
> xive_source_notify()
> {
> xnc->notify(xsrc->xive, srcno, true);
> }
>
> IMHO the XIVE code is complex enough that we should maybe make
> this really explicit with a comment and assert(xrc->[sg]et_pq)
> in the PQ bits helpers.
I will simply implement the hooks for pseries. No big deal.
>
>> int xive_router_get_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>> XiveEND *end)
>> {
>> @@ -1686,7 +1704,7 @@ do_escalation:
>> xive_get_field32(END_W5_ESC_END_DATA, end.w5));
>> }
>>
>> -void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>> +void xive_router_notify(XiveNotifier *xn, uint32_t lisn, bool pq_checked)
>> {
>> XiveRouter *xrtr = XIVE_ROUTER(xn);
>> uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
>> @@ -1699,11 +1717,27 @@ void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>> return;
>> }
>>
>> - /*
>> - * The IVRE checks the State Bit Cache at this point. We skip the
>> - * SBC lookup because the state bits of the sources are modeled
>> - * internally in QEMU.
>> - */
>> + if (!pq_checked) {
>> + bool notify;
>> + uint8_t pq;
>> +
>> + /* PQ cache lookup */
>> + if (xive_router_get_pq(xrtr, eas_blk, eas_idx, &pq)) {
>> + /* Set FIR */
>> + g_assert_not_reached();
>
> I'm not sure about the intent of this assert... Actual QEMU
> bug that should never happen ever
It should never happen because the call above to xive_router_get_eas() would
have errored.
C.
> or guest misuse of XIVE
> that we might try to handle one day ?
>
> In the former case, I'd rather assert() in the PQ bits
> machine hooks when the error condition is first detected.
>
> Apart from that, rest looks good.
>
>> + }
>> +
>> + notify = xive_esb_trigger(&pq);
>> +
>> + if (xive_router_set_pq(xrtr, eas_blk, eas_idx, &pq)) {
>> + /* Set FIR */
>> + g_assert_not_reached();
>> + }
>> +
>> + if (!notify) {
>> + return;
>> + }
>> + }
>>
>> if (!xive_eas_is_valid(&eas)) {
>> qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %x\n", lisn);
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index ac824f877cf1..c451819bfd52 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1241,14 +1241,19 @@ static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
>> return phb->bus_path;
>> }
>>
>> -static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno)
>> +static void pnv_phb4_xive_notify(XiveNotifier *xf, uint32_t srcno,
>> + bool pq_checked)
>> {
>> PnvPHB4 *phb = PNV_PHB4(xf);
>> uint64_t notif_port = phb->regs[PHB_INT_NOTIFY_ADDR >> 3];
>> uint32_t offset = phb->regs[PHB_INT_NOTIFY_INDEX >> 3];
>> - uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
>> + uint64_t data = offset | srcno;
>> MemTxResult result;
>>
>> + if (pq_checked) {
>> + data |= XIVE_TRIGGER_PQ;
>> + }
>> +
>> address_space_stq_be(&address_space_memory, notif_port, data,
>> MEMTXATTRS_UNSPECIFIED, &result);
>> if (result != MEMTX_OK) {
>> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
>> index c34a49b000f2..63804f28f45e 100644
>> --- a/hw/ppc/pnv_psi.c
>> +++ b/hw/ppc/pnv_psi.c
>> @@ -652,7 +652,7 @@ static const TypeInfo pnv_psi_power8_info = {
>> #define PSIHB9_IRQ_STAT_DIO PPC_BIT(12)
>> #define PSIHB9_IRQ_STAT_PSU PPC_BIT(13)
>>
>> -static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
>> +static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno, bool pq_checked)
>> {
>> PnvPsi *psi = PNV_PSI(xf);
>> uint64_t notif_port = psi->regs[PSIHB_REG(PSIHB9_ESB_NOTIF_ADDR)];
>> @@ -661,9 +661,13 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
>>
>> uint32_t offset =
>> (psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT);
>> - uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
>> + uint64_t data = offset | srcno;
>> MemTxResult result;
>>
>> + if (pq_checked) {
>> + data |= XIVE_TRIGGER_PQ;
>> + }
>> +
>> if (!valid) {
>> return;
>> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-25 10:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 16:46 [PATCH v2 0/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 1/2] ppc/xive: export PQ routines Cédric Le Goater
2020-04-01 16:46 ` [PATCH v2 2/2] ppc/xive: Add support for PQ state bits offload Cédric Le Goater
2020-04-02 10:51 ` Greg Kurz
2020-05-25 10:04 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.