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

From: David Gibson <david@gibson.dropbear.id.au>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: Gustavo Romero <gromero@linux.ibm.com>,
	qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	Greg Kurz <groug@kaod.org>
Subject: Re: [PATCH v2 0/6] spapr/xive: Activate StoreEOI in P10 compat guests
Date: Mon, 12 Oct 2020 16:38:46 +1100	[thread overview]
Message-ID: <20201012053846.GE4787@yekko.fritz.box> (raw)
In-Reply-To: <5a3af480-8e84-ddb8-e40e-e3050f6c2978@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 2718 bytes --]

On Fri, Oct 09, 2020 at 07:57:32AM +0200, Cédric Le Goater wrote:
> 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.

no-op as in the load will have regular semantics, or as in the whole
load won't do anything?

I assume this meanse XIVE code needs to be updated to use that special
load for all accesses to XIVE registers... 

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

.. which I guess this does?

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

Ok.

> 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

Ok, seems reasonable

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

Drat, I guess ozlabs.org fell off the net for a while.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]