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

[Qemu-devel] [PATCH 00/10] 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 support for the XIVE
interrupt controller on POWER9 processor. 

The following changes fix some parts of the XIVE model and provide
support for escalations and resend. This mechanism is used by KVM to
kick a vCPU when it is not dispatched on a HW thread.

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.1

Thanks,

C.

Cédric Le Goater (10):
  ppc/xive: Force the Physical CAM line value to group mode
  ppc/xive: Make the PIPR register readonly
  ppc/pnv: Rework cache watch model of PnvXIVE
  ppc/xive: Fix TM_PULL_POOL_CTX special operation
  ppc/xive: Implement TM_PULL_OS_CTX special command
  ppc/xive: Provide escalation support
  ppc/xive: Improve 'info pic' support
  ppc/xive: Extend XiveTCTX with an router object pointer
  ppc/xive: Synthesize interrupt from the saved IPB in the NVT
  ppc/pnv: Dump the XIVE NVT table

 include/hw/ppc/xive.h      |  23 ++-
 include/hw/ppc/xive_regs.h |  13 ++
 hw/intc/pnv_xive.c         | 211 +++++++++++++++++++----
 hw/intc/spapr_xive.c       |   1 -
 hw/intc/xive.c             | 341 +++++++++++++++++++++++++++++--------
 hw/ppc/pnv.c               |   2 +-
 hw/ppc/spapr_irq.c         |   2 +-
 7 files changed, 479 insertions(+), 114 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 01/10] ppc/xive: Force the Physical CAM line value to group mode

[Cédric Le Goater]

When an interrupt needs to be delivered, the XIVE interrupt controller
presenter scans the CAM lines of the thread interrupt contexts of the
HW threads of the chip to find a matching vCPU. The interrupt context
is composed of 4 different sets of registers: Physical, HV, OS and
User.

The encoding of the Physical CAM line depends on the mode in which the
interrupt controller is operating: CAM mode or block group mode.
Block group mode being the default configuration today on POWER9 and
the only one available on the next POWER10 generation, enforce this
encoding in the Physical CAM line :

    chip << 19 | 0000000 0 0001 thread (7Bit)

It fits the overall encoding of the NVT ids and simplifies the matching
algorithm in the presenter.

Fixes: d514c48d41fb ("ppc/xive: hardwire the Physical CAM line of the thread context")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 6250c0414de8..3b1f9520ae9f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1229,27 +1229,16 @@ XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
 }
 
 /*
- * By default on P9, the HW CAM line (23bits) is hardwired to :
+ * Encode the HW CAM line in the block group mode format :
  *
- *   0x000||0b1||4Bit chip number||7Bit Thread number.
- *
- * When the block grouping is enabled, the CAM line is changed to :
- *
- *   4Bit chip number||0x001||7Bit Thread number.
+ *   chip << 19 | 0000000 0 0001 thread (7Bit)
  */
-static uint32_t hw_cam_line(uint8_t chip_id, uint8_t tid)
-{
-    return 1 << 11 | (chip_id & 0xf) << 7 | (tid & 0x7f);
-}
-
-static bool xive_presenter_tctx_match_hw(XiveTCTX *tctx,
-                                         uint8_t nvt_blk, uint32_t nvt_idx)
+static uint32_t xive_tctx_hw_cam_line(XiveTCTX *tctx)
 {
     CPUPPCState *env = &POWERPC_CPU(tctx->cs)->env;
     uint32_t pir = env->spr_cb[SPR_PIR].default_value;
 
-    return hw_cam_line((pir >> 8) & 0xf, pir & 0x7f) ==
-        hw_cam_line(nvt_blk, nvt_idx);
+    return xive_nvt_cam_line((pir >> 8) & 0xf, 1 << 7 | (pir & 0x7f));
 }
 
 /*
@@ -1285,7 +1274,7 @@ static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
 
         /* PHYS ring */
         if ((be32_to_cpu(qw3w2) & TM_QW3W2_VT) &&
-            xive_presenter_tctx_match_hw(tctx, nvt_blk, nvt_idx)) {
+            cam == xive_tctx_hw_cam_line(tctx)) {
             return TM_QW3_HV_PHYS;
         }
 
-- 
2.21.0

[Qemu-devel] [PATCH 02/10] ppc/xive: Make the PIPR register readonly

[Cédric Le Goater]

When the hypervisor (KVM) dispatches a vCPU on a HW thread, it restores
its thread interrupt context. The Pending Interrupt Priority Register
(PIPR) is computed from the Interrupt Pending Buffer (IPB) and stores
should not be allowed to change its value.

Fixes: 207d9fe98510 ("ppc/xive: introduce the XIVE interrupt thread context")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 3b1f9520ae9f..534f56f86bd5 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -182,31 +182,31 @@ static uint64_t xive_tm_vt_poll(XiveTCTX *tctx, hwaddr offset, unsigned size)
  */
 
 static const uint8_t xive_tm_hw_view[] = {
-    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-1 OS   */   3, 3, 3, 3,   3, 3, 0, 3,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-2 POOL */   0, 0, 3, 3,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-3 PHYS */   3, 3, 3, 3,   0, 3, 0, 3,   3, 0, 0, 3,   3, 3, 3, 0,
+    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
+    3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 OS   */
+    0, 0, 3, 3,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-2 POOL */
+    3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   3, 3, 3, 0, /* QW-3 PHYS */
 };
 
 static const uint8_t xive_tm_hv_view[] = {
-    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-1 OS   */   3, 3, 3, 3,   3, 3, 0, 3,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-2 POOL */   0, 0, 3, 3,   0, 0, 0, 0,   0, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-3 PHYS */   3, 3, 3, 3,   0, 3, 0, 3,   3, 0, 0, 3,   0, 0, 0, 0,
+    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
+    3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 OS   */
+    0, 0, 3, 3,   0, 0, 0, 0,   0, 3, 3, 3,   0, 0, 0, 0, /* QW-2 POOL */
+    3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   0, 0, 0, 0, /* QW-3 PHYS */
 };
 
 static const uint8_t xive_tm_os_view[] = {
-    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
-    /* QW-1 OS   */   2, 3, 2, 2,   2, 2, 0, 2,   0, 0, 0, 0,   0, 0, 0, 0,
-    /* QW-2 POOL */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
-    /* QW-3 PHYS */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
+    2, 3, 2, 2,   2, 2, 0, 2,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-1 OS   */
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-2 POOL */
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-3 PHYS */
 };
 
 static const uint8_t xive_tm_user_view[] = {
-    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
-    /* QW-1 OS   */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
-    /* QW-2 POOL */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
-    /* QW-3 PHYS */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
+    3, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-0 User */
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-1 OS   */
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-2 POOL */
+    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-3 PHYS */
 };
 
 /*
-- 
2.21.0

[Qemu-devel] [PATCH 03/10] ppc/pnv: Rework cache watch model of PnvXIVE

[Cédric Le Goater]

When the software modifies the XIVE internal structures, ESB, EAS,
END, NVT, it also must update the caches of the different XIVE
sub-engines. HW offers a set of common interface for such purpose.

The CWATCH_SPEC register defines the block/index of the target and a
set of flags to perform a full update and to watch for update
conflicts.

The cache watch CWATCH_DATAX registers are then loaded with the target
data with a first read on CWATCH_DATA0. Writing back is done in the
opposit order, CWATCH_DATA0 triggering the update.

The SCRUB_TRIG registers are used to flush the cache in RAM, and to
possibly invalidate it. Cache disablement is also an option but as we
do not model the cache, these registers are no-ops

Today, the modeling of these registers is incorrect but it did not
impact the set up of a baremetal system. However, running KVM requires
a rework.

Fixes: 2dfa91a2aa5a ("ppc/pnv: add a XIVE interrupt controller model for POWER9")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/pnv_xive.c | 142 +++++++++++++++++++++++++++++++++------------
 1 file changed, 106 insertions(+), 36 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 9ab77feee9d8..4dc92ef1e372 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -169,7 +169,7 @@ 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)) {
-        xive_error(xive, "VST: invalid %s entry %x !?", info->name, 0);
+        xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
         return 0;
     }
 
@@ -190,7 +190,7 @@ 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)) {
-            xive_error(xive, "VST: invalid %s entry %x !?", info->name, 0);
+            xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
             return 0;
         }
 
@@ -294,8 +294,12 @@ static int pnv_xive_write_end(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
                               word_number);
 }
 
-static int pnv_xive_end_update(PnvXive *xive, uint8_t blk, uint32_t idx)
+static int pnv_xive_end_update(PnvXive *xive)
 {
+    uint8_t  blk = GETFIELD(VC_EQC_CWATCH_BLOCKID,
+                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
+    uint32_t idx = GETFIELD(VC_EQC_CWATCH_OFFSET,
+                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
     int i;
     uint64_t eqc_watch[4];
 
@@ -307,6 +311,24 @@ static int pnv_xive_end_update(PnvXive *xive, uint8_t blk, uint32_t idx)
                               XIVE_VST_WORD_ALL);
 }
 
+static void pnv_xive_end_cache_load(PnvXive *xive)
+{
+    uint8_t  blk = GETFIELD(VC_EQC_CWATCH_BLOCKID,
+                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
+    uint32_t idx = GETFIELD(VC_EQC_CWATCH_OFFSET,
+                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
+    uint64_t eqc_watch[4] = { 0 };
+    int i;
+
+    if (pnv_xive_vst_read(xive, VST_TSEL_EQDT, blk, idx, eqc_watch)) {
+        xive_error(xive, "VST: no END entry %x/%x !?", blk, idx);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(eqc_watch); i++) {
+        xive->regs[(VC_EQC_CWATCH_DAT0 >> 3) + i] = be64_to_cpu(eqc_watch[i]);
+    }
+}
+
 static int pnv_xive_get_nvt(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
                             XiveNVT *nvt)
 {
@@ -320,8 +342,12 @@ static int pnv_xive_write_nvt(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
                               word_number);
 }
 
-static int pnv_xive_nvt_update(PnvXive *xive, uint8_t blk, uint32_t idx)
+static int pnv_xive_nvt_update(PnvXive *xive)
 {
+    uint8_t  blk = GETFIELD(PC_VPC_CWATCH_BLOCKID,
+                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
+    uint32_t idx = GETFIELD(PC_VPC_CWATCH_OFFSET,
+                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
     int i;
     uint64_t vpc_watch[8];
 
@@ -333,6 +359,24 @@ static int pnv_xive_nvt_update(PnvXive *xive, uint8_t blk, uint32_t idx)
                               XIVE_VST_WORD_ALL);
 }
 
+static void pnv_xive_nvt_cache_load(PnvXive *xive)
+{
+    uint8_t  blk = GETFIELD(PC_VPC_CWATCH_BLOCKID,
+                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
+    uint32_t idx = GETFIELD(PC_VPC_CWATCH_OFFSET,
+                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
+    uint64_t vpc_watch[8] = { 0 };
+    int i;
+
+    if (pnv_xive_vst_read(xive, VST_TSEL_VPDT, blk, idx, vpc_watch)) {
+        xive_error(xive, "VST: no NVT entry %x/%x !?", blk, idx);
+    }
+
+    for (i = 0; i < ARRAY_SIZE(vpc_watch); i++) {
+        xive->regs[(PC_VPC_CWATCH_DAT0 >> 3) + i] = be64_to_cpu(vpc_watch[i]);
+    }
+}
+
 static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
                             XiveEAS *eas)
 {
@@ -346,12 +390,6 @@ 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_eas_update(PnvXive *xive, uint8_t blk, uint32_t idx)
-{
-    /* All done. */
-    return 0;
-}
-
 static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -950,28 +988,43 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
      * XIVE PC & VC cache updates for EAS, NVT and END
      */
     case VC_IVC_SCRUB_MASK:
