diff mbox series

[v2,4/6] spapr/xive: Enforce load-after-store ordering

Message ID 20201005165147.526426-5-clg@kaod.org
State New, archived
Headers show
Series spapr/xive: Activate StoreEOI in P10 compat guests | expand

Commit Message

Cédric Le Goater Oct. 5, 2020, 4:51 p.m. UTC
The XIVE_ESB_SET_PQ_10 load operation is used to disable temporarily
an interrupt source. If StoreEOI is active, a source could be left
enabled if the load and store operations come out of order.

QEMU makes use of this offset to quiesce the sources before a
migration. Enforce the load-after-store ordering always when doing so
without querying the characteristics of the sources on the host. The
performance penalty will be very small for QEMU.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h    |  8 ++++++++
 hw/intc/spapr_xive_kvm.c | 12 ++++++++++++
 hw/intc/xive.c           |  6 ++++++
 3 files changed, 26 insertions(+)

Comments

Greg Kurz Oct. 6, 2020, 5:02 p.m. UTC | #1
On Mon, 5 Oct 2020 18:51:45 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> The XIVE_ESB_SET_PQ_10 load operation is used to disable temporarily
> an interrupt source. If StoreEOI is active, a source could be left
> enabled if the load and store operations come out of order.
> 
> QEMU makes use of this offset to quiesce the sources before a
> migration. Enforce the load-after-store ordering always when doing so
> without querying the characteristics of the sources on the host. The
> performance penalty will be very small for QEMU.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  include/hw/ppc/xive.h    |  8 ++++++++
>  hw/intc/spapr_xive_kvm.c | 12 ++++++++++++
>  hw/intc/xive.c           |  6 ++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 445eccfe6b73..39cd273f86d5 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -279,6 +279,14 @@ static inline hwaddr xive_source_esb_mgmt(XiveSource *xsrc, int srcno)
>  #define XIVE_ESB_SET_PQ_10      0xe00 /* Load */
>  #define XIVE_ESB_SET_PQ_11      0xf00 /* Load */
>  
> +/*
> + * Load-after-store ordering
> + *
> + * Adding this offset to the load address will enforce
> + * load-after-store ordering. This is required to use with StoreEOI.
> + */
> +#define XIVE_ESB_LD_ST_MO       0x40 /* Load-after-store ordering */
> +
>  uint8_t xive_source_esb_get(XiveSource *xsrc, uint32_t srcno);
>  uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
>  
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 66bf4c06fe55..d428422a7b72 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -357,6 +357,18 @@ static uint64_t xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
>  
>  static uint8_t xive_esb_read(XiveSource *xsrc, int srcno, uint32_t offset)
>  {
> +    /*
> +     * The XIVE_ESB_SET_PQ_10 load operation is used to disable
> +     * temporarily an interrupt source. If StoreEOI is active, a
> +     * source could be left enabled if the load and store operations
> +     * come out of order.
> +     *
> +     * Enforce the load-after-store ordering always.
> +     */
> +    if (offset == XIVE_ESB_SET_PQ_10) {
> +        offset |= XIVE_ESB_LD_ST_MO;
> +    }
> +
>      return xive_esb_rw(xsrc, srcno, offset, 0, 0) & 0x3;
>  }
>  
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 489e6256ef70..b710ba2df095 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -998,6 +998,12 @@ static uint64_t xive_source_esb_read(void *opaque, hwaddr addr, unsigned size)
>      case XIVE_ESB_SET_PQ_01 ... XIVE_ESB_SET_PQ_01 + 0x0FF:
>      case XIVE_ESB_SET_PQ_10 ... XIVE_ESB_SET_PQ_10 + 0x0FF:
>      case XIVE_ESB_SET_PQ_11 ... XIVE_ESB_SET_PQ_11 + 0x0FF:
> +        if (offset == XIVE_ESB_SET_PQ_10 &&
> +            xsrc->esb_flags & XIVE_SRC_STORE_EOI) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: load-after-store ordering "
> +                          "not enforced with Store EOI active for IRQ %d\n",
> +                          srcno);
> +        }
>          ret = xive_source_esb_set(xsrc, srcno, (offset >> 8) & 0x3);
>          break;
>      default:
diff mbox series

Patch

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 445eccfe6b73..39cd273f86d5 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -279,6 +279,14 @@  static inline hwaddr xive_source_esb_mgmt(XiveSource *xsrc, int srcno)
 #define XIVE_ESB_SET_PQ_10      0xe00 /* Load */
 #define XIVE_ESB_SET_PQ_11      0xf00 /* Load */
 
+/*
+ * Load-after-store ordering
+ *
+ * Adding this offset to the load address will enforce
+ * load-after-store ordering. This is required to use with StoreEOI.
+ */
+#define XIVE_ESB_LD_ST_MO       0x40 /* Load-after-store ordering */
+
 uint8_t xive_source_esb_get(XiveSource *xsrc, uint32_t srcno);
 uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
 
diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index 66bf4c06fe55..d428422a7b72 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -357,6 +357,18 @@  static uint64_t xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
 
 static uint8_t xive_esb_read(XiveSource *xsrc, int srcno, uint32_t offset)
 {
+    /*
+     * The XIVE_ESB_SET_PQ_10 load operation is used to disable
+     * temporarily an interrupt source. If StoreEOI is active, a
+     * source could be left enabled if the load and store operations
+     * come out of order.
+     *
+     * Enforce the load-after-store ordering always.
+     */
+    if (offset == XIVE_ESB_SET_PQ_10) {
+        offset |= XIVE_ESB_LD_ST_MO;
+    }
+
     return xive_esb_rw(xsrc, srcno, offset, 0, 0) & 0x3;
 }
 
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 489e6256ef70..b710ba2df095 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -998,6 +998,12 @@  static uint64_t xive_source_esb_read(void *opaque, hwaddr addr, unsigned size)
     case XIVE_ESB_SET_PQ_01 ... XIVE_ESB_SET_PQ_01 + 0x0FF:
     case XIVE_ESB_SET_PQ_10 ... XIVE_ESB_SET_PQ_10 + 0x0FF:
     case XIVE_ESB_SET_PQ_11 ... XIVE_ESB_SET_PQ_11 + 0x0FF:
+        if (offset == XIVE_ESB_SET_PQ_10 &&
+            xsrc->esb_flags & XIVE_SRC_STORE_EOI) {
+            qemu_log_mask(LOG_GUEST_ERROR, "XIVE: load-after-store ordering "
+                          "not enforced with Store EOI active for IRQ %d\n",
+                          srcno);
+        }
         ret = xive_source_esb_set(xsrc, srcno, (offset >> 8) & 0x3);
         break;
     default: