target/ppc: Improve accuracy of guest HTM availability on P8s
cover target/ppc: Improve accuracy of guest HTM availability on P8s
Hi QEMU,
See the patch itself for a description of the issue it's fixing.
Additionally, I've done some investigation on the effect of the patch on older kernels. The discussion below only refers to the situation in which the existing workaround would have an effect (system is P8, KVM is HV and KVM does not indicate support for HTM):
PPC_FEATURE2_HTM has existed since mid 2013 [1], and at that time it was unconditionally set for P8: nothing will change here because the new test will always be true, always allowing the workaround to activate. The patch doesn't help here.
In early 2016 [2] PPC_FEATURE2_HTM was linked to the HTM bit of ibm,pa-features: the patch will help from here onwards.
So the patch doesn't fix all situations but it doesn't break any either, and it fixes versions going forward.
Cheers,
Sam.
1: Around kernel commit cbbc6f1b1433ef553d57826eee87a84ca49645ce (v3.10-rc1) 2: Around kernel commit 4705e02498d6d5a7ab98dfee9595cd5e91db2017 (v4.6-rc1)
Sam Bobroff (1):
target/ppc: Improve accuracy of guest HTM availability on P8s
target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Signature · 1 line
2.12.1.382.gc0f9c7058
PATCH 1/1 target/ppc: Improve accuracy of guest HTM availability on P8s
On Power8 hosts it is currently theoretically possible for QEMU/KVM-HV guests to receive a ibm,pa-features property indicating that HTM support is available when it is not. The situation would occur if the platform firmware of a Power8 host cleared the HTM bit of the ibm,pa-features property. QEMU would query KVM for the availability of HTM, which will return no support, but workaround code in kvm_arch_init_vcpu() would then re-enable it because KVM_HV is in use and the processor is P8.
This patch adjusts the workaround in kvm_arch_init_vcpu() so that it does not enable HTM (in the above case) unless the host kernel indicates to the QEMU process, via the auxiliary vector, that userspace can use HTM (via the HWCAP2 bit KVM_FEATURE2_HTM).
The reason to use the value from the auxiliary vector is that it is set based only on what the host kernel found in the ibm,pa-features HTM bit at boot time.
--- target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
target/ppc/kvm.c +6 -2
@@ -49,6 +49,7 @@#if defined(TARGET_PPC64 )#include "hw/ppc/spapr_cpu_core.h" #endif +#include "elf.h" //#define DEBUG_KVM
@@ -509,8 +510,11 @@ int kvm_arch_init_vcpu(CPUState *cs)case POWERPC_MMU_2_07 :if (!cap_htm && !kvmppc_is_pr (cs->kvm_state)) {/* KVM-HV has transactional memory on POWER8 also without the -* KVM_CAP_PPC_HTM extension, so enable it here instead. */ - cap_htm = true; +* KVM_CAP_PPC_HTM extension, so enable it here instead as +* long as it's availble to userspace on the host. */ +if (qemu_getauxval (AT_HWCAP2 ) &PPC_FEATURE2_HAS_HTM ) { + cap_htm = true; + } }break ;default :
Signature · 1 line
2.12.1.382.gc0f9c7058
Quoted 4 lines · Sam Bobroff
On Power8 hosts it is currently theoretically possible for QEMU/KVM-HV guests to receive a ibm,pa-features property indicating that HTM support is available when it is not. The situation would occur if the platform firmware of a Power8 host cleared the HTM bit of the ibm,pa-features property.
Out of curiosity: Is there a machine out there where this happens?
Quoted 31 lines
QEMU would query KVM for the availability of HTM, which will return no support, but workaround code in
kvm_arch_init_vcpu()would then re-enable it becauseKVM_HVis in use and the processor is P8.This patch adjusts the workaround in
kvm_arch_init_vcpu()so that it does not enable HTM (in the above case) unless the host kernel indicates to the QEMU process, via the auxiliary vector, that userspace can use HTM (via the HWCAP2 bitKVM_FEATURE2_HTM).The reason to use the value from the auxiliary vector is that it is set based only on what the host kernel found in the ibm,pa-features HTM bit at boot time.
Signed-off-by Sam Bobroff <[email protected]>--- target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target/ppc/kvm.c +1
-0@@ -49,6 +49,7 @@#if defined(TARGET_PPC64 )#include "hw/ppc/spapr_cpu_core.h" #endif +#include "elf.h" //#define DEBUG_KVM
target/ppc/kvm.c int kvm_arch_init_vcpu(CPUState *cs) · line 517
Quoted 10 lines
target/ppc/kvm.c +5
-2@@ -509,8 +510,11 @@ int kvm_arch_init_vcpu(CPUState *cs)case POWERPC_MMU_2_07 :if (!cap_htm && !kvmppc_is_pr (cs->kvm_state)) {/* KVM-HV has transactional memory on POWER8 also without the -* KVM_CAP_PPC_HTM extension, so enable it here instead. */ - cap_htm = true; +* KVM_CAP_PPC_HTM extension, so enable it here instead as +* long as it's availble to userspace on the host. */ +if (qemu_getauxval (AT_HWCAP2 ) &PPC_FEATURE2_HAS_HTM ) { + cap_htm = true; + }
That's a very good idea! ... but I think you could also merge the two if-statements into one to save one level of indentation.
Thomas
David Gibson
AppliedQuoted 13 lines · Sam Bobroff
On Power8 hosts it is currently theoretically possible for QEMU/KVM-HV guests to receive a ibm,pa-features property indicating that HTM support is available when it is not. The situation would occur if the platform firmware of a Power8 host cleared the HTM bit of the ibm,pa-features property. QEMU would query KVM for the availability of HTM, which will return no support, but workaround code in
kvm_arch_init_vcpu()would then re-enable it becauseKVM_HVis in use and the processor is P8.This patch adjusts the workaround in
kvm_arch_init_vcpu()so that it does not enable HTM (in the above case) unless the host kernel indicates to the QEMU process, via the auxiliary vector, that userspace can use HTM (via the HWCAP2 bitKVM_FEATURE2_HTM).
Quoted 5 lines
The reason to use the value from the auxiliary vector is that it is set based only on what the host kernel found in the ibm,pa-features HTM bit at boot time.
Signed-off-by Sam Bobroff <[email protected]>
Applied to ppc-for-2.9.
Quoted 30 lines
--- target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target/ppc/kvm.c +6
-2@@ -49,6 +49,7 @@#if defined(TARGET_PPC64 )#include "hw/ppc/spapr_cpu_core.h" #endif +#include "elf.h" //#define DEBUG_KVM @@ -509,8 +510,11 @@ int kvm_arch_init_vcpu(CPUState *cs)case POWERPC_MMU_2_07 :if (!cap_htm && !kvmppc_is_pr (cs->kvm_state)) {/* KVM-HV has transactional memory on POWER8 also without the -* KVM_CAP_PPC_HTM extension, so enable it here instead. */ - cap_htm = true; +* KVM_CAP_PPC_HTM extension, so enable it here instead as +* long as it's availble to userspace on the host. */ +if (qemu_getauxval (AT_HWCAP2 ) &PPC_FEATURE2_HAS_HTM ) { + cap_htm = true; + } }break ;default :
Signature · 4 lines
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 7 lines · Thomas Huth
Quoted 4 lines · Sam Bobroff
On Power8 hosts it is currently theoretically possible for QEMU/KVM-HV guests to receive a ibm,pa-features property indicating that HTM support is available when it is not. The situation would occur if the platform firmware of a Power8 host cleared the HTM bit of the ibm,pa-features property.
Out of curiosity: Is there a machine out there where this happens?
Not that I know of... just the one who's firmware I broke on purpose for testing ;-)
Quoted 46 lines
Quoted 31 lines
QEMU would query KVM for the availability of HTM, which will return no support, but workaround code in
kvm_arch_init_vcpu()would then re-enable it becauseKVM_HVis in use and the processor is P8.This patch adjusts the workaround in
kvm_arch_init_vcpu()so that it does not enable HTM (in the above case) unless the host kernel indicates to the QEMU process, via the auxiliary vector, that userspace can use HTM (via the HWCAP2 bitKVM_FEATURE2_HTM).The reason to use the value from the auxiliary vector is that it is set based only on what the host kernel found in the ibm,pa-features HTM bit at boot time.
Signed-off-by Sam Bobroff <[email protected]>--- target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target/ppc/kvm.c +1
-0@@ -49,6 +49,7 @@#if defined(TARGET_PPC64 )#include "hw/ppc/spapr_cpu_core.h" #endif +#include "elf.h" //#define DEBUG_KVM target/ppc/kvm.c int kvm_arch_init_vcpu(CPUState *cs) · line 517
Quoted 10 lines
target/ppc/kvm.c +5
-2@@ -509,8 +510,11 @@ int kvm_arch_init_vcpu(CPUState *cs)case POWERPC_MMU_2_07 :if (!cap_htm && !kvmppc_is_pr (cs->kvm_state)) {/* KVM-HV has transactional memory on POWER8 also without the -* KVM_CAP_PPC_HTM extension, so enable it here instead. */ - cap_htm = true; +* KVM_CAP_PPC_HTM extension, so enable it here instead as +* long as it's availble to userspace on the host. */ +if (qemu_getauxval (AT_HWCAP2 ) &PPC_FEATURE2_HAS_HTM ) { + cap_htm = true; + }That's a very good idea! ... but I think you could also merge the two if-statements into one to save one level of indentation.
Thomas
Quoted 11 lines · Sam Bobroff
Quoted 7 lines · Thomas Huth
Quoted 4 lines · Sam Bobroff
On Power8 hosts it is currently theoretically possible for QEMU/KVM-HV guests to receive a ibm,pa-features property indicating that HTM support is available when it is not. The situation would occur if the platform firmware of a Power8 host cleared the HTM bit of the ibm,pa-features property.
Out of curiosity: Is there a machine out there where this happens?
Not that I know of... just the one who's firmware I broke on purpose for testing ;-)
Ok, given that, I'm moving this fix from ppc-for-2.9 to ppc-for-2.10.
Quoted 48 lines
Quoted 47 lines
Quoted 32 lines
QEMU would query KVM for the availability of HTM, which will return no support, but workaround code in
kvm_arch_init_vcpu()would then re-enable it becauseKVM_HVis in use and the processor is P8.This patch adjusts the workaround in
kvm_arch_init_vcpu()so that it does not enable HTM (in the above case) unless the host kernel indicates to the QEMU process, via the auxiliary vector, that userspace can use HTM (via the HWCAP2 bitKVM_FEATURE2_HTM).The reason to use the value from the auxiliary vector is that it is set based only on what the host kernel found in the ibm,pa-features HTM bit at boot time.
Signed-off-by Sam Bobroff <[email protected]>--- target/ppc/kvm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)target/ppc/kvm.c +1
-0@@ -49,6 +49,7 @@#if defined(TARGET_PPC64 )#include "hw/ppc/spapr_cpu_core.h" #endif +#include "elf.h" //#define DEBUG_KVM target/ppc/kvm.c int kvm_arch_init_vcpu(CPUState *cs) · line 517
Quoted 10 lines
target/ppc/kvm.c +5
-2@@ -509,8 +510,11 @@ int kvm_arch_init_vcpu(CPUState *cs)case POWERPC_MMU_2_07 :if (!cap_htm && !kvmppc_is_pr (cs->kvm_state)) {/* KVM-HV has transactional memory on POWER8 also without the -* KVM_CAP_PPC_HTM extension, so enable it here instead. */ - cap_htm = true; +* KVM_CAP_PPC_HTM extension, so enable it here instead as +* long as it's availble to userspace on the host. */ +if (qemu_getauxval (AT_HWCAP2 ) &PPC_FEATURE2_HAS_HTM ) { + cap_htm = true; + }That's a very good idea! ... but I think you could also merge the two if-statements into one to save one level of indentation.
Thomas
Signature · 4 lines
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