[Qemu-devel] [PATCH v4 00/25] ppc/pnv: add XIVE support for KVM guests

[Qemu-devel] [PATCH v4 00/25] ppc/pnv: add XIVE support for KVM guests

[Cédric Le Goater]

Hello,

The QEMU PowerNV machine emulates a baremetal OpenPOWER system and
acts as an hypervisor (L0). Supporting emulation of KVM to run guests
(L1) requires a few more extensions, among which guest support for the
XIVE interrupt controller on POWER9 processor.

The following changes add new per-CPU PowerNV machines and extend the
XIVE models with the new XiveFabric and XivePresenter interfaces to
provide support for XIVE escalations and interrupt resend. This
mechanism is used by XIVE to notify the hypervisor that a vCPU is not
dispatched on a HW thread. Tested on a QEMU PowerNV machine and a
simple QEMU pseries guest doing network on a local bridge.

The XIVE interrupt controller offers a way to increase the XIVE
resources per chip by configuring multiple XIVE blocks on a chip. This
is not currently supported by the model. However, some configurations,
such as OPAL/skiboot, use one block-per-chip configuration with some
optimizations. One of them is to override the hardwired chip ID by the
block id in the PowerBUS operations and for CAM line compares. This
patchset improves the support for this setup. Tested with 4 chips.

A series from Suraj adding guest support in the Radix MMU model of the
QEMU PowerNV machine is still required and will be send later. The
whole patchset can be found under :

  https://github.com/legoater/qemu/tree/powernv-4.2

Thanks,

C.

Changes since v3:

 - reworked the patches introducing the XiveFabric and XivePresenter
   interfaces
 - moved the get_block_id() handler to the XiveRouter
 - new small addons related to the format of the trigger data
 
Changes since v2:

 - introduced the XiveFabric and XivePresenter interfaces
 - removed the need of a XiveRouter pointer under XiveTCTX

Changes since v1:

 - minor extra fixes 
 - split the escalation support in different patches
 - kept the XiveRouter type for XiveTCTX back pointer (will address
   this in P10)
 - removed pnv_xive_vst_size(). Really broken on indirect tables.
 - improved the dump of the NVT table
 - introduce pnv_xive_get_block_id()


Cédric Le Goater (25):
  ppc/xive: Introduce a XivePresenter interface
  ppc/xive: Implement the XivePresenter interface
  ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper
  ppc/pnv: Introduce a pnv_xive_is_cpu_enabled() helper
  ppc/xive: Introduce a XiveFabric interface
  ppc/pnv: Implement the XiveFabric interface
  ppc/spapr: Implement the XiveFabric interface
  ppc/xive: Use the XiveFabric and XivePresenter interfaces
  ppc/xive: Extend the TIMA operation with a XivePresenter parameter
  ppc/pnv: Clarify how the TIMA is accessed on a multichip system
  ppc/xive: Move the TIMA operations to the controller model
  ppc/xive: Remove the get_tctx() XiveRouter handler
  ppc/xive: Introduce a xive_tctx_ipb_update() helper
  ppc/xive: Introduce helpers for the NVT id
  ppc/xive: Synthesize interrupt from the saved IPB in the NVT
  ppc/pnv: Remove pnv_xive_vst_size() routine
  ppc/pnv: Dump the XIVE NVT table
  ppc/pnv: Skip empty slots of the XIVE NVT table
  ppc/pnv: Introduce a pnv_xive_block_id() helper
  ppc/pnv: Extend XiveRouter with a get_block_id() handler
  ppc/pnv: Quiesce some XIVE errors
  ppc/xive: Introduce a xive_os_cam_decode() helper
  ppc/xive: Check V bit in TM_PULL_POOL_CTX
  ppc/pnv: Improve trigger data definition
  ppc/pnv: Use the EAS trigger bit when triggering an interrupt from PSI

 include/hw/ppc/pnv.h       |  13 ++
 include/hw/ppc/pnv_xive.h  |   3 -
 include/hw/ppc/spapr_irq.h |   6 +
 include/hw/ppc/xive.h      |  72 ++++++--
 include/hw/ppc/xive_regs.h |  48 ++++-
 hw/intc/pnv_xive.c         | 370 +++++++++++++++++++++++++------------
 hw/intc/spapr_xive.c       |  80 +++++++-
 hw/intc/xive.c             | 328 +++++++++++++++++---------------
 hw/ppc/pnv.c               |  32 ++++
 hw/ppc/pnv_psi.c           |  15 +-
 hw/ppc/spapr.c             |  34 ++++
 hw/ppc/spapr_irq.c         |  25 +++
 12 files changed, 730 insertions(+), 296 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v4 01/25] ppc/xive: Introduce a XivePresenter interface

[Cédric Le Goater]

When the XIVE IVRE sub-engine (XiveRouter) looks for a Notification
Virtual Target (NVT) to notify, it broadcasts a message on the
PowerBUS to find an XIVE IVPE sub-engine (Presenter) with the NVT
dispatched on one of its HW threads, and then forwards the
notification if any response was received.

The current XIVE presenter model is sufficient for the pseries machine
because it has a single interrupt controller device, but the PowerNV
machine can have multiple chips each having its own interrupt
controller. In this case, the XIVE presenter model is too simple and
the CAM line matching should scan all chips of the system.

To start fixing this issue, we first extend the XIVE Router model with
a new XivePresenter QOM interface representing the XIVE IVPE
sub-engine. This interface exposes a 'match_nvt' handler which the
sPAPR and PowerNV XIVE Router models will need to implement to perform
the CAM line matching.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 32 ++++++++++++++++++++++++++++++++
 hw/intc/xive.c        | 26 +++++++++++++++++---------
 2 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 6d38755f8459..39de45b87cb9 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -367,6 +367,38 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
 XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
 void xive_router_notify(XiveNotifier *xn, uint32_t lisn);
 
+/*
+ * XIVE Presenter
+ */
+
+typedef struct XiveTCTXMatch {
+    XiveTCTX *tctx;
+    uint8_t ring;
+} XiveTCTXMatch;
+
+typedef struct XivePresenter XivePresenter;
+
+#define TYPE_XIVE_PRESENTER "xive-presenter"
+#define XIVE_PRESENTER(obj)                                     \
+    INTERFACE_CHECK(XivePresenter, (obj), TYPE_XIVE_PRESENTER)
+#define XIVE_PRESENTER_CLASS(klass)                                     \
+    OBJECT_CLASS_CHECK(XivePresenterClass, (klass), TYPE_XIVE_PRESENTER)
+#define XIVE_PRESENTER_GET_CLASS(obj)                                   \
+    OBJECT_GET_CLASS(XivePresenterClass, (obj), TYPE_XIVE_PRESENTER)
+
+typedef struct XivePresenterClass {
+    InterfaceClass parent;
+    int (*match_nvt)(XivePresenter *xptr, uint8_t format,
+                     uint8_t nvt_blk, uint32_t nvt_idx,
+                     bool cam_ignore, uint8_t priority,
+                     uint32_t logic_serv, XiveTCTXMatch *match);
+} XivePresenterClass;
+
+int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
+                              uint8_t format,
+                              uint8_t nvt_blk, uint32_t nvt_idx,
+                              bool cam_ignore, uint32_t logic_serv);
+
 /*
  * XIVE END ESBs
  */
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index b7417210d817..aa45ac2e06cb 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1316,9 +1316,10 @@ static uint32_t xive_tctx_hw_cam_line(XiveTCTX *tctx)
 /*
  * The thread context register words are in big-endian format.
  */
-static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
-                                     uint8_t nvt_blk, uint32_t nvt_idx,
-                                     bool cam_ignore, uint32_t logic_serv)
+int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
+                              uint8_t format,
+                              uint8_t nvt_blk, uint32_t nvt_idx,
+                              bool cam_ignore, uint32_t logic_serv)
 {
     uint32_t cam = xive_nvt_cam_line(nvt_blk, nvt_idx);
     uint32_t qw3w2 = xive_tctx_word2(&tctx->regs[TM_QW3_HV_PHYS]);
@@ -1375,11 +1376,6 @@ static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
     return -1;
 }
 
-typedef struct XiveTCTXMatch {
-    XiveTCTX *tctx;
-    uint8_t ring;
-} XiveTCTXMatch;
-
 static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
                                  uint8_t nvt_blk, uint32_t nvt_idx,
                                  bool cam_ignore, uint8_t priority,
@@ -1405,7 +1401,8 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
          * Check the thread context CAM lines and record matches. We
          * will handle CPU exception delivery later
          */
-        ring = xive_presenter_tctx_match(tctx, format, nvt_blk, nvt_idx,
+        ring = xive_presenter_tctx_match(XIVE_PRESENTER(xrtr), tctx, format,
+                                         nvt_blk, nvt_idx,
                                          cam_ignore, logic_serv);
         /*
          * Save the context and follow on to catch duplicates, that we
@@ -1692,6 +1689,7 @@ static const TypeInfo xive_router_info = {
     .class_init    = xive_router_class_init,
     .interfaces    = (InterfaceInfo[]) {
         { TYPE_XIVE_NOTIFIER },
+        { TYPE_XIVE_PRESENTER },
         { }
     }
 };
@@ -1863,10 +1861,20 @@ static const TypeInfo xive_notifier_info = {
     .class_size = sizeof(XiveNotifierClass),
 };
 
+/*
+ * XIVE Presenter
+ */
+static const TypeInfo xive_presenter_info = {
+    .name = TYPE_XIVE_PRESENTER,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(XivePresenterClass),
+};
+
 static void xive_register_types(void)
 {
     type_register_static(&xive_source_info);
     type_register_static(&xive_notifier_info);
+    type_register_static(&xive_presenter_info);
     type_register_static(&xive_router_info);
     type_register_static(&xive_end_source_info);
     type_register_static(&xive_tctx_info);
-- 
2.21.0

[Qemu-devel] [PATCH v4 02/25] ppc/xive: Implement the XivePresenter interface

[Cédric Le Goater]

Each XIVE Router model, sPAPR and PowerNV, now implements the 'match_nvt'
handler of the XivePresenter QOM interface. This is simply moving code
and taking into account the new API.

To be noted that the xive_router_get_tctx() helper is not used anymore
when doing CAM matching and will be removed later on after other changes.

The XIVE presenter model is still too simple for the PowerNV machine
and the CAM matching algo is not correct on multichip system. Subsequent
patches will introduce more changes to scan all chips of the system.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c   | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/intc/spapr_xive.c | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/intc/xive.c       | 43 +++++++------------------------------------
 3 files changed, 89 insertions(+), 36 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index ed6e9d71bbfa..ae449aa1119b 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -392,6 +392,45 @@ 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_match_nvt(XivePresenter *xptr, uint8_t format,
+                              uint8_t nvt_blk, uint32_t nvt_idx,
+                              bool cam_ignore, uint8_t priority,
+                              uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    CPUState *cs;
+    int count = 0;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+        XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
+        int ring;
+
+        /*
+         * Check the thread context CAM lines and record matches.
+         */
+        ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk, nvt_idx,
+                                         cam_ignore, logic_serv);
+        /*
+         * Save the context and follow on to catch duplicates, that we
+         * don't support yet.
+         */
+        if (ring != -1) {
+            if (match->tctx) {
+                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a "
+                              "thread context NVT %x/%x\n",
+                              nvt_blk, nvt_idx);
+                return -1;
+            }
+
+            match->ring = ring;
+            match->tctx = tctx;
+            count++;
+        }
+    }
+
+    return count;
+}
+
 static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -1797,6 +1836,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
     PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
     XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
     XiveNotifierClass *xnc = XIVE_NOTIFIER_CLASS(klass);
+    XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
 
     xdc->dt_xscom = pnv_xive_dt_xscom;
 
@@ -1812,6 +1852,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
     xrc->get_tctx = pnv_xive_get_tctx;
 
     xnc->notify = pnv_xive_notify;
+    xpc->match_nvt  = pnv_xive_match_nvt;
 };
 
 static const TypeInfo pnv_xive_info = {
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index c1c97192a7d2..eefc0d4c36b9 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -422,6 +422,44 @@ static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
     return spapr_cpu_state(cpu)->tctx;
 }
 
+static int spapr_xive_match_nvt(XivePresenter *xptr, uint8_t format,
+                                uint8_t nvt_blk, uint32_t nvt_idx,
+                                bool cam_ignore, uint8_t priority,
+                                uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    CPUState *cs;
+    int count = 0;
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+        XiveTCTX *tctx = spapr_cpu_state(cpu)->tctx;
+        int ring;
+
+        /*
+         * Check the thread context CAM lines and record matches.
+         */
+        ring = xive_presenter_tctx_match(xptr, tctx, format, nvt_blk, nvt_idx,
+                                         cam_ignore, logic_serv);
+        /*
+         * Save the matching thread interrupt context and follow on to
+         * check for duplicates which are invalid.
+         */
+        if (ring != -1) {
+            if (match->tctx) {
+                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread "
+                              "context NVT %x/%x\n", nvt_blk, nvt_idx);
+                return -1;
+            }
+
+            match->ring = ring;
+            match->tctx = tctx;
+            count++;
+        }
+    }
+
+    return count;
+}
+
 static const VMStateDescription vmstate_spapr_xive_end = {
     .name = TYPE_SPAPR_XIVE "/end",
     .version_id = 1,
@@ -499,6 +537,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass);
+    XivePresenterClass *xpc = XIVE_PRESENTER_CLASS(klass);
 
     dc->desc    = "sPAPR XIVE Interrupt Controller";
     dc->props   = spapr_xive_properties;
@@ -511,6 +550,8 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
     xrc->get_nvt = spapr_xive_get_nvt;
     xrc->write_nvt = spapr_xive_write_nvt;
     xrc->get_tctx = spapr_xive_get_tctx;
+
+    xpc->match_nvt  = spapr_xive_match_nvt;
 }
 
 static const TypeInfo spapr_xive_info = {
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index aa45ac2e06cb..fff50429f8ac 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1381,43 +1381,14 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
                                  bool cam_ignore, uint8_t priority,
                                  uint32_t logic_serv, XiveTCTXMatch *match)
 {
-    CPUState *cs;
+    XivePresenter *xptr = XIVE_PRESENTER(xrtr);
+    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
+    int count;
 
-    /*
-     * TODO (PowerNV): handle chip_id overwrite of block field for
-     * hardwired CAM compares
-     */
-
-    CPU_FOREACH(cs) {
-        XiveTCTX *tctx = xive_router_get_tctx(xrtr, cs);
-        int ring;
-
-        /*
-         * HW checks that the CPU is enabled in the Physical Thread
-         * Enable Register (PTER).
-         */
-
-        /*
-         * Check the thread context CAM lines and record matches. We
-         * will handle CPU exception delivery later
-         */
-        ring = xive_presenter_tctx_match(XIVE_PRESENTER(xrtr), tctx, format,
-                                         nvt_blk, nvt_idx,
-                                         cam_ignore, logic_serv);
-        /*
-         * Save the context and follow on to catch duplicates, that we
-         * don't support yet.
-         */
-        if (ring != -1) {
-            if (match->tctx) {
-                qemu_log_mask(LOG_GUEST_ERROR, "XIVE: already found a thread "
-                              "context NVT %x/%x\n", nvt_blk, nvt_idx);
-                return false;
-            }
-
-            match->ring = ring;
-            match->tctx = tctx;
-        }
+    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
+                           priority, logic_serv, match);
+    if (count < 0) {
+        return false;
     }
 
     if (!match->tctx) {
-- 
2.21.0

[Qemu-devel] [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper

[Cédric Le Goater]

As there is now easy way to loop on the CPUs belonging to a chip, add
a helper to filter out external CPUs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index ae449aa1119b..e1c15b6b5b71 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -392,15 +392,36 @@ 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 cpu_pir(PowerPCCPU *cpu)
+{
+    CPUPPCState *env = &cpu->env;
+    return env->spr_cb[SPR_PIR].default_value;
+}
+
+static int cpu_chip_id(PowerPCCPU *cpu)
+{
+    int pir = cpu_pir(cpu);
+    return (pir >> 8) & 0x7f;
+}
+
+#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
+    CPU_FOREACH(cs)                                                     \
+        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
+
 static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
                               uint8_t nvt_blk, uint32_t nvt_idx,
                               bool cam_ignore, uint8_t priority,
                               uint32_t logic_serv, XiveTCTXMatch *match)
 {
+    PnvXive *xive = PNV_XIVE(xptr);
     CPUState *cs;
     int count = 0;
 
-    CPU_FOREACH(cs) {
+    /*
+     * Loop on all CPUs of the machine and filter out the CPUs
+     * belonging to another chip.
+     */
+    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
         PowerPCCPU *cpu = POWERPC_CPU(cs);
         XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
         int ring;
-- 
2.21.0

[Qemu-devel] [PATCH v4 04/25] ppc/pnv: Introduce a pnv_xive_is_cpu_enabled() helper

[Cédric Le Goater]

and use this helper to exclude CPUs which were not enabled by the XIVE
controller.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index e1c15b6b5b71..5c97ccda1cad 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -408,6 +408,14 @@ static int cpu_chip_id(PowerPCCPU *cpu)
     CPU_FOREACH(cs)                                                     \
         if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
 
+static bool pnv_xive_is_cpu_enabled(PnvXive *xive, PowerPCCPU *cpu)
+{
+    int pir = cpu_pir(cpu);
+    int thrd_id = pir & 0x7f;
+
+    return xive->regs[PC_THREAD_EN_REG0 >> 3] & PPC_BIT(thrd_id);
+}
+
 static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
                               uint8_t nvt_blk, uint32_t nvt_idx,
                               bool cam_ignore, uint8_t priority,
@@ -426,6 +434,10 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
         XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
         int ring;
 
+        if (!pnv_xive_is_cpu_enabled(xive, cpu)) {
+            continue;
+        }
+
         /*
          * Check the thread context CAM lines and record matches.
          */
-- 
2.21.0

[Qemu-devel] [PATCH v4 05/25] ppc/xive: Introduce a XiveFabric interface

[Cédric Le Goater]

The XiveFabric QOM interface should be implemented by the machine. It
acts as the PowerBUS interface between the interrupt controller and
the system. On HW, the XIVE sub-engine is responsible for the
communication with the other chip is the Common Queue (CQ) bridge
unit.

This interface offers a 'match_nvt' handler to perform the CAM line
matching when looking for a XIVE Presenter with a dispatched NVT.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 22 ++++++++++++++++++++++
 hw/intc/xive.c        | 10 ++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 39de45b87cb9..3c2910e10e25 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -399,6 +399,28 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
                               uint8_t nvt_blk, uint32_t nvt_idx,
                               bool cam_ignore, uint32_t logic_serv);
 
+/*
+ * XIVE Fabric (Interface between Interrupt Controller and Machine)
+ */
+
+typedef struct XiveFabric XiveFabric;
+
+#define TYPE_XIVE_FABRIC "xive-fabric"
+#define XIVE_FABRIC(obj)                                     \
+    INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
+#define XIVE_FABRIC_CLASS(klass)                                     \
+    OBJECT_CLASS_CHECK(XiveFabricClass, (klass), TYPE_XIVE_FABRIC)
+#define XIVE_FABRIC_GET_CLASS(obj)                                   \
+    OBJECT_GET_CLASS(XiveFabricClass, (obj), TYPE_XIVE_FABRIC)
+
+typedef struct XiveFabricClass {
+    InterfaceClass parent;
+    int (*match_nvt)(XiveFabric *xfb, uint8_t format,
+                     uint8_t nvt_blk, uint32_t nvt_idx,
+                     bool cam_ignore, uint8_t priority,
+                     uint32_t logic_serv, XiveTCTXMatch *match);
+} XiveFabricClass;
+
 /*
  * XIVE END ESBs
  */
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index fff50429f8ac..7a15a64ed7fe 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1841,8 +1841,18 @@ static const TypeInfo xive_presenter_info = {
     .class_size = sizeof(XivePresenterClass),
 };
 
+/*
+ * XIVE Fabric
+ */
+static const TypeInfo xive_fabric_info = {
+    .name = TYPE_XIVE_FABRIC,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(XiveFabricClass),
+};
+
 static void xive_register_types(void)
 {
+    type_register_static(&xive_fabric_info);
     type_register_static(&xive_source_info);
     type_register_static(&xive_notifier_info);
     type_register_static(&xive_presenter_info);
-- 
2.21.0

[Qemu-devel] [PATCH v4 06/25] ppc/pnv: Implement the XiveFabric interface

[Cédric Le Goater]

The CAM line matching on the PowerNV machine now scans all chips of
the system and all CPUs of a chip to find a dispatched NVT in the
thread contexts.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 77a86c6a2301..ca24dd62df23 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1378,6 +1378,35 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
     }
 }
 
+static int pnv_xive_match_nvt(XiveFabric *xfb, uint8_t format,
+                               uint8_t nvt_blk, uint32_t nvt_idx,
+                               bool cam_ignore, uint8_t priority,
+                               uint32_t logic_serv,
+                               XiveTCTXMatch *match)
+{
+    PnvMachineState *pnv = PNV_MACHINE(xfb);
+    int total_count = 0;
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
+        XivePresenter *xptr = XIVE_PRESENTER(&chip9->xive);
+        XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
+        int count;
+
+        count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
+                               priority, logic_serv, match);
+
+        if (count < 0) {
+            return count;
+        }
+
+        total_count += count;
+    }
+
+    return total_count;
+}
+
 static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
@@ -1441,9 +1470,11 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
 static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
 
     mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
     mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
+    xfc->match_nvt = pnv_xive_match_nvt;
 
     mc->alias = "powernv";
 }
@@ -1495,6 +1526,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
         .interfaces = (InterfaceInfo[]) {               \
             { TYPE_XICS_FABRIC },                       \
             { TYPE_INTERRUPT_STATS_PROVIDER },          \
+            { TYPE_XIVE_FABRIC },                       \
             { },                                        \
         },                                              \
     }
-- 
2.21.0

[Qemu-devel] [PATCH v4 07/25] ppc/spapr: Implement the XiveFabric interface

[Cédric Le Goater]

The CAM line matching sequence in the pseries machine does not change
much apart from the use of the new QOM interfaces. There is an extra
indirection because of the sPAPR IRQ backend of the machine. Only the
XIVE backend implements the new 'match_nvt' handler.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/spapr_irq.h |  6 ++++++
 hw/ppc/spapr.c             | 34 ++++++++++++++++++++++++++++++++++
 hw/ppc/spapr_irq.c         | 25 +++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index 5db305165ce2..859780efaf95 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -31,6 +31,8 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align,
                         Error **errp);
 void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num);
 
+struct XiveTCTXMatch;
+
 typedef struct SpaprIrq {
     uint32_t    nr_irqs;
     uint32_t    nr_msis;
@@ -50,6 +52,10 @@ typedef struct SpaprIrq {
     void (*set_irq)(void *opaque, int srcno, int val);
     const char *(*get_nodename)(SpaprMachineState *spapr);
     void (*init_kvm)(SpaprMachineState *spapr, Error **errp);
+    int (*match_nvt)(SpaprMachineState *spapr, uint8_t format,
+                     uint8_t nvt_blk, uint32_t nvt_idx,
+                     bool cam_ignore, uint8_t priority,
+                     uint32_t logic_serv, struct XiveTCTXMatch *match);
 } SpaprIrq;
 
 extern SpaprIrq spapr_irq_xics;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2725b139a7f0..90f6f5fb9536 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4360,6 +4360,37 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
                    kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
 }
 
+static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
+                                uint8_t nvt_blk, uint32_t nvt_idx,
+                                bool cam_ignore, uint8_t priority,
+                                uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
+    int count;
+
+    count = spapr->irq->match_nvt(spapr, format, nvt_blk, nvt_idx, cam_ignore,
+                                  priority, logic_serv, match);
+    if (count < 0) {
+        return count;
+    }
+
+    /*
+     * When we implement the save and restore of the thread interrupt
+     * contexts in the enter/exit CPU handlers of the machine and the
+     * escalations in QEMU, we should be able to handle non dispatched
+     * vCPUs.
+     *
+     * Until this is done, the sPAPR machine should find at least one
+     * matching context always.
+     */
+    if (count == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
+                      nvt_blk, nvt_idx);
+    }
+
+    return count;
+}
+
 int spapr_get_vcpu_id(PowerPCCPU *cpu)
 {
     return cpu->vcpu_id;
@@ -4456,6 +4487,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
     XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
     InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
+    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
 
     mc->desc = "pSeries Logical Partition (PAPR compliant)";
     mc->ignore_boot_device_suffixes = true;
@@ -4514,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
      */
     mc->numa_mem_align_shift = 28;
     mc->numa_mem_supported = true;
+    xfc->match_nvt = spapr_xive_match_nvt;
 
     smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
     smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
@@ -4547,6 +4580,7 @@ static const TypeInfo spapr_machine_info = {
         { TYPE_PPC_VIRTUAL_HYPERVISOR },
         { TYPE_XICS_FABRIC },
         { TYPE_INTERRUPT_STATS_PROVIDER },
+        { TYPE_XIVE_FABRIC },
         { }
     },
 };
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index d8f46b6797f8..8a6d79a59af2 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -257,6 +257,7 @@ SpaprIrq spapr_irq_xics = {
     .set_irq     = spapr_irq_set_irq_xics,
     .get_nodename = spapr_irq_get_nodename_xics,
     .init_kvm    = spapr_irq_init_kvm_xics,
+    .match_nvt   = NULL, /* should not be used */
 };
 
 /*
@@ -406,6 +407,18 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp)
     }
 }
 
+static int spapr_irq_match_nvt_xive(SpaprMachineState *spapr, uint8_t format,
+                                    uint8_t nvt_blk, uint32_t nvt_idx,
+                                    bool cam_ignore, uint8_t priority,
+                                    uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
+    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
+
+    return xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
+                          priority, logic_serv, match);
+}
+
 /*
  * XIVE uses the full IRQ number space. Set it to 8K to be compatible
  * with XICS.
@@ -431,6 +444,7 @@ SpaprIrq spapr_irq_xive = {
     .set_irq     = spapr_irq_set_irq_xive,
     .get_nodename = spapr_irq_get_nodename_xive,
     .init_kvm    = spapr_irq_init_kvm_xive,
+    .match_nvt   = spapr_irq_match_nvt_xive,
 };
 
 /*
@@ -585,6 +599,15 @@ static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
     return spapr_irq_current(spapr)->get_nodename(spapr);
 }
 
+static int spapr_irq_match_nvt_dual(SpaprMachineState *spapr, uint8_t format,
+                                    uint8_t nvt_blk, uint32_t nvt_idx,
+                                    bool cam_ignore, uint8_t priority,
+                                    uint32_t logic_serv, XiveTCTXMatch *match)
+{
+    return spapr_irq_current(spapr)->match_nvt(spapr, format, nvt_blk, nvt_idx,
+                                     cam_ignore, priority, logic_serv, match);
+}
+
 /*
  * Define values in sync with the XIVE and XICS backend
  */
@@ -608,6 +631,7 @@ SpaprIrq spapr_irq_dual = {
     .set_irq     = spapr_irq_set_irq_dual,
     .get_nodename = spapr_irq_get_nodename_dual,
     .init_kvm    = NULL, /* should not be used */
+    .match_nvt   = spapr_irq_match_nvt_dual,
 };
 
 
@@ -825,4 +849,5 @@ SpaprIrq spapr_irq_xics_legacy = {
     .set_irq     = spapr_irq_set_irq_xics,
     .get_nodename = spapr_irq_get_nodename_xics,
     .init_kvm    = spapr_irq_init_kvm_xics,
+    .match_nvt   = NULL, /* should not be used */
 };
-- 
2.21.0

[Qemu-devel] [PATCH v4 08/25] ppc/xive: Use the XiveFabric and XivePresenter interfaces

[Cédric Le Goater]

Now that the machines have handlers implementing the XiveFabric and
XivePresenter interfaces, remove xive_presenter_match() and make use
of the 'match_nvt' handler of the machine.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 48 +++++++++++++++++-------------------------------
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 7a15a64ed7fe..f951ad8cb08d 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1376,30 +1376,6 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
     return -1;
 }
 
-static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
-                                 uint8_t nvt_blk, uint32_t nvt_idx,
-                                 bool cam_ignore, uint8_t priority,
-                                 uint32_t logic_serv, XiveTCTXMatch *match)
-{
-    XivePresenter *xptr = XIVE_PRESENTER(xrtr);
-    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
-    int count;
-
-    count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
-                           priority, logic_serv, match);
-    if (count < 0) {
-        return false;
-    }
-
-    if (!match->tctx) {
-        qemu_log_mask(LOG_UNIMP, "XIVE: NVT %x/%x is not dispatched\n",
-                      nvt_blk, nvt_idx);
-        return false;
-    }
-
-    return true;
-}
-
 /*
  * This is our simple Xive Presenter Engine model. It is merged in the
  * Router as it does not require an extra object.
@@ -1415,22 +1391,32 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
  *
  * The parameters represent what is sent on the PowerBus
  */
-static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format,
+static bool xive_presenter_notify(uint8_t format,
                                   uint8_t nvt_blk, uint32_t nvt_idx,
                                   bool cam_ignore, uint8_t priority,
                                   uint32_t logic_serv)
 {
+    XiveFabric *xfb = XIVE_FABRIC(qdev_get_machine());
+    XiveFabricClass *xfc = XIVE_FABRIC_GET_CLASS(xfb);
     XiveTCTXMatch match = { .tctx = NULL, .ring = 0 };
-    bool found;
+    int count;
 
-    found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore,
-                                 priority, logic_serv, &match);
-    if (found) {
+    /*
+     * Ask the machine to scan the interrupt controllers for a match
+     */
+    count = xfc->match_nvt(xfb, format, nvt_blk, nvt_idx, cam_ignore,
+                           priority, logic_serv, &match);
+    if (count < 0) {
+        return false;
+    }
+
+    /* handle CPU exception delivery */
+    if (count) {
         ipb_update(&match.tctx->regs[match.ring], priority);
         xive_tctx_notify(match.tctx, match.ring);
     }
 
-    return found;
+    return count;
 }
 
 /*
@@ -1543,7 +1529,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
         return;
     }
 
-    found = xive_presenter_notify(xrtr, format, nvt_blk, nvt_idx,
+    found = xive_presenter_notify(format, nvt_blk, nvt_idx,
                           xive_get_field32(END_W7_F0_IGNORE, end.w7),
                           priority,
                           xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
-- 
2.21.0

[Qemu-devel] [PATCH v4 09/25] ppc/xive: Extend the TIMA operation with a XivePresenter parameter

[Cédric Le Goater]

The TIMA operations are performed on behalf of the XIVE IVPE sub-engine
(Presenter) on the thread interrupt context registers. The current
operations the model supports are simple and do not require access to
the controller but more complex operations will need access to the
controller NVT table and to its configuration.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  7 +++---
 hw/intc/pnv_xive.c    |  4 +--
 hw/intc/xive.c        | 58 ++++++++++++++++++++++++-------------------
 3 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 3c2910e10e25..536deea8c622 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -463,9 +463,10 @@ typedef struct XiveENDSource {
 #define XIVE_TM_USER_PAGE       0x3
 
 extern const MemoryRegionOps xive_tm_ops;
-void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
-                        unsigned size);
-uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
+void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
+                        uint64_t value, unsigned size);
+uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
+                           unsigned size);
 
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
 Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 5c97ccda1cad..5c9483b394ab 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1444,7 +1444,7 @@ static void xive_tm_indirect_write(void *opaque, hwaddr offset,
 {
     XiveTCTX *tctx = pnv_xive_get_indirect_tctx(PNV_XIVE(opaque));
 
-    xive_tctx_tm_write(tctx, offset, value, size);
+    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
 }
 
 static uint64_t xive_tm_indirect_read(void *opaque, hwaddr offset,
@@ -1452,7 +1452,7 @@ static uint64_t xive_tm_indirect_read(void *opaque, hwaddr offset,
 {
     XiveTCTX *tctx = pnv_xive_get_indirect_tctx(PNV_XIVE(opaque));
 
-    return xive_tctx_tm_read(tctx, offset, size);
+    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
 }
 
 static const MemoryRegionOps xive_tm_indirect_ops = {
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index f951ad8cb08d..9bb09ed6ee7b 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -144,19 +144,20 @@ static inline uint32_t xive_tctx_word2(uint8_t *ring)
  * XIVE Thread Interrupt Management Area (TIMA)
  */
 
-static void xive_tm_set_hv_cppr(XiveTCTX *tctx, hwaddr offset,
-                                uint64_t value, unsigned size)
+static void xive_tm_set_hv_cppr(XivePresenter *xptr, XiveTCTX *tctx,
+                                hwaddr offset, uint64_t value, unsigned size)
 {
     xive_tctx_set_cppr(tctx, TM_QW3_HV_PHYS, value & 0xff);
 }
 
-static uint64_t xive_tm_ack_hv_reg(XiveTCTX *tctx, hwaddr offset, unsigned size)
+static uint64_t xive_tm_ack_hv_reg(XivePresenter *xptr, XiveTCTX *tctx,
+                                   hwaddr offset, unsigned size)
 {
     return xive_tctx_accept(tctx, TM_QW3_HV_PHYS);
 }
 
-static uint64_t xive_tm_pull_pool_ctx(XiveTCTX *tctx, hwaddr offset,
-                                      unsigned size)
+static uint64_t xive_tm_pull_pool_ctx(XivePresenter *xptr, XiveTCTX *tctx,
+                                      hwaddr offset, unsigned size)
 {
     uint32_t qw2w2_prev = xive_tctx_word2(&tctx->regs[TM_QW2_HV_POOL]);
     uint32_t qw2w2;
@@ -166,13 +167,14 @@ static uint64_t xive_tm_pull_pool_ctx(XiveTCTX *tctx, hwaddr offset,
     return qw2w2;
 }
 
-static void xive_tm_vt_push(XiveTCTX *tctx, hwaddr offset,
+static void xive_tm_vt_push(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
                             uint64_t value, unsigned size)
 {
     tctx->regs[TM_QW3_HV_PHYS + TM_WORD2] = value & 0xff;
 }
 
-static uint64_t xive_tm_vt_poll(XiveTCTX *tctx, hwaddr offset, unsigned size)
+static uint64_t xive_tm_vt_poll(XivePresenter *xptr, XiveTCTX *tctx,
+                                hwaddr offset, unsigned size)
 {
     return tctx->regs[TM_QW3_HV_PHYS + TM_WORD2] & 0xff;
 }
@@ -315,13 +317,14 @@ static uint64_t xive_tm_raw_read(XiveTCTX *tctx, hwaddr offset, unsigned size)
  * state changes (side effects) in addition to setting/returning the
  * interrupt management area context of the processor thread.
  */
-static uint64_t xive_tm_ack_os_reg(XiveTCTX *tctx, hwaddr offset, unsigned size)
+static uint64_t xive_tm_ack_os_reg(XivePresenter *xptr, XiveTCTX *tctx,
+                                   hwaddr offset, unsigned size)
 {
     return xive_tctx_accept(tctx, TM_QW1_OS);
 }
 
-static void xive_tm_set_os_cppr(XiveTCTX *tctx, hwaddr offset,
-                                uint64_t value, unsigned size)
+static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
+                                hwaddr offset, uint64_t value, unsigned size)
 {
     xive_tctx_set_cppr(tctx, TM_QW1_OS, value & 0xff);
 }
@@ -330,15 +333,15 @@ static void xive_tm_set_os_cppr(XiveTCTX *tctx, hwaddr offset,
  * Adjust the IPB to allow a CPU to process event queues of other
  * priorities during one physical interrupt cycle.
  */
-static void xive_tm_set_os_pending(XiveTCTX *tctx, hwaddr offset,
-                                   uint64_t value, unsigned size)
+static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
+                                   hwaddr offset, uint64_t value, unsigned size)
 {
     ipb_update(&tctx->regs[TM_QW1_OS], value & 0xff);
     xive_tctx_notify(tctx, TM_QW1_OS);
 }
 
-static uint64_t xive_tm_pull_os_ctx(XiveTCTX *tctx, hwaddr offset,
-                                    unsigned size)
+static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
+                                    hwaddr offset, unsigned size)
 {
     uint32_t qw1w2_prev = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
     uint32_t qw1w2;
@@ -356,9 +359,11 @@ typedef struct XiveTmOp {
     uint8_t  page_offset;
     uint32_t op_offset;
     unsigned size;
-    void     (*write_handler)(XiveTCTX *tctx, hwaddr offset, uint64_t value,
-                              unsigned size);
-    uint64_t (*read_handler)(XiveTCTX *tctx, hwaddr offset, unsigned size);
+    void     (*write_handler)(XivePresenter *xptr, XiveTCTX *tctx,
+                              hwaddr offset,
+                              uint64_t value, unsigned size);
+    uint64_t (*read_handler)(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
+                             unsigned size);
 } XiveTmOp;
 
 static const XiveTmOp xive_tm_operations[] = {
@@ -404,8 +409,8 @@ static const XiveTmOp *xive_tm_find_op(hwaddr offset, unsigned size, bool write)
 /*
  * TIMA MMIO handlers
  */
-void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
-                        unsigned size)
+void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
+                        uint64_t value, unsigned size)
 {
     const XiveTmOp *xto;
 
@@ -422,7 +427,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
             qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA "
                           "@%"HWADDR_PRIx"\n", offset);
         } else {
-            xto->write_handler(tctx, offset, value, size);
+            xto->write_handler(xptr, tctx, offset, value, size);
         }
         return;
     }
@@ -432,7 +437,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
      */
     xto = xive_tm_find_op(offset, size, true);
     if (xto) {
-        xto->write_handler(tctx, offset, value, size);
+        xto->write_handler(xptr, tctx, offset, value, size);
         return;
     }
 
@@ -442,7 +447,8 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
     xive_tm_raw_write(tctx, offset, value, size);
 }
 
-uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size)
+uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
+                           unsigned size)
 {
     const XiveTmOp *xto;
 
@@ -460,7 +466,7 @@ uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size)
                           "@%"HWADDR_PRIx"\n", offset);
             return -1;
         }
-        return xto->read_handler(tctx, offset, size);
+        return xto->read_handler(xptr, tctx, offset, size);
     }
 
     /*
@@ -468,7 +474,7 @@ uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size)
      */
     xto = xive_tm_find_op(offset, size, false);
     if (xto) {
-        return xto->read_handler(tctx, offset, size);
+        return xto->read_handler(xptr, tctx, offset, size);
     }
 
     /*
@@ -482,14 +488,14 @@ static void xive_tm_write(void *opaque, hwaddr offset,
 {
     XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
 
-    xive_tctx_tm_write(tctx, offset, value, size);
+    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
 }
 
 static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
 {
     XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
 
-    return xive_tctx_tm_read(tctx, offset, size);
+    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
 }
 
 const MemoryRegionOps xive_tm_ops = {
-- 
2.21.0

[Qemu-devel] [PATCH v4 10/25] ppc/pnv: Clarify how the TIMA is accessed on a multichip system

[Cédric Le Goater]

The TIMA MMIO space is shared among the chips. To identify the chip
from which the access is being done, the PowerBUS uses a 'chip' field
in the load/store messages. QEMU does not model these messages,
instead, we extract the chip id from the CPU PIR and do a lookup at
the machine level to fetch the targeted interrupt controller.

Introduce pnv_get_chip() and pnv_xive_tm_get_xive() helpers to clarify
this process in pnv_xive_get_tctx(). The latter will be removed in the
subsequent patches but the same principle will be kept.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv.h | 13 +++++++++++++
 hw/intc/pnv_xive.c   | 40 +++++++++++++++++++++++-----------------
 2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 1cdbe55bf86c..5e01a9f3df95 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -185,6 +185,19 @@ static inline bool pnv_is_power9(PnvMachineState *pnv)
     return pnv_chip_is_power9(pnv->chips[0]);
 }
 
+static inline PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id)
+{
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        PnvChip *chip = pnv->chips[i];
+        if (chip->chip_id == chip_id) {
+            return chip;
+        }
+    }
+    return NULL;
+}
+
 #define PNV_FDT_ADDR          0x01000000
 #define PNV_TIMEBASE_FREQ     512000000ULL
 
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 5c9483b394ab..3d6fcf9ac139 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -464,31 +464,37 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
     return count;
 }
 
+/*
+ * The TIMA MMIO space is shared among the chips and to identify the
+ * chip from which the access is being done, we extract the chip id
+ * from the PIR.
+ */
+static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    PnvChip *chip;
+    PnvXive *xive;
+
+    chip = pnv_get_chip(pnv, cpu_chip_id(cpu));
+    assert(chip);
+    xive = &PNV9_CHIP(chip)->xive;
+
+    if (!pnv_xive_is_cpu_enabled(xive, cpu)) {
+        xive_error(xive, "IC: CPU %x is not enabled", cpu_pir(cpu));
+    }
+    return xive;
+}
+
 static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
-    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
-    PnvXive *xive = NULL;
-    CPUPPCState *env = &cpu->env;
-    int pir = env->spr_cb[SPR_PIR].default_value;
+    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
 
-    /*
-     * Perform an extra check on the HW thread enablement.
-     *
-     * The TIMA is shared among the chips and to identify the chip
-     * from which the access is being done, we extract the chip id
-     * from the PIR.
-     */
-    xive = pnv_xive_get_ic((pir >> 8) & 0xf);
     if (!xive) {
         return NULL;
     }
 
-    if (!(xive->regs[PC_THREAD_EN_REG0 >> 3] & PPC_BIT(pir & 0x3f))) {
-        xive_error(PNV_XIVE(xrtr), "IC: CPU %x is not enabled", pir);
-    }
-
-    return tctx;
+    return XIVE_TCTX(pnv_cpu_state(cpu)->intc);
 }
 
 /*
-- 
2.21.0

[Qemu-devel] [PATCH v4 11/25] ppc/xive: Move the TIMA operations to the controller model

[Cédric Le Goater]

This also removes the need of the get_tctx() XiveRouter handler in the
core XIVE framework.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  1 -
 hw/intc/pnv_xive.c    | 35 ++++++++++++++++++++++++++++++++++-
 hw/intc/spapr_xive.c  | 33 +++++++++++++++++++++++++++++++--
 hw/intc/xive.c        | 29 -----------------------------
 4 files changed, 65 insertions(+), 33 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 536deea8c622..9d9cd88dd17e 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -462,7 +462,6 @@ typedef struct XiveENDSource {
 #define XIVE_TM_OS_PAGE         0x2
 #define XIVE_TM_USER_PAGE       0x3
 
-extern const MemoryRegionOps xive_tm_ops;
 void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
                         uint64_t value, unsigned size);
 uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 3d6fcf9ac139..40e18fb44811 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1475,6 +1475,39 @@ static const MemoryRegionOps xive_tm_indirect_ops = {
     },
 };
 
+static void pnv_xive_tm_write(void *opaque, hwaddr offset,
+                              uint64_t value, unsigned size)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
+    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
+    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
+
+    xive_tctx_tm_write(XIVE_PRESENTER(xive), tctx, offset, value, size);
+}
+
+static uint64_t pnv_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
+    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
+    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
+
+    return xive_tctx_tm_read(XIVE_PRESENTER(xive), tctx, offset, size);
+}
+
+const MemoryRegionOps pnv_xive_tm_ops = {
+    .read = pnv_xive_tm_read,
+    .write = pnv_xive_tm_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+};
+
 /*
  * Interrupt controller XSCOM region.
  */
@@ -1832,7 +1865,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
                           "xive-pc", PNV9_XIVE_PC_SIZE);
 
     /* Thread Interrupt Management Area (Direct) */
-    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops,
+    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &pnv_xive_tm_ops,
                           xive, "xive-tima", PNV9_XIVE_TM_SIZE);
 
     qemu_register_reset(pnv_xive_reset, dev);
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index eefc0d4c36b9..e00a9bdd901b 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -222,6 +222,35 @@ void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
     memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
 }
 
