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

["Cédric Le Goater" <clg@kaod.org>]

On 10/9/20 2:23 AM, David Gibson wrote:
> 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?

It's a special form of load that has the ordering, only on available 
on P10. It's a no-op on P9. 

Linux commit b1f9be9392f0 ("powerpc/xive: Enforce load-after-store  
ordering when StoreEOI is active") introduced the Load-after-Store 
ordering offset and P10 support was added in the same 5.8 release.

This is why StoreEOI should be advertised on P10 compat kernels only. 
I would have preferred to introduce some extra CAS bits. that would 
have been cleaner than mix the two.


The basic requirement is to advertise StoreEOI when the CPU compat
allows it. I have used the capabilities to toggle the feature on/off.
It seemed a clean way to cover all the extra needs : 

 - switch it off on P10 if needed
 - switch it on on P9 for tests

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


Yes. I have received a few ot these : 
 
The original message was received at Mon, 5 Oct 2020 12:51:56 -0400
from m0098419.ppops.net [127.0.0.1]

   ----- The following addresses had permanent fatal errors -----
<david@gibson.dropbear.id.au>

   ----- Transcript of session follows -----
<david@gibson.dropbear.id.au>... Deferred: Name server: gibson.dropbear.id.au.: host name lookup failure
Message could not be delivered for 1 day
Message will be deleted from queue


Reporting-MTA: dns; mx0b-001b2d01.pphosted.com
Arrival-Date: Mon, 5 Oct 2020 12:51:56 -0400

Final-Recipient: RFC822; david@gibson.dropbear.id.au
Action: failed
Status: 4.4.7
Remote-MTA: DNS; gibson.dropbear.id.au
Last-Attempt-Date: Tue, 6 Oct 2020 13:06:28 -0400


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