| Message ID | 20201005165147.526426-5-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | spapr/xive: Activate StoreEOI in P10 compat guests | expand |
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 --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:
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(+)