+static void spapr_xive_tm_write(void *opaque, hwaddr offset,
+                          uint64_t value, unsigned size)
+{
+    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
+
+    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
+}
+
+static uint64_t spapr_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
+{
+    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
+
+    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
+}
+
+const MemoryRegionOps spapr_xive_tm_ops = {
+    .read = spapr_xive_tm_read,
+    .write = spapr_xive_tm_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
+};
+
 static void spapr_xive_end_reset(XiveEND *end)
 {
     memset(end, 0, sizeof(*end));
@@ -331,8 +360,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
     qemu_register_reset(spapr_xive_reset, dev);
 
     /* TIMA initialization */
-    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops, xive,
-                          "xive.tima", 4ull << TM_SHIFT);
+    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &spapr_xive_tm_ops,
+                          xive, "xive.tima", 4ull << TM_SHIFT);
     sysbus_init_mmio(SYS_BUS_DEVICE(xive), &xive->tm_mmio);
 
     /*
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 9bb09ed6ee7b..11432f04f5c3 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -483,35 +483,6 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
     return xive_tm_raw_read(tctx, offset, size);
 }
 
-static void xive_tm_write(void *opaque, hwaddr offset,
-                          uint64_t value, unsigned size)
-{
-    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
-
-    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
-}
-
-static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
-{
-    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
-
-    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
-}
-
-const MemoryRegionOps xive_tm_ops = {
-    .read = xive_tm_read,
-    .write = xive_tm_write,
-    .endianness = DEVICE_BIG_ENDIAN,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-    },
-    .impl = {
-        .min_access_size = 1,
-        .max_access_size = 8,
-    },
-};
-
 static char *xive_tctx_ring_print(uint8_t *ring)
 {
     uint32_t w2 = xive_tctx_word2(ring);
-- 
2.21.0

[Qemu-devel] [PATCH v4 12/25] ppc/xive: Remove the get_tctx() XiveRouter handler

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  2 --
 hw/intc/pnv_xive.c    | 13 -------------
 hw/intc/spapr_xive.c  |  8 --------
 hw/intc/xive.c        |  7 -------
 4 files changed, 30 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 9d9cd88dd17e..f35ff3b64791 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -351,7 +351,6 @@ typedef struct XiveRouterClass {
                    XiveNVT *nvt);
     int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                      XiveNVT *nvt, uint8_t word_number);
-    XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
 } XiveRouterClass;
 
 int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
@@ -364,7 +363,6 @@ 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);
-XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs);
 void xive_router_notify(XiveNotifier *xn, uint32_t lisn);
 
 /*
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 40e18fb44811..74d6ccbea3d6 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -485,18 +485,6 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
     return xive;
 }
 
-static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
-{
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
-
-    if (!xive) {
-        return NULL;
-    }
-
-    return XIVE_TCTX(pnv_cpu_state(cpu)->intc);
-}
-
 /*
  * The internal sources (IPIs) of the interrupt controller have no
  * knowledge of the XIVE chip on which they reside. Encode the block
@@ -1921,7 +1909,6 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
     xrc->write_end = pnv_xive_write_end;
     xrc->get_nvt = pnv_xive_get_nvt;
     xrc->write_nvt = pnv_xive_write_nvt;
-    xrc->get_tctx = pnv_xive_get_tctx;
 
     xnc->notify = pnv_xive_notify;
     xpc->match_nvt  = pnv_xive_match_nvt;
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index e00a9bdd901b..864f50167c65 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -444,13 +444,6 @@ static int spapr_xive_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk,
     g_assert_not_reached();
 }
 
-static XiveTCTX *spapr_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
-{
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-
-    return spapr_cpu_state(cpu)->tctx;
-}
-
 static int spapr_xive_match_nvt(XivePresenter *xptr, uint8_t format,
                                 uint8_t nvt_blk, uint32_t nvt_idx,
                                 bool cam_ignore, uint8_t priority,
@@ -578,7 +571,6 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
     xrc->write_end = spapr_xive_write_end;
     xrc->get_nvt = spapr_xive_get_nvt;
     xrc->write_nvt = spapr_xive_write_nvt;
-    xrc->get_tctx = spapr_xive_get_tctx;
 
     xpc->match_nvt  = spapr_xive_match_nvt;
 }
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 11432f04f5c3..68d3361d1c3f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1270,13 +1270,6 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
    return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
 }
 
-XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
-{
-    XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
-
-    return xrc->get_tctx(xrtr, cs);
-}
-
 /*
  * Encode the HW CAM line in the block group mode format :
  *
-- 
2.21.0

[Qemu-devel] [PATCH v4 13/25] ppc/xive: Introduce a xive_tctx_ipb_update() helper

[Cédric Le Goater]

We will use it to resend missed interrupts when a vCPU context is
pushed a HW thread.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  1 +
 hw/intc/xive.c        | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index f35ff3b64791..a461753f5da5 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -467,6 +467,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
 
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
 Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
+void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
 
 static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
 {
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 68d3361d1c3f..5f7c37b091a7 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -135,6 +135,15 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, uint8_t cppr)
     xive_tctx_notify(tctx, ring);
 }
 
+void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
+{
+    uint8_t *regs = &tctx->regs[ring];
+
+    regs[TM_IPB] |= ipb;
+    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
+    xive_tctx_notify(tctx, ring);
+}
+
 static inline uint32_t xive_tctx_word2(uint8_t *ring)
 {
     return *((uint32_t *) &ring[TM_WORD2]);
@@ -336,8 +345,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
 static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
                                    hwaddr offset, uint64_t value, unsigned size)
 {
-    ipb_update(&tctx->regs[TM_QW1_OS], value & 0xff);
-    xive_tctx_notify(tctx, TM_QW1_OS);
+    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
 }
 
 static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
@@ -1382,8 +1390,7 @@ static bool xive_presenter_notify(uint8_t format,
 
     /* handle CPU exception delivery */
     if (count) {
-        ipb_update(&match.tctx->regs[match.ring], priority);
-        xive_tctx_notify(match.tctx, match.ring);
+        xive_tctx_ipb_update(match.tctx, match.ring, priority_to_ipb(priority));
     }
 
     return count;
-- 
2.21.0

[Qemu-devel] [PATCH v4 14/25] ppc/xive: Introduce helpers for the NVT id

[Cédric Le Goater]

The NVT space is 19 bits wide, giving a maximum of 512K per chip. When
dispatched on a HW thread, the NVT identifier of a vCPU is pushed/stored
in the CAM line (word2) of the thread interrupt context.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h      |  5 -----
 include/hw/ppc/xive_regs.h | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index a461753f5da5..794dfcaae0f8 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -469,11 +469,6 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
 Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
 void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
 
-static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
-{
-    return (nvt_blk << 19) | nvt_idx;
-}
-
 /*
  * KVM XIVE device helpers
  */
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 08c8bf7172e2..3d7b6fd09664 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -251,4 +251,25 @@ typedef struct XiveNVT {
 
 #define xive_nvt_is_valid(nvt)    (be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
 
+/*
+ * The VP number space in a block is defined by the END_W6_NVT_INDEX
+ * field of the XIVE END
+ */
+#define XIVE_NVT_SHIFT                19
+
+static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
+{
+    return (nvt_blk << XIVE_NVT_SHIFT) | nvt_idx;
+}
+
+static inline uint32_t xive_nvt_idx(uint32_t cam_line)
+{
+    return cam_line & ((1 << XIVE_NVT_SHIFT) - 1);
+}
+
+static inline uint32_t xive_nvt_blk(uint32_t cam_line)
+{
+    return (cam_line >> XIVE_NVT_SHIFT) & 0xf;
+}
+
 #endif /* PPC_XIVE_REGS_H */
-- 
2.21.0

[Qemu-devel] [PATCH v4 15/25] ppc/xive: Synthesize interrupt from the saved IPB in the NVT

[Cédric Le Goater]

When an interrupt can not be presented to a vCPU, the XIVE presenter
updates the Interrupt Pending Buffer of the XIVE NVT if backlog is
activated in the END.

Later, when the same vCPU is dispatched, its context is pushed in the
thread context registers and the VO bit is set in the CAM line word to
activate the context. The HW grabs the associated NVT to pull the
pending bits, and merges them with the IPB of the TIMA. If interrupts
were missed while the vCPU was not dispatched, these are synthesized
in this sequence.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive_regs.h |  1 +
 hw/intc/xive.c             | 67 +++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 3d7b6fd09664..dea0318e7e89 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -235,6 +235,7 @@ typedef struct XiveNVT {
         uint32_t        w2;
         uint32_t        w3;
         uint32_t        w4;
+#define NVT_W4_IPB               PPC_BITMASK32(16, 23)
         uint32_t        w5;
         uint32_t        w6;
         uint32_t        w7;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 5f7c37b091a7..f47b0cf2b053 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -47,12 +47,6 @@ static uint8_t ipb_to_pipr(uint8_t ibp)
     return ibp ? clz32((uint32_t)ibp << 24) : 0xff;
 }
 
-static void ipb_update(uint8_t *regs, uint8_t priority)
-{
-    regs[TM_IPB] |= priority_to_ipb(priority);
-    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
-}
-
 static uint8_t exception_mask(uint8_t ring)
 {
     switch (ring) {
@@ -359,6 +353,55 @@ static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
     return qw1w2;
 }
 
+static void xive_tctx_need_resend(XiveRouter *xrtr, XiveTCTX *tctx,
+                                  uint8_t nvt_blk, uint32_t nvt_idx)
+{
+    XiveNVT nvt;
+    uint8_t ipb;
+
+    /*
+     * Grab the associated NVT to pull the pending bits, and merge
+     * them with the IPB of the thread interrupt context registers
+     */
+    if (xive_router_get_nvt(xrtr, nvt_blk, nvt_idx, &nvt)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid NVT %x/%x\n",
+                          nvt_blk, nvt_idx);
+        return;
+    }
+
+    ipb = xive_get_field32(NVT_W4_IPB, nvt.w4);
+
+    if (ipb) {
+        /* Reset the NVT value */
+        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, 0);
+        xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
+
+        /* Merge in current context */
+        xive_tctx_ipb_update(tctx, TM_QW1_OS, ipb);
+    }
+}
+
+/*
+ * Updating the OS CAM line can trigger a resend of interrupt
+ */
+static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
+                                hwaddr offset, uint64_t value, unsigned size)
+{
+    uint32_t qw1w2 = value;
+    uint8_t nvt_blk = xive_nvt_blk(qw1w2);
+    uint32_t nvt_idx = xive_nvt_idx(qw1w2);
+    bool vo = !!(qw1w2 & TM_QW1W2_VO);
+
+    /* First update the registers */
+    qw1w2 = cpu_to_be32(qw1w2);
+    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+
+    /* Check the interrupt pending bits */
+    if (vo) {
+        xive_tctx_need_resend(XIVE_ROUTER(xptr), tctx, nvt_blk, nvt_idx);
+    }
+}
+
 /*
  * Define a mapping of "special" operations depending on the TIMA page
  * offset and the size of the operation.
@@ -380,6 +423,7 @@ static const XiveTmOp xive_tm_operations[] = {
      * effects
      */
     { XIVE_TM_OS_PAGE, TM_QW1_OS + TM_CPPR,   1, xive_tm_set_os_cppr, NULL },
+    { XIVE_TM_HV_PAGE, TM_QW1_OS + TM_WORD2,     4, xive_tm_push_os_ctx, NULL },
     { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_CPPR, 1, xive_tm_set_hv_cppr, NULL },
     { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, xive_tm_vt_push, NULL },
     { XIVE_TM_HV_PAGE, TM_QW3_HV_PHYS + TM_WORD2, 1, NULL, xive_tm_vt_poll },
@@ -1523,14 +1567,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
      * - logical server : forward request to IVPE (not supported)
      */
     if (xive_end_is_backlog(&end)) {
+        uint8_t ipb;
+
         if (format == 1) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "XIVE: END %x/%x invalid config: F1 & backlog\n",
                           end_blk, end_idx);
             return;
         }
-        /* Record the IPB in the associated NVT structure */
-        ipb_update((uint8_t *) &nvt.w4, priority);
+        /*
+         * Record the IPB in the associated NVT structure for later
+         * use. The presenter will resend the interrupt when the vCPU
+         * is dispatched again on a HW thread.
+         */
+        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
+        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
         xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
 
         /*
-- 
2.21.0

[Qemu-devel] [PATCH v4 16/25] ppc/pnv: Remove pnv_xive_vst_size() routine

[Cédric Le Goater]

pnv_xive_vst_size() tries to compute the size of a VSD table from the
information given by FW. The number of entries of the table are
deduced from the result and the MMIO regions of the ESBs and the ENDS
are also resized with the computed value.

But for indirect tables, the result is incorrect. An indirect table is
a one page array of VSDs pointing to subpages containing XIVE virtual
structures. The number of first level VSD entries is page aligned and
the overall computed size of the table is too large. It can also be
completely wrong when the first VSD entry is not yet initialized.

Remove pnv_xive_vst_size() and use a simpler form for direct tables.
This is only useful when outputting the XIVE sources on the monitor
and to resize the ESB MMIO window.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 112 +++++++++++++++++----------------------------
 1 file changed, 43 insertions(+), 69 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 74d6ccbea3d6..b7d505839e68 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -123,36 +123,22 @@ static uint64_t pnv_xive_vst_page_size_allowed(uint32_t page_shift)
          page_shift == 21 || page_shift == 24;
 }
 
-static uint64_t pnv_xive_vst_size(uint64_t vsd)
-{
-    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
-
-    /*
-     * Read the first descriptor to get the page size of the indirect
-     * table.
-     */
-    if (VSD_INDIRECT & vsd) {
-        uint32_t nr_pages = vst_tsize / XIVE_VSD_SIZE;
-        uint32_t page_shift;
-
-        vsd = ldq_be_dma(&address_space_memory, vsd & VSD_ADDRESS_MASK);
-        page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
-
-        if (!pnv_xive_vst_page_size_allowed(page_shift)) {
-            return 0;
-        }
-
-        return nr_pages * (1ull << page_shift);
-    }
-
-    return vst_tsize;
-}
-
 static uint64_t pnv_xive_vst_addr_direct(PnvXive *xive, uint32_t type,
                                          uint64_t vsd, uint32_t idx)
 {
     const XiveVstInfo *info = &vst_infos[type];
     uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
+    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
+    uint32_t idx_max;
+
+    idx_max = vst_tsize / info->size - 1;
+    if (idx > idx_max) {
+#ifdef XIVE_DEBUG
+        xive_error(xive, "VST: %s entry %x out of range [ 0 .. %x ] !?",
+                   info->name, idx, idx_max);
+#endif
+        return 0;
+    }
 
     return vst_addr + idx * info->size;
 }
@@ -215,7 +201,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
 {
     const XiveVstInfo *info = &vst_infos[type];
     uint64_t vsd;
-    uint32_t idx_max;
 
     if (blk >= info->max_blocks) {
         xive_error(xive, "VST: invalid block id %d for VST %s %d !?",
@@ -232,15 +217,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
         return xive ? pnv_xive_vst_addr(xive, type, blk, idx) : 0;
     }
 
-    idx_max = pnv_xive_vst_size(vsd) / info->size - 1;
-    if (idx > idx_max) {
-#ifdef XIVE_DEBUG
-        xive_error(xive, "VST: %s entry %x/%x out of range [ 0 .. %x ] !?",
-                   info->name, blk, idx, idx_max);
-#endif
-        return 0;
-    }
-
     if (VSD_INDIRECT & vsd) {
         return pnv_xive_vst_addr_indirect(xive, type, vsd, idx);
     }
@@ -519,19 +495,12 @@ static uint64_t pnv_xive_pc_size(PnvXive *xive)
     return (~xive->regs[CQ_PC_BARM >> 3] + 1) & CQ_PC_BARM_MASK;
 }
 
-static uint32_t pnv_xive_nr_ipis(PnvXive *xive)
+static uint32_t pnv_xive_nr_ipis(PnvXive *xive, uint8_t blk)
 {
-    uint8_t blk = xive->chip->chip_id;
-
-    return pnv_xive_vst_size(xive->vsds[VST_TSEL_SBE][blk]) * SBE_PER_BYTE;
-}
-
-static uint32_t pnv_xive_nr_ends(PnvXive *xive)
-{
-    uint8_t blk = xive->chip->chip_id;
+    uint64_t vsd = xive->vsds[VST_TSEL_SBE][blk];
+    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
 
-    return pnv_xive_vst_size(xive->vsds[VST_TSEL_EQDT][blk])
-        / vst_infos[VST_TSEL_EQDT].size;
+    return VSD_INDIRECT & vsd ? 0 : vst_tsize * SBE_PER_BYTE;
 }
 
 /*
@@ -664,6 +633,7 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
     XiveSource *xsrc = &xive->ipi_source;
     const XiveVstInfo *info = &vst_infos[type];
     uint32_t page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
+    uint64_t vst_tsize = 1ull << page_shift;
     uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
 
     /* Basic checks */
@@ -699,11 +669,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
 
     case VST_TSEL_EQDT:
         /*
-         * Backing store pages for the END. Compute the number of ENDs
-         * provisioned by FW and resize the END ESB window accordingly.
+         * Backing store pages for the END.
+         *
+         * If the table is direct, we can compute the number of PQ
+         * entries provisioned by FW (such as skiboot) and resize the
+         * END ESB window accordingly.
          */
-        memory_region_set_size(&end_xsrc->esb_mmio, pnv_xive_nr_ends(xive) *
-                               (1ull << (end_xsrc->esb_shift + 1)));
+        if (!(VSD_INDIRECT & vsd)) {
+            memory_region_set_size(&end_xsrc->esb_mmio, (vst_tsize / info->size)
+                                   * (1ull << xsrc->esb_shift));
+        }
         memory_region_add_subregion(&xive->end_edt_mmio, 0,
                                     &end_xsrc->esb_mmio);
         break;
@@ -712,11 +687,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
         /*
          * Backing store pages for the source PQ bits. The model does
          * not use these PQ bits backed in RAM because the XiveSource
-         * model has its own. Compute the number of IRQs provisioned
-         * by FW and resize the IPI ESB window accordingly.
+         * model has its own.
+         *
+         * If the table is direct, we can compute the number of PQ
+         * entries provisioned by FW (such as skiboot) and resize the
+         * ESB window accordingly.
          */
-        memory_region_set_size(&xsrc->esb_mmio, pnv_xive_nr_ipis(xive) *
-                               (1ull << xsrc->esb_shift));
+        if (!(VSD_INDIRECT & vsd)) {
+            memory_region_set_size(&xsrc->esb_mmio, vst_tsize * SBE_PER_BYTE
+                                   * (1ull << xsrc->esb_shift));
+        }
         memory_region_add_subregion(&xive->ipi_edt_mmio, 0, &xsrc->esb_mmio);
         break;
 
@@ -1666,8 +1646,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     XiveRouter *xrtr = XIVE_ROUTER(xive);
     uint8_t blk = xive->chip->chip_id;
     uint32_t srcno0 = XIVE_SRCNO(blk, 0);
-    uint32_t nr_ipis = pnv_xive_nr_ipis(xive);
-    uint32_t nr_ends = pnv_xive_nr_ends(xive);
+    uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
     XiveEAS eas;
     XiveEND end;
     int i;
@@ -1687,21 +1666,16 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
         }
     }
 
-    monitor_printf(mon, "XIVE[%x] ENDT %08x .. %08x\n", blk, 0, nr_ends - 1);
-    for (i = 0; i < nr_ends; i++) {
-        if (xive_router_get_end(xrtr, blk, i, &end)) {
-            break;
-        }
-        xive_end_pic_print_info(&end, i, mon);
+    monitor_printf(mon, "XIVE[%x] ENDT\n", blk);
+    i = 0;
+    while (!xive_router_get_end(xrtr, blk, i, &end)) {
+        xive_end_pic_print_info(&end, i++, mon);
     }
 
-    monitor_printf(mon, "XIVE[%x] END Escalation %08x .. %08x\n", blk, 0,
-                   nr_ends - 1);
-    for (i = 0; i < nr_ends; i++) {
-        if (xive_router_get_end(xrtr, blk, i, &end)) {
-            break;
-        }
-        xive_end_eas_pic_print_info(&end, i, mon);
+    monitor_printf(mon, "XIVE[%x] END Escalation EAT\n", blk);
+    i = 0;
+    while (!xive_router_get_end(xrtr, blk, i, &end)) {
+        xive_end_eas_pic_print_info(&end, i++, mon);
     }
 }
 
-- 
2.21.0

[Qemu-devel] [PATCH v4 17/25] ppc/pnv: Dump the XIVE NVT table

[Cédric Le Goater]

This is to track the configuration of the base END index of the vCPU
and the Interrupt Pending Buffer. The NVT IPB is updated when an
interrupt can not be presented to a vCPU.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive_regs.h |  2 ++
 hw/intc/pnv_xive.c         | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index dea0318e7e89..dd42c33cef35 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -232,6 +232,8 @@ typedef struct XiveNVT {
         uint32_t        w0;
 #define NVT_W0_VALID             PPC_BIT32(0)
         uint32_t        w1;
+#define NVT_W1_EQ_BLOCK          PPC_BITMASK32(0, 3)
+#define NVT_W1_EQ_INDEX          PPC_BITMASK32(4, 31)
         uint32_t        w2;
         uint32_t        w3;
         uint32_t        w4;
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index b7d505839e68..782775136288 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1641,6 +1641,21 @@ static const MemoryRegionOps pnv_xive_pc_ops = {
     },
 };
 
+static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
+                                    Monitor *mon)
+{
+    uint8_t  eq_blk = xive_get_field32(NVT_W1_EQ_BLOCK, nvt->w1);
+    uint32_t eq_idx = xive_get_field32(NVT_W1_EQ_INDEX, nvt->w1);
+
+    if (!xive_nvt_is_valid(nvt)) {
+        return;
+    }
+
+    monitor_printf(mon, "  %08x end:%02x/%04x IPB:%02x\n", nvt_idx,
+                   eq_blk, eq_idx,
+                   xive_get_field32(NVT_W4_IPB, nvt->w4));
+}
+
 void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
 {
     XiveRouter *xrtr = XIVE_ROUTER(xive);
@@ -1649,6 +1664,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
     XiveEAS eas;
     XiveEND end;
+    XiveNVT nvt;
     int i;
 
     monitor_printf(mon, "XIVE[%x] Source %08x .. %08x\n", blk, srcno0,
@@ -1677,6 +1693,12 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     while (!xive_router_get_end(xrtr, blk, i, &end)) {
         xive_end_eas_pic_print_info(&end, i++, mon);
     }
+
+    monitor_printf(mon, "XIVE[%x] NVTT\n", blk);
+    i = 0;
+    while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
+        xive_nvt_pic_print_info(&nvt, i++, mon);
+    }
 }
 
 static void pnv_xive_reset(void *dev)
-- 
2.21.0

[Qemu-devel] [PATCH v4 18/25] ppc/pnv: Skip empty slots of the XIVE NVT table

[Cédric Le Goater]

We try to loop on the full table skipping empty indirect pages which
are not necessarily allocated. This is useful to dump the contexts of
the KVM vCPUs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 782775136288..a986d4ed9364 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1641,6 +1641,12 @@ static const MemoryRegionOps pnv_xive_pc_ops = {
     },
 };
 
+/*
+ * skiboot uses an indirect NVT table with 64k subpages
+ */
+#define XIVE_NVT_COUNT          (1 << XIVE_NVT_SHIFT)
+#define XIVE_NVT_PER_PAGE       (0x10000 / sizeof(XiveNVT))
+
 static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
                                     Monitor *mon)
 {
@@ -1694,10 +1700,12 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
         xive_end_eas_pic_print_info(&end, i++, mon);
     }
 
-    monitor_printf(mon, "XIVE[%x] NVTT\n", blk);
-    i = 0;
-    while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
-        xive_nvt_pic_print_info(&nvt, i++, mon);
+    monitor_printf(mon, "XIVE[%x] NVTT %08x .. %08x\n", blk, 0,
+                   XIVE_NVT_COUNT - 1);
+    for (i = 0; i < XIVE_NVT_COUNT; i += XIVE_NVT_PER_PAGE) {
+        while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
+            xive_nvt_pic_print_info(&nvt, i++, mon);
+        }
     }
 }
 
-- 
2.21.0

[Qemu-devel] [PATCH v4 19/25] ppc/pnv: Introduce a pnv_xive_block_id() helper

[Cédric Le Goater]

When PC_TCTXT_CHIPID_OVERRIDE is configured, the PC_TCTXT_CHIPID field
overrides the hardwired chip ID in the Powerbus operations and for CAM
compares. This is typically used in the one block-per-chip configuration
to associate a unique block id number to each IC of the system.

Simplify the model with a pnv_xive_block_id() helper and remove
'tctx_chipid' which becomes useless.

The model does support multiple blocks per chip.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv_xive.h |  3 --
 hw/intc/pnv_xive.c        | 68 ++++++++++++++++++++-------------------
 2 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