-        break;
     case VC_IVC_SCRUB_TRIG:
-        pnv_xive_eas_update(xive, GETFIELD(PC_SCRUB_BLOCK_ID, val),
-                            GETFIELD(VC_SCRUB_OFFSET, val));
         break;
 
-    case VC_EQC_SCRUB_MASK:
     case VC_EQC_CWATCH_SPEC:
-    case VC_EQC_CWATCH_DAT0 ... VC_EQC_CWATCH_DAT3:
+        val &= ~VC_EQC_CWATCH_CONFLICT; /* HW resets this bit */
+        break;
+    case VC_EQC_CWATCH_DAT1 ... VC_EQC_CWATCH_DAT3:
         break;
+    case VC_EQC_CWATCH_DAT0:
+        /* writing to DATA0 triggers the cache write */
+        xive->regs[reg] = val;
+        pnv_xive_end_update(xive);
+        break;
+    case VC_EQC_SCRUB_MASK:
     case VC_EQC_SCRUB_TRIG:
-        pnv_xive_end_update(xive, GETFIELD(VC_SCRUB_BLOCK_ID, val),
-                            GETFIELD(VC_SCRUB_OFFSET, val));
+        /*
+         * The scrubbing registers flush the cache in RAM and can also
+         * invalidate.
+         */
         break;
 
-    case PC_VPC_SCRUB_MASK:
     case PC_VPC_CWATCH_SPEC:
-    case PC_VPC_CWATCH_DAT0 ... PC_VPC_CWATCH_DAT7:
+        val &= ~PC_VPC_CWATCH_CONFLICT; /* HW resets this bit */
+        break;
+    case PC_VPC_CWATCH_DAT1 ... PC_VPC_CWATCH_DAT7:
         break;
+    case PC_VPC_CWATCH_DAT0:
+        /* writing to DATA0 triggers the cache write */
+        xive->regs[reg] = val;
+        pnv_xive_nvt_update(xive);
+        break;
+    case PC_VPC_SCRUB_MASK:
     case PC_VPC_SCRUB_TRIG:
-        pnv_xive_nvt_update(xive, GETFIELD(PC_SCRUB_BLOCK_ID, val),
-                           GETFIELD(PC_SCRUB_OFFSET, val));
+        /*
+         * The scrubbing registers flush the cache in RAM and can also
+         * invalidate.
+         */
         break;
 
 
@@ -1022,15 +1075,6 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
     case PC_GLOBAL_CONFIG:
 
     case PC_VPC_SCRUB_MASK:
-    case PC_VPC_CWATCH_SPEC:
-    case PC_VPC_CWATCH_DAT0:
-    case PC_VPC_CWATCH_DAT1:
-    case PC_VPC_CWATCH_DAT2:
-    case PC_VPC_CWATCH_DAT3:
-    case PC_VPC_CWATCH_DAT4:
-    case PC_VPC_CWATCH_DAT5:
-    case PC_VPC_CWATCH_DAT6:
-    case PC_VPC_CWATCH_DAT7:
 
     case VC_GLOBAL_CONFIG:
     case VC_AIB_TX_ORDER_TAG2:
@@ -1043,12 +1087,6 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
     case VC_IRQ_CONFIG_IPI_CASC:
 
     case VC_EQC_SCRUB_MASK:
-    case VC_EQC_CWATCH_DAT0:
-    case VC_EQC_CWATCH_DAT1:
-    case VC_EQC_CWATCH_DAT2:
-    case VC_EQC_CWATCH_DAT3:
-
-    case VC_EQC_CWATCH_SPEC:
     case VC_IVC_SCRUB_MASK:
     case VC_SBC_CONFIG:
     case VC_AT_MACRO_KILL_MASK:
@@ -1080,6 +1118,38 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
     /*
      * XIVE PC & VC cache updates for EAS, NVT and END
      */
+    case VC_EQC_CWATCH_SPEC:
+        xive->regs[reg] = ~(VC_EQC_CWATCH_FULL | VC_EQC_CWATCH_CONFLICT);
+        val = xive->regs[reg];
+        break;
+    case VC_EQC_CWATCH_DAT0:
+        /*
+         * Load DATA registers from cache with data requested by the
+         * SPEC register
+         */
+        pnv_xive_end_cache_load(xive);
+        val = xive->regs[reg];
+        break;
+    case VC_EQC_CWATCH_DAT1 ... VC_EQC_CWATCH_DAT3:
+        val = xive->regs[reg];
+        break;
+
+    case PC_VPC_CWATCH_SPEC:
+        xive->regs[reg] = ~(PC_VPC_CWATCH_FULL | PC_VPC_CWATCH_CONFLICT);
+        val = xive->regs[reg];
+        break;
+    case PC_VPC_CWATCH_DAT0:
+        /*
+         * Load DATA registers from cache with data requested by the
+         * SPEC register
+         */
+        pnv_xive_nvt_cache_load(xive);
+        val = xive->regs[reg];
+        break;
+    case PC_VPC_CWATCH_DAT1 ... PC_VPC_CWATCH_DAT7:
+        val = xive->regs[reg];
+        break;
+
     case PC_VPC_SCRUB_TRIG:
     case VC_IVC_SCRUB_TRIG:
     case VC_EQC_SCRUB_TRIG:
-- 
2.21.0

[Qemu-devel] [PATCH 04/10] ppc/xive: Fix TM_PULL_POOL_CTX special operation

[Cédric Le Goater]

When a CPU is reseted, the hypervisor (Linux or OPAL) invalidates the
POOL interrupt context of a CPU with this special command. It returns
the POOL CAM line value and resets the VP bit.

Fixes: 4836b45510aa ("ppc/xive: activate HV support")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/intc/xive.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 534f56f86bd5..cf77bdb7d34a 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -132,6 +132,11 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, uint8_t cppr)
     xive_tctx_notify(tctx, ring);
 }
 
+static inline uint32_t xive_tctx_word2(uint8_t *ring)
+{
+    return *((uint32_t *) &ring[TM_WORD2]);
+}
+
 /*
  * XIVE Thread Interrupt Management Area (TIMA)
  */
@@ -150,11 +155,12 @@ static uint64_t xive_tm_ack_hv_reg(XiveTCTX *tctx, hwaddr offset, unsigned size)
 static uint64_t xive_tm_pull_pool_ctx(XiveTCTX *tctx, hwaddr offset,
                                       unsigned size)
 {
-    uint64_t ret;
+    uint32_t qw2w2_prev = xive_tctx_word2(&tctx->regs[TM_QW2_HV_POOL]);
+    uint32_t qw2w2;
 
-    ret = tctx->regs[TM_QW2_HV_POOL + TM_WORD2] & TM_QW2W2_POOL_CAM;
-    tctx->regs[TM_QW2_HV_POOL + TM_WORD2] &= ~TM_QW2W2_POOL_CAM;
-    return ret;
+    qw2w2 = xive_set_field32(TM_QW2W2_VP, qw2w2_prev, 0);
+    memcpy(&tctx->regs[TM_QW2_HV_POOL + TM_WORD2], &qw2w2, 4);
+    return qw2w2;
 }
 
 static void xive_tm_vt_push(XiveTCTX *tctx, hwaddr offset,
@@ -484,11 +490,6 @@ const MemoryRegionOps xive_tm_ops = {
     },
 };
 
-static inline uint32_t xive_tctx_word2(uint8_t *ring)
-{
-    return *((uint32_t *) &ring[TM_WORD2]);
-}
-
 static char *xive_tctx_ring_print(uint8_t *ring)
 {
     uint32_t w2 = xive_tctx_word2(ring);
-- 
2.21.0

[Qemu-devel] [PATCH 05/10] ppc/xive: Implement TM_PULL_OS_CTX special command

[Cédric Le Goater]

When a vCPU is not dispatched anymore on a HW thread, the Hypervisor
(KVM) invalidates the OS interrupt context of a vCPU with this special
command. It returns the OS CAM line value and resets the VO bit.

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

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index cf77bdb7d34a..592c0b70f197 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -334,6 +334,17 @@ static void xive_tm_set_os_pending(XiveTCTX *tctx, hwaddr offset,
     xive_tctx_notify(tctx, TM_QW1_OS);
 }
 
+static uint64_t xive_tm_pull_os_ctx(XiveTCTX *tctx, hwaddr offset,
+                                    unsigned size)
+{
+    uint32_t qw1w2_prev = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
+    uint32_t qw1w2;
+
+    qw1w2 = xive_set_field32(TM_QW1W2_VO, qw1w2_prev, 0);
+    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+    return qw1w2;
+}
+
 /*
  * Define a mapping of "special" operations depending on the TIMA page
  * offset and the size of the operation.
@@ -360,6 +371,8 @@ static const XiveTmOp xive_tm_operations[] = {
     /* MMIOs above 2K : special operations with side effects */
     { XIVE_TM_OS_PAGE, TM_SPC_ACK_OS_REG,     2, NULL, xive_tm_ack_os_reg },
     { XIVE_TM_OS_PAGE, TM_SPC_SET_OS_PENDING, 1, xive_tm_set_os_pending, NULL },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    4, NULL, xive_tm_pull_os_ctx },
+    { XIVE_TM_HV_PAGE, TM_SPC_PULL_OS_CTX,    8, NULL, xive_tm_pull_os_ctx },
     { XIVE_TM_HV_PAGE, TM_SPC_ACK_HV_REG,     2, NULL, xive_tm_ack_hv_reg },
     { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  4, NULL, xive_tm_pull_pool_ctx },
     { XIVE_TM_HV_PAGE, TM_SPC_PULL_POOL_CTX,  8, NULL, xive_tm_pull_pool_ctx },
