KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs. An
option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/smp.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..a737a2f87c67 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,14 @@ static int pseries_smp_prepare_cpu(int cpu)
return 0;
}
+static void (*cause_ipi_offcore)(int cpu) __ro_after_init;
+
static void smp_pseries_cause_ipi(int cpu)
{
- /* POWER9 should not use this handler */
if (doorbell_try_core_ipi(cpu))
return;
- icp_ops->cause_ipi(cpu);
+ cause_ipi_offcore(cpu);
}
static int pseries_cause_nmi_ipi(int cpu)@@ -222,10 +223,7 @@ static __init void pSeries_smp_probe_xics(void)
{
xics_smp_probe();
- if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
- smp_ops->cause_ipi = smp_pseries_cause_ipi;
- else
- smp_ops->cause_ipi = icp_ops->cause_ipi;
+ smp_ops->cause_ipi = icp_ops->cause_ipi;
}
static __init void pSeries_smp_probe(void)@@ -238,6 +236,18 @@ static __init void pSeries_smp_probe(void)
xive_smp_probe();
else
pSeries_smp_probe_xics();
+
+ /*
+ * KVM emulates doorbells by reading the instruction, which
+ * can't be done if the guest is secure. If a secure guest
+ * runs under PowerVM, it could use msgsndp but would need a
+ * way to distinguish.
+ */
+ if (cpu_has_feature(CPU_FTR_DBELL) &&
+ cpu_has_feature(CPU_FTR_SMT) && !is_secure_guest()) {
+ cause_ipi_offcore = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = smp_pseries_cause_ipi;
+ }
}
static struct smp_ops_t pseries_smp_ops = {--
2.23.0
Nicholas Piggin [off-list ref] writes:
KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs.
You have to take explicit steps to configure KVM in that way with qemu.
eg. "qemu .. -smp 8" will give you 8 SMT1 CPUs by default.
An option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
Qemu/KVM populates /proc/device-tree/hypervisor, so we *could* look at
that. Though adding PowerVM/KVM specific hacks is obviously a very
slippery slope.
quoted hunk
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..a737a2f87c67 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,14 @@ static int pseries_smp_prepare_cpu(int cpu)
return 0;
}
+static void (*cause_ipi_offcore)(int cpu) __ro_after_init;
+
static void smp_pseries_cause_ipi(int cpu)
This is static so the name could be more descriptive, it doesn't need
the "smp_pseries" prefix.
{
- /* POWER9 should not use this handler */
if (doorbell_try_core_ipi(cpu))
return;
Seems like it would be worth making that static inline so we can avoid
the function call overhead.
quoted hunk
- icp_ops->cause_ipi(cpu);
+ cause_ipi_offcore(cpu);
}
static int pseries_cause_nmi_ipi(int cpu)
@@ -222,10 +223,7 @@ static __init void pSeries_smp_probe_xics(void)
{
xics_smp_probe();
- if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
- smp_ops->cause_ipi = smp_pseries_cause_ipi;
- else
- smp_ops->cause_ipi = icp_ops->cause_ipi;
+ smp_ops->cause_ipi = icp_ops->cause_ipi;
}
static __init void pSeries_smp_probe(void)@@ -238,6 +236,18 @@ static __init void pSeries_smp_probe(void)
The comment just above here says:
/*
* Don't use P9 doorbells when XIVE is enabled. IPIs
* using MMIOs should be faster
*/
xive_smp_probe();
Which is no longer true.
else
pSeries_smp_probe_xics();
I think you should just fold this in, it would make the logic slightly
easier to follow.
+ /*
+ * KVM emulates doorbells by reading the instruction, which
+ * can't be done if the guest is secure. If a secure guest
+ * runs under PowerVM, it could use msgsndp but would need a
+ * way to distinguish.
+ */
It's not clear what it needs to distinguish: That it's running under
PowerVM and therefore *can* use msgsndp even though it's secure.
Also the comment just talks about the is_secure_guest() test, which is
not obvious on first reading.
+ if (cpu_has_feature(CPU_FTR_DBELL) &&
+ cpu_has_feature(CPU_FTR_SMT) && !is_secure_guest()) {
+ cause_ipi_offcore = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = smp_pseries_cause_ipi;
+ }
Because we're at the tail of the function I think this would be clearer
if it used early returns, eg:
// If the CPU doesn't have doorbells then we must use xics/xive
if (!cpu_has_feature(CPU_FTR_DBELL))
return;
// If the CPU doesn't have SMT then doorbells don't help us
if (!cpu_has_feature(CPU_FTR_SMT))
return;
// Secure guests can't use doorbells because ...
if (!is_secure_guest()
return;
/*
* Otherwise we want to use doorbells for sibling threads and
* xics/xive for IPIs off the core, because it performs better
* on large systems ...
*/
cause_ipi_offcore = smp_ops->cause_ipi;
smp_ops->cause_ipi = smp_pseries_cause_ipi;
}
cheers
[ ... ]
quoted
quoted
An option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
An option vector would require a PAPR change. Unless the architecture
reserves some bits for the implementation, but I don't think so. Same
for CAS.
quoted
Qemu/KVM populates /proc/device-tree/hypervisor, so we *could* look at
that. Though adding PowerVM/KVM specific hacks is obviously a very
slippery slope.
QEMU could advertise a property "emulated-msgsndp", or something similar,
which would be interpreted by Linux as a CPU feature and taken into account
when doing the IPIs.
This is really a PowerVM optimization.
The CPU setup for XIVE needs a cleanup also. There is no need to allocate
interrupts for IPIs anymore in that case.
We need to keep these for the other cores. The XIVE layer is unchanged.
C.
quoted
quoted
An option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
An option vector would require a PAPR change. Unless the architecture
reserves some bits for the implementation, but I don't think so. Same
for CAS.
quoted
Qemu/KVM populates /proc/device-tree/hypervisor, so we *could* look at
that. Though adding PowerVM/KVM specific hacks is obviously a very
slippery slope.
QEMU could advertise a property "emulated-msgsndp", or something similar,
which would be interpreted by Linux as a CPU feature and taken into account
when doing the IPIs.
Could we remove msgsndp support from HFSCR in KVM and test it in pseries ?
C.
Adding David,
On 6/25/20 3:11 AM, Michael Ellerman wrote:
Nicholas Piggin [off-list ref] writes:
quoted
KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs.
You have to take explicit steps to configure KVM in that way with qemu.
eg. "qemu .. -smp 8" will give you 8 SMT1 CPUs by default.
quoted
An option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
An option vector would require a PAPR change. Unless the architecture
reserves some bits for the implementation, but I don't think so. Same
for CAS.
Qemu/KVM populates /proc/device-tree/hypervisor, so we *could* look at
that. Though adding PowerVM/KVM specific hacks is obviously a very
slippery slope.
QEMU could advertise a property "emulated-msgsndp", or something similar,
which would be interpreted by Linux as a CPU feature and taken into account
when doing the IPIs.
The CPU setup for XIVE needs a cleanup also. There is no need to allocate
interrupts for IPIs anymore in that case.
Thanks,
C.
quoted
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 6891710833be..a737a2f87c67 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -188,13 +188,14 @@ static int pseries_smp_prepare_cpu(int cpu)
return 0;
}
+static void (*cause_ipi_offcore)(int cpu) __ro_after_init;
+
static void smp_pseries_cause_ipi(int cpu)
This is static so the name could be more descriptive, it doesn't need
the "smp_pseries" prefix.
quoted
{
- /* POWER9 should not use this handler */
if (doorbell_try_core_ipi(cpu))
return;
Seems like it would be worth making that static inline so we can avoid
the function call overhead.
quoted
- icp_ops->cause_ipi(cpu);
+ cause_ipi_offcore(cpu);
}
static int pseries_cause_nmi_ipi(int cpu)
@@ -222,10 +223,7 @@ static __init void pSeries_smp_probe_xics(void)
{
xics_smp_probe();
- if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
- smp_ops->cause_ipi = smp_pseries_cause_ipi;
- else
- smp_ops->cause_ipi = icp_ops->cause_ipi;
+ smp_ops->cause_ipi = icp_ops->cause_ipi;
}
static __init void pSeries_smp_probe(void)@@ -238,6 +236,18 @@ static __init void pSeries_smp_probe(void)
The comment just above here says:
/*
* Don't use P9 doorbells when XIVE is enabled. IPIs
* using MMIOs should be faster
*/
quoted
xive_smp_probe();
Which is no longer true.
quoted
else
pSeries_smp_probe_xics();
I think you should just fold this in, it would make the logic slightly
easier to follow.
quoted
+ /*
+ * KVM emulates doorbells by reading the instruction, which
+ * can't be done if the guest is secure. If a secure guest
+ * runs under PowerVM, it could use msgsndp but would need a
+ * way to distinguish.
+ */
It's not clear what it needs to distinguish: That it's running under
PowerVM and therefore *can* use msgsndp even though it's secure.
Also the comment just talks about the is_secure_guest() test, which is
not obvious on first reading.
quoted
+ if (cpu_has_feature(CPU_FTR_DBELL) &&
+ cpu_has_feature(CPU_FTR_SMT) && !is_secure_guest()) {
+ cause_ipi_offcore = smp_ops->cause_ipi;
+ smp_ops->cause_ipi = smp_pseries_cause_ipi;
+ }
Because we're at the tail of the function I think this would be clearer
if it used early returns, eg:
// If the CPU doesn't have doorbells then we must use xics/xive
if (!cpu_has_feature(CPU_FTR_DBELL))
return;
// If the CPU doesn't have SMT then doorbells don't help us
if (!cpu_has_feature(CPU_FTR_SMT))
return;
// Secure guests can't use doorbells because ...
if (!is_secure_guest()
return;
/*
* Otherwise we want to use doorbells for sibling threads and
* xics/xive for IPIs off the core, because it performs better
* on large systems ...
*/
cause_ipi_offcore = smp_ops->cause_ipi;
smp_ops->cause_ipi = smp_pseries_cause_ipi;
}
cheers
Excerpts from Cédric Le Goater's message of June 26, 2020 5:17 pm:
Adding David,
On 6/25/20 3:11 AM, Michael Ellerman wrote:
quoted
Nicholas Piggin [off-list ref] writes:
quoted
KVM supports msgsndp in guests by trapping and emulating the
instruction, so it was decided to always use XIVE for IPIs if it is
available. However on PowerVM systems, msgsndp can be used and gives
better performance. On large systems, high XIVE interrupt rates can
have sub-linear scaling, and using msgsndp can reduce the load on
the interrupt controller.
So switch to using core local doorbells even if XIVE is available.
This reduces performance for KVM guests with an SMT topology by
about 50% for ping-pong context switching between SMT vCPUs.
You have to take explicit steps to configure KVM in that way with qemu.
eg. "qemu .. -smp 8" will give you 8 SMT1 CPUs by default.
quoted
An option vector (or dt-cpu-ftrs) could be defined to disable msgsndp
to get KVM performance back.
An option vector would require a PAPR change. Unless the architecture
reserves some bits for the implementation, but I don't think so. Same
for CAS.
quoted
Qemu/KVM populates /proc/device-tree/hypervisor, so we *could* look at
that. Though adding PowerVM/KVM specific hacks is obviously a very
slippery slope.
QEMU could advertise a property "emulated-msgsndp", or something similar,
which would be interpreted by Linux as a CPU feature and taken into account
when doing the IPIs.
What I'm going to do is detect KVM here (we already have a KVM detection
test using that dt property). The IPI setup code already has KVM hacks
in it, so I don't really see the problem with puting them behind a KVM
test.
I think doing cpu ftrs or some specific entry for msgsndp in particular
is the right way to go, but in the interests of making existing KVM work
I'll do this.
Thanks,
Nick