index 4fdaa9247d65..f4c7caad40ee 100644
--- a/include/hw/ppc/pnv_xive.h
+++ b/include/hw/ppc/pnv_xive.h
@@ -72,9 +72,6 @@ typedef struct PnvXive {
     /* Interrupt controller registers */
     uint64_t      regs[0x300];
 
-    /* Can be configured by FW */
-    uint32_t      tctx_chipid;
-
     /*
      * Virtual Structure Descriptor tables : EAT, SBE, ENDT, NVTT, IRQ
      * These are in a SRAM protected by ECC.
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index a986d4ed9364..8c352315f6f5 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -85,13 +85,30 @@ static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
     return (word & ~mask) | ((value << ctz64(mask)) & mask);
 }
 
+/*
+ * When PC_TCTXT_CHIPID_OVERRIDE is configured, the PC_TCTXT_CHIPID
+ * field overrides the hardwired chip ID in the Powerbus operations
+ * and for CAM compares
+ */
+static uint8_t pnv_xive_block_id(PnvXive *xive)
+{
+    uint8_t blk = xive->chip->chip_id;
+    uint64_t cfg_val = xive->regs[PC_TCTXT_CFG >> 3];
+
+    if (cfg_val & PC_TCTXT_CHIPID_OVERRIDE) {
+        blk = GETFIELD(PC_TCTXT_CHIPID, cfg_val);
+    }
+
+    return blk;
+}
+
 /*
  * Remote access to controllers. HW uses MMIOs. For now, a simple scan
  * of the chips is good enough.
  *
  * TODO: Block scope support
  */
-static PnvXive *pnv_xive_get_ic(uint8_t blk)
+static PnvXive *pnv_xive_get_remote(uint8_t blk)
 {
     PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
     int i;
@@ -100,7 +117,7 @@ static PnvXive *pnv_xive_get_ic(uint8_t blk)
         Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
         PnvXive *xive = &chip9->xive;
 
-        if (xive->chip->chip_id == blk) {
+        if (pnv_xive_block_id(xive) == blk) {
             return xive;
         }
     }
@@ -212,7 +229,7 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
 
     /* Remote VST access */
     if (GETFIELD(VSD_MODE, vsd) == VSD_MODE_FORWARD) {
-        xive = pnv_xive_get_ic(blk);
+        xive = pnv_xive_get_remote(blk);
 
         return xive ? pnv_xive_vst_addr(xive, type, blk, idx) : 0;
     }
@@ -360,7 +377,10 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
 {
     PnvXive *xive = PNV_XIVE(xrtr);
 
-    if (pnv_xive_get_ic(blk) != xive) {
+    /*
+     * EAT lookups should be local to the IC
+     */
+    if (pnv_xive_block_id(xive) != blk) {
         xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
         return -1;
     }
@@ -471,7 +491,7 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
 static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
 {
     PnvXive *xive = PNV_XIVE(xn);
-    uint8_t blk = xive->chip->chip_id;
+    uint8_t blk = pnv_xive_block_id(xive);
 
     xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
 }
@@ -835,20 +855,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
     case PC_TCTXT_CFG:
         /*
          * TODO: block group support
-         *
-         * PC_TCTXT_CFG_BLKGRP_EN
-         * PC_TCTXT_CFG_HARD_CHIPID_BLK :
-         *   Moves the chipid into block field for hardwired CAM compares.
-         *   Block offset value is adjusted to 0b0..01 & ThrdId
-         *
-         *   Will require changes in xive_presenter_tctx_match(). I am
-         *   not sure how to handle that yet.
          */
-
-        /* Overrides hardwired chip ID with the chip ID field */
-        if (val & PC_TCTXT_CHIPID_OVERRIDE) {
-            xive->tctx_chipid = GETFIELD(PC_TCTXT_CHIPID, val);
-        }
         break;
     case PC_TCTXT_TRACK:
         /*
@@ -1665,7 +1672,8 @@ static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
 void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
 {
     XiveRouter *xrtr = XIVE_ROUTER(xive);
-    uint8_t blk = xive->chip->chip_id;
+    uint8_t blk = pnv_xive_block_id(xive);
+    uint8_t chip_id = xive->chip->chip_id;
     uint32_t srcno0 = XIVE_SRCNO(blk, 0);
     uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
     XiveEAS eas;
@@ -1673,12 +1681,12 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     XiveNVT nvt;
     int i;
 
-    monitor_printf(mon, "XIVE[%x] Source %08x .. %08x\n", blk, srcno0,
-                   srcno0 + nr_ipis - 1);
+    monitor_printf(mon, "XIVE[%x] #%d Source %08x .. %08x\n", chip_id, blk,
+                   srcno0, srcno0 + nr_ipis - 1);
     xive_source_pic_print_info(&xive->ipi_source, srcno0, mon);
 
-    monitor_printf(mon, "XIVE[%x] EAT %08x .. %08x\n", blk, srcno0,
-                   srcno0 + nr_ipis - 1);
+    monitor_printf(mon, "XIVE[%x] #%d EAT %08x .. %08x\n", chip_id, blk,
+                   srcno0, srcno0 + nr_ipis - 1);
     for (i = 0; i < nr_ipis; i++) {
         if (xive_router_get_eas(xrtr, blk, i, &eas)) {
             break;
@@ -1688,20 +1696,20 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
         }
     }
 
-    monitor_printf(mon, "XIVE[%x] ENDT\n", blk);
+    monitor_printf(mon, "XIVE[%x] #%d ENDT\n", chip_id, blk);
     i = 0;
     while (!xive_router_get_end(xrtr, blk, i, &end)) {
         xive_end_pic_print_info(&end, i++, mon);
     }
 
-    monitor_printf(mon, "XIVE[%x] END Escalation EAT\n", blk);
+    monitor_printf(mon, "XIVE[%x] #%d END Escalation EAT\n", chip_id, blk);
     i = 0;
     while (!xive_router_get_end(xrtr, blk, i, &end)) {
         xive_end_eas_pic_print_info(&end, i++, mon);
     }
 
-    monitor_printf(mon, "XIVE[%x] NVTT %08x .. %08x\n", blk, 0,
-                   XIVE_NVT_COUNT - 1);
+    monitor_printf(mon, "XIVE[%x] #%d NVTT %08x .. %08x\n", chip_id, blk,
+                   0, XIVE_NVT_COUNT - 1);
     for (i = 0; i < XIVE_NVT_COUNT; i += XIVE_NVT_PER_PAGE) {
         while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
             xive_nvt_pic_print_info(&nvt, i++, mon);
@@ -1715,12 +1723,6 @@ static void pnv_xive_reset(void *dev)
     XiveSource *xsrc = &xive->ipi_source;
     XiveENDSource *end_xsrc = &xive->end_source;
 
-    /*
-     * Use the PnvChip id to identify the XIVE interrupt controller.
-     * It can be overriden by configuration at runtime.
-     */
-    xive->tctx_chipid = xive->chip->chip_id;
-
     /* Default page size (Should be changed at runtime to 64k) */
     xive->ic_shift = xive->vc_shift = xive->pc_shift = 12;
 
-- 
2.21.0

[Qemu-devel] [PATCH v4 20/25] ppc/pnv: Extend XiveRouter with a get_block_id() handler

[Cédric Le Goater]

When doing CAM line compares, fetch the block id from the interrupt
controller which can have set the PC_TCTXT_CHIPID field.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h |  2 +-
 hw/intc/pnv_xive.c    |  6 ++++++
 hw/intc/spapr_xive.c  |  6 ++++++
 hw/intc/xive.c        | 21 ++++++++++++++++-----
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 794dfcaae0f8..1f084b6e13a5 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -351,6 +351,7 @@ typedef struct XiveRouterClass {
                    XiveNVT *nvt);
     int (*write_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                      XiveNVT *nvt, uint8_t word_number);
+    uint8_t (*get_block_id)(XiveRouter *xrtr);
 } XiveRouterClass;
 
 int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
@@ -431,7 +432,6 @@ typedef struct XiveENDSource {
     DeviceState parent;
 
     uint32_t        nr_ends;
-    uint8_t         block_id;
 
     /* ESB memory region */
     uint32_t        esb_shift;
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 8c352315f6f5..8fa78e1c6cd9 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -460,6 +460,11 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
     return count;
 }
 
+static uint8_t pnv_xive_get_block_id(XiveRouter *xrtr)
+{
+    return pnv_xive_block_id(PNV_XIVE(xrtr));
+}
+
 /*
  * The TIMA MMIO space is shared among the chips and to identify the
  * chip from which the access is being done, we extract the chip id
@@ -1915,6 +1920,7 @@ static void pnv_xive_class_init(ObjectClass *klass, void *data)
     xrc->write_end = pnv_xive_write_end;
     xrc->get_nvt = pnv_xive_get_nvt;
     xrc->write_nvt = pnv_xive_write_nvt;
+    xrc->get_block_id = pnv_xive_get_block_id;
 
     xnc->notify = pnv_xive_notify;
     xpc->match_nvt  = pnv_xive_match_nvt;
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 864f50167c65..1a2475811257 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -482,6 +482,11 @@ static int spapr_xive_match_nvt(XivePresenter *xptr, uint8_t format,
     return count;
 }
 
+static uint8_t spapr_xive_get_block_id(XiveRouter *xrtr)
+{
+    return SPAPR_XIVE_BLOCK_ID;
+}
+
 static const VMStateDescription vmstate_spapr_xive_end = {
     .name = TYPE_SPAPR_XIVE "/end",
     .version_id = 1,
@@ -571,6 +576,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
     xrc->write_end = spapr_xive_write_end;
     xrc->get_nvt = spapr_xive_get_nvt;
     xrc->write_nvt = spapr_xive_write_nvt;
+    xrc->get_block_id = spapr_xive_get_block_id;
 
     xpc->match_nvt  = spapr_xive_match_nvt;
 }
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index f47b0cf2b053..dfae584a319f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1322,17 +1322,25 @@ int xive_router_write_nvt(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
    return xrc->write_nvt(xrtr, nvt_blk, nvt_idx, nvt, word_number);
 }
 
+static int xive_router_get_block_id(XiveRouter *xrtr)
+{
+   XiveRouterClass *xrc = XIVE_ROUTER_GET_CLASS(xrtr);
+
+   return xrc->get_block_id(xrtr);
+}
+
 /*
  * Encode the HW CAM line in the block group mode format :
  *
  *   chip << 19 | 0000000 0 0001 thread (7Bit)
  */
-static uint32_t xive_tctx_hw_cam_line(XiveTCTX *tctx)
+static uint32_t xive_tctx_hw_cam_line(XivePresenter *xptr, XiveTCTX *tctx)
 {
     CPUPPCState *env = &POWERPC_CPU(tctx->cs)->env;
     uint32_t pir = env->spr_cb[SPR_PIR].default_value;
+    uint8_t blk = xive_router_get_block_id(XIVE_ROUTER(xptr));
 
-    return xive_nvt_cam_line((pir >> 8) & 0xf, 1 << 7 | (pir & 0x7f));
+    return xive_nvt_cam_line(blk, 1 << 7 | (pir & 0x7f));
 }
 
 /*
@@ -1369,7 +1377,7 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
 
         /* PHYS ring */
         if ((be32_to_cpu(qw3w2) & TM_QW3W2_VT) &&
-            cam == xive_tctx_hw_cam_line(tctx)) {
+            cam == xive_tctx_hw_cam_line(xptr, tctx)) {
             return TM_QW3_HV_PHYS;
         }
 
@@ -1706,7 +1714,11 @@ static uint64_t xive_end_source_read(void *opaque, hwaddr addr, unsigned size)
     uint8_t pq;
     uint64_t ret = -1;
 
-    end_blk = xsrc->block_id;
+    /*
+     * The block id should be deduced from the load address on the END
+     * ESB MMIO but our model only supports a single block per XIVE chip.
+     */
+    end_blk = xive_router_get_block_id(xsrc->xrtr);
     end_idx = addr >> (xsrc->esb_shift + 1);
 
     if (xive_router_get_end(xsrc->xrtr, end_blk, end_idx, &end)) {
@@ -1815,7 +1827,6 @@ static void xive_end_source_realize(DeviceState *dev, Error **errp)
 }
 
 static Property xive_end_source_properties[] = {
-    DEFINE_PROP_UINT8("block-id", XiveENDSource, block_id, 0),
     DEFINE_PROP_UINT32("nr-ends", XiveENDSource, nr_ends, 0),
     DEFINE_PROP_UINT32("shift", XiveENDSource, esb_shift, XIVE_ESB_64K),
     DEFINE_PROP_END_OF_LIST(),
-- 
2.21.0

[Qemu-devel] [PATCH v4 21/25] ppc/pnv: Quiesce some XIVE errors

[Cédric Le Goater]

When dumping the END and NVT tables, the error logging is too noisy.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 8fa78e1c6cd9..4c1fa024cdf5 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -29,7 +29,7 @@
 
 #include "pnv_xive_regs.h"
 
-#define XIVE_DEBUG
+#undef XIVE_DEBUG
 
 /*
  * Virtual structures table (VST)
@@ -174,7 +174,9 @@ static uint64_t pnv_xive_vst_addr_indirect(PnvXive *xive, uint32_t type,
     vsd = ldq_be_dma(&address_space_memory, vsd_addr);
 
     if (!(vsd & VSD_ADDRESS_MASK)) {
+#ifdef XIVE_DEBUG
         xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
+#endif
         return 0;
     }
 
@@ -195,7 +197,9 @@ static uint64_t pnv_xive_vst_addr_indirect(PnvXive *xive, uint32_t type,
         vsd = ldq_be_dma(&address_space_memory, vsd_addr);
 
         if (!(vsd & VSD_ADDRESS_MASK)) {
+#ifdef XIVE_DEBUG
             xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
+#endif
             return 0;
         }
 
-- 
2.21.0

[Qemu-devel] [PATCH v4 22/25] ppc/xive: Introduce a xive_os_cam_decode() helper

[Cédric Le Goater]

The OS CAM line has a special encoding exploited by the HW. Provide a
helper routine to hide the details to the TIMA command handlers. This
also clarifies the endian ness of different variables : 'qw1w2' is
big-endian and 'cam' is native.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index dfae584a319f..cdc4ea8b0e51 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -342,14 +342,29 @@ static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
     xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
 }
 
+static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
+                               uint32_t *nvt_idx, bool *vo)
+{
+    *nvt_blk = xive_nvt_blk(cam);
+    *nvt_idx = xive_nvt_idx(cam);
+    *vo = !!(cam & TM_QW1W2_VO);
+}
+
 static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
                                     hwaddr offset, unsigned size)
 {
-    uint32_t qw1w2_prev = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
-    uint32_t qw1w2;
+    uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
+    uint32_t qw1w2_new;
+    uint32_t cam = be32_to_cpu(qw1w2);
+    uint8_t nvt_blk;
+    uint32_t nvt_idx;
+    bool vo;
 
-    qw1w2 = xive_set_field32(TM_QW1W2_VO, qw1w2_prev, 0);
-    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
+
+    /* Invalidate CAM line */
+    qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
+    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2_new, 4);
     return qw1w2;
 }
 
@@ -387,13 +402,15 @@ static void xive_tctx_need_resend(XiveRouter *xrtr, XiveTCTX *tctx,
 static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
                                 hwaddr offset, uint64_t value, unsigned size)
 {
-    uint32_t qw1w2 = value;
-    uint8_t nvt_blk = xive_nvt_blk(qw1w2);
-    uint32_t nvt_idx = xive_nvt_idx(qw1w2);
-    bool vo = !!(qw1w2 & TM_QW1W2_VO);
+    uint32_t cam = value;
+    uint32_t qw1w2 = cpu_to_be32(cam);
+    uint8_t nvt_blk;
+    uint32_t nvt_idx;
+    bool vo;
+
+    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
 
     /* First update the registers */
-    qw1w2 = cpu_to_be32(qw1w2);
     memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
 
     /* Check the interrupt pending bits */
-- 
2.21.0

[Qemu-devel] [PATCH v4 23/25] ppc/xive: Check V bit in TM_PULL_POOL_CTX

[Cédric Le Goater]

A context should be 'valid' when pulled from the thread interrupt
context registers.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index cdc4ea8b0e51..07b7c3586c12 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -362,6 +362,11 @@ static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
 
     xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
 
+    if (!vo) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: pulling invalid NVT %x/%x !?\n",
+                      nvt_blk, nvt_idx);
+    }
+
     /* Invalidate CAM line */
     qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
     memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2_new, 4);
-- 
2.21.0

[Qemu-devel] [PATCH v4 24/25] ppc/pnv: Improve trigger data definition

[Cédric Le Goater]

The trigger definition is used for triggers both for HW source
interrupts, PHB, PSI, as well as for rerouting interrupts between
Interrupt Controller.

HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
information (EAS block + EAS index) in the 8 byte data and not END
information, and bit1 of word0 to ‘1’ to signal that the state bit
check has been performed.

Introduce these new trigger bits and rename the XIVE_SRCNO macros in
XIVE_EAS to reflect better the nature of the data. This is breaking
the notification for the PSI model which will be fixed in the next
patch.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive_regs.h | 24 +++++++++++++++++++++---
 hw/intc/pnv_xive.c         | 16 ++++++++++++----
 hw/intc/xive.c             |  4 ++--
 3 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index dd42c33cef35..83a2f2cc1318 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -22,9 +22,27 @@
 /*
  * Interrupt source number encoding on PowerBUS
  */
-#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
-#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
-#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
+/*
+ * Trigger data definition
+ *
+ * The trigger definition is used for triggers both for HW source
+ * interrupts (PHB, PSI), as well as for rerouting interrupts between
+ * Interrupt Controller.
+ *
+ * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
+ * information (EAS block + EAS index) in the 8 byte data and not END
+ * information, and bit1 of word0 to ‘1’ to signal that the state bit
+ * check has been performed.
+ */
+#define XIVE_TRIGGER_END        PPC_BIT(0)
+#define XIVE_TRIGGER_EAS        PPC_BIT(1)
+
+/*
+ * QEMU macros to manipulate the trigger payload in native endian
+ */
+#define XIVE_EAS_BLOCK(n)       (((n) >> 28) & 0xf)
+#define XIVE_EAS_INDEX(n)       ((n) & 0x0fffffff)
+#define XIVE_EAS(blk, idx)      ((uint32_t)(blk) << 28 | (idx))
 
 #define TM_SHIFT                16
 
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 4c1fa024cdf5..61af3f23000f 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
      * EAT lookups should be local to the IC
      */
     if (pnv_xive_block_id(xive) != blk) {
-        xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
+        xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
         return -1;
     }
 
@@ -502,7 +502,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
     PnvXive *xive = PNV_XIVE(xn);
     uint8_t blk = pnv_xive_block_id(xive);
 
-    xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
+    xive_router_notify(xn, XIVE_EAS(blk, srcno));
 }
 
 /*
@@ -1287,12 +1287,20 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = {
 
 static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
 {
+    uint8_t blk = XIVE_EAS_BLOCK(val);
+    uint32_t idx = XIVE_EAS_INDEX(val);
+
     /*
      * Forward the source event notification directly to the Router.
      * The source interrupt number should already be correctly encoded
      * with the chip block id by the sending device (PHB, PSI).
      */
-    xive_router_notify(XIVE_NOTIFIER(xive), val);
+    if (val & XIVE_TRIGGER_EAS) {
+        xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
+    } else {
+        xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64,
+                   addr, val);
+    }
 }
 
 static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
@@ -1683,7 +1691,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     XiveRouter *xrtr = XIVE_ROUTER(xive);
     uint8_t blk = pnv_xive_block_id(xive);
     uint8_t chip_id = xive->chip->chip_id;
-    uint32_t srcno0 = XIVE_SRCNO(blk, 0);
+    uint32_t srcno0 = XIVE_EAS(blk, 0);
     uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
     XiveEAS eas;
     XiveEND end;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 07b7c3586c12..6702f32be601 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1652,8 +1652,8 @@ do_escalation:
 void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
 {
     XiveRouter *xrtr = XIVE_ROUTER(xn);
-    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
-    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
+    uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
+    uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
     XiveEAS eas;
 
     /* EAS cache lookup */
-- 
2.21.0

[Qemu-devel] [PATCH v4 25/25] ppc/pnv: Use the EAS trigger bit when triggering an interrupt from PSI

[Cédric Le Goater]

and replace the cpu_physical_memory_write() by a address_space_stq_be().

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv_psi.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 88ba8e7b9b0a..3b0f4b02f9bf 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -660,10 +660,19 @@ 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 lisn = cpu_to_be64(offset + srcno);
+    uint64_t data = XIVE_TRIGGER_EAS | offset | srcno;
+    MemTxResult result;
 
-    if (valid) {
-        cpu_physical_memory_write(notify_addr, &lisn, sizeof(lisn));
+    if (!valid) {
+        return;
+    }
+
+    address_space_stq_be(&address_space_memory, notify_addr, data,
+                         MEMTXATTRS_UNSPECIFIED, &result);
+    if (result != MEMTX_OK) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: trigger failed @%"
+                      HWADDR_PRIx "\n", __func__, notif_port);
+        return;
     }
 }
 
-- 
2.21.0

Re: [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

On Wed, Sep 18, 2019 at 06:06:23PM +0200, Cédric Le Goater wrote:
> As there is now easy way to loop on the CPUs belonging to a chip, add
> a helper to filter out external CPUs.

This seems a somewhat odd way to go about it, given that the chip does
have a cores array and the cores then have a threads array.  What's
the difficulty with using that rather than looping through all vcpus
and filtering?

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index ae449aa1119b..e1c15b6b5b71 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -392,15 +392,36 @@ 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 cpu_pir(PowerPCCPU *cpu)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    return env->spr_cb[SPR_PIR].default_value;
> +}
> +
> +static int cpu_chip_id(PowerPCCPU *cpu)
> +{
> +    int pir = cpu_pir(cpu);
> +    return (pir >> 8) & 0x7f;
> +}
> +
> +#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
> +    CPU_FOREACH(cs)                                                     \
> +        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
> +
>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>                                uint8_t nvt_blk, uint32_t nvt_idx,
>                                bool cam_ignore, uint8_t priority,
>                                uint32_t logic_serv, XiveTCTXMatch *match)
>  {
> +    PnvXive *xive = PNV_XIVE(xptr);
>      CPUState *cs;
>      int count = 0;
>  
> -    CPU_FOREACH(cs) {
> +    /*
> +     * Loop on all CPUs of the machine and filter out the CPUs
> +     * belonging to another chip.
> +     */
> +    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>          int ring;

-- 
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 --]

Re: [PATCH v4 04/25] ppc/pnv: Introduce a pnv_xive_is_cpu_enabled() helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]

On Wed, Sep 18, 2019 at 06:06:24PM +0200, Cédric Le Goater wrote:
> and use this helper to exclude CPUs which were not enabled by the XIVE
> controller.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/intc/pnv_xive.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index e1c15b6b5b71..5c97ccda1cad 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -408,6 +408,14 @@ static int cpu_chip_id(PowerPCCPU *cpu)
>      CPU_FOREACH(cs)                                                     \
>          if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
>  
> +static bool pnv_xive_is_cpu_enabled(PnvXive *xive, PowerPCCPU *cpu)
> +{
> +    int pir = cpu_pir(cpu);
> +    int thrd_id = pir & 0x7f;
> +
> +    return xive->regs[PC_THREAD_EN_REG0 >> 3] & PPC_BIT(thrd_id);
> +}
> +
>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>                                uint8_t nvt_blk, uint32_t nvt_idx,
>                                bool cam_ignore, uint8_t priority,
> @@ -426,6 +434,10 @@ static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>          XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>          int ring;
>  
> +        if (!pnv_xive_is_cpu_enabled(xive, cpu)) {
> +            continue;
> +        }
> +
>          /*
>           * Check the thread context CAM lines and record matches.
>           */

-- 
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 --]

Re: [PATCH v4 05/25] ppc/xive: Introduce a XiveFabric interface

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3153 bytes --]

On Wed, Sep 18, 2019 at 06:06:25PM +0200, Cédric Le Goater wrote:
> The XiveFabric QOM interface should be implemented by the machine. It
> acts as the PowerBUS interface between the interrupt controller and
> the system. On HW, the XIVE sub-engine is responsible for the
> communication with the other chip is the Common Queue (CQ) bridge
> unit.
> 
> This interface offers a 'match_nvt' handler to perform the CAM line
> matching when looking for a XIVE Presenter with a dispatched NVT.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Um.. this has basically the same single method as the XivePresented
interface you just introduced.  What's the distinction between them?

> ---
>  include/hw/ppc/xive.h | 22 ++++++++++++++++++++++
>  hw/intc/xive.c        | 10 ++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 39de45b87cb9..3c2910e10e25 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -399,6 +399,28 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>                                uint8_t nvt_blk, uint32_t nvt_idx,
>                                bool cam_ignore, uint32_t logic_serv);
>  
> +/*
> + * XIVE Fabric (Interface between Interrupt Controller and Machine)
> + */
> +
> +typedef struct XiveFabric XiveFabric;
> +
> +#define TYPE_XIVE_FABRIC "xive-fabric"
> +#define XIVE_FABRIC(obj)                                     \
> +    INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
> +#define XIVE_FABRIC_CLASS(klass)                                     \
> +    OBJECT_CLASS_CHECK(XiveFabricClass, (klass), TYPE_XIVE_FABRIC)
> +#define XIVE_FABRIC_GET_CLASS(obj)                                   \
> +    OBJECT_GET_CLASS(XiveFabricClass, (obj), TYPE_XIVE_FABRIC)
> +
> +typedef struct XiveFabricClass {
> +    InterfaceClass parent;
> +    int (*match_nvt)(XiveFabric *xfb, uint8_t format,
> +                     uint8_t nvt_blk, uint32_t nvt_idx,
> +                     bool cam_ignore, uint8_t priority,
> +                     uint32_t logic_serv, XiveTCTXMatch *match);
> +} XiveFabricClass;
> +
>  /*
>   * XIVE END ESBs
>   */
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index fff50429f8ac..7a15a64ed7fe 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1841,8 +1841,18 @@ static const TypeInfo xive_presenter_info = {
>      .class_size = sizeof(XivePresenterClass),
>  };
>  
> +/*
> + * XIVE Fabric
> + */
> +static const TypeInfo xive_fabric_info = {
> +    .name = TYPE_XIVE_FABRIC,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(XiveFabricClass),
> +};
> +
>  static void xive_register_types(void)
>  {
> +    type_register_static(&xive_fabric_info);
>      type_register_static(&xive_source_info);
>      type_register_static(&xive_notifier_info);
>      type_register_static(&xive_presenter_info);

-- 
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 --]

Re: [PATCH v4 06/25] ppc/pnv: Implement the XiveFabric interface

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]

On Wed, Sep 18, 2019 at 06:06:26PM +0200, Cédric Le Goater wrote:
> The CAM line matching on the PowerNV machine now scans all chips of
> the system and all CPUs of a chip to find a dispatched NVT in the
> thread contexts.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 77a86c6a2301..ca24dd62df23 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1378,6 +1378,35 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
>      }
>  }
>  
> +static int pnv_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> +                               uint8_t nvt_blk, uint32_t nvt_idx,
> +                               bool cam_ignore, uint8_t priority,
> +                               uint32_t logic_serv,
> +                               XiveTCTXMatch *match)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(xfb);
> +    int total_count = 0;
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        XivePresenter *xptr = XIVE_PRESENTER(&chip9->xive);
> +        XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +        int count;
> +
> +        count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                               priority, logic_serv, match);
> +
> +        if (count < 0) {
> +            return count;
> +        }
> +
> +        total_count += count;
> +    }
> +
> +    return total_count;
> +}
> +
>  static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
>                                void *opaque, Error **errp)
>  {
> @@ -1441,9 +1470,11 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
>  static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>  
>      mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> +    xfc->match_nvt = pnv_xive_match_nvt;
>  
>      mc->alias = "powernv";
>  }
> @@ -1495,6 +1526,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>          .interfaces = (InterfaceInfo[]) {               \
>              { TYPE_XICS_FABRIC },                       \
>              { TYPE_INTERRUPT_STATS_PROVIDER },          \
> +            { TYPE_XIVE_FABRIC },                       \

Hmm.. this should probably be on the powernv9 machine only, not
powernv8 as well, yes?

>              { },                                        \
>          },                                              \
>      }

-- 
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 --]

Re: [PATCH v4 07/25] ppc/spapr: Implement the XiveFabric interface

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7653 bytes --]

On Wed, Sep 18, 2019 at 06:06:27PM +0200, Cédric Le Goater wrote:
> The CAM line matching sequence in the pseries machine does not change
> much apart from the use of the new QOM interfaces. There is an extra
> indirection because of the sPAPR IRQ backend of the machine. Only the
> XIVE backend implements the new 'match_nvt' handler.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/spapr_irq.h |  6 ++++++
>  hw/ppc/spapr.c             | 34 ++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c         | 25 +++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 5db305165ce2..859780efaf95 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -31,6 +31,8 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align,
>                          Error **errp);
>  void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num);
>  
> +struct XiveTCTXMatch;
> +
>  typedef struct SpaprIrq {
>      uint32_t    nr_irqs;
>      uint32_t    nr_msis;
> @@ -50,6 +52,10 @@ typedef struct SpaprIrq {
>      void (*set_irq)(void *opaque, int srcno, int val);
>      const char *(*get_nodename)(SpaprMachineState *spapr);
>      void (*init_kvm)(SpaprMachineState *spapr, Error **errp);
> +    int (*match_nvt)(SpaprMachineState *spapr, uint8_t format,
> +                     uint8_t nvt_blk, uint32_t nvt_idx,
> +                     bool cam_ignore, uint8_t priority,
> +                     uint32_t logic_serv, struct XiveTCTXMatch *match);

Obviously this will need some rework against my stuff.

But more importantly, I don't see the point of indirecting via here,
when the method is only relevant for the xive case.  Why not just
assert that XIVE is in use in the XiveFabric hook, and go directly  to
the XIVE matching code.

>  } SpaprIrq;
>  
>  extern SpaprIrq spapr_irq_xics;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2725b139a7f0..90f6f5fb9536 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4360,6 +4360,37 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>  }
>  
> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> +                                uint8_t nvt_blk, uint32_t nvt_idx,
> +                                bool cam_ignore, uint8_t priority,
> +                                uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
> +    int count;
> +
> +    count = spapr->irq->match_nvt(spapr, format, nvt_blk, nvt_idx, cam_ignore,
> +                                  priority, logic_serv, match);
> +    if (count < 0) {
> +        return count;
> +    }
> +
> +    /*
> +     * When we implement the save and restore of the thread interrupt
> +     * contexts in the enter/exit CPU handlers of the machine and the
> +     * escalations in QEMU, we should be able to handle non dispatched
> +     * vCPUs.
> +     *
> +     * Until this is done, the sPAPR machine should find at least one
> +     * matching context always.
> +     */
> +    if (count == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
> +                      nvt_blk, nvt_idx);
> +    }
> +
> +    return count;
> +}
> +
>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>  {
>      return cpu->vcpu_id;
> @@ -4456,6 +4487,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>      mc->ignore_boot_device_suffixes = true;
> @@ -4514,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>       */
>      mc->numa_mem_align_shift = 28;
>      mc->numa_mem_supported = true;
> +    xfc->match_nvt = spapr_xive_match_nvt;
>  
>      smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>      smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
> @@ -4547,6 +4580,7 @@ static const TypeInfo spapr_machine_info = {
>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>          { TYPE_XICS_FABRIC },
>          { TYPE_INTERRUPT_STATS_PROVIDER },
> +        { TYPE_XIVE_FABRIC },
>          { }
>      },
>  };
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d8f46b6797f8..8a6d79a59af2 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -257,6 +257,7 @@ SpaprIrq spapr_irq_xics = {
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_nodename = spapr_irq_get_nodename_xics,
>      .init_kvm    = spapr_irq_init_kvm_xics,
> +    .match_nvt   = NULL, /* should not be used */
>  };
>  
>  /*
> @@ -406,6 +407,18 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp)
>      }
>  }
>  
> +static int spapr_irq_match_nvt_xive(SpaprMachineState *spapr, uint8_t format,
> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
> +                                    bool cam_ignore, uint8_t priority,
> +                                    uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +
> +    return xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                          priority, logic_serv, match);
> +}
> +
>  /*
>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>   * with XICS.
> @@ -431,6 +444,7 @@ SpaprIrq spapr_irq_xive = {
>      .set_irq     = spapr_irq_set_irq_xive,
>      .get_nodename = spapr_irq_get_nodename_xive,
>      .init_kvm    = spapr_irq_init_kvm_xive,
> +    .match_nvt   = spapr_irq_match_nvt_xive,
>  };
>  
>  /*
> @@ -585,6 +599,15 @@ static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
>      return spapr_irq_current(spapr)->get_nodename(spapr);
>  }
>  
> +static int spapr_irq_match_nvt_dual(SpaprMachineState *spapr, uint8_t format,
> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
> +                                    bool cam_ignore, uint8_t priority,
> +                                    uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    return spapr_irq_current(spapr)->match_nvt(spapr, format, nvt_blk, nvt_idx,
> +                                     cam_ignore, priority, logic_serv, match);
> +}
> +
>  /*
>   * Define values in sync with the XIVE and XICS backend
>   */
> @@ -608,6 +631,7 @@ SpaprIrq spapr_irq_dual = {
>      .set_irq     = spapr_irq_set_irq_dual,
>      .get_nodename = spapr_irq_get_nodename_dual,
>      .init_kvm    = NULL, /* should not be used */
> +    .match_nvt   = spapr_irq_match_nvt_dual,
>  };
>  
>  
> @@ -825,4 +849,5 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_nodename = spapr_irq_get_nodename_xics,
>      .init_kvm    = spapr_irq_init_kvm_xics,
> +    .match_nvt   = NULL, /* should not be used */
>  };

-- 
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 --]

Re: [PATCH v4 11/25] ppc/xive: Move the TIMA operations to the controller model

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6965 bytes --]

On Wed, Sep 18, 2019 at 06:06:31PM +0200, Cédric Le Goater wrote:
> This also removes the need of the get_tctx() XiveRouter handler in the
> core XIVE framework.

In general these commit messages could really do with more context.
What exactly is the "controller model"?  Where were the TIMA
operations before now.  Why is having them in the controller model
better?

I could probably answer those with some investigation, but the point
is that the commit message is supposed to make it easy to find that
information.

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive.h |  1 -
>  hw/intc/pnv_xive.c    | 35 ++++++++++++++++++++++++++++++++++-
>  hw/intc/spapr_xive.c  | 33 +++++++++++++++++++++++++++++++--
>  hw/intc/xive.c        | 29 -----------------------------
>  4 files changed, 65 insertions(+), 33 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 536deea8c622..9d9cd88dd17e 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -462,7 +462,6 @@ typedef struct XiveENDSource {
>  #define XIVE_TM_OS_PAGE         0x2
>  #define XIVE_TM_USER_PAGE       0x3
>  
> -extern const MemoryRegionOps xive_tm_ops;
>  void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>                          uint64_t value, unsigned size);
>  uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 3d6fcf9ac139..40e18fb44811 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1475,6 +1475,39 @@ static const MemoryRegionOps xive_tm_indirect_ops = {
>      },
>  };
>  
> +static void pnv_xive_tm_write(void *opaque, hwaddr offset,
> +                              uint64_t value, unsigned size)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +
> +    xive_tctx_tm_write(XIVE_PRESENTER(xive), tctx, offset, value, size);
> +}
> +
> +static uint64_t pnv_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +
> +    return xive_tctx_tm_read(XIVE_PRESENTER(xive), tctx, offset, size);

You're not using the opaque pointer in either of these cases.  So
couldn't you make it point to the presenter for pnv as well, then...

> +}
> +
> +const MemoryRegionOps pnv_xive_tm_ops = {
> +    .read = pnv_xive_tm_read,
> +    .write = pnv_xive_tm_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +};
> +
>  /*
>   * Interrupt controller XSCOM region.
>   */
> @@ -1832,7 +1865,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
>                            "xive-pc", PNV9_XIVE_PC_SIZE);
>  
>      /* Thread Interrupt Management Area (Direct) */
> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops,
> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &pnv_xive_tm_ops,
>                            xive, "xive-tima", PNV9_XIVE_TM_SIZE);
>  
>      qemu_register_reset(pnv_xive_reset, dev);
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index eefc0d4c36b9..e00a9bdd901b 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -222,6 +222,35 @@ void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
>      memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
>  }
>  
> +static void spapr_xive_tm_write(void *opaque, hwaddr offset,
> +                          uint64_t value, unsigned size)
> +{
> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
> +
> +    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value,
> size);

... there would be no difference from the spapr versions, AFAICT.

> +}
> +
> +static uint64_t spapr_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
> +
> +    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
> +}
> +
> +const MemoryRegionOps spapr_xive_tm_ops = {
> +    .read = spapr_xive_tm_read,
> +    .write = spapr_xive_tm_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +};
> +
>  static void spapr_xive_end_reset(XiveEND *end)
>  {
>      memset(end, 0, sizeof(*end));
> @@ -331,8 +360,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
>      qemu_register_reset(spapr_xive_reset, dev);
>  
>      /* TIMA initialization */
> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops, xive,
> -                          "xive.tima", 4ull << TM_SHIFT);
> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &spapr_xive_tm_ops,
> +                          xive, "xive.tima", 4ull << TM_SHIFT);
>      sysbus_init_mmio(SYS_BUS_DEVICE(xive), &xive->tm_mmio);
>  
>      /*
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 9bb09ed6ee7b..11432f04f5c3 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -483,35 +483,6 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>      return xive_tm_raw_read(tctx, offset, size);
>  }
>  
> -static void xive_tm_write(void *opaque, hwaddr offset,
> -                          uint64_t value, unsigned size)
> -{
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> -
> -    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
> -}
> -
> -static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> -{
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> -
> -    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
> -}
> -
> -const MemoryRegionOps xive_tm_ops = {
> -    .read = xive_tm_read,
> -    .write = xive_tm_write,
> -    .endianness = DEVICE_BIG_ENDIAN,
> -    .valid = {
> -        .min_access_size = 1,
> -        .max_access_size = 8,
> -    },
> -    .impl = {
> -        .min_access_size = 1,
> -        .max_access_size = 8,
> -    },
> -};
> -
>  static char *xive_tctx_ring_print(uint8_t *ring)
>  {
>      uint32_t w2 = xive_tctx_word2(ring);

-- 
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 --]

Re: [PATCH v4 13/25] ppc/xive: Introduce a xive_tctx_ipb_update() helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

On Wed, Sep 18, 2019 at 06:06:33PM +0200, Cédric Le Goater wrote:
> We will use it to resend missed interrupts when a vCPU context is
> pushed a HW thread.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive.h |  1 +
>  hw/intc/xive.c        | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index f35ff3b64791..a461753f5da5 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -467,6 +467,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>  
>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
> +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
>  
>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>  {
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 68d3361d1c3f..5f7c37b091a7 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -135,6 +135,15 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, uint8_t cppr)
>      xive_tctx_notify(tctx, ring);
>  }
>  
> +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
> +{
> +    uint8_t *regs = &tctx->regs[ring];
> +
> +    regs[TM_IPB] |= ipb;
> +    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);

You're opencoding the existing ipb_update() here, but not eliminating
it.  That doesn't seem like a good idea.

> +    xive_tctx_notify(tctx, ring);
> +}
> +
>  static inline uint32_t xive_tctx_word2(uint8_t *ring)
>  {
>      return *((uint32_t *) &ring[TM_WORD2]);
> @@ -336,8 +345,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
>  static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
>                                     hwaddr offset, uint64_t value, unsigned size)
>  {
> -    ipb_update(&tctx->regs[TM_QW1_OS], value & 0xff);
> -    xive_tctx_notify(tctx, TM_QW1_OS);
> +    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
>  }
>  
>  static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
> @@ -1382,8 +1390,7 @@ static bool xive_presenter_notify(uint8_t format,
>  
>      /* handle CPU exception delivery */
>      if (count) {
> -        ipb_update(&match.tctx->regs[match.ring], priority);
> -        xive_tctx_notify(match.tctx, match.ring);
> +        xive_tctx_ipb_update(match.tctx, match.ring, priority_to_ipb(priority));
>      }
>  
>      return count;

-- 
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 --]

Re: [PATCH v4 14/25] ppc/xive: Introduce helpers for the NVT id

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]

On Wed, Sep 18, 2019 at 06:06:34PM +0200, Cédric Le Goater wrote:
> The NVT space is 19 bits wide, giving a maximum of 512K per chip. When
> dispatched on a HW thread, the NVT identifier of a vCPU is pushed/stored
> in the CAM line (word2) of the thread interrupt context.

Ok, that's interesting, but how does it lead to this patch?  Connect
the dots, please.

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive.h      |  5 -----
>  include/hw/ppc/xive_regs.h | 21 +++++++++++++++++++++
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index a461753f5da5..794dfcaae0f8 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -469,11 +469,6 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>  void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
>  
> -static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> -{
> -    return (nvt_blk << 19) | nvt_idx;
> -}
> -
>  /*
>   * KVM XIVE device helpers
>   */
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index 08c8bf7172e2..3d7b6fd09664 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -251,4 +251,25 @@ typedef struct XiveNVT {
>  
>  #define xive_nvt_is_valid(nvt)    (be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
>  
> +/*
> + * The VP number space in a block is defined by the END_W6_NVT_INDEX
> + * field of the XIVE END
> + */
> +#define XIVE_NVT_SHIFT                19
> +
> +static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
> +{
> +    return (nvt_blk << XIVE_NVT_SHIFT) | nvt_idx;
> +}
> +
> +static inline uint32_t xive_nvt_idx(uint32_t cam_line)
> +{
> +    return cam_line & ((1 << XIVE_NVT_SHIFT) - 1);
> +}
> +
> +static inline uint32_t xive_nvt_blk(uint32_t cam_line)
> +{
> +    return (cam_line >> XIVE_NVT_SHIFT) & 0xf;
> +}
> +
>  #endif /* PPC_XIVE_REGS_H */

-- 
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 --]

Re: [PATCH v4 16/25] ppc/pnv: Remove pnv_xive_vst_size() routine

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 9115 bytes --]

On Wed, Sep 18, 2019 at 06:06:36PM +0200, Cédric Le Goater wrote:
> pnv_xive_vst_size() tries to compute the size of a VSD table from the
> information given by FW. The number of entries of the table are
> deduced from the result and the MMIO regions of the ESBs and the ENDS
> are also resized with the computed value.
> 
> But for indirect tables, the result is incorrect. An indirect table is
> a one page array of VSDs pointing to subpages containing XIVE virtual
> structures. The number of first level VSD entries is page aligned and
> the overall computed size of the table is too large. It can also be
> completely wrong when the first VSD entry is not yet initialized.
> 
> Remove pnv_xive_vst_size() and use a simpler form for direct tables.
> This is only useful when outputting the XIVE sources on the monitor
> and to resize the ESB MMIO window.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Do.. does this actually correct the value for indirect tables?  Or
just remove the calculation on the grounds that it was already broken
and we'll fix later?  If it's the latter, we should surely have some
error_report()s or something if the guest does attempt to use an
indirect table.

