[PATCH 0/9] ppc/pnv: XIVE cleanup and fixes

[PATCH 0/9] ppc/pnv: XIVE cleanup and fixes

[Cédric Le Goater]

Hello,

Here is a short series adding the cleanups and fixes of the bigger
series "ppc/pnv: add XIVE support for KVM guests". There is still some
rework to be done on the XivePresenter before it can be resent.

These are valuable changes which can come first and which should not
impact the work yet to come. Comments have been addressed. They were
mostly on the changelog.

Thanks,

C.

Cédric Le Goater (9):
  ppc/pnv: Improve trigger data definition
  ppc/pnv: Use address_space_stq_be() when triggering an interrupt from
    PSI
  ppc/xive: Record the IPB in the associated NVT
  ppc/xive: Introduce helpers for the NVT id
  ppc/pnv: Remove pnv_xive_vst_size() routine
  ppc/pnv: Dump the XIVE NVT table
  ppc/pnv: Quiesce some XIVE errors
  ppc/xive: Introduce OS CAM line helpers
  ppc/xive: Check V bit in TM_PULL_POOL_CTX

 include/hw/ppc/xive.h      |   5 --
 include/hw/ppc/xive_regs.h |  50 ++++++++++-
 hw/intc/pnv_xive.c         | 166 +++++++++++++++++++++----------------
 hw/intc/xive.c             |  61 ++++++++++++--
 hw/ppc/pnv_psi.c           |  15 +++-
 5 files changed, 206 insertions(+), 91 deletions(-)

-- 
2.21.0

[PATCH 1/9] ppc/pnv: Improve trigger data definition

[Cédric Le Goater]

The trigger data is used for both triggers of a HW source interrupts,
PHB, PSI, and triggers for rerouting interrupts between interrupt
controllers.

When an interrupt is rerouted, the trigger data follows an "END
trigger" format. In that case, the remote IC needs EAS containing an
END index to perform a lookup of an END.

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

             |0123|4567|0123|4567|0123|4567|0123|4567|
    W0 E=1   |1P--|BLOC|          END IDX            |
    W1 E=1   |M   |           END DATA               |

An EAS is defined as :

             |0123|4567|0123|4567|0123|4567|0123|4567|
    W0       |V---|BLOC|          END IDX            |
    W1       |M   |          END DATA                |

The END trigger adds an extra 'PQ' bit, bit1 of word0 set to '1',
signaling that the PQ bits have been checked. That bit is unused in
the initial EAS definition.

When a HW device performs the trigger, the trigger data follows an
"EAS trigger" format because the trigger data in that case contains an
EAS index which the IC needs to look for.

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

             |0123|4567|0123|4567|0123|4567|0123|4567|
    W0 E=0   |0P--|---- ---- ---- ---- ---- ---- ----|
    W1 E=0   |BLOC|            EAS INDEX             |

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

Introduce these new trigger bits and rename the XIVE_SRCNO macros in
XIVE_EAS to reflect better the nature of the data.

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

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 08c8bf7172e2..55307cd1533c 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -22,9 +22,29 @@
 /*
  * Interrupt source number encoding on PowerBUS
  */
-#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
-#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
-#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
+/*
+ * Trigger data definition
+ *
+ * The trigger definition is used for triggers both for HW source
+ * interrupts (PHB, PSI), as well as for rerouting interrupts between
+ * Interrupt Controller.
+ *
+ * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
+ * information (EAS block + EAS index) in the 8 byte data and not END
+ * information, which is use for rerouting interrupts.
+ *
+ * bit1 of word0 to ‘1’ signals that the state bit check has been
+ * performed.
+ */
+#define XIVE_TRIGGER_END        PPC_BIT(0)
+#define XIVE_TRIGGER_PQ         PPC_BIT(1)
+
+/*
+ * QEMU macros to manipulate the trigger payload in native endian
+ */
+#define XIVE_EAS_BLOCK(n)       (((n) >> 28) & 0xf)
+#define XIVE_EAS_INDEX(n)       ((n) & 0x0fffffff)
+#define XIVE_EAS(blk, idx)      ((uint32_t)(blk) << 28 | (idx))
 
 #define TM_SHIFT                16
 
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index ed6e9d71bbfa..348f2fdd263d 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
     PnvXive *xive = PNV_XIVE(xrtr);
 
     if (pnv_xive_get_ic(blk) != xive) {
-        xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
+        xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
         return -1;
     }
 