@@ -403,7 +416,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
     if (offset & 0x800) {
         xto = xive_tm_find_op(offset, size, true);
         if (!xto) {
-            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA"
+            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid write access at TIMA "
                           "@%"HWADDR_PRIx"\n", offset);
         } else {
             xto->write_handler(tctx, offset, value, size);
-- 
2.21.0

[Qemu-devel] [PATCH 06/10] ppc/xive: Provide escalation support

[Cédric Le Goater]

If the XIVE presenter can not find the NVT dispatched on any of the HW
threads, it can not deliver the interrupt. XIVE offers a mechanism to
handle such scenarios and inform the hypervisor that an action should
be taken.

The first action is to keep track of the pending priority of the
missed event. It is recorded in the IPB field of the NVT for a later
resend if backlog is activated ('b' bit) on the END.

An END can also escalate if configured: 'e' bit and setting of the EAS
in word 4 & 5 to let the HW look for the escalation END on which to
trigger a new event. Escalation has its own options to program
different behaviors :

 - unconditional escalation ('u' bit) with which the ESe PQ bits are
   not used.
 - silent/gather escalation ('s' bit), the sequence skips the
   notification process and jumps directly to the escalation.

KVM uses a combination of these. The first level END is configured to
enqueue, unconditionally notify, backlog and escalate and points to an
escalation END which is configured to escalate silently.

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

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 1a8c5b5e64f0..69af326ebf2c 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -207,6 +207,10 @@ typedef struct XiveEND {
 #define xive_end_is_notify(end)   (be32_to_cpu((end)->w0) & END_W0_UCOND_NOTIFY)
 #define xive_end_is_backlog(end)  (be32_to_cpu((end)->w0) & END_W0_BACKLOG)
 #define xive_end_is_escalate(end) (be32_to_cpu((end)->w0) & END_W0_ESCALATE_CTL)
+#define xive_end_is_uncond_escalation(end)              \
+    (be32_to_cpu((end)->w0) & END_W0_UNCOND_ESCALATE)
+#define xive_end_is_silent_escalation(end)              \
+    (be32_to_cpu((end)->w0) & END_W0_SILENT_ESCALATE)
 
 static inline uint64_t xive_end_qaddr(XiveEND *end)
 {
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 592c0b70f197..3970999f4837 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1389,7 +1389,7 @@ static bool xive_presenter_match(XiveRouter *xrtr, uint8_t format,
  *
  * The parameters represent what is sent on the PowerBus
  */
-static void xive_presenter_notify(XiveRouter *xrtr, uint8_t format,
+static bool xive_presenter_notify(XiveRouter *xrtr, uint8_t format,
                                   uint8_t nvt_blk, uint32_t nvt_idx,
                                   bool cam_ignore, uint8_t priority,
                                   uint32_t logic_serv)
@@ -1402,13 +1402,13 @@ static void xive_presenter_notify(XiveRouter *xrtr, uint8_t format,
     if (xive_router_get_nvt(xrtr, nvt_blk, nvt_idx, &nvt)) {
         qemu_log_mask(LOG_GUEST_ERROR, "XIVE: no NVT %x/%x\n",
                       nvt_blk, nvt_idx);
-        return;
+        return false;
     }
 
     if (!xive_nvt_is_valid(&nvt)) {
         qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is invalid\n",
                       nvt_blk, nvt_idx);
-        return;
+        return false;
     }
 
     found = xive_presenter_match(xrtr, format, nvt_blk, nvt_idx, cam_ignore,
@@ -1416,19 +1416,55 @@ static void xive_presenter_notify(XiveRouter *xrtr, uint8_t format,
     if (found) {
         ipb_update(&match.tctx->regs[match.ring], priority);
         xive_tctx_notify(match.tctx, match.ring);
+    }
+
+    return found;
+}
+
+static void xive_router_end_backlog(XiveRouter *xrtr,
+                                    uint8_t nvt_blk, uint32_t nvt_idx,
+                                    uint8_t priority)
+{
+    XiveNVT nvt;
+
+    /* NVT cache lookup */
+    if (xive_router_get_nvt(xrtr, nvt_blk, nvt_idx, &nvt)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: no NVT %x/%x\n",
+                      nvt_blk, nvt_idx);
+        return;
+    }
+
+    if (!xive_nvt_is_valid(&nvt)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is invalid\n",
+                      nvt_blk, nvt_idx);
         return;
     }
 
     /* Record the IPB in the associated NVT structure */
     ipb_update((uint8_t *) &nvt.w4, priority);
     xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
+}
 
-    /*
-     * If no matching NVT is dispatched on a HW thread :
-     * - update the NVT structure if backlog is activated
-     * - escalate (ESe PQ bits and EAS in w4-5) if escalation is
-     *   activated
-     */
+
+/*
+ * Notification using the END ESe/ESn bit (Event State Buffer for
+ * escalation and notification). Profide futher coalescing in the
+ * Router.
+ */
+static bool xive_router_end_es_notify(XiveRouter *xrtr, uint8_t end_blk,
+                                      uint32_t end_idx, XiveEND *end,
+                                      uint32_t end_esmask)
+{
+    uint8_t pq = xive_get_field32(end_esmask, end->w1);
+    bool notify = xive_esb_trigger(&pq);
+
+    if (pq != xive_get_field32(end_esmask, end->w1)) {
+        end->w1 = xive_set_field32(end_esmask, end->w1, pq);
+        xive_router_write_end(xrtr, end_blk, end_idx, end, 1);
+    }
+
+    /* ESe/n[Q]=1 : end of notification */
+    return notify;
 }
 
 /*
@@ -1442,6 +1478,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
     XiveEND end;
     uint8_t priority;
     uint8_t format;
+    bool found;
 
     /* END cache lookup */
     if (xive_router_get_end(xrtr, end_blk, end_idx, &end)) {
@@ -1462,6 +1499,13 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
         xive_router_write_end(xrtr, end_blk, end_idx, &end, 1);
     }
 
+    /*
+     * When the END is silent, we skip the notification part.
+     */
+    if (xive_end_is_silent_escalation(&end)) {
+        goto do_escalation;
+    }
+
     /*
      * The W7 format depends on the F bit in W6. It defines the type
      * of the notification :
@@ -1483,16 +1527,9 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
      * even futher coalescing in the Router
      */
     if (!xive_end_is_notify(&end)) {
-        uint8_t pq = xive_get_field32(END_W1_ESn, end.w1);
-        bool notify = xive_esb_trigger(&pq);
-
-        if (pq != xive_get_field32(END_W1_ESn, end.w1)) {
-            end.w1 = xive_set_field32(END_W1_ESn, end.w1, pq);
-            xive_router_write_end(xrtr, end_blk, end_idx, &end, 1);
-        }
-
         /* ESn[Q]=1 : end of notification */
-        if (!notify) {
+        if (!xive_router_end_es_notify(xrtr, end_blk, end_idx,
+                                       &end, END_W1_ESn)) {
             return;
         }
     }
@@ -1500,7 +1537,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
     /*
      * Follows IVPE notification
      */
-    xive_presenter_notify(xrtr, format,
+    found = xive_presenter_notify(xrtr, format,
                           xive_get_field32(END_W6_NVT_BLOCK, end.w6),
                           xive_get_field32(END_W6_NVT_INDEX, end.w6),
                           xive_get_field32(END_W7_F0_IGNORE, end.w7),
@@ -1508,6 +1545,61 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
                           xive_get_field32(END_W7_F1_LOG_SERVER_ID, end.w7));
 
     /* TODO: Auto EOI. */
+
+    if (found) {
+        return;
+    }
+
+    /*
+     * If no matching NVT is dispatched on a HW thread :
+     * - specific VP: update the NVT structure if backlog is activated
+     * - logical server : forward request to IVPE (not supported)
+     */
+    if (xive_end_is_backlog(&end)) {
+        if (format == 1) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "XIVE: END %x/%x invalid config: F1 & backlog\n",
+                          end_blk, end_idx);
+            return;
+        }
+        xive_router_end_backlog(xrtr,
+                                xive_get_field32(END_W6_NVT_BLOCK, end.w6),
+                                xive_get_field32(END_W6_NVT_INDEX, end.w6),
+                                priority);
+
+        /*
+         * On HW, follows a "Broadcast Backlog" to IVPEs
+         */
+    }
+
+do_escalation:
+    /*
+     * If activated, escalate notification using the ESe PQ bits and
+     * the EAS in w4-5
+     */
+    if (!xive_end_is_escalate(&end)) {
+        return;
+    }
+
+    /*
+     * Check the END ESe (Event State Buffer for escalation) for even
+     * futher coalescing in the Router
+     */
+    if (!xive_end_is_uncond_escalation(&end)) {
+        /* ESe[Q]=1 : end of notification */
+        if (!xive_router_end_es_notify(xrtr, end_blk, end_idx,
+                                       &end, END_W1_ESe)) {
+            return;
+        }
+    }
+
+    /*
+     * The END trigger becomes an Escalation trigger
+     */
+    xive_router_end_notify(xrtr,
+                           xive_get_field32(END_W4_ESC_END_BLOCK, end.w4),
+                           xive_get_field32(END_W4_ESC_END_INDEX, end.w4),
+                           xive_get_field32(END_W5_ESC_END_DATA,  end.w5));
 }
 
 void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
-- 
2.21.0

[Qemu-devel] [PATCH 07/10] ppc/xive: Improve 'info pic' support

[Cédric Le Goater]

Provide a better output of the XIVE END structures including the
escalation information and extend the PowerNV machine 'info pic'
command with a dump of the END EAS table used for escalations.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h      |  5 ----
 include/hw/ppc/xive_regs.h |  6 +++++
 hw/intc/pnv_xive.c         |  9 +++++++
 hw/intc/spapr_xive.c       |  1 -
 hw/intc/xive.c             | 48 +++++++++++++++++++++++++++++++++-----
 5 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index a6ee7e831d8b..d922524982d3 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -356,8 +356,6 @@ typedef struct XiveRouterClass {
     XiveTCTX *(*get_tctx)(XiveRouter *xrtr, CPUState *cs);
 } XiveRouterClass;
 
-void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
-
 int xive_router_get_eas(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
                         XiveEAS *eas);
 int xive_router_get_end(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
@@ -399,9 +397,6 @@ typedef struct XiveENDSource {
  */
 #define XIVE_PRIORITY_MAX  7
 
-void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon);
-void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor *mon);
-
 /*
  * XIVE Thread Interrupt Management Aera (TIMA)
  *
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 69af326ebf2c..3fdf1a83b9b6 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -128,6 +128,8 @@ typedef struct XiveEAS {
 #define xive_eas_is_valid(eas)   (be64_to_cpu((eas)->w) & EAS_VALID)
 #define xive_eas_is_masked(eas)  (be64_to_cpu((eas)->w) & EAS_MASKED)
 
+void xive_eas_pic_print_info(XiveEAS *eas, uint32_t lisn, Monitor *mon);
+
 static inline uint64_t xive_get_field64(uint64_t mask, uint64_t word)
 {
     return (be64_to_cpu(word) & mask) >> ctz64(mask);
@@ -218,6 +220,10 @@ static inline uint64_t xive_end_qaddr(XiveEND *end)
         be32_to_cpu(end->w3);
 }
 
+void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon);
+void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor *mon);
+void xive_end_eas_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon);
+
 /* Notification Virtual Target (NVT) */
 typedef struct XiveNVT {
         uint32_t        w0;
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 4dc92ef1e372..ff1226485983 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1593,6 +1593,15 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
         }
         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);
+    }
 }
 
 static void pnv_xive_reset(void *dev)
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 58c2e5d890bd..48cd50192f61 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -143,7 +143,6 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
                    priority, qindex, qentries, qaddr_base, qgen);
 
     xive_end_queue_pic_print_info(end, 6, mon);