> ---
>  hw/intc/pnv_xive.c | 112 +++++++++++++++++----------------------------
>  1 file changed, 43 insertions(+), 69 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 74d6ccbea3d6..b7d505839e68 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -123,36 +123,22 @@ static uint64_t pnv_xive_vst_page_size_allowed(uint32_t page_shift)
>           page_shift == 21 || page_shift == 24;
>  }
>  
> -static uint64_t pnv_xive_vst_size(uint64_t vsd)
> -{
> -    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
> -
> -    /*
> -     * Read the first descriptor to get the page size of the indirect
> -     * table.
> -     */
> -    if (VSD_INDIRECT & vsd) {
> -        uint32_t nr_pages = vst_tsize / XIVE_VSD_SIZE;
> -        uint32_t page_shift;
> -
> -        vsd = ldq_be_dma(&address_space_memory, vsd & VSD_ADDRESS_MASK);
> -        page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
> -
> -        if (!pnv_xive_vst_page_size_allowed(page_shift)) {
> -            return 0;
> -        }
> -
> -        return nr_pages * (1ull << page_shift);
> -    }
> -
> -    return vst_tsize;
> -}
> -
>  static uint64_t pnv_xive_vst_addr_direct(PnvXive *xive, uint32_t type,
>                                           uint64_t vsd, uint32_t idx)
>  {
>      const XiveVstInfo *info = &vst_infos[type];
>      uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
> +    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
> +    uint32_t idx_max;
> +
> +    idx_max = vst_tsize / info->size - 1;
> +    if (idx > idx_max) {
> +#ifdef XIVE_DEBUG
> +        xive_error(xive, "VST: %s entry %x out of range [ 0 .. %x ] !?",
> +                   info->name, idx, idx_max);
> +#endif
> +        return 0;
> +    }
>  
>      return vst_addr + idx * info->size;
>  }
> @@ -215,7 +201,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
>  {
>      const XiveVstInfo *info = &vst_infos[type];
>      uint64_t vsd;
> -    uint32_t idx_max;
>  
>      if (blk >= info->max_blocks) {
>          xive_error(xive, "VST: invalid block id %d for VST %s %d !?",
> @@ -232,15 +217,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
>          return xive ? pnv_xive_vst_addr(xive, type, blk, idx) : 0;
>      }
>  
> -    idx_max = pnv_xive_vst_size(vsd) / info->size - 1;
> -    if (idx > idx_max) {
> -#ifdef XIVE_DEBUG
> -        xive_error(xive, "VST: %s entry %x/%x out of range [ 0 .. %x ] !?",
> -                   info->name, blk, idx, idx_max);
> -#endif
> -        return 0;
> -    }
> -
>      if (VSD_INDIRECT & vsd) {
>          return pnv_xive_vst_addr_indirect(xive, type, vsd, idx);
>      }
> @@ -519,19 +495,12 @@ static uint64_t pnv_xive_pc_size(PnvXive *xive)
>      return (~xive->regs[CQ_PC_BARM >> 3] + 1) & CQ_PC_BARM_MASK;
>  }
>  
> -static uint32_t pnv_xive_nr_ipis(PnvXive *xive)
> +static uint32_t pnv_xive_nr_ipis(PnvXive *xive, uint8_t blk)
>  {
> -    uint8_t blk = xive->chip->chip_id;
> -
> -    return pnv_xive_vst_size(xive->vsds[VST_TSEL_SBE][blk]) * SBE_PER_BYTE;
> -}
> -
> -static uint32_t pnv_xive_nr_ends(PnvXive *xive)
> -{
> -    uint8_t blk = xive->chip->chip_id;
> +    uint64_t vsd = xive->vsds[VST_TSEL_SBE][blk];
> +    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
>  
> -    return pnv_xive_vst_size(xive->vsds[VST_TSEL_EQDT][blk])
> -        / vst_infos[VST_TSEL_EQDT].size;
> +    return VSD_INDIRECT & vsd ? 0 : vst_tsize * SBE_PER_BYTE;
>  }
>  
>  /*
> @@ -664,6 +633,7 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>      XiveSource *xsrc = &xive->ipi_source;
>      const XiveVstInfo *info = &vst_infos[type];
>      uint32_t page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
> +    uint64_t vst_tsize = 1ull << page_shift;
>      uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
>  
>      /* Basic checks */
> @@ -699,11 +669,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>  
>      case VST_TSEL_EQDT:
>          /*
> -         * Backing store pages for the END. Compute the number of ENDs
> -         * provisioned by FW and resize the END ESB window accordingly.
> +         * Backing store pages for the END.
> +         *
> +         * If the table is direct, we can compute the number of PQ
> +         * entries provisioned by FW (such as skiboot) and resize the
> +         * END ESB window accordingly.
>           */
> -        memory_region_set_size(&end_xsrc->esb_mmio, pnv_xive_nr_ends(xive) *
> -                               (1ull << (end_xsrc->esb_shift + 1)));
> +        if (!(VSD_INDIRECT & vsd)) {
> +            memory_region_set_size(&end_xsrc->esb_mmio, (vst_tsize / info->size)
> +                                   * (1ull << xsrc->esb_shift));
> +        }
>          memory_region_add_subregion(&xive->end_edt_mmio, 0,
>                                      &end_xsrc->esb_mmio);
>          break;
> @@ -712,11 +687,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>          /*
>           * Backing store pages for the source PQ bits. The model does
>           * not use these PQ bits backed in RAM because the XiveSource
> -         * model has its own. Compute the number of IRQs provisioned
> -         * by FW and resize the IPI ESB window accordingly.
> +         * model has its own.
> +         *
> +         * If the table is direct, we can compute the number of PQ
> +         * entries provisioned by FW (such as skiboot) and resize the
> +         * ESB window accordingly.
>           */
> -        memory_region_set_size(&xsrc->esb_mmio, pnv_xive_nr_ipis(xive) *
> -                               (1ull << xsrc->esb_shift));
> +        if (!(VSD_INDIRECT & vsd)) {
> +            memory_region_set_size(&xsrc->esb_mmio, vst_tsize * SBE_PER_BYTE
> +                                   * (1ull << xsrc->esb_shift));
> +        }
>          memory_region_add_subregion(&xive->ipi_edt_mmio, 0, &xsrc->esb_mmio);
>          break;
>  
> @@ -1666,8 +1646,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>      XiveRouter *xrtr = XIVE_ROUTER(xive);
>      uint8_t blk = xive->chip->chip_id;
>      uint32_t srcno0 = XIVE_SRCNO(blk, 0);
> -    uint32_t nr_ipis = pnv_xive_nr_ipis(xive);
> -    uint32_t nr_ends = pnv_xive_nr_ends(xive);
> +    uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
>      XiveEAS eas;
>      XiveEND end;
>      int i;
> @@ -1687,21 +1666,16 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>          }
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] ENDT %08x .. %08x\n", blk, 0, nr_ends - 1);
> -    for (i = 0; i < nr_ends; i++) {
> -        if (xive_router_get_end(xrtr, blk, i, &end)) {
> -            break;
> -        }
> -        xive_end_pic_print_info(&end, i, mon);
> +    monitor_printf(mon, "XIVE[%x] ENDT\n", blk);
> +    i = 0;
> +    while (!xive_router_get_end(xrtr, blk, i, &end)) {
> +        xive_end_pic_print_info(&end, i++, mon);
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] END Escalation %08x .. %08x\n", blk, 0,
> -                   nr_ends - 1);
> -    for (i = 0; i < nr_ends; i++) {
> -        if (xive_router_get_end(xrtr, blk, i, &end)) {
> -            break;
> -        }
> -        xive_end_eas_pic_print_info(&end, i, mon);
> +    monitor_printf(mon, "XIVE[%x] END Escalation EAT\n", blk);
> +    i = 0;
> +    while (!xive_router_get_end(xrtr, blk, i, &end)) {
> +        xive_end_eas_pic_print_info(&end, i++, mon);
>      }
>  }
>  

-- 
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 --]

Re: [PATCH v4 18/25] ppc/pnv: Skip empty slots of the XIVE NVT table

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

On Wed, Sep 18, 2019 at 06:06:38PM +0200, Cédric Le Goater wrote:
> We try to loop on the full table skipping empty indirect pages which
> are not necessarily allocated. This is useful to dump the contexts of
> the KVM vCPUs.

I think this patch can be merged into the previous one.  They're both
short, so we might as well get the dumping code correct first time.

> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/intc/pnv_xive.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 782775136288..a986d4ed9364 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1641,6 +1641,12 @@ static const MemoryRegionOps pnv_xive_pc_ops = {
>      },
>  };
>  
> +/*
> + * skiboot uses an indirect NVT table with 64k subpages
> + */
> +#define XIVE_NVT_COUNT          (1 << XIVE_NVT_SHIFT)
> +#define XIVE_NVT_PER_PAGE       (0x10000 / sizeof(XiveNVT))
> +
>  static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
>                                      Monitor *mon)
>  {
> @@ -1694,10 +1700,12 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>          xive_end_eas_pic_print_info(&end, i++, mon);
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] NVTT\n", blk);
> -    i = 0;
> -    while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
> -        xive_nvt_pic_print_info(&nvt, i++, mon);
> +    monitor_printf(mon, "XIVE[%x] NVTT %08x .. %08x\n", blk, 0,
> +                   XIVE_NVT_COUNT - 1);
> +    for (i = 0; i < XIVE_NVT_COUNT; i += XIVE_NVT_PER_PAGE) {
> +        while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
> +            xive_nvt_pic_print_info(&nvt, i++, mon);
> +        }
>      }
>  }
>  

-- 
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 --]

Re: [PATCH v4 19/25] ppc/pnv: Introduce a pnv_xive_block_id() helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7907 bytes --]

On Wed, Sep 18, 2019 at 06:06:39PM +0200, Cédric Le Goater wrote:
> When PC_TCTXT_CHIPID_OVERRIDE is configured, the PC_TCTXT_CHIPID field
> overrides the hardwired chip ID in the Powerbus operations and for CAM
> compares. This is typically used in the one block-per-chip configuration
> to associate a unique block id number to each IC of the system.
> 
> Simplify the model with a pnv_xive_block_id() helper and remove
> 'tctx_chipid' which becomes useless.
> 
> The model does support multiple blocks per chip.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/hw/ppc/pnv_xive.h |  3 --
>  hw/intc/pnv_xive.c        | 68 ++++++++++++++++++++-------------------
>  2 files changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv_xive.h b/include/hw/ppc/pnv_xive.h
> index 4fdaa9247d65..f4c7caad40ee 100644
> --- a/include/hw/ppc/pnv_xive.h
> +++ b/include/hw/ppc/pnv_xive.h
> @@ -72,9 +72,6 @@ typedef struct PnvXive {
>      /* Interrupt controller registers */
>      uint64_t      regs[0x300];
>  
> -    /* Can be configured by FW */
> -    uint32_t      tctx_chipid;
> -
>      /*
>       * Virtual Structure Descriptor tables : EAT, SBE, ENDT, NVTT, IRQ
>       * These are in a SRAM protected by ECC.
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index a986d4ed9364..8c352315f6f5 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -85,13 +85,30 @@ static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
>      return (word & ~mask) | ((value << ctz64(mask)) & mask);
>  }
>  
> +/*
> + * When PC_TCTXT_CHIPID_OVERRIDE is configured, the PC_TCTXT_CHIPID
> + * field overrides the hardwired chip ID in the Powerbus operations
> + * and for CAM compares
> + */
> +static uint8_t pnv_xive_block_id(PnvXive *xive)
> +{
> +    uint8_t blk = xive->chip->chip_id;
> +    uint64_t cfg_val = xive->regs[PC_TCTXT_CFG >> 3];
> +
> +    if (cfg_val & PC_TCTXT_CHIPID_OVERRIDE) {
> +        blk = GETFIELD(PC_TCTXT_CHIPID, cfg_val);
> +    }
> +
> +    return blk;
> +}
> +
>  /*
>   * Remote access to controllers. HW uses MMIOs. For now, a simple scan
>   * of the chips is good enough.
>   *
>   * TODO: Block scope support
>   */
> -static PnvXive *pnv_xive_get_ic(uint8_t blk)
> +static PnvXive *pnv_xive_get_remote(uint8_t blk)
>  {
>      PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>      int i;
> @@ -100,7 +117,7 @@ static PnvXive *pnv_xive_get_ic(uint8_t blk)
>          Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
>          PnvXive *xive = &chip9->xive;
>  
> -        if (xive->chip->chip_id == blk) {
> +        if (pnv_xive_block_id(xive) == blk) {
>              return xive;
>          }
>      }
> @@ -212,7 +229,7 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
>  
>      /* Remote VST access */
>      if (GETFIELD(VSD_MODE, vsd) == VSD_MODE_FORWARD) {
> -        xive = pnv_xive_get_ic(blk);
> +        xive = pnv_xive_get_remote(blk);
>  
>          return xive ? pnv_xive_vst_addr(xive, type, blk, idx) : 0;
>      }
> @@ -360,7 +377,10 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>  {
>      PnvXive *xive = PNV_XIVE(xrtr);
>  
> -    if (pnv_xive_get_ic(blk) != xive) {
> +    /*
> +     * EAT lookups should be local to the IC
> +     */
> +    if (pnv_xive_block_id(xive) != blk) {
>          xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
>          return -1;
>      }
> @@ -471,7 +491,7 @@ static PnvXive *pnv_xive_tm_get_xive(PowerPCCPU *cpu)
>  static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
>  {
>      PnvXive *xive = PNV_XIVE(xn);
> -    uint8_t blk = xive->chip->chip_id;
> +    uint8_t blk = pnv_xive_block_id(xive);
>  
>      xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
>  }
> @@ -835,20 +855,7 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
>      case PC_TCTXT_CFG:
>          /*
>           * TODO: block group support
> -         *
> -         * PC_TCTXT_CFG_BLKGRP_EN
> -         * PC_TCTXT_CFG_HARD_CHIPID_BLK :
> -         *   Moves the chipid into block field for hardwired CAM compares.
> -         *   Block offset value is adjusted to 0b0..01 & ThrdId
> -         *
> -         *   Will require changes in xive_presenter_tctx_match(). I am
> -         *   not sure how to handle that yet.
>           */
> -
> -        /* Overrides hardwired chip ID with the chip ID field */
> -        if (val & PC_TCTXT_CHIPID_OVERRIDE) {
> -            xive->tctx_chipid = GETFIELD(PC_TCTXT_CHIPID, val);
> -        }
>          break;
>      case PC_TCTXT_TRACK:
>          /*
> @@ -1665,7 +1672,8 @@ static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
>  void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>  {
>      XiveRouter *xrtr = XIVE_ROUTER(xive);
> -    uint8_t blk = xive->chip->chip_id;
> +    uint8_t blk = pnv_xive_block_id(xive);
> +    uint8_t chip_id = xive->chip->chip_id;
>      uint32_t srcno0 = XIVE_SRCNO(blk, 0);
>      uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
>      XiveEAS eas;
> @@ -1673,12 +1681,12 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>      XiveNVT nvt;
>      int i;
>  
> -    monitor_printf(mon, "XIVE[%x] Source %08x .. %08x\n", blk, srcno0,
> -                   srcno0 + nr_ipis - 1);
> +    monitor_printf(mon, "XIVE[%x] #%d Source %08x .. %08x\n", chip_id, blk,
> +                   srcno0, srcno0 + nr_ipis - 1);
>      xive_source_pic_print_info(&xive->ipi_source, srcno0, mon);
>  
> -    monitor_printf(mon, "XIVE[%x] EAT %08x .. %08x\n", blk, srcno0,
> -                   srcno0 + nr_ipis - 1);
> +    monitor_printf(mon, "XIVE[%x] #%d EAT %08x .. %08x\n", chip_id, blk,
> +                   srcno0, srcno0 + nr_ipis - 1);
>      for (i = 0; i < nr_ipis; i++) {
>          if (xive_router_get_eas(xrtr, blk, i, &eas)) {
>              break;
> @@ -1688,20 +1696,20 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>          }
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] ENDT\n", blk);
> +    monitor_printf(mon, "XIVE[%x] #%d ENDT\n", chip_id, blk);
>      i = 0;
>      while (!xive_router_get_end(xrtr, blk, i, &end)) {
>          xive_end_pic_print_info(&end, i++, mon);
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] END Escalation EAT\n", blk);
> +    monitor_printf(mon, "XIVE[%x] #%d END Escalation EAT\n", chip_id, blk);
>      i = 0;
>      while (!xive_router_get_end(xrtr, blk, i, &end)) {
>          xive_end_eas_pic_print_info(&end, i++, mon);
>      }
>  
> -    monitor_printf(mon, "XIVE[%x] NVTT %08x .. %08x\n", blk, 0,
> -                   XIVE_NVT_COUNT - 1);
> +    monitor_printf(mon, "XIVE[%x] #%d NVTT %08x .. %08x\n", chip_id, blk,
> +                   0, XIVE_NVT_COUNT - 1);
>      for (i = 0; i < XIVE_NVT_COUNT; i += XIVE_NVT_PER_PAGE) {
>          while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
>              xive_nvt_pic_print_info(&nvt, i++, mon);
> @@ -1715,12 +1723,6 @@ static void pnv_xive_reset(void *dev)
>      XiveSource *xsrc = &xive->ipi_source;
>      XiveENDSource *end_xsrc = &xive->end_source;
>  
> -    /*
> -     * Use the PnvChip id to identify the XIVE interrupt controller.
> -     * It can be overriden by configuration at runtime.
> -     */
> -    xive->tctx_chipid = xive->chip->chip_id;
> -
>      /* Default page size (Should be changed at runtime to 64k) */
>      xive->ic_shift = xive->vc_shift = xive->pc_shift = 12;
>  

-- 
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 --]

Re: [PATCH v4 22/25] ppc/xive: Introduce a xive_os_cam_decode() helper

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

On Wed, Sep 18, 2019 at 06:06:42PM +0200, Cédric Le Goater wrote:
> The OS CAM line has a special encoding exploited by the HW. Provide a
> helper routine to hide the details to the TIMA command handlers. This
> also clarifies the endian ness of different variables : 'qw1w2' is
> big-endian and 'cam' is native.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/intc/xive.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index dfae584a319f..cdc4ea8b0e51 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -342,14 +342,29 @@ static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
>      xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
>  }
>  
> +static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
> +                               uint32_t *nvt_idx, bool *vo)
> +{
> +    *nvt_blk = xive_nvt_blk(cam);
> +    *nvt_idx = xive_nvt_idx(cam);
> +    *vo = !!(cam & TM_QW1W2_VO);
> +}
> +
>  static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>                                      hwaddr offset, unsigned size)
>  {
> -    uint32_t qw1w2_prev = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
> -    uint32_t qw1w2;
> +    uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
> +    uint32_t qw1w2_new;
> +    uint32_t cam = be32_to_cpu(qw1w2);
> +    uint8_t nvt_blk;
> +    uint32_t nvt_idx;
> +    bool vo;
>  
> -    qw1w2 = xive_set_field32(TM_QW1W2_VO, qw1w2_prev, 0);
> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);

I'd kind of prefer to fold both the memcpy and the endian swizzle into
a read/write_register_word helper of some sort.

> +    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
> +
> +    /* Invalidate CAM line */
> +    qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
> +    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2_new, 4);
>      return qw1w2;
>  }
>  
> @@ -387,13 +402,15 @@ static void xive_tctx_need_resend(XiveRouter *xrtr, XiveTCTX *tctx,
>  static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>                                  hwaddr offset, uint64_t value, unsigned size)
>  {
> -    uint32_t qw1w2 = value;
> -    uint8_t nvt_blk = xive_nvt_blk(qw1w2);
> -    uint32_t nvt_idx = xive_nvt_idx(qw1w2);
> -    bool vo = !!(qw1w2 & TM_QW1W2_VO);
> +    uint32_t cam = value;
> +    uint32_t qw1w2 = cpu_to_be32(cam);
> +    uint8_t nvt_blk;
> +    uint32_t nvt_idx;
> +    bool vo;
> +
> +    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
>  
>      /* First update the registers */
> -    qw1w2 = cpu_to_be32(qw1w2);
>      memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
>  
>      /* Check the interrupt pending bits */

-- 
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 --]

Re: [PATCH v4 24/25] ppc/pnv: Improve trigger data definition

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5736 bytes --]

On Wed, Sep 18, 2019 at 06:06:44PM +0200, Cédric Le Goater wrote:
> The trigger definition is used for triggers both for HW source
> interrupts, PHB, PSI, as well as for rerouting interrupts between
> Interrupt Controller.
> 
> HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
> information (EAS block + EAS index) in the 8 byte data and not END
> information, and bit1 of word0 to ‘1’ to signal that the state bit
> check has been performed.
> 
> Introduce these new trigger bits and rename the XIVE_SRCNO macros in
> XIVE_EAS to reflect better the nature of the data. This is breaking
> the notification for the PSI model which will be fixed in the next
> patch.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/ppc/xive_regs.h | 24 +++++++++++++++++++++---
>  hw/intc/pnv_xive.c         | 16 ++++++++++++----
>  hw/intc/xive.c             |  4 ++--
>  3 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index dd42c33cef35..83a2f2cc1318 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -22,9 +22,27 @@
>  /*
>   * Interrupt source number encoding on PowerBUS
>   */
> -#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
> -#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
> -#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
> +/*
> + * Trigger data definition
> + *
> + * The trigger definition is used for triggers both for HW source
> + * interrupts (PHB, PSI), as well as for rerouting interrupts between
> + * Interrupt Controller.
> + *
> + * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
> + * information (EAS block + EAS index) in the 8 byte data and not END
> + * information, and bit1 of word0 to ‘1’ to signal that the state bit
> + * check has been performed.
> + */
> +#define XIVE_TRIGGER_END        PPC_BIT(0)
> +#define XIVE_TRIGGER_EAS        PPC_BIT(1)

These names seem really confusing in light of the comment above.  HW
source controllers set the XIVE_TRIGGER_END bit in order to indicate
that they're providing EAS info, not END info??  And also set the
XIVE_TRIGGER_EAS bit to indicate something different?

Also, the comment implies that HW source controllers set both bits,
but I'm not really sure if that's what you meant.  Expanding on what
non-HW source controllers might set could be helpful, too..

> +
> +/*
> + * QEMU macros to manipulate the trigger payload in native endian
> + */
> +#define XIVE_EAS_BLOCK(n)       (((n) >> 28) & 0xf)
> +#define XIVE_EAS_INDEX(n)       ((n) & 0x0fffffff)
> +#define XIVE_EAS(blk, idx)      ((uint32_t)(blk) << 28 | (idx))
>  
>  #define TM_SHIFT                16
>  
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 4c1fa024cdf5..61af3f23000f 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>       * EAT lookups should be local to the IC
>       */
>      if (pnv_xive_block_id(xive) != blk) {
> -        xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
> +        xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
>          return -1;
>      }
>  
> @@ -502,7 +502,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
>      PnvXive *xive = PNV_XIVE(xn);
>      uint8_t blk = pnv_xive_block_id(xive);
>  
> -    xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
> +    xive_router_notify(xn, XIVE_EAS(blk, srcno));
>  }
>  
>  /*
> @@ -1287,12 +1287,20 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = {
>  
>  static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
>  {
> +    uint8_t blk = XIVE_EAS_BLOCK(val);
> +    uint32_t idx = XIVE_EAS_INDEX(val);
> +
>      /*
>       * Forward the source event notification directly to the Router.
>       * The source interrupt number should already be correctly encoded
>       * with the chip block id by the sending device (PHB, PSI).
>       */
> -    xive_router_notify(XIVE_NOTIFIER(xive), val);
> +    if (val & XIVE_TRIGGER_EAS) {
> +        xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
> +    } else {
> +        xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64,
> +                   addr, val);
> +    }
>  }
>  
>  static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
> @@ -1683,7 +1691,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>      XiveRouter *xrtr = XIVE_ROUTER(xive);
>      uint8_t blk = pnv_xive_block_id(xive);
>      uint8_t chip_id = xive->chip->chip_id;
> -    uint32_t srcno0 = XIVE_SRCNO(blk, 0);
> +    uint32_t srcno0 = XIVE_EAS(blk, 0);
>      uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
>      XiveEAS eas;
>      XiveEND end;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 07b7c3586c12..6702f32be601 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1652,8 +1652,8 @@ do_escalation:
>  void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>  {
>      XiveRouter *xrtr = XIVE_ROUTER(xn);
> -    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
> -    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
> +    uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
> +    uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
>      XiveEAS eas;
>  
>      /* EAS cache lookup */

-- 
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 --]

Re: [PATCH v4 24/25] ppc/pnv: Improve trigger data definition

[Cédric Le Goater]

On 03/10/2019 04:41, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:44PM +0200, Cédric Le Goater wrote:
>> The trigger definition is used for triggers both for HW source
>> interrupts, PHB, PSI, as well as for rerouting interrupts between
>> Interrupt Controller.
>>
>> HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
>> information (EAS block + EAS index) in the 8 byte data and not END
>> information, and bit1 of word0 to ‘1’ to signal that the state bit
>> check has been performed.
>>
>> Introduce these new trigger bits and rename the XIVE_SRCNO macros in
>> XIVE_EAS to reflect better the nature of the data. This is breaking
>> the notification for the PSI model which will be fixed in the next
>> patch.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive_regs.h | 24 +++++++++++++++++++++---
>>  hw/intc/pnv_xive.c         | 16 ++++++++++++----
>>  hw/intc/xive.c             |  4 ++--
>>  3 files changed, 35 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index dd42c33cef35..83a2f2cc1318 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -22,9 +22,27 @@
>>  /*
>>   * Interrupt source number encoding on PowerBUS
>>   */
>> -#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
>> -#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
>> -#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
>> +/*
>> + * Trigger data definition
>> + *
>> + * The trigger definition is used for triggers both for HW source
>> + * interrupts (PHB, PSI), as well as for rerouting interrupts between
>> + * Interrupt Controller.
>> + *
>> + * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
>> + * information (EAS block + EAS index) in the 8 byte data and not END
>> + * information, and bit1 of word0 to ‘1’ to signal that the state bit
>> + * check has been performed.
>> + */
>> +#define XIVE_TRIGGER_END        PPC_BIT(0)
>> +#define XIVE_TRIGGER_EAS        PPC_BIT(1)
> 
> These names seem really confusing in light of the comment above.  HW
> source controllers set the XIVE_TRIGGER_END bit in order to indicate
> that they're providing EAS info, not END info?? 