@@ -431,7 +431,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
     PnvXive *xive = PNV_XIVE(xn);
     uint8_t blk = xive->chip->chip_id;
 
-    xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
+    xive_router_notify(xn, XIVE_EAS(blk, srcno));
 }
 
 /*
@@ -1225,12 +1225,24 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = {
 
 static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
 {
+    uint8_t blk;
+    uint32_t idx;
+
+    if (val & XIVE_TRIGGER_END) {
+        xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64,
+                   addr, val);
+        return;
+    }
+
     /*
      * Forward the source event notification directly to the Router.
      * The source interrupt number should already be correctly encoded
      * with the chip block id by the sending device (PHB, PSI).
      */
-    xive_router_notify(XIVE_NOTIFIER(xive), val);
+    blk = XIVE_EAS_BLOCK(val);
+    idx = XIVE_EAS_INDEX(val);
+
+    xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
 }
 
 static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
@@ -1566,7 +1578,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
 {
     XiveRouter *xrtr = XIVE_ROUTER(xive);
     uint8_t blk = xive->chip->chip_id;
-    uint32_t srcno0 = XIVE_SRCNO(blk, 0);
+    uint32_t srcno0 = XIVE_EAS(blk, 0);
     uint32_t nr_ipis = pnv_xive_nr_ipis(xive);
     uint32_t nr_ends = pnv_xive_nr_ends(xive);
     XiveEAS eas;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 29df06df1136..cbe4ae6c294d 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1648,8 +1648,8 @@ do_escalation:
 void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
 {
     XiveRouter *xrtr = XIVE_ROUTER(xn);
-    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
-    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
+    uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
+    uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
     XiveEAS eas;
 
     /* EAS cache lookup */
-- 
2.21.0

Re: [PATCH 1/9] ppc/pnv: Improve trigger data definition

[David Gibson]

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

On Mon, Oct 07, 2019 at 10:40:54AM +0200, Cédric Le Goater wrote:
> The trigger data is used for both triggers of a HW source interrupts,
> PHB, PSI, and triggers for rerouting interrupts between interrupt
> controllers.
> 
> When an interrupt is rerouted, the trigger data follows an "END
> trigger" format. In that case, the remote IC needs EAS containing an
> END index to perform a lookup of an END.
> 
> An END trigger, bit0 of word0 set to '1', is defined as :
> 
>              |0123|4567|0123|4567|0123|4567|0123|4567|
>     W0 E=1   |1P--|BLOC|          END IDX            |
>     W1 E=1   |M   |           END DATA               |
> 
> An EAS is defined as :
> 
>              |0123|4567|0123|4567|0123|4567|0123|4567|
>     W0       |V---|BLOC|          END IDX            |
>     W1       |M   |          END DATA                |
> 
> The END trigger adds an extra 'PQ' bit, bit1 of word0 set to '1',
> signaling that the PQ bits have been checked. That bit is unused in
> the initial EAS definition.
> 
> When a HW device performs the trigger, the trigger data follows an
> "EAS trigger" format because the trigger data in that case contains an
> EAS index which the IC needs to look for.
> 
> An EAS trigger, bit0 of word0 set to '0', is defined as :
> 
>              |0123|4567|0123|4567|0123|4567|0123|4567|
>     W0 E=0   |0P--|---- ---- ---- ---- ---- ---- ----|
>     W1 E=0   |BLOC|            EAS INDEX             |
> 
> There is also a 'PQ' bit, bit1 of word0 to '1', signaling that the
> PQ bits have been checked.
> 
> Introduce these new trigger bits and rename the XIVE_SRCNO macros in
> XIVE_EAS to reflect better the nature of the data.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-4.2, thanks.

> ---
>  include/hw/ppc/xive_regs.h | 26 +++++++++++++++++++++++---
>  hw/intc/pnv_xive.c         | 20 ++++++++++++++++----
>  hw/intc/xive.c             |  4 ++--
>  3 files changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index 08c8bf7172e2..55307cd1533c 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -22,9 +22,29 @@
>  /*
>   * Interrupt source number encoding on PowerBUS
>   */
> -#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf)
> -#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff)
> -#define XIVE_SRCNO(blk, idx)    ((uint32_t)(blk) << 28 | (idx))
> +/*
> + * Trigger data definition
> + *
> + * The trigger definition is used for triggers both for HW source
> + * interrupts (PHB, PSI), as well as for rerouting interrupts between
> + * Interrupt Controller.
> + *
> + * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS
> + * information (EAS block + EAS index) in the 8 byte data and not END
> + * information, which is use for rerouting interrupts.
> + *
> + * bit1 of word0 to ‘1’ signals that the state bit check has been
> + * performed.
> + */
> +#define XIVE_TRIGGER_END        PPC_BIT(0)
> +#define XIVE_TRIGGER_PQ         PPC_BIT(1)
> +
> +/*
> + * QEMU macros to manipulate the trigger payload in native endian
> + */
> +#define XIVE_EAS_BLOCK(n)       (((n) >> 28) & 0xf)
> +#define XIVE_EAS_INDEX(n)       ((n) & 0x0fffffff)
> +#define XIVE_EAS(blk, idx)      ((uint32_t)(blk) << 28 | (idx))
>  
>  #define TM_SHIFT                16
>  
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index ed6e9d71bbfa..348f2fdd263d 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx,
>      PnvXive *xive = PNV_XIVE(xrtr);
>  
>      if (pnv_xive_get_ic(blk) != xive) {
> -        xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx));
> +        xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx));
>          return -1;
>      }
>  
> @@ -431,7 +431,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno)
>      PnvXive *xive = PNV_XIVE(xn);
>      uint8_t blk = xive->chip->chip_id;
>  
> -    xive_router_notify(xn, XIVE_SRCNO(blk, srcno));
> +    xive_router_notify(xn, XIVE_EAS(blk, srcno));
>  }
>  
>  /*
> @@ -1225,12 +1225,24 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = {
>  
>  static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val)
>  {
> +    uint8_t blk;
> +    uint32_t idx;
> +
> +    if (val & XIVE_TRIGGER_END) {
> +        xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64,
> +                   addr, val);
> +        return;
> +    }
> +
>      /*
>       * Forward the source event notification directly to the Router.
>       * The source interrupt number should already be correctly encoded
>       * with the chip block id by the sending device (PHB, PSI).
>       */
> -    xive_router_notify(XIVE_NOTIFIER(xive), val);
> +    blk = XIVE_EAS_BLOCK(val);
> +    idx = XIVE_EAS_INDEX(val);
> +
> +    xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx));
>  }
>  
>  static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val,
> @@ -1566,7 +1578,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
>  {
>      XiveRouter *xrtr = XIVE_ROUTER(xive);
>      uint8_t blk = xive->chip->chip_id;
> -    uint32_t srcno0 = XIVE_SRCNO(blk, 0);
> +    uint32_t srcno0 = XIVE_EAS(blk, 0);
>      uint32_t nr_ipis = pnv_xive_nr_ipis(xive);
>      uint32_t nr_ends = pnv_xive_nr_ends(xive);
>      XiveEAS eas;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 29df06df1136..cbe4ae6c294d 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1648,8 +1648,8 @@ do_escalation:
>  void xive_router_notify(XiveNotifier *xn, uint32_t lisn)
>  {
>      XiveRouter *xrtr = XIVE_ROUTER(xn);
> -    uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn);
> -    uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn);
> +    uint8_t eas_blk = XIVE_EAS_BLOCK(lisn);
> +    uint32_t eas_idx = XIVE_EAS_INDEX(lisn);
>      XiveEAS eas;
>  
>      /* EAS cache lookup */

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

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