-    monitor_printf(mon, "]");
 }
 
 void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon)
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 3970999f4837..f7ba1c3b622f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1136,6 +1136,7 @@ void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor *mon)
                        be32_to_cpu(qdata));
         qindex = (qindex + 1) & (qentries - 1);
     }
+    monitor_printf(mon, "]");
 }
 
 void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon)
@@ -1146,24 +1147,36 @@ void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon)
     uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
     uint32_t qentries = 1 << (qsize + 10);
 
-    uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
+    uint32_t nvt_blk = xive_get_field32(END_W6_NVT_BLOCK, end->w6);
+    uint32_t nvt_idx = xive_get_field32(END_W6_NVT_INDEX, end->w6);
     uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
+    uint8_t pq;
 
     if (!xive_end_is_valid(end)) {
         return;
     }
 
-    monitor_printf(mon, "  %08x %c%c%c%c%c prio:%d nvt:%04x eq:@%08"PRIx64
-                   "% 6d/%5d ^%d", end_idx,
+    pq = xive_get_field32(END_W1_ESn, end->w1);
+
+    monitor_printf(mon, "  %08x %c%c %c%c%c%c%c%c%c prio:%d nvt:%02x/%04x",
+                   end_idx,
+                   pq & XIVE_ESB_VAL_P ? 'P' : '-',
+                   pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
                    xive_end_is_valid(end)    ? 'v' : '-',
                    xive_end_is_enqueue(end)  ? 'q' : '-',
                    xive_end_is_notify(end)   ? 'n' : '-',
                    xive_end_is_backlog(end)  ? 'b' : '-',
                    xive_end_is_escalate(end) ? 'e' : '-',
-                   priority, nvt, qaddr_base, qindex, qentries, qgen);
+                   xive_end_is_uncond_escalation(end)   ? 'u' : '-',
+                   xive_end_is_silent_escalation(end)   ? 's' : '-',
+                   priority, nvt_blk, nvt_idx);
 
-    xive_end_queue_pic_print_info(end, 6, mon);
-    monitor_printf(mon, "]\n");
+    if (qaddr_base) {
+        monitor_printf(mon, " eq:@%08"PRIx64"% 6d/%5d ^%d",
+                       qaddr_base, qindex, qentries, qgen);
+        xive_end_queue_pic_print_info(end, 6, mon);
+    }
+    monitor_printf(mon, "\n");
 }
 
 static void xive_end_enqueue(XiveEND *end, uint32_t data)
@@ -1191,6 +1204,29 @@ static void xive_end_enqueue(XiveEND *end, uint32_t data)
     end->w1 = xive_set_field32(END_W1_PAGE_OFF, end->w1, qindex);
 }
 
+void xive_end_eas_pic_print_info(XiveEND *end, uint32_t end_idx,
+                                   Monitor *mon)
+{
+    XiveEAS *eas = (XiveEAS *) &end->w4;
+    uint8_t pq;
+
+    if (!xive_end_is_escalate(end)) {
+        return;
+    }
+
+    pq = xive_get_field32(END_W1_ESe, end->w1);
+
+    monitor_printf(mon, "  %08x %c%c %c%c end:%02x/%04x data:%08x\n",
+                   end_idx,
+                   pq & XIVE_ESB_VAL_P ? 'P' : '-',
+                   pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
+                   xive_eas_is_valid(eas) ? 'V' : ' ',
+                   xive_eas_is_masked(eas) ? 'M' : ' ',
+                   (uint8_t)  xive_get_field64(EAS_END_BLOCK, eas->w),
+                   (uint32_t) xive_get_field64(EAS_END_INDEX, eas->w),
+                   (uint32_t) xive_get_field64(EAS_END_DATA, eas->w));
+}
+
 /*
  * XIVE Router (aka. Virtualization Controller or IVRE)
  */
-- 
2.21.0

[Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[Cédric Le Goater]

This is to perform lookups in the NVT table when a vCPU is dispatched
and possibly resend interrupts.

Future XIVE chip will use a different class for the model of the
interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.

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

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index d922524982d3..b764e1e4e6d4 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -321,6 +321,8 @@ typedef struct XiveTCTX {
     qemu_irq    os_output;
 
     uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
+
+    Object      *xrtr;
 } XiveTCTX;
 
 /*
@@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
 uint64_t xive_tctx_tm_read(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);
+Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
 
 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 f7ba1c3b622f..56700681884f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
     Object *obj;
     Error *local_err = NULL;
 
+    obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
+    if (!obj) {
+        error_propagate(errp, local_err);
+        error_prepend(errp, "required link 'xrtr' not found: ");
+        return;
+    }
+    tctx->xrtr = obj;
+
     obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
     if (!obj) {
         error_propagate(errp, local_err);
@@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
     .class_init    = xive_tctx_class_init,
 };
 
-Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
+Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
 {
     Error *local_err = NULL;
     Object *obj;
@@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
     object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
     object_unref(obj);
     object_property_add_const_link(obj, "cpu", cpu, &error_abort);
+    object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
     object_property_set_bool(obj, true, "realized", &local_err);
     if (local_err) {
         goto error;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index b87e01e5b925..11916dc273c2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
      * controller object is initialized afterwards. Hopefully, it's
      * only used at runtime.
      */
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
+    obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index b2b01e850de8..5b3c3c50967b 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -353,7 +353,7 @@ static void spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
     Object *obj;
     SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
 
-    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
+    obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
-- 
2.21.0

[Qemu-devel] [PATCH 09/10] 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. The HW grabs the associated NVT to pull the pending bits, and
merge 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.h      | 14 ++++++++
 include/hw/ppc/xive_regs.h |  1 +
 hw/intc/xive.c             | 67 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index b764e1e4e6d4..e4dcaa7a10e9 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -420,11 +420,25 @@ uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size);
 void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon);
 Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
 
+/*
+ * The VP number space in a block is defined by the END_W6_NVT_INDEX
+ * field of the XIVE END
+ */
 static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
 {
     return (nvt_blk << 19) | nvt_idx;
 }
 
+static inline uint32_t xive_nvt_idx(uint32_t cam_line)
+{
+    return cam_line & 0x7ffff;
+}
+
+static inline uint32_t xive_nvt_blk(uint32_t cam_line)
+{
+    return (cam_line >> 19) & 0xf;
+}
+
 /*
  * KVM XIVE device helpers
  */
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 3fdf1a83b9b6..7ba0fb055174 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -232,6 +232,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 56700681884f..2225183e0e16 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -345,6 +345,62 @@ static uint64_t xive_tm_pull_os_ctx(XiveTCTX *tctx, hwaddr offset,
     return qw1w2;
 }
 
+static void xive_tctx_need_resend(XiveTCTX *tctx, uint8_t nvt_blk,
+                                 uint32_t nvt_idx)
+{
+    XiveNVT nvt;
+    uint8_t ipb;
+    XiveRouter *xrtr = XIVE_ROUTER(tctx->xrtr);
+
+    /*
+     * 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) {
+        uint8_t *regs = &tctx->regs[TM_QW1_OS];
+
+        /* 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 */
+        regs[TM_IPB] |= ipb;
+        regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
+
+        /* Possibly resend */
+        xive_tctx_notify(tctx, TM_QW1_OS);
+    }
+}
+
+/*
+ * Updating the OS CAM line can trigger a resend of interrupt
+ */
+static void xive_tm_push_os_cam(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(tctx, nvt_blk, nvt_idx);
+    }
+}
+
 /*
  * Define a mapping of "special" operations depending on the TIMA page
  * offset and the size of the operation.
@@ -364,6 +420,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_cam, 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 },
@@ -1471,6 +1528,7 @@ static void xive_router_end_backlog(XiveRouter *xrtr,
                                     uint8_t priority)
 {
     XiveNVT nvt;
+    uint8_t ipb;
 
     /* NVT cache lookup */
     if (xive_router_get_nvt(xrtr, nvt_blk, nvt_idx, &nvt)) {
@@ -1485,8 +1543,13 @@ static void xive_router_end_backlog(XiveRouter *xrtr,
         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 10/10] 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.

We try to loop on the full table skipping empty indirect pages which
are not necessarily allocated.

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

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 7ba0fb055174..50802bbdaab0 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -229,6 +229,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 ff1226485983..8778c11623dc 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -121,6 +121,20 @@ 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_indirect_page_shift(uint64_t vsd)
+{
+    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 page_shift;
+}
+
 static uint64_t pnv_xive_vst_size(uint64_t vsd)
 {
     uint64_t vst_tsize = 1ull << (GETFIELD(VSD_TSIZE, vsd) + 12);
@@ -466,6 +480,24 @@ static uint32_t pnv_xive_nr_ends(PnvXive *xive)
         / vst_infos[VST_TSEL_EQDT].size;
 }
 
+static uint32_t pnv_xive_nr_indirect(PnvXive *xive, uint32_t type)
+{
+    const XiveVstInfo *info = &vst_infos[type];
+    uint8_t blk = xive->chip->chip_id;
+    uint32_t page_shift =
+        pnv_xive_vst_indirect_page_shift(xive->vsds[type][blk]);
+
+    return (1ull << page_shift) / info->size;
+}
+
+static uint32_t pnv_xive_nr_nvts(PnvXive *xive)
+{
+    uint8_t blk = xive->chip->chip_id;
+
+    return pnv_xive_vst_size(xive->vsds[VST_TSEL_VPDT][blk])
+        / vst_infos[VST_TSEL_VPDT].size;
+}
+
 /*
  * EDT Table
  *
@@ -1560,6 +1592,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);
@@ -1567,8 +1614,11 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     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_nvts = pnv_xive_nr_nvts(xive);
+    uint32_t nr_indirect_nvts = pnv_xive_nr_indirect(xive, VST_TSEL_VPDT);
     XiveEAS eas;
     XiveEND end;
+    XiveNVT nvt;
     int i;
 
     monitor_printf(mon, "XIVE[%x] Source %08x .. %08x\n", blk, srcno0,
@@ -1602,6 +1652,16 @@ 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 %08x .. %08x\n", blk, 0, nr_nvts - 1);
+    for (i = 0; i < nr_nvts; i++) {
+        if (xive_router_get_nvt(xrtr, blk, i, &nvt)) {
+            /* skip an indirect page */
+            i += nr_indirect_nvts - 1;
+            continue;
+        }
+        xive_nvt_pic_print_info(&nvt, i, mon);
+    }
 }
 
 static void pnv_xive_reset(void *dev)
-- 
2.21.0

Re: [Qemu-devel] [PATCH 01/10] ppc/xive: Force the Physical CAM line value to group mode

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:52PM +0200, Cédric Le Goater wrote:
> When an interrupt needs to be delivered, the XIVE interrupt controller
> presenter scans the CAM lines of the thread interrupt contexts of the
> HW threads of the chip to find a matching vCPU. The interrupt context
> is composed of 4 different sets of registers: Physical, HV, OS and
> User.
> 
> The encoding of the Physical CAM line depends on the mode in which the
> interrupt controller is operating: CAM mode or block group mode.
> Block group mode being the default configuration today on POWER9 and
> the only one available on the next POWER10 generation, enforce this
> encoding in the Physical CAM line :
> 
>     chip << 19 | 0000000 0 0001 thread (7Bit)
> 
> It fits the overall encoding of the NVT ids and simplifies the matching
> algorithm in the presenter.
> 
> Fixes: d514c48d41fb ("ppc/xive: hardwire the Physical CAM line of the thread context")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Since this appears to be a fix, applied to ppc-for-4.1.

> ---
>  hw/intc/xive.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 6250c0414de8..3b1f9520ae9f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1229,27 +1229,16 @@ XiveTCTX *xive_router_get_tctx(XiveRouter *xrtr, CPUState *cs)
>  }
>  
>  /*
> - * By default on P9, the HW CAM line (23bits) is hardwired to :
> + * Encode the HW CAM line in the block group mode format :
>   *
> - *   0x000||0b1||4Bit chip number||7Bit Thread number.
> - *
> - * When the block grouping is enabled, the CAM line is changed to :
> - *
> - *   4Bit chip number||0x001||7Bit Thread number.
> + *   chip << 19 | 0000000 0 0001 thread (7Bit)
>   */
> -static uint32_t hw_cam_line(uint8_t chip_id, uint8_t tid)
> -{
> -    return 1 << 11 | (chip_id & 0xf) << 7 | (tid & 0x7f);
> -}
> -
> -static bool xive_presenter_tctx_match_hw(XiveTCTX *tctx,
> -                                         uint8_t nvt_blk, uint32_t nvt_idx)
> +static uint32_t xive_tctx_hw_cam_line(XiveTCTX *tctx)
>  {
>      CPUPPCState *env = &POWERPC_CPU(tctx->cs)->env;
>      uint32_t pir = env->spr_cb[SPR_PIR].default_value;
>  
> -    return hw_cam_line((pir >> 8) & 0xf, pir & 0x7f) ==
> -        hw_cam_line(nvt_blk, nvt_idx);
> +    return xive_nvt_cam_line((pir >> 8) & 0xf, 1 << 7 | (pir & 0x7f));
>  }
>  
>  /*
> @@ -1285,7 +1274,7 @@ static int xive_presenter_tctx_match(XiveTCTX *tctx, uint8_t format,
>  
>          /* PHYS ring */
>          if ((be32_to_cpu(qw3w2) & TM_QW3W2_VT) &&
> -            xive_presenter_tctx_match_hw(tctx, nvt_blk, nvt_idx)) {
> +            cam == xive_tctx_hw_cam_line(tctx)) {
>              return TM_QW3_HV_PHYS;
>          }
>  

-- 
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: [Qemu-devel] [PATCH 02/10] ppc/xive: Make the PIPR register readonly

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:53PM +0200, Cédric Le Goater wrote:
> When the hypervisor (KVM) dispatches a vCPU on a HW thread, it restores
> its thread interrupt context. The Pending Interrupt Priority Register
> (PIPR) is computed from the Interrupt Pending Buffer (IPB) and stores
> should not be allowed to change its value.
> 
> Fixes: 207d9fe98510 ("ppc/xive: introduce the XIVE interrupt thread context")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-4.1.

> ---
>  hw/intc/xive.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 3b1f9520ae9f..534f56f86bd5 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -182,31 +182,31 @@ static uint64_t xive_tm_vt_poll(XiveTCTX *tctx, hwaddr offset, unsigned size)
>   */
>  
>  static const uint8_t xive_tm_hw_view[] = {
> -    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-1 OS   */   3, 3, 3, 3,   3, 3, 0, 3,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-2 POOL */   0, 0, 3, 3,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-3 PHYS */   3, 3, 3, 3,   0, 3, 0, 3,   3, 0, 0, 3,   3, 3, 3, 0,
> +    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
> +    3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 OS   */
> +    0, 0, 3, 3,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-2 POOL */
> +    3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   3, 3, 3, 0, /* QW-3 PHYS */
>  };
>  
>  static const uint8_t xive_tm_hv_view[] = {
> -    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-1 OS   */   3, 3, 3, 3,   3, 3, 0, 3,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-2 POOL */   0, 0, 3, 3,   0, 0, 0, 0,   0, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-3 PHYS */   3, 3, 3, 3,   0, 3, 0, 3,   3, 0, 0, 3,   0, 0, 0, 0,
> +    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
> +    3, 3, 3, 3,   3, 3, 0, 2,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-1 OS   */
> +    0, 0, 3, 3,   0, 0, 0, 0,   0, 3, 3, 3,   0, 0, 0, 0, /* QW-2 POOL */
> +    3, 3, 3, 3,   0, 3, 0, 2,   3, 0, 0, 3,   0, 0, 0, 0, /* QW-3 PHYS */
>  };
>  
>  static const uint8_t xive_tm_os_view[] = {
> -    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0,
> -    /* QW-1 OS   */   2, 3, 2, 2,   2, 2, 0, 2,   0, 0, 0, 0,   0, 0, 0, 0,
> -    /* QW-2 POOL */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> -    /* QW-3 PHYS */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> +    3, 0, 0, 0,   0, 0, 0, 0,   3, 3, 3, 3,   0, 0, 0, 0, /* QW-0 User */
> +    2, 3, 2, 2,   2, 2, 0, 2,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-1 OS   */
> +    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-2 POOL */
> +    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-3 PHYS */
>  };
>  
>  static const uint8_t xive_tm_user_view[] = {
> -    /* QW-0 User */   3, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> -    /* QW-1 OS   */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> -    /* QW-2 POOL */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> -    /* QW-3 PHYS */   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,
> +    3, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-0 User */
> +    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-1 OS   */
> +    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-2 POOL */
> +    0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0,   0, 0, 0, 0, /* QW-3 PHYS */
>  };
>  
>  /*

-- 
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: [Qemu-devel] [PATCH 03/10] ppc/pnv: Rework cache watch model of PnvXIVE

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:54PM +0200, Cédric Le Goater wrote:
> When the software modifies the XIVE internal structures, ESB, EAS,
> END, NVT, it also must update the caches of the different XIVE
> sub-engines. HW offers a set of common interface for such purpose.
> 
> The CWATCH_SPEC register defines the block/index of the target and a
> set of flags to perform a full update and to watch for update
> conflicts.
> 
> The cache watch CWATCH_DATAX registers are then loaded with the target
> data with a first read on CWATCH_DATA0. Writing back is done in the
> opposit order, CWATCH_DATA0 triggering the update.
> 
> The SCRUB_TRIG registers are used to flush the cache in RAM, and to
> possibly invalidate it. Cache disablement is also an option but as we
> do not model the cache, these registers are no-ops
> 
> Today, the modeling of these registers is incorrect but it did not
> impact the set up of a baremetal system. However, running KVM requires
> a rework.
> 
> Fixes: 2dfa91a2aa5a ("ppc/pnv: add a XIVE interrupt controller model for POWER9")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Pushing it for what counts as a bugfix this close to freeze, but I'll
take it.  Applied to ppc-for-4.1.

> ---
>  hw/intc/pnv_xive.c | 142 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 106 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index 9ab77feee9d8..4dc92ef1e372 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -169,7 +169,7 @@ 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)) {
> -        xive_error(xive, "VST: invalid %s entry %x !?", info->name, 0);
> +        xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
>          return 0;
>      }
>  
> @@ -190,7 +190,7 @@ 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)) {
> -            xive_error(xive, "VST: invalid %s entry %x !?", info->name, 0);
> +            xive_error(xive, "VST: invalid %s entry %x !?", info->name, idx);
>              return 0;
>          }
>  
> @@ -294,8 +294,12 @@ static int pnv_xive_write_end(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>                                word_number);
>  }
>  
> -static int pnv_xive_end_update(PnvXive *xive, uint8_t blk, uint32_t idx)
> +static int pnv_xive_end_update(PnvXive *xive)
>  {
> +    uint8_t  blk = GETFIELD(VC_EQC_CWATCH_BLOCKID,
> +                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
> +    uint32_t idx = GETFIELD(VC_EQC_CWATCH_OFFSET,
> +                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
>      int i;
>      uint64_t eqc_watch[4];
>  
> @@ -307,6 +311,24 @@ static int pnv_xive_end_update(PnvXive *xive, uint8_t blk, uint32_t idx)
>                                XIVE_VST_WORD_ALL);
>  }
>  
> +static void pnv_xive_end_cache_load(PnvXive *xive)
> +{
> +    uint8_t  blk = GETFIELD(VC_EQC_CWATCH_BLOCKID,
> +                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
> +    uint32_t idx = GETFIELD(VC_EQC_CWATCH_OFFSET,
> +                           xive->regs[(VC_EQC_CWATCH_SPEC >> 3)]);
> +    uint64_t eqc_watch[4] = { 0 };
> +    int i;
> +
> +    if (pnv_xive_vst_read(xive, VST_TSEL_EQDT, blk, idx, eqc_watch)) {
> +        xive_error(xive, "VST: no END entry %x/%x !?", blk, idx);
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(eqc_watch); i++) {
> +        xive->regs[(VC_EQC_CWATCH_DAT0 >> 3) + i] = be64_to_cpu(eqc_watch[i]);
> +    }
> +}
> +
>  static int pnv_xive_get_nvt(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>                              XiveNVT *nvt)
>  {
> @@ -320,8 +342,12 @@ static int pnv_xive_write_nvt(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>                                word_number);
>  }
>  
> -static int pnv_xive_nvt_update(PnvXive *xive, uint8_t blk, uint32_t idx)
> +static int pnv_xive_nvt_update(PnvXive *xive)
>  {
> +    uint8_t  blk = GETFIELD(PC_VPC_CWATCH_BLOCKID,
> +                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
> +    uint32_t idx = GETFIELD(PC_VPC_CWATCH_OFFSET,
> +                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
>      int i;
>      uint64_t vpc_watch[8];
>  
> @@ -333,6 +359,24 @@ static int pnv_xive_nvt_update(PnvXive *xive, uint8_t blk, uint32_t idx)
>                                XIVE_VST_WORD_ALL);
>  }
>  
> +static void pnv_xive_nvt_cache_load(PnvXive *xive)
> +{
> +    uint8_t  blk = GETFIELD(PC_VPC_CWATCH_BLOCKID,
> +                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
> +    uint32_t idx = GETFIELD(PC_VPC_CWATCH_OFFSET,
> +                           xive->regs[(PC_VPC_CWATCH_SPEC >> 3)]);
> +    uint64_t vpc_watch[8] = { 0 };
> +    int i;
> +
> +    if (pnv_xive_vst_read(xive, VST_TSEL_VPDT, blk, idx, vpc_watch)) {
> +        xive_error(xive, "VST: no NVT entry %x/%x !?", blk, idx);
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(vpc_watch); i++) {
> +        xive->regs[(PC_VPC_CWATCH_DAT0 >> 3) + i] = be64_to_cpu(vpc_watch[i]);
> +    }
> +}
> +
>  static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>                              XiveEAS *eas)
>  {
> @@ -346,12 +390,6 @@ 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_eas_update(PnvXive *xive, uint8_t blk, uint32_t idx)
> -{
> -    /* All done. */
> -    return 0;
> -}
> -
>  static XiveTCTX *pnv_xive_get_tctx(XiveRouter *xrtr, CPUState *cs)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -950,28 +988,43 @@ static void pnv_xive_ic_reg_write(void *opaque, hwaddr offset,
>       * XIVE PC & VC cache updates for EAS, NVT and END
>       */
>      case VC_IVC_SCRUB_MASK:
> -        break;
>      case VC_IVC_SCRUB_TRIG:
> -        pnv_xive_eas_update(xive, GETFIELD(PC_SCRUB_BLOCK_ID, val),
> -                            GETFIELD(VC_SCRUB_OFFSET, val));
>          break;
>  
> -    case VC_EQC_SCRUB_MASK:
>      case VC_EQC_CWATCH_SPEC:
> -    case VC_EQC_CWATCH_DAT0 ... VC_EQC_CWATCH_DAT3:
> +        val &= ~VC_EQC_CWATCH_CONFLICT; /* HW resets this bit */
> +        break;
> +    case VC_EQC_CWATCH_DAT1 ... VC_EQC_CWATCH_DAT3:
>          break;
> +    case VC_EQC_CWATCH_DAT0:
> +        /* writing to DATA0 triggers the cache write */
> +        xive->regs[reg] = val;
> +        pnv_xive_end_update(xive);
> +        break;
> +    case VC_EQC_SCRUB_MASK:
>      case VC_EQC_SCRUB_TRIG:
> -        pnv_xive_end_update(xive, GETFIELD(VC_SCRUB_BLOCK_ID, val),
> -                            GETFIELD(VC_SCRUB_OFFSET, val));
> +        /*
> +         * The scrubbing registers flush the cache in RAM and can also
> +         * invalidate.
> +         */
>          break;
>  
> -    case PC_VPC_SCRUB_MASK:
>      case PC_VPC_CWATCH_SPEC:
> -    case PC_VPC_CWATCH_DAT0 ... PC_VPC_CWATCH_DAT7:
> +        val &= ~PC_VPC_CWATCH_CONFLICT; /* HW resets this bit */
> +        break;
> +    case PC_VPC_CWATCH_DAT1 ... PC_VPC_CWATCH_DAT7:
>          break;
> +    case PC_VPC_CWATCH_DAT0:
> +        /* writing to DATA0 triggers the cache write */
> +        xive->regs[reg] = val;
> +        pnv_xive_nvt_update(xive);
> +        break;
> +    case PC_VPC_SCRUB_MASK:
>      case PC_VPC_SCRUB_TRIG:
> -        pnv_xive_nvt_update(xive, GETFIELD(PC_SCRUB_BLOCK_ID, val),
> -                           GETFIELD(PC_SCRUB_OFFSET, val));
> +        /*
> +         * The scrubbing registers flush the cache in RAM and can also
> +         * invalidate.
> +         */
>          break;
>  
>  
> @@ -1022,15 +1075,6 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
>      case PC_GLOBAL_CONFIG:
>  
>      case PC_VPC_SCRUB_MASK:
> -    case PC_VPC_CWATCH_SPEC:
> -    case PC_VPC_CWATCH_DAT0:
> -    case PC_VPC_CWATCH_DAT1:
> -    case PC_VPC_CWATCH_DAT2:
> -    case PC_VPC_CWATCH_DAT3:
> -    case PC_VPC_CWATCH_DAT4:
> -    case PC_VPC_CWATCH_DAT5:
> -    case PC_VPC_CWATCH_DAT6:
> -    case PC_VPC_CWATCH_DAT7:
>  
>      case VC_GLOBAL_CONFIG:
>      case VC_AIB_TX_ORDER_TAG2:
> @@ -1043,12 +1087,6 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
>      case VC_IRQ_CONFIG_IPI_CASC:
>  
>      case VC_EQC_SCRUB_MASK:
> -    case VC_EQC_CWATCH_DAT0:
> -    case VC_EQC_CWATCH_DAT1:
> -    case VC_EQC_CWATCH_DAT2:
> -    case VC_EQC_CWATCH_DAT3:
> -
> -    case VC_EQC_CWATCH_SPEC:
>      case VC_IVC_SCRUB_MASK:
>      case VC_SBC_CONFIG:
>      case VC_AT_MACRO_KILL_MASK:
> @@ -1080,6 +1118,38 @@ static uint64_t pnv_xive_ic_reg_read(void *opaque, hwaddr offset, unsigned size)
>      /*
>       * XIVE PC & VC cache updates for EAS, NVT and END
>       */
> +    case VC_EQC_CWATCH_SPEC:
> +        xive->regs[reg] = ~(VC_EQC_CWATCH_FULL | VC_EQC_CWATCH_CONFLICT);
> +        val = xive->regs[reg];
> +        break;
> +    case VC_EQC_CWATCH_DAT0:
> +        /*
> +         * Load DATA registers from cache with data requested by the
> +         * SPEC register
> +         */
> +        pnv_xive_end_cache_load(xive);
> +        val = xive->regs[reg];
> +        break;
> +    case VC_EQC_CWATCH_DAT1 ... VC_EQC_CWATCH_DAT3:
> +        val = xive->regs[reg];
> +        break;
> +
> +    case PC_VPC_CWATCH_SPEC:
> +        xive->regs[reg] = ~(PC_VPC_CWATCH_FULL | PC_VPC_CWATCH_CONFLICT);
> +        val = xive->regs[reg];
> +        break;
> +    case PC_VPC_CWATCH_DAT0:
> +        /*
> +         * Load DATA registers from cache with data requested by the
> +         * SPEC register
> +         */
> +        pnv_xive_nvt_cache_load(xive);
> +        val = xive->regs[reg];
> +        break;
> +    case PC_VPC_CWATCH_DAT1 ... PC_VPC_CWATCH_DAT7:
> +        val = xive->regs[reg];
> +        break;
> +
>      case PC_VPC_SCRUB_TRIG:
>      case VC_IVC_SCRUB_TRIG:
>      case VC_EQC_SCRUB_TRIG:

-- 
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: [Qemu-devel] [PATCH 04/10] ppc/xive: Fix TM_PULL_POOL_CTX special operation

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:55PM +0200, Cédric Le Goater wrote:
> When a CPU is reseted, the hypervisor (Linux or OPAL) invalidates the
> POOL interrupt context of a CPU with this special command. It returns
> the POOL CAM line value and resets the VP bit.
> 
> Fixes: 4836b45510aa ("ppc/xive: activate HV support")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-4.1.

> ---
>  hw/intc/xive.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 534f56f86bd5..cf77bdb7d34a 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -132,6 +132,11 @@ static void xive_tctx_set_cppr(XiveTCTX *tctx, uint8_t ring, uint8_t cppr)
>      xive_tctx_notify(tctx, ring);
>  }
>  
> +static inline uint32_t xive_tctx_word2(uint8_t *ring)
> +{
> +    return *((uint32_t *) &ring[TM_WORD2]);
> +}
> +
>  /*
>   * XIVE Thread Interrupt Management Area (TIMA)
>   */
> @@ -150,11 +155,12 @@ static uint64_t xive_tm_ack_hv_reg(XiveTCTX *tctx, hwaddr offset, unsigned size)
>  static uint64_t xive_tm_pull_pool_ctx(XiveTCTX *tctx, hwaddr offset,
>                                        unsigned size)
>  {
> -    uint64_t ret;
> +    uint32_t qw2w2_prev = xive_tctx_word2(&tctx->regs[TM_QW2_HV_POOL]);
> +    uint32_t qw2w2;
>  
> -    ret = tctx->regs[TM_QW2_HV_POOL + TM_WORD2] & TM_QW2W2_POOL_CAM;
> -    tctx->regs[TM_QW2_HV_POOL + TM_WORD2] &= ~TM_QW2W2_POOL_CAM;
> -    return ret;
> +    qw2w2 = xive_set_field32(TM_QW2W2_VP, qw2w2_prev, 0);
> +    memcpy(&tctx->regs[TM_QW2_HV_POOL + TM_WORD2], &qw2w2, 4);
> +    return qw2w2;
>  }
>  
>  static void xive_tm_vt_push(XiveTCTX *tctx, hwaddr offset,
> @@ -484,11 +490,6 @@ const MemoryRegionOps xive_tm_ops = {
>      },
>  };
>  
> -static inline uint32_t xive_tctx_word2(uint8_t *ring)
> -{
> -    return *((uint32_t *) &ring[TM_WORD2]);
> -}
> -
>  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: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> This is to perform lookups in the NVT table when a vCPU is dispatched
> and possibly resend interrupts.

I'm slightly confused by this one.  Aren't there multiple router
objects, each of which can deliver to any thread?  In which case what
router object is associated with a specific TCTX?

> Future XIVE chip will use a different class for the model of the
> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.

This seems odd to me, shouldn't it be an interface pointer or
something in that case?

> Signed-off-by: Cédric Le Goater <clg@kaod.org>

> ---
>  include/hw/ppc/xive.h |  4 +++-
>  hw/intc/xive.c        | 11 ++++++++++-
>  hw/ppc/pnv.c          |  2 +-
>  hw/ppc/spapr_irq.c    |  2 +-
>  4 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index d922524982d3..b764e1e4e6d4 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
>      qemu_irq    os_output;
>  
>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> +
> +    Object      *xrtr;
>  } XiveTCTX;
>  
>  /*
> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>  uint64_t xive_tctx_tm_read(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);
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
>  
>  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 f7ba1c3b622f..56700681884f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
>      Object *obj;
>      Error *local_err = NULL;
>  
> +    obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
> +    if (!obj) {
> +        error_propagate(errp, local_err);
> +        error_prepend(errp, "required link 'xrtr' not found: ");
> +        return;
> +    }
> +    tctx->xrtr = obj;
> +
>      obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
>      if (!obj) {
>          error_propagate(errp, local_err);
> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
>      .class_init    = xive_tctx_class_init,
>  };
>  
> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
>  {
>      Error *local_err = NULL;
>      Object *obj;
> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>      object_unref(obj);
>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> +    object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
>          goto error;
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b87e01e5b925..11916dc273c2 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>       * controller object is initialized afterwards. Hopefully, it's
>       * only used at runtime.
>       */
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b2b01e850de8..5b3c3c50967b 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -353,7 +353,7 @@ static void spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
>      Object *obj;
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>  
> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;

-- 
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: [Qemu-devel] [PATCH 00/10] ppc/pnv: add XIVE support for KVM guests

[David Gibson]

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

On Sun, Jun 30, 2019 at 10:45:51PM +0200, Cédric Le Goater wrote:
> 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 support for the XIVE
> interrupt controller on POWER9 processor. 
> 
> The following changes fix some parts of the XIVE model and provide
> support for escalations and resend. This mechanism is used by KVM to
> kick a vCPU when it is not dispatched on a HW thread.
> 
> 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.1

The rest of this series is essentially a new feature, and so will have
to wait until 4.2 now we're in soft freeze.

> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (10):
>   ppc/xive: Force the Physical CAM line value to group mode
>   ppc/xive: Make the PIPR register readonly
>   ppc/pnv: Rework cache watch model of PnvXIVE
>   ppc/xive: Fix TM_PULL_POOL_CTX special operation
>   ppc/xive: Implement TM_PULL_OS_CTX special command
>   ppc/xive: Provide escalation support
>   ppc/xive: Improve 'info pic' support
>   ppc/xive: Extend XiveTCTX with an router object pointer
>   ppc/xive: Synthesize interrupt from the saved IPB in the NVT
>   ppc/pnv: Dump the XIVE NVT table
> 
>  include/hw/ppc/xive.h      |  23 ++-
>  include/hw/ppc/xive_regs.h |  13 ++
>  hw/intc/pnv_xive.c         | 211 +++++++++++++++++++----
>  hw/intc/spapr_xive.c       |   1 -
>  hw/intc/xive.c             | 341 +++++++++++++++++++++++++++++--------
>  hw/ppc/pnv.c               |   2 +-
>  hw/ppc/spapr_irq.c         |   2 +-
>  7 files changed, 479 insertions(+), 114 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[Cédric Le Goater]

On 03/07/2019 04:07, David Gibson wrote:
> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>> This is to perform lookups in the NVT table when a vCPU is dispatched
>> and possibly resend interrupts.
> 
> I'm slightly confused by this one.  Aren't there multiple router
> objects, each of which can deliver to any thread?  In which case what
> router object is associated with a specific TCTX?

when a vCPU is dispatched on a HW thread, the hypervisor does a store 
on the CAM line to store the VP id. At that time, it checks the IPB in 
the associated NVT structure and notifies the thread if an interrupt is 
pending. 

We need to do a NVT lookup, just like the presenter in HW, hence the 
router pointer. You should look at the following patch which clarifies 
the resend sequence.

 
>> Future XIVE chip will use a different class for the model of the
>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
> 
> This seems odd to me, shouldn't it be an interface pointer or
> something in that case?

I have duplicated most of the XIVE models for P10 because the internal 
structures have changed. I managed to keep the XiveSource and XiveTCTX 
but we now have a Xive10Router, this is the reason why.

If I was to duplicate XiveTCTX also, I will switch it back to a XiveRouter 
pointer in the P9 version. 

C.


>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
>> ---
>>  include/hw/ppc/xive.h |  4 +++-
>>  hw/intc/xive.c        | 11 ++++++++++-
>>  hw/ppc/pnv.c          |  2 +-
>>  hw/ppc/spapr_irq.c    |  2 +-
>>  4 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index d922524982d3..b764e1e4e6d4 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
>>      qemu_irq    os_output;
>>  
>>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
>> +
>> +    Object      *xrtr;
>>  } XiveTCTX;
>>  
>>  /*
>> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
>>  uint64_t xive_tctx_tm_read(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);
>> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
>>  
>>  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 f7ba1c3b622f..56700681884f 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
>>      Object *obj;
>>      Error *local_err = NULL;
>>  
>> +    obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
>> +    if (!obj) {
>> +        error_propagate(errp, local_err);
>> +        error_prepend(errp, "required link 'xrtr' not found: ");
>> +        return;
>> +    }
>> +    tctx->xrtr = obj;
>> +
>>      obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
>>      if (!obj) {
>>          error_propagate(errp, local_err);
>> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
>>      .class_init    = xive_tctx_class_init,
>>  };
>>  
>> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
>>  {
>>      Error *local_err = NULL;
>>      Object *obj;
>> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
>>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
>>      object_unref(obj);
>>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
>> +    object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
>>      object_property_set_bool(obj, true, "realized", &local_err);
>>      if (local_err) {
>>          goto error;
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index b87e01e5b925..11916dc273c2 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>>       * controller object is initialized afterwards. Hopefully, it's
>>       * only used at runtime.
>>       */
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
>> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>>          return;
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index b2b01e850de8..5b3c3c50967b 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -353,7 +353,7 @@ static void spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
>>      Object *obj;
>>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>>  
>> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
>> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>>          return;
> 

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

[Cédric Le Goater]

On 03/07/2019 04:10, David Gibson wrote:
> On Sun, Jun 30, 2019 at 10:45:51PM +0200, Cédric Le Goater wrote:
>> 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 support for the XIVE
>> interrupt controller on POWER9 processor. 
>>
>> The following changes fix some parts of the XIVE model and provide
>> support for escalations and resend. This mechanism is used by KVM to
>> kick a vCPU when it is not dispatched on a HW thread.
>>
>> 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.1
> 
> The rest of this series is essentially a new feature, and so will have
> to wait until 4.2 now we're in soft freeze.

Yes. That's fine. This is to let you know what is in preparation:
escalations, KVM under PowerNV, PowerNV POWER10. It's all in the 
pipe.

Thanks,

C.

Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[David Gibson]

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

On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
> On 03/07/2019 04:07, David Gibson wrote:
> > On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> >> This is to perform lookups in the NVT table when a vCPU is dispatched
> >> and possibly resend interrupts.
> > 
> > I'm slightly confused by this one.  Aren't there multiple router
> > objects, each of which can deliver to any thread?  In which case what
> > router object is associated with a specific TCTX?
> 
> when a vCPU is dispatched on a HW thread, the hypervisor does a store 
> on the CAM line to store the VP id. At that time, it checks the IPB in 
> the associated NVT structure and notifies the thread if an interrupt is 
> pending. 
> 
> We need to do a NVT lookup, just like the presenter in HW, hence the 
> router pointer. You should look at the following patch which clarifies 
> the resend sequence.

Hm, ok.

> >> Future XIVE chip will use a different class for the model of the
> >> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
> > 
> > This seems odd to me, shouldn't it be an interface pointer or
> > something in that case?
> 
> I have duplicated most of the XIVE models for P10 because the internal 
> structures have changed. I managed to keep the XiveSource and XiveTCTX 
> but we now have a Xive10Router, this is the reason why.

Right, but XiveRouter and Xive10Router must have something in common
if they can both be used here.  Usually that's expressed as a shared
QOM interface - in which case you can use a pointer to the interface,
rathe than using Object * which kind of implies *anything* can go
here.

> 
> If I was to duplicate XiveTCTX also, I will switch it back to a XiveRouter 
> pointer in the P9 version. 
> 
> C.
> 
> 
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> >> ---
> >>  include/hw/ppc/xive.h |  4 +++-
> >>  hw/intc/xive.c        | 11 ++++++++++-
> >>  hw/ppc/pnv.c          |  2 +-
> >>  hw/ppc/spapr_irq.c    |  2 +-
> >>  4 files changed, 15 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> >> index d922524982d3..b764e1e4e6d4 100644
> >> --- a/include/hw/ppc/xive.h
> >> +++ b/include/hw/ppc/xive.h
> >> @@ -321,6 +321,8 @@ typedef struct XiveTCTX {
> >>      qemu_irq    os_output;
> >>  
> >>      uint8_t     regs[XIVE_TM_RING_COUNT * XIVE_TM_RING_SIZE];
> >> +
> >> +    Object      *xrtr;
> >>  } XiveTCTX;
> >>  
> >>  /*
> >> @@ -416,7 +418,7 @@ void xive_tctx_tm_write(XiveTCTX *tctx, hwaddr offset, uint64_t value,
> >>  uint64_t xive_tctx_tm_read(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);
> >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp);
> >>  
> >>  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 f7ba1c3b622f..56700681884f 100644
> >> --- a/hw/intc/xive.c
> >> +++ b/hw/intc/xive.c
> >> @@ -573,6 +573,14 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp)
> >>      Object *obj;
> >>      Error *local_err = NULL;
> >>  
> >> +    obj = object_property_get_link(OBJECT(dev), "xrtr", &local_err);
> >> +    if (!obj) {
> >> +        error_propagate(errp, local_err);
> >> +        error_prepend(errp, "required link 'xrtr' not found: ");
> >> +        return;
> >> +    }
> >> +    tctx->xrtr = obj;
> >> +
> >>      obj = object_property_get_link(OBJECT(dev), "cpu", &local_err);
> >>      if (!obj) {
> >>          error_propagate(errp, local_err);
> >> @@ -657,7 +665,7 @@ static const TypeInfo xive_tctx_info = {
> >>      .class_init    = xive_tctx_class_init,
> >>  };
> >>  
> >> -Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> >> +Object *xive_tctx_create(Object *cpu, Object *xrtr, Error **errp)
> >>  {
> >>      Error *local_err = NULL;
> >>      Object *obj;
> >> @@ -666,6 +674,7 @@ Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp)
> >>      object_property_add_child(cpu, TYPE_XIVE_TCTX, obj, &error_abort);
> >>      object_unref(obj);
> >>      object_property_add_const_link(obj, "cpu", cpu, &error_abort);
> >> +    object_property_add_const_link(obj, "xrtr", xrtr, &error_abort);
> >>      object_property_set_bool(obj, true, "realized", &local_err);
> >>      if (local_err) {
> >>          goto error;
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> index b87e01e5b925..11916dc273c2 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -765,7 +765,7 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
> >>       * controller object is initialized afterwards. Hopefully, it's
> >>       * only used at runtime.
> >>       */
> >> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(&chip9->xive), &local_err);
> >> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(&chip9->xive), &local_err);
> >>      if (local_err) {
> >>          error_propagate(errp, local_err);
> >>          return;
> >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> >> index b2b01e850de8..5b3c3c50967b 100644
> >> --- a/hw/ppc/spapr_irq.c
> >> +++ b/hw/ppc/spapr_irq.c
> >> @@ -353,7 +353,7 @@ static void spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr,
> >>      Object *obj;
> >>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> >>  
> >> -    obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err);
> >> +    obj = xive_tctx_create(OBJECT(cpu), OBJECT(spapr->xive), &local_err);
> >>      if (local_err) {
> >>          error_propagate(errp, local_err);
> >>          return;
> > 
> 

-- 
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: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[Cédric Le Goater]

On 12/07/2019 03:15, David Gibson wrote:
> On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
>> On 03/07/2019 04:07, David Gibson wrote:
>>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>>>> This is to perform lookups in the NVT table when a vCPU is dispatched
>>>> and possibly resend interrupts.
>>>
>>> I'm slightly confused by this one.  Aren't there multiple router
>>> objects, each of which can deliver to any thread?  In which case what
>>> router object is associated with a specific TCTX?
>>
>> when a vCPU is dispatched on a HW thread, the hypervisor does a store 
>> on the CAM line to store the VP id. At that time, it checks the IPB in 
>> the associated NVT structure and notifies the thread if an interrupt is 
>> pending. 
>>
>> We need to do a NVT lookup, just like the presenter in HW, hence the 
>> router pointer. You should look at the following patch which clarifies 
>> the resend sequence.
> 
> Hm, ok.
> 
>>>> Future XIVE chip will use a different class for the model of the
>>>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>>>
>>> This seems odd to me, shouldn't it be an interface pointer or
>>> something in that case?
>>
>> I have duplicated most of the XIVE models for P10 because the internal 
>> structures have changed. I managed to keep the XiveSource and XiveTCTX 
>> but we now have a Xive10Router, this is the reason why.
> 
> Right, but XiveRouter and Xive10Router must have something in common
> if they can both be used here.  Usually that's expressed as a shared
> QOM interface - in which case you can use a pointer to the interface,
> rathe than using Object * which kind of implies *anything* can go
> here.

Yeah. I also think it would be better to have a common base object but
the class don't have much in common. Here is what I have for now :

P9:

typedef struct XiveRouterClass {
    SysBusDeviceClass parent;

    /* XIVE table accessors */
    int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
                   XiveEAS *eas);
    int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
                   XiveEND *end);
    int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
                     XiveEND *end, uint8_t word_number);
    int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                   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);
    uint8_t (*get_block_id)(XiveRouter *xrtr);
} XiveRouterClass;