The END bit defines an END trigger, which is a forward trigger between 
ICs. In that case, the remote IC needs to do a lookup of an END and it 
needs an EAS containing an END index.

An END trigger, bit0 of word0 set to '1', is defined as :

         01234567 01234567 01234567 01234567 

W0 E=1   1P--BLOC          END IDX        
W1 E=1   M             END DATA         
    
An EAS is defined as :

         01234567 01234567 01234567 01234567 
W0       V---BLOC          END IDX
W1       M             END DATA              

Same layout as you can see. There is an extra 'PQ' bit, bit1 of word0 
set to '1', signaling that the PQ bits have been checked. But that bit 
is unused in the initial EAS definition.

> And also set the XIVE_TRIGGER_EAS bit to indicate something different?

That's where the confusion comes from. 

I have called it 'EAS' because the trigger data in that case contains 
an EAS index which the IC needs to look for. This is the format the HW 
devices use to perform the trigger.    

An EAS trigger, bit0 of word0 set to '0', is defined as :

         01234567 01234567 01234567 01234567 
W0 E=0   0P------ -------- -------- --------                              
W1 E=0   BLOC          EAS INDEX            


There is also a 'PQ' bit, bit1 of word0 to '1', signaling that the 
PQ bits have been checked. 


> Also, the comment implies that HW source controllers set both bits,
> but I'm not really sure if that's what you meant.  

END trigger starts with 0b11 (IC <-> IC)
EAS trigger starts with 0b01 (HW -> IC)

> Expanding on what non-HW source controllers might set could be helpful
> too ..

what do you mean ? this is all HW ! guests ? 

C. 


> 
>> +
>> +/*
>> + * QEMU macros to manipulate the trigger payload in native endian
>> + */
>> +#define XIVE_EAS_BLOCK(n)       (((n) >> 28) & 0xf)
>> +#define XIVE_EAS_INDEX(n)       ((n) & 0x0fffffff)
>> +#define XIVE_EAS(blk, idx)      ((uint32_t)(blk) << 28 | (idx))
>>  
>>  #define TM_SHIFT                16
>>  
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 4c1fa024cdf5..61af3f23000f 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>>       * EAT lookups should be local to the IC
>>       */
>>      if (pnv_xive_block_id(xive) != blk) {
>> -        xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
>> +        xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
>>          return -1;
>>      }
>>  
>> @@ -502,7 +502,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
>>      PnvXive *xive = PNV_XIVE(xn);
>>      uint8_t blk = pnv_xive_block_id(xive);
>>  
>> -    xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
>> +    xive_router_notify(xn, XIVE_EAS(blk, srcno));
>>  }
>>  
>>  /*
>> @@ -1287,12 +1287,20 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = {
>>  
>>  static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
>>  {
>> +    uint8_t blk = XIVE_EAS_BLOCK(val);
>> +    uint32_t idx = XIVE_EAS_INDEX(val);
>> +
>>      /*
>>       * Forward the source event notification directly to the Router.
>>       * The source interrupt number should already be correctly encoded
>>       * with the chip block id by the sending device (PHB, PSI).
>>       */
>> -    xive_router_notify(XIVE_NOTIFIER(xive), val);
>> +    if (val & XIVE_TRIGGER_EAS) {
>> +        xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
>> +    } else {
>> +        xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64,
>> +                   addr, val);
>> +    }
>>  }
>>  
>>  static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
>> @@ -1683,7 +1691,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>>      XiveRouter *xrtr = XIVE_ROUTER(xive);
>>      uint8_t blk = pnv_xive_block_id(xive);
>>      uint8_t chip_id = xive->chip->chip_id;
>> -    uint32_t srcno0 = XIVE_SRCNO(blk, 0);
>> +    uint32_t srcno0 = XIVE_EAS(blk, 0);
>>      uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
>>      XiveEAS eas;
>>      XiveEND end;
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 07b7c3586c12..6702f32be601 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -1652,8 +1652,8 @@ do_escalation:
>>  void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>>  {
>>      XiveRouter *xrtr = XIVE_ROUTER(xn);
>> -    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
>> -    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
>> +    uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
>> +    uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
>>      XiveEAS eas;
>>  
>>      /* EAS cache lookup */
> 

Re: [PATCH v4 22/25] ppc/xive: Introduce a xive_os_cam_decode() helper

[Cédric Le Goater]

On 03/10/2019 04:34, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:42PM +0200, Cédric Le Goater wrote:
>> The OS CAM line has a special encoding exploited by the HW. Provide a
>> helper routine to hide the details to the TIMA command handlers. This
>> also clarifies the endian ness of different variables : 'qw1w2' is
>> big-endian and 'cam' is native.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/intc/xive.c | 35 ++++++++++++++++++++++++++---------
>>  1 file changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index dfae584a319f..cdc4ea8b0e51 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -342,14 +342,29 @@ static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
>>      xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
>>  }
>>  
>> +static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
>> +                               uint32_t *nvt_idx, bool *vo)
>> +{
>> +    *nvt_blk = xive_nvt_blk(cam);
>> +    *nvt_idx = xive_nvt_idx(cam);
>> +    *vo = !!(cam & TM_QW1W2_VO);
>> +}
>> +
>>  static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>>                                      hwaddr offset, unsigned size)
>>  {
>> -    uint32_t qw1w2_prev = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
>> -    uint32_t qw1w2;
>> +    uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
>> +    uint32_t qw1w2_new;
>> +    uint32_t cam = be32_to_cpu(qw1w2);
>> +    uint8_t nvt_blk;
>> +    uint32_t nvt_idx;
>> +    bool vo;
>>  
>> -    qw1w2 = xive_set_field32(TM_QW1W2_VO, qw1w2_prev, 0);
>> -    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
> 
> I'd kind of prefer to fold both the memcpy and the endian swizzle into
> a read/write_register_word helper of some sort.

ok. I will see if I can improve that. The goal being to get and set
the relevant fields of the CAM line and hide away the ugly details.


C.

> 
>> +    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
>> +
>> +    /* Invalidate CAM line */
>> +    qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
>> +    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2_new, 4);
>>      return qw1w2;
>>  }
>>  
>> @@ -387,13 +402,15 @@ static void xive_tctx_need_resend(XiveRouter *xrtr, XiveTCTX *tctx,
>>  static void xive_tm_push_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>>                                  hwaddr offset, uint64_t value, unsigned size)
>>  {
>> -    uint32_t qw1w2 = value;
>> -    uint8_t nvt_blk = xive_nvt_blk(qw1w2);
>> -    uint32_t nvt_idx = xive_nvt_idx(qw1w2);
>> -    bool vo = !!(qw1w2 & TM_QW1W2_VO);
>> +    uint32_t cam = value;
>> +    uint32_t qw1w2 = cpu_to_be32(cam);
>> +    uint8_t nvt_blk;
>> +    uint32_t nvt_idx;
>> +    bool vo;
>> +
>> +    xive_os_cam_decode(cam, &nvt_blk, &nvt_idx, &vo);
>>  
>>      /* First update the registers */
>> -    qw1w2 = cpu_to_be32(qw1w2);
>>      memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
>>  
>>      /* Check the interrupt pending bits */
> 

Re: [PATCH v4 18/25] ppc/pnv: Skip empty slots of the XIVE NVT table

[Cédric Le Goater]

On 03/10/2019 04:22, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:38PM +0200, Cédric Le Goater wrote:
>> We try to loop on the full table skipping empty indirect pages which
>> are not necessarily allocated. This is useful to dump the contexts of
>> the KVM vCPUs.
> 
> I think this patch can be merged into the previous one.  They're both
> short, so we might as well get the dumping code correct first time.

ok.

C. 

Re: [PATCH v4 16/25] ppc/pnv: Remove pnv_xive_vst_size() routine

[Cédric Le Goater]

On 03/10/2019 04:20, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:36PM +0200, Cédric Le Goater wrote:
>> pnv_xive_vst_size() tries to compute the size of a VSD table from the
>> information given by FW. The number of entries of the table are
>> deduced from the result and the MMIO regions of the ESBs and the ENDS
>> are also resized with the computed value.
>>
>> But for indirect tables, the result is incorrect. An indirect table is
>> a one page array of VSDs pointing to subpages containing XIVE virtual
>> structures. The number of first level VSD entries is page aligned and
>> the overall computed size of the table is too large. It can also be
>> completely wrong when the first VSD entry is not yet initialized.
>>
>> Remove pnv_xive_vst_size() and use a simpler form for direct tables.
>> This is only useful when outputting the XIVE sources on the monitor
>> and to resize the ESB MMIO window.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Do.. does this actually correct the value for indirect tables? 

well, we can not before hand. It's a runtime value as Linux/OPAL
will populate the indirect entries depending on the number of vCPUs
requested. Max value is :

 (PG_SIZE / sizeof(vsd)) * (PG_SIZE / sizeof(end|vp))

That's 16M for ENDs, 8M for VPs and that's too big for the associated
BARS;

> Or
> just remove the calculation on the grounds that it was already broken
> and we'll fix later?  

We are removing the calculation and not fixing later, because of the
above reasons.

It impacts the underlying END ESB MMIO region which will keep its maximum 
initial size. The segmentation introduced by the translation set tables 
(EDT) at runtime will still reduce the accessible part : 

address-space: xive-vc-end
  0000000000000000-0000007fffffffff (prio 0, i/o): xive-vc-end
    0000000000000000-0000001fffffffff (prio 0, i/o): xive-vc-end-edt
      0000000000000000-0000007fffffffff (prio 0, i/o): xive.end

However, the IPI ESB MMIO region is still resized because the PQ table
is direct. So we can deduce from the table size the MMIO region.

address-space: xive-vc-ipi
  0000000000000000-0000007fffffffff (prio 0, i/o): xive-vc-ipi
    0000000000000000-0000005fffffffff (prio 0, i/o): xive-vc-ipi-edt
      0000000000000000-0000001fffffffff (prio 0, i/o): xive.esb


> If it's the latter, we should surely have some
> error_report()s or something if the guest does attempt to use an
> indirect table.

We have checks in pnv_xive_vst_addr_indirect() which computes the
address of a XIVE structure in an indirect table.

I think we can improve error reporting by using address_space_* 
routines however.

C.

>> ---
>>  hw/intc/pnv_xive.c | 112 +++++++++++++++++----------------------------
>>  1 file changed, 43 insertions(+), 69 deletions(-)
>>
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 74d6ccbea3d6..b7d505839e68 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -123,36 +123,22 @@ static uint64_t pnv_xive_vst_page_size_allowed(uint32_t page_shift)
>>           page_shift == 21 || page_shift == 24;
>>  }
>>  
>> -static uint64_t pnv_xive_vst_size(uint64_t vsd)
>> -{
>> -    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
>> -
>> -    /*
>> -     * Read the first descriptor to get the page size of the indirect
>> -     * table.
>> -     */
>> -    if (VSD_INDIRECT & vsd) {
>> -        uint32_t nr_pages = vst_tsize / XIVE_VSD_SIZE;
>> -        uint32_t page_shift;
>> -
>> -        vsd = ldq_be_dma(&address_space_memory, vsd & VSD_ADDRESS_MASK);
>> -        page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
>> -
>> -        if (!pnv_xive_vst_page_size_allowed(page_shift)) {
>> -            return 0;
>> -        }
>> -
>> -        return nr_pages * (1ull << page_shift);
>> -    }
>> -
>> -    return vst_tsize;
>> -}
>> -
>>  static uint64_t pnv_xive_vst_addr_direct(PnvXive *xive, uint32_t type,
>>                                           uint64_t vsd, uint32_t idx)
>>  {
>>      const XiveVstInfo *info = &vst_infos[type];
>>      uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
>> +    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
>> +    uint32_t idx_max;
>> +
>> +    idx_max = vst_tsize / info->size - 1;
>> +    if (idx > idx_max) {
>> +#ifdef XIVE_DEBUG
>> +        xive_error(xive, "VST: %s entry %x out of range [ 0 .. %x ] !?",
>> +                   info->name, idx, idx_max);
>> +#endif
>> +        return 0;
>> +    }
>>  
>>      return vst_addr + idx * info->size;
>>  }
>> @@ -215,7 +201,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
>>  {
>>      const XiveVstInfo *info = &vst_infos[type];
>>      uint64_t vsd;
>> -    uint32_t idx_max;
>>  
>>      if (blk >= info->max_blocks) {
>>          xive_error(xive, "VST: invalid block id %d for VST %s %d !?",
>> @@ -232,15 +217,6 @@ static uint64_t pnv_xive_vst_addr(PnvXive *xive, uint32_t type, uint8_t blk,
>>          return xive ? pnv_xive_vst_addr(xive, type, blk, idx) : 0;
>>      }
>>  
>> -    idx_max = pnv_xive_vst_size(vsd) / info->size - 1;
>> -    if (idx > idx_max) {
>> -#ifdef XIVE_DEBUG
>> -        xive_error(xive, "VST: %s entry %x/%x out of range [ 0 .. %x ] !?",
>> -                   info->name, blk, idx, idx_max);
>> -#endif
>> -        return 0;
>> -    }
>> -
>>      if (VSD_INDIRECT & vsd) {
>>          return pnv_xive_vst_addr_indirect(xive, type, vsd, idx);
>>      }
>> @@ -519,19 +495,12 @@ static uint64_t pnv_xive_pc_size(PnvXive *xive)
>>      return (~xive->regs[CQ_PC_BARM >> 3] + 1) & CQ_PC_BARM_MASK;
>>  }
>>  
>> -static uint32_t pnv_xive_nr_ipis(PnvXive *xive)
>> +static uint32_t pnv_xive_nr_ipis(PnvXive *xive, uint8_t blk)
>>  {
>> -    uint8_t blk = xive->chip->chip_id;
>> -
>> -    return pnv_xive_vst_size(xive->vsds[VST_TSEL_SBE][blk]) * SBE_PER_BYTE;
>> -}
>> -
>> -static uint32_t pnv_xive_nr_ends(PnvXive *xive)
>> -{
>> -    uint8_t blk = xive->chip->chip_id;
>> +    uint64_t vsd = xive->vsds[VST_TSEL_SBE][blk];
>> +    uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
>>  
>> -    return pnv_xive_vst_size(xive->vsds[VST_TSEL_EQDT][blk])
>> -        / vst_infos[VST_TSEL_EQDT].size;
>> +    return VSD_INDIRECT & vsd ? 0 : vst_tsize * SBE_PER_BYTE;
>>  }
>>  
>>  /*
>> @@ -664,6 +633,7 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>>      XiveSource *xsrc = &xive->ipi_source;
>>      const XiveVstInfo *info = &vst_infos[type];
>>      uint32_t page_shift = GETFIELD(VSD_TSIZE, vsd) + 12;
>> +    uint64_t vst_tsize = 1ull << page_shift;
>>      uint64_t vst_addr = vsd & VSD_ADDRESS_MASK;
>>  
>>      /* Basic checks */
>> @@ -699,11 +669,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>>  
>>      case VST_TSEL_EQDT:
>>          /*
>> -         * Backing store pages for the END. Compute the number of ENDs
>> -         * provisioned by FW and resize the END ESB window accordingly.
>> +         * Backing store pages for the END.
>> +         *
>> +         * If the table is direct, we can compute the number of PQ
>> +         * entries provisioned by FW (such as skiboot) and resize the
>> +         * END ESB window accordingly.
>>           */
>> -        memory_region_set_size(&end_xsrc->esb_mmio, pnv_xive_nr_ends(xive) *
>> -                               (1ull << (end_xsrc->esb_shift + 1)));
>> +        if (!(VSD_INDIRECT & vsd)) {
>> +            memory_region_set_size(&end_xsrc->esb_mmio, (vst_tsize / info->size)
>> +                                   * (1ull << xsrc->esb_shift));
>> +        }
>>          memory_region_add_subregion(&xive->end_edt_mmio, 0,
>>                                      &end_xsrc->esb_mmio);
>>          break;
>> @@ -712,11 +687,16 @@ static void pnv_xive_vst_set_exclusive(PnvXive *xive, uint8_t type,
>>          /*
>>           * Backing store pages for the source PQ bits. The model does
>>           * not use these PQ bits backed in RAM because the XiveSource
>> -         * model has its own. Compute the number of IRQs provisioned
>> -         * by FW and resize the IPI ESB window accordingly.
>> +         * model has its own.
>> +         *
>> +         * If the table is direct, we can compute the number of PQ
>> +         * entries provisioned by FW (such as skiboot) and resize the
>> +         * ESB window accordingly.
>>           */
>> -        memory_region_set_size(&xsrc->esb_mmio, pnv_xive_nr_ipis(xive) *
>> -                               (1ull << xsrc->esb_shift));
>> +        if (!(VSD_INDIRECT & vsd)) {
>> +            memory_region_set_size(&xsrc->esb_mmio, vst_tsize * SBE_PER_BYTE
>> +                                   * (1ull << xsrc->esb_shift));
>> +        }
>>          memory_region_add_subregion(&xive->ipi_edt_mmio, 0, &xsrc->esb_mmio);
>>          break;
>>  
>> @@ -1666,8 +1646,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>>      XiveRouter *xrtr = XIVE_ROUTER(xive);
>>      uint8_t blk = xive->chip->chip_id;
>>      uint32_t srcno0 = XIVE_SRCNO(blk, 0);
>> -    uint32_t nr_ipis = pnv_xive_nr_ipis(xive);
>> -    uint32_t nr_ends = pnv_xive_nr_ends(xive);
>> +    uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
>>      XiveEAS eas;
>>      XiveEND end;
>>      int i;
>> @@ -1687,21 +1666,16 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>>          }
>>      }
>>  
>> -    monitor_printf(mon, "XIVE[%x] ENDT %08x .. %08x\n", blk, 0, nr_ends - 1);
>> -    for (i = 0; i < nr_ends; i++) {
>> -        if (xive_router_get_end(xrtr, blk, i, &end)) {
>> -            break;
>> -        }
>> -        xive_end_pic_print_info(&end, i, mon);
>> +    monitor_printf(mon, "XIVE[%x] ENDT\n", blk);
>> +    i = 0;
>> +    while (!xive_router_get_end(xrtr, blk, i, &end)) {
>> +        xive_end_pic_print_info(&end, i++, mon);
>>      }
>>  
>> -    monitor_printf(mon, "XIVE[%x] END Escalation %08x .. %08x\n", blk, 0,
>> -                   nr_ends - 1);
>> -    for (i = 0; i < nr_ends; i++) {
>> -        if (xive_router_get_end(xrtr, blk, i, &end)) {
>> -            break;
>> -        }
>> -        xive_end_eas_pic_print_info(&end, i, mon);
>> +    monitor_printf(mon, "XIVE[%x] END Escalation EAT\n", blk);
>> +    i = 0;
>> +    while (!xive_router_get_end(xrtr, blk, i, &end)) {
>> +        xive_end_eas_pic_print_info(&end, i++, mon);
>>      }
>>  }
>>  
> 

Re: [PATCH v4 14/25] ppc/xive: Introduce helpers for the NVT id

[Cédric Le Goater]

On 03/10/2019 04:12, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:34PM +0200, Cédric Le Goater wrote:
>> The NVT space is 19 bits wide, giving a maximum of 512K per chip. When
>> dispatched on a HW thread, the NVT identifier of a vCPU is pushed/stored
>> in the CAM line (word2) of the thread interrupt context.
> 
> Ok, that's interesting, but how does it lead to this patch?  Connect
> the dots, please.

ok. These are helpers for the next patch. It's a clarification of the
values also.


When a vPCU is dispatched on a HW thread, the NVT identifier is pushed
in the CAM line (QW1W2). This identifier is used to fetch a NVT structure,
in the presenter subengine, which might contain pending interrupts that 
need a resend. So we will use these helpers to do : 

 xive_router_get_nvt(xrtr, nvt_blk, nvt_idx, &nvt)
 xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4)

and notify a cpu.

C.


> 
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive.h      |  5 -----
>>  include/hw/ppc/xive_regs.h | 21 +++++++++++++++++++++
>>  2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index a461753f5da5..794dfcaae0f8 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -469,11 +469,6 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>>  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>>  void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
>>  
>> -static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> -{
>> -    return (nvt_blk << 19) | nvt_idx;
>> -}
>> -
>>  /*
>>   * KVM XIVE device helpers
>>   */
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index 08c8bf7172e2..3d7b6fd09664 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -251,4 +251,25 @@ typedef struct XiveNVT {
>>  
>>  #define xive_nvt_is_valid(nvt)    (be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
>>  
>> +/*
>> + * The VP number space in a block is defined by the END_W6_NVT_INDEX
>> + * field of the XIVE END
>> + */
>> +#define XIVE_NVT_SHIFT                19
>> +
>> +static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>> +{
>> +    return (nvt_blk << XIVE_NVT_SHIFT) | nvt_idx;
>> +}
>> +
>> +static inline uint32_t xive_nvt_idx(uint32_t cam_line)
>> +{
>> +    return cam_line & ((1 << XIVE_NVT_SHIFT) - 1);
>> +}
>> +
>> +static inline uint32_t xive_nvt_blk(uint32_t cam_line)
>> +{
>> +    return (cam_line >> XIVE_NVT_SHIFT) & 0xf;
>> +}
>> +
>>  #endif /* PPC_XIVE_REGS_H */
> 

Re: [PATCH v4 13/25] ppc/xive: Introduce a xive_tctx_ipb_update() helper

[Cédric Le Goater]

On 03/10/2019 04:11, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:33PM +0200, Cédric Le Goater wrote:
>> We will use it to resend missed interrupts when a vCPU context is
>> pushed a HW thread.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive.h |  1 +
>>  hw/intc/xive.c        | 15 +++++++++++----
>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index f35ff3b64791..a461753f5da5 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -467,6 +467,7 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>>  
>>  void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
>>  Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp);
>> +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb);
>>  
>>  static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
>>  {
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 68d3361d1c3f..5f7c37b091a7 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -135,6 +135,15 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, uint8_t cppr)
>>      xive_tctx_notify(tctx, ring);
>>  }
>>  
>> +void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
>> +{
>> +    uint8_t *regs = &tctx->regs[ring];
>> +
>> +    regs[TM_IPB] |= ipb;
>> +    regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
> 
> You're opencoding the existing ipb_update() here, but not eliminating
> it.  That doesn't seem like a good idea.
 It is in patch 15 when the IPB is correctly recorded in the NVT. 

I can move that part before patch 13 to make things clearer.


C. 

>> +    xive_tctx_notify(tctx, ring);
>> +}
>> +
>>  static inline uint32_t xive_tctx_word2(uint8_t *ring)
>>  {
>>      return *((uint32_t *) &ring[TM_WORD2]);
>> @@ -336,8 +345,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
>>  static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
>>                                     hwaddr offset, uint64_t value, unsigned size)
>>  {
>> -    ipb_update(&tctx->regs[TM_QW1_OS], value & 0xff);
>> -    xive_tctx_notify(tctx, TM_QW1_OS);
>> +    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
>>  }
>>  
>>  static uint64_t xive_tm_pull_os_ctx(XivePresenter *xptr, XiveTCTX *tctx,
>> @@ -1382,8 +1390,7 @@ static bool xive_presenter_notify(uint8_t format,
>>  
>>      /* handle CPU exception delivery */
>>      if (count) {
>> -        ipb_update(&match.tctx->regs[match.ring], priority);
>> -        xive_tctx_notify(match.tctx, match.ring);
>> +        xive_tctx_ipb_update(match.tctx, match.ring, priority_to_ipb(priority));
>>      }
>>  
>>      return count;
> 

Re: [PATCH v4 03/25] ppc/pnv: Introduce a PNV_CHIP_CPU_FOREACH() helper

[Cédric Le Goater]

On 03/10/2019 03:50, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:23PM +0200, Cédric Le Goater wrote:
>> As there is now easy way to loop on the CPUs belonging to a chip, add
>> a helper to filter out external CPUs.
> 
> This seems a somewhat odd way to go about it, given that the chip does
> have a cores array and the cores then have a threads array.  What's
> the difficulty with using that rather than looping through all vcpus
> and filtering?

because I find some ugliness in using the chip->cores array I guess. 
See pnv_chip_core_realize(), pnv_chip_quad_realize() etc.

Anyway, I agree this is rather hacky and with Greg's ideas to move 
the XiveTCTX list under the XiveRouter, we should get rid of it.

C. 


>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/intc/pnv_xive.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index ae449aa1119b..e1c15b6b5b71 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -392,15 +392,36 @@ 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 cpu_pir(PowerPCCPU *cpu)
>> +{
>> +    CPUPPCState *env = &cpu->env;
>> +    return env->spr_cb[SPR_PIR].default_value;
>> +}
>> +
>> +static int cpu_chip_id(PowerPCCPU *cpu)
>> +{
>> +    int pir = cpu_pir(cpu);
>> +    return (pir >> 8) & 0x7f;
>> +}
>> +
>> +#define PNV_CHIP_CPU_FOREACH(chip, cs)                                  \
>> +    CPU_FOREACH(cs)                                                     \
>> +        if (chip->chip_id != cpu_chip_id(POWERPC_CPU(cs))) {} else
>> +
>>  static int pnv_xive_match_nvt(XivePresenter *xptr, uint8_t format,
>>                                uint8_t nvt_blk, uint32_t nvt_idx,
>>                                bool cam_ignore, uint8_t priority,
>>                                uint32_t logic_serv, XiveTCTXMatch *match)
>>  {
>> +    PnvXive *xive = PNV_XIVE(xptr);
>>      CPUState *cs;
>>      int count = 0;
>>  
>> -    CPU_FOREACH(cs) {
>> +    /*
>> +     * Loop on all CPUs of the machine and filter out the CPUs
>> +     * belonging to another chip.
>> +     */
>> +    PNV_CHIP_CPU_FOREACH(xive->chip, cs) {
>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>>          XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>>          int ring;
> 

Re: [PATCH v4 05/25] ppc/xive: Introduce a XiveFabric interface

[Cédric Le Goater]

On 03/10/2019 03:54, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:25PM +0200, Cédric Le Goater wrote:
>> The XiveFabric QOM interface should be implemented by the machine. It
>> acts as the PowerBUS interface between the interrupt controller and
>> the system. On HW, the XIVE sub-engine is responsible for the
>> communication with the other chip is the Common Queue (CQ) bridge
>> unit.
>>
>> This interface offers a 'match_nvt' handler to perform the CAM line
>> matching when looking for a XIVE Presenter with a dispatched NVT.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Um.. this has basically the same single method as the XivePresented
> interface you just introduced.  What's the distinction between them?

XivePresenter is the subengine of the controller, XiveFabric is the
bus linking the chips on the machine. PowerNV being multichip, when
looking for a target, we need to scan all presenters in the machine.

C. 

> 
>> ---
>>  include/hw/ppc/xive.h | 22 ++++++++++++++++++++++
>>  hw/intc/xive.c        | 10 ++++++++++
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 39de45b87cb9..3c2910e10e25 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -399,6 +399,28 @@ int xive_presenter_tctx_match(XivePresenter *xptr, XiveTCTX *tctx,
>>                                uint8_t nvt_blk, uint32_t nvt_idx,
>>                                bool cam_ignore, uint32_t logic_serv);
>>  
>> +/*
>> + * XIVE Fabric (Interface between Interrupt Controller and Machine)
>> + */
>> +
>> +typedef struct XiveFabric XiveFabric;
>> +
>> +#define TYPE_XIVE_FABRIC "xive-fabric"
>> +#define XIVE_FABRIC(obj)                                     \
>> +    INTERFACE_CHECK(XiveFabric, (obj), TYPE_XIVE_FABRIC)
>> +#define XIVE_FABRIC_CLASS(klass)                                     \
>> +    OBJECT_CLASS_CHECK(XiveFabricClass, (klass), TYPE_XIVE_FABRIC)
>> +#define XIVE_FABRIC_GET_CLASS(obj)                                   \
>> +    OBJECT_GET_CLASS(XiveFabricClass, (obj), TYPE_XIVE_FABRIC)
>> +
>> +typedef struct XiveFabricClass {
>> +    InterfaceClass parent;
>> +    int (*match_nvt)(XiveFabric *xfb, uint8_t format,
>> +                     uint8_t nvt_blk, uint32_t nvt_idx,
>> +                     bool cam_ignore, uint8_t priority,
>> +                     uint32_t logic_serv, XiveTCTXMatch *match);
>> +} XiveFabricClass;
>> +
>>  /*
>>   * XIVE END ESBs
>>   */
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index fff50429f8ac..7a15a64ed7fe 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -1841,8 +1841,18 @@ static const TypeInfo xive_presenter_info = {
>>      .class_size = sizeof(XivePresenterClass),
>>  };
>>  
>> +/*
>> + * XIVE Fabric
>> + */
>> +static const TypeInfo xive_fabric_info = {
>> +    .name = TYPE_XIVE_FABRIC,
>> +    .parent = TYPE_INTERFACE,
>> +    .class_size = sizeof(XiveFabricClass),
>> +};
>> +
>>  static void xive_register_types(void)
>>  {
>> +    type_register_static(&xive_fabric_info);
>>      type_register_static(&xive_source_info);
>>      type_register_static(&xive_notifier_info);
>>      type_register_static(&xive_presenter_info);
> 

Re: [PATCH v4 06/25] ppc/pnv: Implement the XiveFabric interface

[Cédric Le Goater]

On 03/10/2019 03:55, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:26PM +0200, Cédric Le Goater wrote:
>> The CAM line matching on the PowerNV machine now scans all chips of
>> the system and all CPUs of a chip to find a dispatched NVT in the
>> thread contexts.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/ppc/pnv.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 77a86c6a2301..ca24dd62df23 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -1378,6 +1378,35 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
>>      }
>>  }
>>  
>> +static int pnv_xive_match_nvt(XiveFabric *xfb, uint8_t format,
>> +                               uint8_t nvt_blk, uint32_t nvt_idx,
>> +                               bool cam_ignore, uint8_t priority,
>> +                               uint32_t logic_serv,
>> +                               XiveTCTXMatch *match)
>> +{
>> +    PnvMachineState *pnv = PNV_MACHINE(xfb);
>> +    int total_count = 0;
>> +    int i;
>> +
>> +    for (i = 0; i < pnv->num_chips; i++) {
>> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
>> +        XivePresenter *xptr = XIVE_PRESENTER(&chip9->xive);
>> +        XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>> +        int count;
>> +
>> +        count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
>> +                               priority, logic_serv, match);
>> +
>> +        if (count < 0) {
>> +            return count;
>> +        }
>> +
>> +        total_count += count;
>> +    }
>> +
>> +    return total_count;
>> +}
>> +
>>  static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
>>                                void *opaque, Error **errp)
>>  {
>> @@ -1441,9 +1470,11 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
>>  static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>>  
>>      mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
>>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
>> +    xfc->match_nvt = pnv_xive_match_nvt;
>>  
>>      mc->alias = "powernv";
>>  }
>> @@ -1495,6 +1526,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>>          .interfaces = (InterfaceInfo[]) {               \
>>              { TYPE_XICS_FABRIC },                       \
>>              { TYPE_INTERRUPT_STATS_PROVIDER },          \
>> +            { TYPE_XIVE_FABRIC },                       \
> 
> Hmm.. this should probably be on the powernv9 machine only, not
> powernv8 as well, yes?

you are right. 

Thanks,

C. 


> 
>>              { },                                        \
>>          },                                              \
>>      }
> 

Re: [PATCH v4 07/25] ppc/spapr: Implement the XiveFabric interface

[Cédric Le Goater]

On 03/10/2019 03:58, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:27PM +0200, Cédric Le Goater wrote:
>> The CAM line matching sequence in the pseries machine does not change
>> much apart from the use of the new QOM interfaces. There is an extra
>> indirection because of the sPAPR IRQ backend of the machine. Only the
>> XIVE backend implements the new 'match_nvt' handler.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/spapr_irq.h |  6 ++++++
>>  hw/ppc/spapr.c             | 34 ++++++++++++++++++++++++++++++++++
>>  hw/ppc/spapr_irq.c         | 25 +++++++++++++++++++++++++
>>  3 files changed, 65 insertions(+)
>>
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index 5db305165ce2..859780efaf95 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -31,6 +31,8 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align,
>>                          Error **errp);
>>  void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num);
>>  
>> +struct XiveTCTXMatch;
>> +
>>  typedef struct SpaprIrq {
>>      uint32_t    nr_irqs;
>>      uint32_t    nr_msis;
>> @@ -50,6 +52,10 @@ typedef struct SpaprIrq {
>>      void (*set_irq)(void *opaque, int srcno, int val);
>>      const char *(*get_nodename)(SpaprMachineState *spapr);
>>      void (*init_kvm)(SpaprMachineState *spapr, Error **errp);
>> +    int (*match_nvt)(SpaprMachineState *spapr, uint8_t format,
>> +                     uint8_t nvt_blk, uint32_t nvt_idx,
>> +                     bool cam_ignore, uint8_t priority,
>> +                     uint32_t logic_serv, struct XiveTCTXMatch *match);
> 
> Obviously this will need some rework against my stuff.
> 
> But more importantly, I don't see the point of indirecting via here,
> when the method is only relevant for the xive case.  Why not just
> assert that XIVE is in use in the XiveFabric hook, and go directly  to
> the XIVE matching code.

Yes you might be right. I had to introduce this helper to cover dual 
and xive mode for some reason. I will rebase when your code is merged and 
see how things fit together. I am not worried.

C.


> 
>>  } SpaprIrq;
>>  
>>  extern SpaprIrq spapr_irq_xics;
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 2725b139a7f0..90f6f5fb9536 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4360,6 +4360,37 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj,
>>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>>  }
>>  
>> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
>> +                                uint8_t nvt_blk, uint32_t nvt_idx,
>> +                                bool cam_ignore, uint8_t priority,
>> +                                uint32_t logic_serv, XiveTCTXMatch *match)
>> +{
>> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
>> +    int count;
>> +
>> +    count = spapr->irq->match_nvt(spapr, format, nvt_blk, nvt_idx, cam_ignore,
>> +                                  priority, logic_serv, match);
>> +    if (count < 0) {
>> +        return count;
>> +    }
>> +
>> +    /*
>> +     * When we implement the save and restore of the thread interrupt
>> +     * contexts in the enter/exit CPU handlers of the machine and the
>> +     * escalations in QEMU, we should be able to handle non dispatched
>> +     * vCPUs.
>> +     *
>> +     * Until this is done, the sPAPR machine should find at least one
>> +     * matching context always.
>> +     */
>> +    if (count == 0) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
>> +                      nvt_blk, nvt_idx);
>> +    }
>> +
>> +    return count;
>> +}
>> +
>>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>>  {
>>      return cpu->vcpu_id;
>> @@ -4456,6 +4487,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>>  
>>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>>      mc->ignore_boot_device_suffixes = true;
>> @@ -4514,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>>       */
>>      mc->numa_mem_align_shift = 28;
>>      mc->numa_mem_supported = true;
>> +    xfc->match_nvt = spapr_xive_match_nvt;
>>  
>>      smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>>      smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
>> @@ -4547,6 +4580,7 @@ static const TypeInfo spapr_machine_info = {
>>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>>          { TYPE_XICS_FABRIC },
>>          { TYPE_INTERRUPT_STATS_PROVIDER },
>> +        { TYPE_XIVE_FABRIC },
>>          { }
>>      },
>>  };
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index d8f46b6797f8..8a6d79a59af2 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -257,6 +257,7 @@ SpaprIrq spapr_irq_xics = {
>>      .set_irq     = spapr_irq_set_irq_xics,
>>      .get_nodename = spapr_irq_get_nodename_xics,
>>      .init_kvm    = spapr_irq_init_kvm_xics,
>> +    .match_nvt   = NULL, /* should not be used */
>>  };
>>  
>>  /*
>> @@ -406,6 +407,18 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp)
>>      }
>>  }
>>  
>> +static int spapr_irq_match_nvt_xive(SpaprMachineState *spapr, uint8_t format,
>> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
>> +                                    bool cam_ignore, uint8_t priority,
>> +                                    uint32_t logic_serv, XiveTCTXMatch *match)
>> +{
>> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
>> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
>> +
>> +    return xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
>> +                          priority, logic_serv, match);
>> +}
>> +
>>  /*
>>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>>   * with XICS.
>> @@ -431,6 +444,7 @@ SpaprIrq spapr_irq_xive = {
>>      .set_irq     = spapr_irq_set_irq_xive,
>>      .get_nodename = spapr_irq_get_nodename_xive,
>>      .init_kvm    = spapr_irq_init_kvm_xive,
>> +    .match_nvt   = spapr_irq_match_nvt_xive,
>>  };
>>  
>>  /*
>> @@ -585,6 +599,15 @@ static const char *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
>>      return spapr_irq_current(spapr)->get_nodename(spapr);
>>  }
>>  
>> +static int spapr_irq_match_nvt_dual(SpaprMachineState *spapr, uint8_t format,
>> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
>> +                                    bool cam_ignore, uint8_t priority,
>> +                                    uint32_t logic_serv, XiveTCTXMatch *match)
>> +{
>> +    return spapr_irq_current(spapr)->match_nvt(spapr, format, nvt_blk, nvt_idx,
>> +                                     cam_ignore, priority, logic_serv, match);
>> +}
>> +
>>  /*
>>   * Define values in sync with the XIVE and XICS backend
>>   */
>> @@ -608,6 +631,7 @@ SpaprIrq spapr_irq_dual = {
>>      .set_irq     = spapr_irq_set_irq_dual,
>>      .get_nodename = spapr_irq_get_nodename_dual,
>>      .init_kvm    = NULL, /* should not be used */
>> +    .match_nvt   = spapr_irq_match_nvt_dual,
>>  };
>>  
>>  
>> @@ -825,4 +849,5 @@ SpaprIrq spapr_irq_xics_legacy = {
>>      .set_irq     = spapr_irq_set_irq_xics,
>>      .get_nodename = spapr_irq_get_nodename_xics,
>>      .init_kvm    = spapr_irq_init_kvm_xics,
>> +    .match_nvt   = NULL, /* should not be used */
>>  };
> 