[PATCH 2/9] ppc/pnv: Use address_space_stq_be() when triggering an interrupt from PSI

[Cédric Le Goater]

Include the XIVE_TRIGGER_PQ bit in the trigger data which is how
hardware signals to the IC that the PQ bits of the interrupt source
have been checked.

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

diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index a997f16bb4e6..68d0dfacfe2b 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -660,10 +660,19 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
 
     uint32_t offset =
         (psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT);
-    uint64_t lisn = cpu_to_be64(offset + srcno);
+    uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
+    MemTxResult result;
 
-    if (valid) {
-        cpu_physical_memory_write(notify_addr, &lisn, sizeof(lisn));
+    if (!valid) {
+        return;
+    }
+
+    address_space_stq_be(&address_space_memory, notify_addr, data,
+                         MEMTXATTRS_UNSPECIFIED, &result);
+    if (result != MEMTX_OK) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: trigger failed @%"
+                      HWADDR_PRIx "\n", __func__, notif_port);
+        return;
     }
 }
 
-- 
2.21.0

Re: [PATCH 2/9] ppc/pnv: Use address_space_stq_be() when triggering an interrupt from PSI

[David Gibson]

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

On Mon, Oct 07, 2019 at 10:40:55AM +0200, Cédric Le Goater wrote:
> Include the XIVE_TRIGGER_PQ bit in the trigger data which is how
> hardware signals to the IC that the PQ bits of the interrupt source
> have been checked.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-4.2, thanks.

