I am still slowly wrapping my head around XIVE and it's interaction with KVM but from what I can see this looks good and is needed so we can enable StoreEOI support in future so:
Reviewed-by: Alistair Popple <alist...@popple.id.au> On Thursday, 20 February 2020 7:15:06 PM AEST Cédric Le Goater wrote: > When an interrupt has been handled, the OS notifies the interrupt > controller with a EOI sequence. On a POWER9 system using the XIVE > interrupt controller, this can be done with a load or a store > operation on the ESB interrupt management page of the interrupt. The > StoreEOI operation has less latency and improves interrupt handling > performance but it was deactivated during the POWER9 DD2.0 timeframe > because of ordering issues. We use the LoadEOI today but we plan to > reactivate StoreEOI in future architectures. > > There is usually no need to enforce ordering between ESB load and > store operations as they should lead to the same result. E.g. a store > trigger and a load EOI can be executed in any order. Assuming the > interrupt state is PQ=10, a store trigger followed by a load EOI will > return a Q bit. In the reverse order, it will create a new interrupt > trigger from HW. In both cases, the handler processing interrupts is > notified. > > In some cases, the XIVE_ESB_SET_PQ_10 load operation is used to > disable temporarily the interrupt source (mask/unmask). When the > source is reenabled, the OS can detect if interrupts were received > while the source was disabled and reinject them. This process needs > special care when StoreEOI is activated. The ESB load and store > operations should be correctly ordered because a XIVE_ESB_STORE_EOI > operation could leave the source enabled if it has not completed > before the loads. > > For those cases, we enforce Load-after-Store ordering with a special > load operation offset. To avoid performance impact, this ordering is > only enforced when really needed, that is when interrupt sources are > temporarily disabled with the XIVE_ESB_SET_PQ_10 load. It should not > be needed for other loads. > > Signed-off-by: Cédric Le Goater <c...@kaod.org> > --- > arch/powerpc/include/asm/xive-regs.h | 8 ++++++++ > arch/powerpc/kvm/book3s_xive_native.c | 6 ++++++ > arch/powerpc/kvm/book3s_xive_template.c | 3 +++ > arch/powerpc/sysdev/xive/common.c | 3 +++ > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 5 +++++ > 5 files changed, 25 insertions(+) > > diff --git a/arch/powerpc/include/asm/xive-regs.h > b/arch/powerpc/include/asm/xive-regs.h index f2dfcd50a2d3..b1996fbae59a > 100644 > --- a/arch/powerpc/include/asm/xive-regs.h > +++ b/arch/powerpc/include/asm/xive-regs.h > @@ -37,6 +37,14 @@ > #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 StoreEOI. > + */ > +#define XIVE_ESB_LD_ST_MO 0x40 /* Load-after-store ordering */ > + > #define XIVE_ESB_VAL_P 0x2 > #define XIVE_ESB_VAL_Q 0x1 > > diff --git a/arch/powerpc/kvm/book3s_xive_native.c > b/arch/powerpc/kvm/book3s_xive_native.c index d83adb1e1490..c80b6a447efd > 100644 > --- a/arch/powerpc/kvm/book3s_xive_native.c > +++ b/arch/powerpc/kvm/book3s_xive_native.c > @@ -31,6 +31,12 @@ static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 > offset) { > u64 val; > > + /* > + * The KVM XIVE native device does not use the XIVE_ESB_SET_PQ_10 > + * load operation, so there is no need to enforce load-after-store > + * ordering. > + */ > + > if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG) > offset |= offset << 4; > > diff --git a/arch/powerpc/kvm/book3s_xive_template.c > b/arch/powerpc/kvm/book3s_xive_template.c index a8a900ace1e6..4ad3c0279458 > 100644 > --- a/arch/powerpc/kvm/book3s_xive_template.c > +++ b/arch/powerpc/kvm/book3s_xive_template.c > @@ -58,6 +58,9 @@ static u8 GLUE(X_PFX,esb_load)(struct xive_irq_data *xd, > u32 offset) { > u64 val; > > + if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI) > + offset |= XIVE_ESB_LD_ST_MO; > + > if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG) > offset |= offset << 4; > > diff --git a/arch/powerpc/sysdev/xive/common.c > b/arch/powerpc/sysdev/xive/common.c index f5fadbd2533a..0dc421bb494f 100644 > --- a/arch/powerpc/sysdev/xive/common.c > +++ b/arch/powerpc/sysdev/xive/common.c > @@ -202,6 +202,9 @@ static notrace u8 xive_esb_read(struct xive_irq_data > *xd, u32 offset) { > u64 val; > > + if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI) > + offset |= XIVE_ESB_LD_ST_MO; > + > /* Handle HW errata */ > if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG) > offset |= offset << 4; > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S index e11017897eb0..abe132ff2346 > 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -2911,6 +2911,11 @@ kvm_cede_exit: > beq 4f > li r0, 0 > stb r0, VCPU_CEDED(r9) > + /* > + * The escalation interrupts are special as we don't EOI them. > + * There is no need to use the load-after-store ordering offset > + * to set PQ to 10 as we won't use StoreEOI. > + */ > li r6, XIVE_ESB_SET_PQ_10 > b 5f > 4: li r0, 1