Re: [PATCH v2 0/6] spapr/xive: Activate StoreEOI in P10 compat guests

[David Gibson]

On Mon, Oct 05, 2020 at 06:51:41PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> When an interrupt has been handled, the OS notifies the interrupt
> controller with an EOI sequence. On the XIVE interrupt controller
> (POWER9 and POWER10), 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 time-frame
> because of ordering issues. POWER9 systems use the LoadEOI instead.
> POWER10 has fixed the issue with a special load command which enforces
> Load-after-Store ordering and StoreEOI can be safely used.

Do you mean that ordering is *always* enforced on P10?  Or it's a
special form of load that has the ordering?

Also, weirdly, despite the series being addressed to me, only some of
the patches ended up in my inbox, rather than the list folder :/.

> These changes add a new StoreEOI capability which activate StoreEOI
> support in the flags returned by the hcall H_INT_GET_SOURCE_INFO. When
> the machine is using an emulated interrupt controller, TCG or without
> kernel IRQ chip, there are no limitations and activating StoreEOI is
> not an issue. However, when running with a kernel IRQ chip, some
> verification needs to be done on the host. This is done through the
> DT, which tells us that firmware has configured the HW for StoreEOI,
> but a new KVM capability would be cleaner.
> 
> The last patch introduces a new 'cas' value to the capability which
> lets the hypervisor decide at CAS time if StoreEOI should be
> advertised to the guest OS. P10 compat kernel are considered safe
> because the OS enforces load-after-store ordering but not with P9.
> 
> The StoreEOI capability is a global setting and does not take into
> account the characteristics of a single source. It could be an issue
> if StoreEOI is not supported on a specific source, of a passthrough
> device for instance. In that case, we could either introduce a new KVM
> ioctl to query the characteristics of the source at the HW level (like
> in v1) or deactivate StoreEOI on the machine.
> 
> We are using these patches today on P10 and P9 (with a custom FW
> activating StoreEOI) systems to benchmark interrupt performance on
> large guests but there's no hurry to take them. Let's discuss this new
> approach.
> 
> Thanks,
> 
> C.
> 
> Changes in v2:
> 
>  - completely approach using a capability
> 
> Cédric Le Goater (6):
>   spapr/xive: Introduce a StoreEOI capability
>   spapr/xive: Add a warning when StoreEOI is activated on POWER8 CPUs
>   spapr/xive: Add a warning when StoreEOI is activated on POWER9 CPUs
>   spapr/xive: Enforce load-after-store ordering
>   spapr/xive: Activate StoreEOI at the source level
>   spapr/xive: Introduce a new CAS value for the StoreEOI capability
> 
>  include/hw/ppc/spapr.h      |  5 +++-
>  include/hw/ppc/spapr_xive.h |  1 +
>  include/hw/ppc/xive.h       |  8 +++++
>  target/ppc/kvm_ppc.h        |  6 ++++
>  hw/intc/spapr_xive.c        | 10 +++++++
>  hw/intc/spapr_xive_kvm.c    | 12 ++++++++
>  hw/intc/xive.c              |  6 ++++
>  hw/ppc/spapr.c              |  1 +
>  hw/ppc/spapr_caps.c         | 60 +++++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_hcall.c        |  7 +++++
>  hw/ppc/spapr_irq.c          |  6 ++++
>  target/ppc/kvm.c            | 18 +++++++++++
>  12 files changed, 139 insertions(+), 1 deletion(-)
> 

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

signature.asc
Description: PGP signature