> ---
>  hw/ppc/pnv_psi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index a997f16bb4e6..68d0dfacfe2b 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -660,10 +660,19 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno)
>  
>      uint32_t offset =
>          (psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT);
> -    uint64_t lisn = cpu_to_be64(offset + srcno);
> +    uint64_t data = XIVE_TRIGGER_PQ | offset | srcno;
> +    MemTxResult result;
>  
> -    if (valid) {
> -        cpu_physical_memory_write(notify_addr, &lisn, sizeof(lisn));
> +    if (!valid) {
> +        return;
> +    }
> +
> +    address_space_stq_be(&address_space_memory, notify_addr, data,
> +                         MEMTXATTRS_UNSPECIFIED, &result);
> +    if (result != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: trigger failed @%"
> +                      HWADDR_PRIx "\n", __func__, notif_port);
> +        return;
>      }
>  }
>  

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

[PATCH 3/9] ppc/xive: Record the IPB in the associated 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.

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

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 55307cd1533c..530f232b04f8 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -255,6 +255,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 cbe4ae6c294d..2bf7b4ad7006 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1600,14 +1600,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
      * - logical server : forward request to IVPE (not supported)
      */
     if (xive_end_is_backlog(&end)) {
+        uint8_t ipb;
+
         if (format == 1) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "XIVE: END %x/%x invalid config: F1 & backlog\n",
                           end_blk, end_idx);
             return;
         }
