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(-)
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. 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(-) -- 2.25.4
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
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(-)
>>
>
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
Sorry for the late answer I was out for a couple of weeks.
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 load offset that has the ordering. Oring 0x40 to the load
address :
#define XIVE_ESB_LOAD_EOI 0x000 /* Load */
#define XIVE_ESB_GET 0x800 /* Load */
#define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
#define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
#define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
#define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
will enforce load-after-store ordering.
We only need it for XIVE_ESB_SET_PQ_10. See commit b1f9be9392f0
("powerpc/xive: Enforce load-after-store ordering when StoreEOI is active")
in Linux.
C.
>
> 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(-)
>>
>
On Mon, Nov 02, 2020 at 02:22:35PM +0100, Cédric Le Goater wrote:
> Sorry for the late answer I was out for a couple of weeks.
>
> 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 load offset that has the ordering. Oring 0x40 to the load
> address :
>
> #define XIVE_ESB_LOAD_EOI 0x000 /* Load */
> #define XIVE_ESB_GET 0x800 /* Load */
> #define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
> #define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
> #define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
> #define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
>
> will enforce load-after-store ordering.
Oh... I had assumed the problem was to do with the load/store ordering
within the CPU core itself (or maybe the L1, I guess). But if the
address used can change it, the problem must be within the XIVE, yes?
Or at least somwhere on the Powerbus. So, wasn't this just a plain
XIVE hardware bug? In which case why is there software involvement as
well?
> We only need it for XIVE_ESB_SET_PQ_10. See commit b1f9be9392f0
> ("powerpc/xive: Enforce load-after-store ordering when StoreEOI is active")
> in Linux.
>
> C.
>
>
> >
> > 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
On 11/23/20 7:44 AM, David Gibson wrote:
> On Mon, Nov 02, 2020 at 02:22:35PM +0100, Cédric Le Goater wrote:
>> Sorry for the late answer I was out for a couple of weeks.
>>
>> 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 load offset that has the ordering. Oring 0x40 to the load
>> address :
>>
>> #define XIVE_ESB_LOAD_EOI 0x000 /* Load */
>> #define XIVE_ESB_GET 0x800 /* Load */
>> #define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
>> #define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
>> #define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
>> #define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
>>
>> will enforce load-after-store ordering.
>
> Oh... I had assumed the problem was to do with the load/store ordering
> within the CPU core itself (or maybe the L1, I guess). But if the
> address used can change it, the problem must be within the XIVE, yes?
Yes. It's in the XIVE logic handling the load/store operations on the
PQ bits.
> Or at least somwhere on the Powerbus. So, wasn't this just a plain
> XIVE hardware bug?
It's a theoretical bug in HW. StoreEOI is activated on the P9 systems
we use for performance testing and it never showed up.
> In which case why is there software involvement as well?
Software is involved as an optimization, because only PQ_10 loads need
the ordering enforcement.
commit b1f9be9392f0 in Linux says more :
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.
This ordering is a requirement for StoreEOI.
C.
© 2016 - 2024 Red Hat, Inc.