Re: [PATCH v4 11/25] ppc/xive: Move the TIMA operations to the controller model

[Cédric Le Goater]

On 03/10/2019 04:08, David Gibson wrote:
> On Wed, Sep 18, 2019 at 06:06:31PM +0200, Cédric Le Goater wrote:
>> This also removes the need of the get_tctx() XiveRouter handler in the
>> core XIVE framework.
> 
> In general these commit messages could really do with more context.
> What exactly is the "controller model"?  Where were the TIMA
> operations before now.  Why is having them in the controller model
> better?
> 
> I could probably answer those with some investigation, but the point
> is that the commit message is supposed to make it easy to find that
> information.
> 
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  include/hw/ppc/xive.h |  1 -
>>  hw/intc/pnv_xive.c    | 35 ++++++++++++++++++++++++++++++++++-
>>  hw/intc/spapr_xive.c  | 33 +++++++++++++++++++++++++++++++--
>>  hw/intc/xive.c        | 29 -----------------------------
>>  4 files changed, 65 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 536deea8c622..9d9cd88dd17e 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -462,7 +462,6 @@ typedef struct XiveENDSource {
>>  #define XIVE_TM_OS_PAGE         0x2
>>  #define XIVE_TM_USER_PAGE       0x3
>>  
>> -extern const MemoryRegionOps xive_tm_ops;
>>  void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>>                          uint64_t value, unsigned size);
>>  uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
>> index 3d6fcf9ac139..40e18fb44811 100644
>> --- a/hw/intc/pnv_xive.c
>> +++ b/hw/intc/pnv_xive.c
>> @@ -1475,6 +1475,39 @@ static const MemoryRegionOps xive_tm_indirect_ops = {
>>      },
>>  };
>>  
>> +static void pnv_xive_tm_write(void *opaque, hwaddr offset,
>> +                              uint64_t value, unsigned size)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
>> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>> +
>> +    xive_tctx_tm_write(XIVE_PRESENTER(xive), tctx, offset, value, size);
>> +}
>> +
>> +static uint64_t pnv_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
>> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
>> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
>> +
>> +    return xive_tctx_tm_read(XIVE_PRESENTER(xive), tctx, offset, size);
> 
> You're not using the opaque pointer in either of these cases.  So
> couldn't you make it point to the presenter for pnv as well, then...

On PowerNV, it's the cpu doing the TIMA load/store which determines 
the chip and the XiveTCTX is deduced from the pnv_cpu_state().

The TIMA is only mapped on chip0 in our model. See CQ_TM1_BAR reg.
 
> 
>> +}
>> +
>> +const MemoryRegionOps pnv_xive_tm_ops = {
>> +    .read = pnv_xive_tm_read,
>> +    .write = pnv_xive_tm_write,
>> +    .endianness = DEVICE_BIG_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 8,
>> +    },
>> +    .impl = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 8,
>> +    },
>> +};
>> +
>>  /*
>>   * Interrupt controller XSCOM region.
>>   */
>> @@ -1832,7 +1865,7 @@ static void pnv_xive_realize(DeviceState *dev, Error **errp)
>>                            "xive-pc", PNV9_XIVE_PC_SIZE);
>>  
>>      /* Thread Interrupt Management Area (Direct) */
>> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops,
>> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &pnv_xive_tm_ops,
>>                            xive, "xive-tima", PNV9_XIVE_TM_SIZE);
>>  
>>      qemu_register_reset(pnv_xive_reset, dev);
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index eefc0d4c36b9..e00a9bdd901b 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -222,6 +222,35 @@ void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx)
>>      memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
>>  }
>>  
>> +static void spapr_xive_tm_write(void *opaque, hwaddr offset,
>> +                          uint64_t value, unsigned size)
>> +{
>> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
>> +
>> +    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value,
>> size);
> 
> ... there would be no difference from the spapr versions, AFAICT.

the XiveTCTX is deduced from the spapr_cpu_state().

C.

> 
>> +}
>> +
>> +static uint64_t spapr_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
>> +
>> +    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
>> +}
>> +
>> +const MemoryRegionOps spapr_xive_tm_ops = {
>> +    .read = spapr_xive_tm_read,
>> +    .write = spapr_xive_tm_write,
>> +    .endianness = DEVICE_BIG_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 8,
>> +    },
>> +    .impl = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 8,
>> +    },
>> +};
>> +
>>  static void spapr_xive_end_reset(XiveEND *end)
>>  {
>>      memset(end, 0, sizeof(*end));
>> @@ -331,8 +360,8 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp)
>>      qemu_register_reset(spapr_xive_reset, dev);
>>  
>>      /* TIMA initialization */
>> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops, xive,
>> -                          "xive.tima", 4ull << TM_SHIFT);
>> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &spapr_xive_tm_ops,
>> +                          xive, "xive.tima", 4ull << TM_SHIFT);
>>      sysbus_init_mmio(SYS_BUS_DEVICE(xive), &xive->tm_mmio);
>>  
>>      /*
>> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
>> index 9bb09ed6ee7b..11432f04f5c3 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -483,35 +483,6 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>>      return xive_tm_raw_read(tctx, offset, size);
>>  }
>>  
>> -static void xive_tm_write(void *opaque, hwaddr offset,
>> -                          uint64_t value, unsigned size)
>> -{
>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> -
>> -    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
>> -}
>> -
>> -static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
>> -{
>> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
>> -
>> -    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
>> -}
>> -
>> -const MemoryRegionOps xive_tm_ops = {
>> -    .read = xive_tm_read,
>> -    .write = xive_tm_write,
>> -    .endianness = DEVICE_BIG_ENDIAN,
>> -    .valid = {
>> -        .min_access_size = 1,
>> -        .max_access_size = 8,
>> -    },
>> -    .impl = {
>> -        .min_access_size = 1,
>> -        .max_access_size = 8,
>> -    },
>> -};
>> -
>>  static char *xive_tctx_ring_print(uint8_t *ring)
>>  {
>>      uint32_t w2 = xive_tctx_word2(ring);
> 

Re: [PATCH v4 06/25] ppc/pnv: Implement the XiveFabric interface

[Greg Kurz]

On Wed, 18 Sep 2019 18:06:26 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> The CAM line matching on the PowerNV machine now scans all chips of
> the system and all CPUs of a chip to find a dispatched NVT in the
> thread contexts.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 77a86c6a2301..ca24dd62df23 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1378,6 +1378,35 @@ static void pnv_pic_print_info(InterruptStatsProvider *obj,
>      }
>  }
>  
> +static int pnv_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> +                               uint8_t nvt_blk, uint32_t nvt_idx,
> +                               bool cam_ignore, uint8_t priority,
> +                               uint32_t logic_serv,
> +                               XiveTCTXMatch *match)

Hmm, patch 2 already introduces a function with the same name:

--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -392,6 +392,45 @@ 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_match_nvt(XivePresenter *xptr, uint8_t format,
+                              uint8_t nvt_blk, uint32_t nvt_idx,
+                              bool cam_ignore, uint8_t priority,
+                              uint32_t logic_serv, XiveTCTXMatch *match)
+{

This is valid because both are static functions in different compilation
units, but it usually better to avoid naming collisions (for an easier
gdb experience for example).

Anyway, I guess this isn't a problem if we reparent TM contexts to the
XIVE router, since we wouldn't need patches 1 to 3 anymore if I get it
right.

> +{
> +    PnvMachineState *pnv = PNV_MACHINE(xfb);
> +    int total_count = 0;
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        XivePresenter *xptr = XIVE_PRESENTER(&chip9->xive);
> +        XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +        int count;
> +
> +        count = xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                               priority, logic_serv, match);
> +
> +        if (count < 0) {
> +            return count;
> +        }
> +
> +        total_count += count;
> +    }
> +
> +    return total_count;
> +}
> +
>  static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
>                                void *opaque, Error **errp)
>  {
> @@ -1441,9 +1470,11 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
>  static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>  
>      mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> +    xfc->match_nvt = pnv_xive_match_nvt;
>  
>      mc->alias = "powernv";
>  }
> @@ -1495,6 +1526,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>          .interfaces = (InterfaceInfo[]) {               \
>              { TYPE_XICS_FABRIC },                       \
>              { TYPE_INTERRUPT_STATS_PROVIDER },          \
> +            { TYPE_XIVE_FABRIC },                       \
>              { },                                        \
>          },                                              \
>      }