-        /* Record the IPB in the associated NVT structure */
-        ipb_update((uint8_t *) &nvt.w4, priority);
+        /*
+         * Record the IPB in the associated NVT structure for later
+         * use. The presenter will resend the interrupt when the vCPU
+         * is dispatched again on a HW thread.
+         */
+        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
+        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
         xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
 
         /*
-- 
2.21.0

Re: [PATCH 3/9] ppc/xive: Record the IPB in the associated NVT

[David Gibson]

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

On Mon, Oct 07, 2019 at 10:40:56AM +0200, Cédric Le Goater wrote:
> 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.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

This commit message doesn't really make it clear what was happening
before, which certainly looks like an update to the IPB field in the
NVT, via a different method.

Is this just a cleanup, or a fix?

> ---
>  include/hw/ppc/xive_regs.h |  1 +
>  hw/intc/xive.c             | 11 +++++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index 55307cd1533c..530f232b04f8 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -255,6 +255,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 cbe4ae6c294d..2bf7b4ad7006 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1600,14 +1600,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>       * - logical server : forward request to IVPE (not supported)
>       */
>      if (xive_end_is_backlog(&end)) {
> +        uint8_t ipb;
> +
>          if (format == 1) {
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "XIVE: END %x/%x invalid config: F1 & backlog\n",
>                            end_blk, end_idx);
>              return;
>          }
> -        /* Record the IPB in the associated NVT structure */
> -        ipb_update((uint8_t *) &nvt.w4, priority);
> +        /*
> +         * Record the IPB in the associated NVT structure for later
> +         * use. The presenter will resend the interrupt when the vCPU
> +         * is dispatched again on a HW thread.
> +         */
> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
> +        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>  
>          /*

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

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

Re: [PATCH 3/9] ppc/xive: Record the IPB in the associated NVT

[Cédric Le Goater]

On 14/10/2019 07:32, David Gibson wrote:
> On Mon, Oct 07, 2019 at 10:40:56AM +0200, Cédric Le Goater wrote:
>> 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.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> This commit message doesn't really make it clear what was happening
> before, which certainly looks like an update to the IPB field in the
> NVT, via a different method.
> 
> Is this just a cleanup, or a fix?

This is a small fix. Here is an update changelog :

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

Fix the current model that also updates the PIPR which is not how HW 
operates and use the XIVE field structure routines to get/set the 
IPB register backup value in the XIVE NVT structure.
"

 
>> ---
>>  include/hw/ppc/xive_regs.h |  1 +
>>  hw/intc/xive.c             | 11 +++++++++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
>> index 55307cd1533c..530f232b04f8 100644
>> --- a/include/hw/ppc/xive_regs.h
>> +++ b/include/hw/ppc/xive_regs.h
>> @@ -255,6 +255,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 cbe4ae6c294d..2bf7b4ad7006 100644
>> --- a/hw/intc/xive.c
>> +++ b/hw/intc/xive.c
>> @@ -1600,14 +1600,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>>       * - logical server : forward request to IVPE (not supported)
>>       */
>>      if (xive_end_is_backlog(&end)) {
>> +        uint8_t ipb;
>> +
>>          if (format == 1) {
>>              qemu_log_mask(LOG_GUEST_ERROR,
>>                            "XIVE: END %x/%x invalid config: F1 & backlog\n",
>>                            end_blk, end_idx);
>>              return;
>>          }
>> -        /* Record the IPB in the associated NVT structure */
>> -        ipb_update((uint8_t *) &nvt.w4, priority);
>> +        /*
>> +         * Record the IPB in the associated NVT structure for later
>> +         * use. The presenter will resend the interrupt when the vCPU
>> +         * is dispatched again on a HW thread.
>> +         */
>> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
>> +        nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>>  
>>          /*
> 

[PATCH 4/9] ppc/xive: Introduce helpers for the NVT id

[Cédric Le Goater]

The NVT space is 19 bits wide, giving a maximum of 512K per chip. When
a vPCU is dispatched on a HW thread, the NVT identifier is pushed in
the CAM line (QW1W2). This identifier is used in the presenter
subengine to fetch a NVT structure which might contain pending
interrupts that need a resend.

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

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index fd3319bd3202..c1d274f39f9f 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -416,11 +416,6 @@ 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);
 
-static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
-{
-    return (nvt_blk << 19) | nvt_idx;
-}
-
 /*
  * KVM XIVE device helpers
  */
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 530f232b04f8..1a5622f8ded8 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -272,4 +272,25 @@ typedef struct XiveNVT {
 
 #define xive_nvt_is_valid(nvt)    (be32_to_cpu((nvt)->w0) & NVT_W0_VALID)
 
+/*
+ * The VP number space in a block is defined by the END_W6_NVT_INDEX
+ * field of the XIVE END
+ */
+#define XIVE_NVT_SHIFT                19
+
+static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
+{
+    return (nvt_blk << XIVE_NVT_SHIFT) | nvt_idx;
+}
+
+static inline uint32_t xive_nvt_idx(uint32_t cam_line)
+{
+    return cam_line & ((1 << XIVE_NVT_SHIFT) - 1);
+}
+
+static inline uint32_t xive_nvt_blk(uint32_t cam_line)
+{
+    return (cam_line >> XIVE_NVT_SHIFT) & 0xf;
+}
+
 #endif /* PPC_XIVE_REGS_H */
-- 
2.21.0

[PATCH 5/9] ppc/pnv: Remove pnv_xive_vst_size() routine

[Cédric Le Goater]

pnv_xive_vst_size() tries to compute the size of a VSD table from the
information given by FW. The number of entries of the table are
deduced from the result and the MMIO regions of the ESBs and the END
ESBs are then resized accordingly with the computed value. This
reduces the number of elements that can be addressed by the ESB pages.

The maximum number of elements of a direct table can contain is simply:

   Table size / sizeof(XIVE structure)

An indirect table is a one page array of VSDs pointing to subpages
containing XIVE virtual structures and the maximum number of elements
an indirect table can contain :

   (PAGE_SIZE / sizeof(vsd)) * (PAGE_SIZE / sizeof(XIVE structure))

which gives us 16M for XiveENDs, 8M for XiveNVTs. That's more than the
associated VC and PC BARS can address.

The result returned by pnv_xive_vst_size() for indirect tables is
incorrect and can not be used to reduce the size of the MMIO region of
a XIVE resource using an indirect table, such as ENDs in skiboot.

Remove pnv_xive_vst_size() and use a simpler form for direct tables
only. Keep the resizing of the MMIO region for direct tables only as
this is still useful for the ESB MMIO window.

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

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

[PATCH 6/9] ppc/pnv: Dump the XIVE NVT table

[Cédric Le Goater]

This is useful to dump the contexts of the KVM vCPUs : configuration
of the base END index of the vCPU and the Interrupt Pending Buffer,
which is updated when an interrupt can not be presented.

When dumping the NVT table, we skip 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         | 30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index 1a5622f8ded8..94338b4b551e 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -252,6 +252,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 0b142fd9f112..9f7e0e1662d6 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1554,6 +1554,27 @@ static const MemoryRegionOps pnv_xive_pc_ops = {
     },
 };
 
+/*
+ * skiboot uses an indirect NVT table with 64k subpages
+ */
+#define XIVE_NVT_COUNT          (1 << XIVE_NVT_SHIFT)
+#define XIVE_NVT_PER_PAGE       (0x10000 / sizeof(XiveNVT))
+
+static void xive_nvt_pic_print_info(XiveNVT *nvt, uint32_t nvt_idx,
+                                    Monitor *mon)
+{
+    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);
@@ -1562,6 +1583,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     uint32_t nr_ipis = pnv_xive_nr_ipis(xive, blk);
     XiveEAS eas;
     XiveEND end;
+    XiveNVT nvt;
     int i;
 
     monitor_printf(mon, "XIVE[%x] Source %08x .. %08x\n", blk, srcno0,
@@ -1590,6 +1612,14 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon)
     while (!xive_router_get_end(xrtr, blk, i, &end)) {
         xive_end_eas_pic_print_info(&end, i++, mon);
     }
+
+    monitor_printf(mon, "XIVE[%x] NVTT %08x .. %08x\n", blk, 0,
+                   XIVE_NVT_COUNT - 1);
+    for (i = 0; i < XIVE_NVT_COUNT; i += XIVE_NVT_PER_PAGE) {
+        while (!xive_router_get_nvt(xrtr, blk, i, &nvt)) {
+            xive_nvt_pic_print_info(&nvt, i++, mon);
+        }
+    }
 }
 
 static void pnv_xive_reset(void *dev)