and P10:

typedef struct Xive10RouterClass {
    SysBusDeviceClass parent;

    /* XIVE table accessors */
    int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
                   Xive10EAS *eas);
    int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
                   Xive10END *end);
    int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
                     Xive10END *end, uint8_t word_number);
    int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                   Xive10NVP *nvt);
    int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
                     Xive10NVP *nvt, uint8_t word_number);
    XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
    uint8_t (*get_block_id)(XiveRouter *xrtr);
    uint32_t (*get_config)(Xive10Router *xrtr);
} Xive10RouterClass;

Only get_tctx() is common. 

The XIVE structures (END, NV*) used by the routing algo have changed a lot.
Even the presenter has changed, because all the CAM lines have a slightly 
different format.   

C.

Re: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[David Gibson]

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

On Mon, Jul 15, 2019 at 05:45:38PM +0200, Cédric Le Goater wrote:
> On 12/07/2019 03:15, David Gibson wrote:
> > On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
> >> On 03/07/2019 04:07, David Gibson wrote:
> >>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
> >>>> This is to perform lookups in the NVT table when a vCPU is dispatched
> >>>> and possibly resend interrupts.
> >>>
> >>> I'm slightly confused by this one.  Aren't there multiple router
> >>> objects, each of which can deliver to any thread?  In which case what
> >>> router object is associated with a specific TCTX?
> >>
> >> when a vCPU is dispatched on a HW thread, the hypervisor does a store 
> >> on the CAM line to store the VP id. At that time, it checks the IPB in 
> >> the associated NVT structure and notifies the thread if an interrupt is 
> >> pending. 
> >>
> >> We need to do a NVT lookup, just like the presenter in HW, hence the 
> >> router pointer. You should look at the following patch which clarifies 
> >> the resend sequence.
> > 
> > Hm, ok.
> > 
> >>>> Future XIVE chip will use a different class for the model of the
> >>>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
> >>>
> >>> This seems odd to me, shouldn't it be an interface pointer or
> >>> something in that case?
> >>
> >> I have duplicated most of the XIVE models for P10 because the internal 
> >> structures have changed. I managed to keep the XiveSource and XiveTCTX 
> >> but we now have a Xive10Router, this is the reason why.
> > 
> > Right, but XiveRouter and Xive10Router must have something in common
> > if they can both be used here.  Usually that's expressed as a shared
> > QOM interface - in which case you can use a pointer to the interface,
> > rathe than using Object * which kind of implies *anything* can go
> > here.
> 
> Yeah. I also think it would be better to have a common base object but
> the class don't have much in common. Here is what I have for now :

Uh.. QOM interfaces don't require there to be a common base object,
only common methods.

> 
> P9:
> 
> typedef struct XiveRouterClass {
>     SysBusDeviceClass parent;
> 
>     /* XIVE table accessors */
>     int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>                    XiveEAS *eas);
>     int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>                    XiveEND *end);
>     int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>                      XiveEND *end, uint8_t word_number);
>     int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                    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);
>     uint8_t (*get_block_id)(XiveRouter *xrtr);
> } XiveRouterClass;
> 
> and P10:
> 
> typedef struct Xive10RouterClass {
>     SysBusDeviceClass parent;
> 
>     /* XIVE table accessors */
>     int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>                    Xive10EAS *eas);
>     int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>                    Xive10END *end);
>     int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>                      Xive10END *end, uint8_t word_number);
>     int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                    Xive10NVP *nvt);
>     int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>                      Xive10NVP *nvt, uint8_t word_number);
>     XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
>     uint8_t (*get_block_id)(XiveRouter *xrtr);
>     uint32_t (*get_config)(Xive10Router *xrtr);
> } Xive10RouterClass;
> 
> Only get_tctx() is common. 
> 
> The XIVE structures (END, NV*) used by the routing algo have changed a lot.
> Even the presenter has changed, because all the CAM lines have a slightly 
> different format.   
> 
> C.
> 
> 

-- 
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: [Qemu-devel] [PATCH 08/10] ppc/xive: Extend XiveTCTX with an router object pointer

[Cédric Le Goater]

On 17/07/2019 04:08, David Gibson wrote:
> On Mon, Jul 15, 2019 at 05:45:38PM +0200, Cédric Le Goater wrote:
>> On 12/07/2019 03:15, David Gibson wrote:
>>> On Wed, Jul 03, 2019 at 07:54:57AM +0200, Cédric Le Goater wrote:
>>>> On 03/07/2019 04:07, David Gibson wrote:
>>>>> On Sun, Jun 30, 2019 at 10:45:59PM +0200, Cédric Le Goater wrote:
>>>>>> This is to perform lookups in the NVT table when a vCPU is dispatched
>>>>>> and possibly resend interrupts.
>>>>>
>>>>> I'm slightly confused by this one.  Aren't there multiple router
>>>>> objects, each of which can deliver to any thread?  In which case what
>>>>> router object is associated with a specific TCTX?
>>>>
>>>> when a vCPU is dispatched on a HW thread, the hypervisor does a store 
>>>> on the CAM line to store the VP id. At that time, it checks the IPB in 
>>>> the associated NVT structure and notifies the thread if an interrupt is 
>>>> pending. 
>>>>
>>>> We need to do a NVT lookup, just like the presenter in HW, hence the 
>>>> router pointer. You should look at the following patch which clarifies 
>>>> the resend sequence.
>>>
>>> Hm, ok.
>>>
>>>>>> Future XIVE chip will use a different class for the model of the
>>>>>> interrupt controller. So use an 'Object *' instead of a 'XiveRouter *'.
>>>>>
>>>>> This seems odd to me, shouldn't it be an interface pointer or
>>>>> something in that case?
>>>>
>>>> I have duplicated most of the XIVE models for P10 because the internal 
>>>> structures have changed. I managed to keep the XiveSource and XiveTCTX 
>>>> but we now have a Xive10Router, this is the reason why.
>>>
>>> Right, but XiveRouter and Xive10Router must have something in common
>>> if they can both be used here.  Usually that's expressed as a shared
>>> QOM interface - in which case you can use a pointer to the interface,
>>> rathe than using Object * which kind of implies *anything* can go
>>> here.
>>
>> Yeah. I also think it would be better to have a common base object but
>> the class don't have much in common. Here is what I have for now :
> 
> Uh.. QOM interfaces don't require there to be a common base object,
> only common methods.

It's not a QOM interface today because it already uses an interface. 

>>
>> P9:
>>
>> typedef struct XiveRouterClass {
>>     SysBusDeviceClass parent;
>>
>>     /* XIVE table accessors */
>>     int (*get_eas)(XiveRouter *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>>                    XiveEAS *eas);
>>     int (*get_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>>                    XiveEND *end);
>>     int (*write_end)(XiveRouter *xrtr, uint8_t end_blk, uint32_t end_idx,
>>                      XiveEND *end, uint8_t word_number);
>>     int (*get_nvt)(XiveRouter *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                    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);
>>     uint8_t (*get_block_id)(XiveRouter *xrtr);
>> } XiveRouterClass;
>>
>> and P10:
>>
>> typedef struct Xive10RouterClass {
>>     SysBusDeviceClass parent;
>>
>>     /* XIVE table accessors */
>>     int (*get_eas)(Xive10Router *xrtr, uint8_t eas_blk, uint32_t eas_idx,
>>                    Xive10EAS *eas);
>>     int (*get_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>>                    Xive10END *end);
>>     int (*write_end)(Xive10Router *xrtr, uint8_t end_blk, uint32_t end_idx,
>>                      Xive10END *end, uint8_t word_number);
>>     int (*get_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                    Xive10NVP *nvt);
>>     int (*write_nvp)(Xive10Router *xrtr, uint8_t nvt_blk, uint32_t nvt_idx,
>>                      Xive10NVP *nvt, uint8_t word_number);
>>     XiveTCTX *(*get_tctx)(Xive10Router *xrtr, CPUState *cs);
>>     uint8_t (*get_block_id)(XiveRouter *xrtr);
>>     uint32_t (*get_config)(Xive10Router *xrtr);
>> } Xive10RouterClass;
>>
>> Only get_tctx() is common. 
>>
>> The XIVE structures (END, NV*) used by the routing algo have changed a lot.
>> Even the presenter has changed, because all the CAM lines have a slightly 
>> different format.   
>>
>> C.
>>
>>
>