-- 
2.21.0

[PATCH 7/9] ppc/pnv: Quiesce some XIVE errors

[Cédric Le Goater]

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

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

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

[PATCH 8/9] ppc/xive: Introduce OS CAM line helpers

[Cédric Le Goater]

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

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

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 2bf7b4ad7006..143418c232a2 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -337,14 +337,49 @@ static void xive_tm_set_os_pending(XiveTCTX *tctx, hwaddr offset,
     xive_tctx_notify(tctx, TM_QW1_OS);
 }
 
+static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
+                               uint32_t *nvt_idx, bool *vo)
+{
+    if (nvt_blk) {
+        *nvt_blk = xive_nvt_blk(cam);
+    }
+    if (nvt_idx) {
+        *nvt_idx = xive_nvt_idx(cam);
+    }
+    if (vo) {
+        *vo = !!(cam & TM_QW1W2_VO);
+    }
+}
+
+static uint32_t xive_tctx_get_os_cam(XiveTCTX *tctx, uint8_t *nvt_blk,
+                                     uint32_t *nvt_idx, bool *vo)
+{
+    uint32_t qw1w2 = xive_tctx_word2(&tctx->regs[TM_QW1_OS]);
+    uint32_t cam = be32_to_cpu(qw1w2);
+
+    xive_os_cam_decode(cam, nvt_blk, nvt_idx, vo);
+    return qw1w2;
+}
+
+static void xive_tctx_set_os_cam(XiveTCTX *tctx, uint32_t qw1w2)
+{
+    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+}
+
 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;
+    uint32_t qw1w2_new;
+    uint8_t nvt_blk;
+    uint32_t nvt_idx;
+    bool vo;
 
-    qw1w2 = xive_set_field32(TM_QW1W2_VO, qw1w2_prev, 0);
-    memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4);
+    qw1w2 = xive_tctx_get_os_cam(tctx, &nvt_blk, &nvt_idx, &vo);
+
+    /* Invalidate CAM line */
+    qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
+    xive_tctx_set_os_cam(tctx, qw1w2_new);
     return qw1w2;
 }
 
-- 
2.21.0

[PATCH 9/9] ppc/xive: Check V bit in TM_PULL_POOL_CTX

[Cédric Le Goater]

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

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

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 143418c232a2..91eb7789f329 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -377,6 +377,11 @@ static uint64_t xive_tm_pull_os_ctx(XiveTCTX *tctx, hwaddr offset,
 
     qw1w2 = xive_tctx_get_os_cam(tctx, &nvt_blk, &nvt_idx, &vo);
 
+    if (!vo) {
+        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: pulling invalid NVT %x/%x !?\n",
+                      nvt_blk, nvt_idx);
+    }
+
     /* Invalidate CAM line */
     qw1w2_new = xive_set_field32(TM_QW1W2_VO, qw1w2, 0);
     xive_tctx_set_os_cam(tctx, qw1w2_new);
-- 
2.21.0

Re: [PATCH 0/9] ppc/pnv: XIVE cleanup and fixes

[Cédric Le Goater]

On 07/10/2019 10:40, Cédric Le Goater wrote:
> Hello,
> 
> Here is a short series adding the cleanups and fixes of the bigger
> series "ppc/pnv: add XIVE support for KVM guests". There is still some
> rework to be done on the XivePresenter before it can be resent.
> 
> These are valuable changes which can come first and which should not
> impact the work yet to come. Comments have been addressed. They were
> mostly on the changelog.

David, Do you have some time for these patches ? There are larger
series coming after.

Thanks,

C.
 
> Cédric Le Goater (9):
>   ppc/pnv: Improve trigger data definition
>   ppc/pnv: Use address_space_stq_be() when triggering an interrupt from
>     PSI
>   ppc/xive: Record the IPB in the associated NVT
>   ppc/xive: Introduce helpers for the NVT id
>   ppc/pnv: Remove pnv_xive_vst_size() routine
>   ppc/pnv: Dump the XIVE NVT table
>   ppc/pnv: Quiesce some XIVE errors
>   ppc/xive: Introduce OS CAM line helpers
>   ppc/xive: Check V bit in TM_PULL_POOL_CTX
> 
>  include/hw/ppc/xive.h      |   5 --
>  include/hw/ppc/xive_regs.h |  50 ++++++++++-
>  hw/intc/pnv_xive.c         | 166 +++++++++++++++++++++----------------
>  hw/intc/xive.c             |  61 ++++++++++++--
>  hw/ppc/pnv_psi.c           |  15 +++-
>  5 files changed, 206 insertions(+), 91 deletions(-)
>