Misbehaving guests can cause bus locks to degrade the performance of a system. Non-WB (write-back) and misaligned locked RMW (read-modify-write) instructions are referred to as "bus locks" and require system wide synchronization among all processors to guarantee the atomicity. The bus locks can impose notable performance penalties for all processors within the system.
Support for the Bus Lock Threshold is indicated by CPUID Fn8000_000A_EDX[29] BusLockThreshold=1, the VMCB provides a Bus Lock Threshold enable bit and an unsigned 16-bit Bus Lock Threshold count.
VMCB intercept bit VMCB Offset Bits Function 14h 5 Intercept bus lock operations
Bus lock threshold count VMCB Offset Bits Function 120h 15:0 Bus lock counter
During VMRUN, the bus lock threshold count is fetched and stored in an internal count register. Prior to executing a bus lock within the guest, the processor verifies the count in the bus lock register. If the count is greater than zero, the processor executes the bus lock, reducing the count. However, if the count is zero, the bus lock operation is not performed, and instead, a Bus Lock Threshold #VMEXIT is triggered to transfer control to the Virtual Machine Monitor (VMM).
A Bus Lock Threshold #VMEXIT is reported to the VMM with VMEXIT code 0xA5h, VMEXIT_BUSLOCK. EXITINFO1 and EXITINFO2 are set to 0 on a VMEXIT_BUSLOCK. On a #VMEXIT, the processor writes the current value of the Bus Lock Threshold Counter to the VMCB.
More details about the Bus Lock Threshold feature can be found in AMD APM [1].
v1 -> v2 - Incorporated misc review comments from Sean. - Removed bus_lock_counter module parameter. - Set the value of bus_lock_counter to zero by default and reload the value by 1 in bus lock exit handler. - Add documentation for the behavioral difference for KVM_EXIT_BUS_LOCK. - Improved selftest for buslock to work on SVM and VMX. - Rewrite the commit messages.
Patches are prepared on kvm-next/next (0cdcc99eeaed)
Testing done: - Added a selftest for the Bus Lock Threshold functionality. - The bus lock threshold selftest has been tested on both Intel and AMD platforms. - Tested the Bus Lock Threshold functionality on SEV and SEV-ES guests. - Tested the Bus Lock Threshold functionality on nested guests.
v1: https://lore.kernel.org/kvm/20240709175145.9986-4-manali.shukla@amd.com/T/ [1]: AMD64 Architecture Programmer's Manual Pub. 24593, April 2024, Vol 2, 15.14.5 Bus Lock Threshold. https://bugzilla.kernel.org/attachment.cgi?id=306250
Manali Shukla (3): x86/cpu: Add virt tag in /proc/cpuinfo x86/cpufeatures: Add CPUID feature bit for the Bus Lock Threshold KVM: X86: Add documentation about behavioral difference for KVM_EXIT_BUS_LOCK
Nikunj A Dadhania (2): KVM: SVM: Enable Bus lock threshold exit KVM: selftests: Add bus lock exit test
Documentation/virt/kvm/api.rst | 5 + arch/x86/include/asm/cpufeature.h | 1 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/svm.h | 5 +- arch/x86/include/uapi/asm/svm.h | 2 + arch/x86/kernel/cpu/mkcapflags.sh | 3 + arch/x86/kernel/cpu/proc.c | 5 + arch/x86/kvm/svm/nested.c | 12 ++ arch/x86/kvm/svm/svm.c | 29 ++++ tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/x86_64/kvm_buslock_test.c | 130 ++++++++++++++++++ 11 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/kvm/x86_64/kvm_buslock_test.c
base-commit: 0cdcc99eeaedf2422c80d75760293fdbb476cec1
Add support for generating Virtualization feature names in capflags.c and use the resulting x86_virt_flags to print the virt flags in /proc/cpuinfo.
Currently, it is difficult to check if a feature is supported in _KVM_. Manually querying cpuid to know whether the feature is supported or not can be quite tedious at times.
Print the features supported in KVM hypervisor in a dedicated "virt" line instead of adding them to the common "flags".
To do so, define flags which are needed to be added in a dedicated virt line in /proc/cpuinfo with prefix X86_VIRT_FEATURE_.
Signed-off-by: Manali Shukla manali.shukla@amd.com --- arch/x86/include/asm/cpufeature.h | 1 + arch/x86/kernel/cpu/mkcapflags.sh | 3 +++ arch/x86/kernel/cpu/proc.c | 5 +++++ 3 files changed, 9 insertions(+)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h index 0b9611da6c53..74c52bfd8cf2 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -41,6 +41,7 @@ enum cpuid_leafs #define x86_cap_flag_num(flag) ((flag) >> 5), ((flag) & 31)
extern const char * const x86_cap_flags[NCAPINTS*32]; +extern const char * const x86_virt_flags[NCAPINTS*32]; extern const char * const x86_power_flags[32]; #define X86_CAP_FMT "%s" #define x86_cap_flag(flag) x86_cap_flags[flag] diff --git a/arch/x86/kernel/cpu/mkcapflags.sh b/arch/x86/kernel/cpu/mkcapflags.sh index 68f537347466..3671c7892c56 100644 --- a/arch/x86/kernel/cpu/mkcapflags.sh +++ b/arch/x86/kernel/cpu/mkcapflags.sh @@ -62,6 +62,9 @@ trap 'rm "$OUT"' EXIT dump_array "x86_bug_flags" "NBUGINTS*32" "X86_BUG_" "NCAPINTS*32" $2 echo ""
+ dump_array "x86_virt_flags" "NCAPINTS*32" "X86_VIRT_FEATURE_" "" $2 + echo "" + echo "#ifdef CONFIG_X86_VMX_FEATURE_NAMES" echo "#ifndef _ASM_X86_VMXFEATURES_H" echo "#include <asm/vmxfeatures.h>" diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c index e65fae63660e..3068b0a110e4 100644 --- a/arch/x86/kernel/cpu/proc.c +++ b/arch/x86/kernel/cpu/proc.c @@ -103,6 +103,11 @@ static int show_cpuinfo(struct seq_file *m, void *v) if (cpu_has(c, i) && x86_cap_flags[i] != NULL) seq_printf(m, " %s", x86_cap_flags[i]);
+ seq_puts(m, "\nvirt\t\t:"); + for (i = 0; i < 32*NCAPINTS; i++) + if (cpu_has(c, i) && x86_virt_flags[i] != NULL) + seq_printf(m, " %s", x86_virt_flags[i]); + #ifdef CONFIG_X86_VMX_FEATURE_NAMES if (cpu_has(c, X86_FEATURE_VMX) && c->vmx_capability[0]) { seq_puts(m, "\nvmx flags\t:");
On Tue, Oct 01, 2024, Manali Shukla wrote:
Add support for generating Virtualization feature names in capflags.c and use the resulting x86_virt_flags to print the virt flags in /proc/cpuinfo.
Currently, it is difficult to check if a feature is supported in _KVM_. Manually querying cpuid to know whether the feature is supported or not can be quite tedious at times.
Print the features supported in KVM hypervisor in a dedicated "virt" line instead of adding them to the common "flags".
First off, printing flags in a separate section doesn't magically connect them to KVM support. E.g. if you cut this series after patch 2, BUS_LOCK_THRESHOLD will show up in "virt" despite KVM not supporting the feature.
Second, deviating from the X86_FEATURE_* syntax will make it _harder_ for KVM to manage its configuration.
Third, this is completely orthogonal to supporting bus lock threshold in KVM, i.e. belongs in a separate series.
On 10/1/2024 10:11 PM, Sean Christopherson wrote:
On Tue, Oct 01, 2024, Manali Shukla wrote:
Add support for generating Virtualization feature names in capflags.c and use the resulting x86_virt_flags to print the virt flags in /proc/cpuinfo.
Currently, it is difficult to check if a feature is supported in _KVM_. Manually querying cpuid to know whether the feature is supported or not can be quite tedious at times.
Print the features supported in KVM hypervisor in a dedicated "virt" line instead of adding them to the common "flags".
First off, printing flags in a separate section doesn't magically connect them to KVM support. E.g. if you cut this series after patch 2, BUS_LOCK_THRESHOLD will show up in "virt" despite KVM not supporting the feature.
Second, deviating from the X86_FEATURE_* syntax will make it _harder_ for KVM to manage its configuration.
Third, this is completely orthogonal to supporting bus lock threshold in KVM, i.e. belongs in a separate series.
Okay. I agree to your point. I will remove the patch from this series.
-Manali
Misbehaving guests can cause bus locks to degrade the performance of the system. The Bus Lock Threshold feature can be used to address this issue by providing capability to the hypervisor to limit guest's ability to generate bus lock, thereby preventing system slowdown due to performance penalities.
When the Bus Lock Threshold feature is enabled, the processor checks the bus lock threshold count before executing the buslock and decides whether to trigger bus lock exit or not.
The value of the bus lock threshold count '0' generates bus lock exits, and if the value is greater than '0', the bus lock is executed successfully and the bus lock threshold count is decremented.
Presence of the Bus Lock threshold feature is indicated via CPUID function 0x8000000A_EDX[29].
Signed-off-by: Manali Shukla manali.shukla@amd.com --- arch/x86/include/asm/cpufeatures.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index dd4682857c12..20f389ee0079 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -382,6 +382,7 @@ #define X86_FEATURE_V_SPEC_CTRL (15*32+20) /* "v_spec_ctrl" Virtual SPEC_CTRL */ #define X86_FEATURE_VNMI (15*32+25) /* "vnmi" Virtual NMI */ #define X86_FEATURE_SVME_ADDR_CHK (15*32+28) /* SVME addr check */ +#define X86_VIRT_FEATURE_BUS_LOCK_THRESHOLD (15*32+29) /* "buslock" Bus lock threshold */
/* Intel-defined CPU features, CPUID level 0x00000007:0 (ECX), word 16 */ #define X86_FEATURE_AVX512VBMI (16*32+ 1) /* "avx512vbmi" AVX512 Vector Bit Manipulation instructions*/
From: Nikunj A Dadhania nikunj@amd.com
Virtual machines can exploit bus locks to degrade the performance of the system. Bus locks can be caused by Non-WB(Write back) and misaligned locked RMW (Read-modify-Write) instructions and require systemwide synchronization among all processors which can result into significant performance penalties.
To address this issue, the Bus Lock Threshold feature is introduced to provide ability to hypervisor to restrict guests' capability of initiating mulitple buslocks, thereby preventing system slowdowns.
Support for the buslock threshold is indicated via CPUID function 0x8000000A_EDX[29].
On the processors that support the Bus Lock Threshold feature, the VMCB provides a Bus Lock Threshold enable bit and an unsigned 16-bit Bus Lock threshold count.
VMCB intercept bit VMCB Offset Bits Function 14h 5 Intercept bus lock operations (occurs after guest instruction finishes)
Bus lock threshold count VMCB Offset Bits Function 120h 15:0 Bus lock counter
When a VMRUN instruction is executed, the bus lock threshold count is loaded into an internal count register. Before the processor executes a bus lock in the guest, it checks the value of this register:
- If the value is greater than '0', the processor successfully executes the bus lock and decrements the count.
- If the value is '0', the bus lock is not executed, and a #VMEXIT to the VMM is taken.
The bus lock threshold #VMEXIT is reported to the VMM with the VMEXIT code A5h, SVM_EXIT_BUS_LOCK.
When SVM_EXIT_BUS_LOCK occurs, the value of the current Bus Lock Threshold counter, which is '0', is loaded into the VMCB. This value must be reset to a default greater than '0' in bus_lock_exit handler in hypervisor, because the behavior of SVM_EXIT_BUS_LOCK is fault like, so the bus lock exit to userspace happens with the RIP pointing to the offending instruction (which generates buslocks).
So, if the value of the Bus Lock Threshold counter remains '0', the guest continuously exits with SVM_EXIT_BUS_LOCK.
The generated SVM_EXIT_BUS_LOCK in kvm will exit to user space by setting the KVM_RUN_BUS_LOCK flag in vcpu->run->flags, indicating to the user space that a bus lock has been detected in the guest.
Use the already available KVM capability KVM_CAP_X86_BUS_LOCK_EXIT to enable the feature. This feature is disabled by default, it can be enabled explicitly from user space.
More details about the Bus Lock Threshold feature can be found in AMD APM [1].
[1]: AMD64 Architecture Programmer's Manual Pub. 24593, April 2024, Vol 2, 15.14.5 Bus Lock Threshold. https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Nikunj A Dadhania nikunj@amd.com Co-developed-by: Manali Shukla manali.shukla@amd.com Signed-off-by: Manali Shukla manali.shukla@amd.com --- arch/x86/include/asm/svm.h | 5 ++++- arch/x86/include/uapi/asm/svm.h | 2 ++ arch/x86/kvm/svm/nested.c | 12 ++++++++++++ arch/x86/kvm/svm/svm.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index f0dea3750ca9..bad9723f40e1 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -116,6 +116,7 @@ enum { INTERCEPT_INVPCID, INTERCEPT_MCOMMIT, INTERCEPT_TLBSYNC, + INTERCEPT_BUSLOCK, };
@@ -158,7 +159,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area { u64 avic_physical_id; /* Offset 0xf8 */ u8 reserved_7[8]; u64 vmsa_pa; /* Used for an SEV-ES guest */ - u8 reserved_8[720]; + u8 reserved_8[16]; + u16 bus_lock_counter; /* Offset 0x120 */ + u8 reserved_9[702]; /* * Offset 0x3e0, 32 bytes reserved * for use by hypervisor/software. diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index 1814b413fd57..abf6aed88cee 100644 --- a/arch/x86/include/uapi/asm/svm.h +++ b/arch/x86/include/uapi/asm/svm.h @@ -95,6 +95,7 @@ #define SVM_EXIT_CR14_WRITE_TRAP 0x09e #define SVM_EXIT_CR15_WRITE_TRAP 0x09f #define SVM_EXIT_INVPCID 0x0a2 +#define SVM_EXIT_BUS_LOCK 0x0a5 #define SVM_EXIT_NPF 0x400 #define SVM_EXIT_AVIC_INCOMPLETE_IPI 0x401 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS 0x402 @@ -224,6 +225,7 @@ { SVM_EXIT_CR4_WRITE_TRAP, "write_cr4_trap" }, \ { SVM_EXIT_CR8_WRITE_TRAP, "write_cr8_trap" }, \ { SVM_EXIT_INVPCID, "invpcid" }, \ + { SVM_EXIT_BUS_LOCK, "buslock" }, \ { SVM_EXIT_NPF, "npf" }, \ { SVM_EXIT_AVIC_INCOMPLETE_IPI, "avic_incomplete_ipi" }, \ { SVM_EXIT_AVIC_UNACCELERATED_ACCESS, "avic_unaccelerated_access" }, \ diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 6f704c1037e5..670092d31f77 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -669,6 +669,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm, vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa; vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
+ /* + * The bus lock threshold count should keep running across nested + * transitions. + * Copied the buslock threshold count from vmcb01 to vmcb02. + */ + vmcb02->control.bus_lock_counter = vmcb01->control.bus_lock_counter; /* Done at vmrun: asid. */
/* Also overwritten later if necessary. */ @@ -1035,6 +1041,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
}
+ /* + * The bus lock threshold count should keep running across nested + * transitions. + * Copied the buslock threshold count from vmcb02 to vmcb01. + */ + vmcb01->control.bus_lock_counter = vmcb02->control.bus_lock_counter; nested_svm_copy_common_state(svm->nested.vmcb02.ptr, svm->vmcb01.ptr);
svm_switch_vmcb(svm, &svm->vmcb01); diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 5ab2c92c7331..41c773a40f99 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -1372,6 +1372,11 @@ static void init_vmcb(struct kvm_vcpu *vcpu) svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK; }
+ if (vcpu->kvm->arch.bus_lock_detection_enabled) + svm_set_intercept(svm, INTERCEPT_BUSLOCK); + else + svm_clr_intercept(svm, INTERCEPT_BUSLOCK); + if (sev_guest(vcpu->kvm)) sev_init_vmcb(svm);
@@ -3283,6 +3288,24 @@ static int invpcid_interception(struct kvm_vcpu *vcpu) return kvm_handle_invpcid(vcpu, type, gva); }
+static int bus_lock_exit(struct kvm_vcpu *vcpu) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK; + vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK; + + /* + * Reload the counter with value greater than '0'. + * The bus lock exit on SVM happens with RIP pointing to the guilty + * instruction. So, reloading the value of bus_lock_counter to '0' + * results into generating continuous bus lock exits. + */ + svm->vmcb->control.bus_lock_counter = 1; + + return 0; +} + static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_READ_CR0] = cr_interception, [SVM_EXIT_READ_CR3] = cr_interception, @@ -3350,6 +3373,7 @@ static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_CR4_WRITE_TRAP] = cr_trap, [SVM_EXIT_CR8_WRITE_TRAP] = cr_trap, [SVM_EXIT_INVPCID] = invpcid_interception, + [SVM_EXIT_BUS_LOCK] = bus_lock_exit, [SVM_EXIT_NPF] = npf_interception, [SVM_EXIT_RSM] = rsm_interception, [SVM_EXIT_AVIC_INCOMPLETE_IPI] = avic_incomplete_ipi_interception, @@ -5212,6 +5236,11 @@ static __init void svm_set_cpu_caps(void) kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK); }
+ if (cpu_feature_enabled(X86_VIRT_FEATURE_BUS_LOCK_THRESHOLD)) { + pr_info("Bus Lock Threashold supported\n"); + kvm_caps.has_bus_lock_exit = true; + } + /* CPUID 0x80000008 */ if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) || boot_cpu_has(X86_FEATURE_AMD_SSBD))
On 10/1/24 01:34, Manali Shukla wrote:
From: Nikunj A Dadhania nikunj@amd.com
Virtual machines can exploit bus locks to degrade the performance of the system. Bus locks can be caused by Non-WB(Write back) and misaligned locked RMW (Read-modify-Write) instructions and require systemwide synchronization among all processors which can result into significant performance penalties.
To address this issue, the Bus Lock Threshold feature is introduced to provide ability to hypervisor to restrict guests' capability of initiating mulitple buslocks, thereby preventing system slowdowns.
Support for the buslock threshold is indicated via CPUID function 0x8000000A_EDX[29].
On the processors that support the Bus Lock Threshold feature, the VMCB provides a Bus Lock Threshold enable bit and an unsigned 16-bit Bus Lock threshold count.
VMCB intercept bit VMCB Offset Bits Function 14h 5 Intercept bus lock operations (occurs after guest instruction finishes)
Bus lock threshold count VMCB Offset Bits Function 120h 15:0 Bus lock counter
When a VMRUN instruction is executed, the bus lock threshold count is loaded into an internal count register. Before the processor executes a bus lock in the guest, it checks the value of this register:
If the value is greater than '0', the processor successfully executes the bus lock and decrements the count.
If the value is '0', the bus lock is not executed, and a #VMEXIT to the VMM is taken.
The bus lock threshold #VMEXIT is reported to the VMM with the VMEXIT code A5h, SVM_EXIT_BUS_LOCK.
When SVM_EXIT_BUS_LOCK occurs, the value of the current Bus Lock Threshold counter, which is '0', is loaded into the VMCB. This value must be reset to a default greater than '0' in bus_lock_exit handler in hypervisor, because the behavior of SVM_EXIT_BUS_LOCK is fault like, so the bus lock exit to userspace happens with the RIP pointing to the offending instruction (which generates buslocks).
So, if the value of the Bus Lock Threshold counter remains '0', the guest continuously exits with SVM_EXIT_BUS_LOCK.
The generated SVM_EXIT_BUS_LOCK in kvm will exit to user space by setting the KVM_RUN_BUS_LOCK flag in vcpu->run->flags, indicating to the user space that a bus lock has been detected in the guest.
Use the already available KVM capability KVM_CAP_X86_BUS_LOCK_EXIT to enable the feature. This feature is disabled by default, it can be enabled explicitly from user space.
More details about the Bus Lock Threshold feature can be found in AMD APM [1].
You should mention in the commit message that this implementation is set up to intercept every guest bus lock. The initial value of the threshold is 0 in the VMCB, so the very first bus lock will exit, set the threshold to 1 (so that the offending instruction can make forward progress) but then the value is at 0 again so the next bus lock will exit.
[1]: AMD64 Architecture Programmer's Manual Pub. 24593, April 2024, Vol 2, 15.14.5 Bus Lock Threshold. https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Nikunj A Dadhania nikunj@amd.com Co-developed-by: Manali Shukla manali.shukla@amd.com Signed-off-by: Manali Shukla manali.shukla@amd.com
arch/x86/include/asm/svm.h | 5 ++++- arch/x86/include/uapi/asm/svm.h | 2 ++ arch/x86/kvm/svm/nested.c | 12 ++++++++++++ arch/x86/kvm/svm/svm.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index f0dea3750ca9..bad9723f40e1 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -116,6 +116,7 @@ enum { INTERCEPT_INVPCID, INTERCEPT_MCOMMIT, INTERCEPT_TLBSYNC,
- INTERCEPT_BUSLOCK,
}; @@ -158,7 +159,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area { u64 avic_physical_id; /* Offset 0xf8 */ u8 reserved_7[8]; u64 vmsa_pa; /* Used for an SEV-ES guest */
- u8 reserved_8[720];
- u8 reserved_8[16];
- u16 bus_lock_counter; /* Offset 0x120 */
- u8 reserved_9[702]; /*
- Offset 0x3e0, 32 bytes reserved
- for use by hypervisor/software.
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index 1814b413fd57..abf6aed88cee 100644 --- a/arch/x86/include/uapi/asm/svm.h +++ b/arch/x86/include/uapi/asm/svm.h @@ -95,6 +95,7 @@ #define SVM_EXIT_CR14_WRITE_TRAP 0x09e #define SVM_EXIT_CR15_WRITE_TRAP 0x09f #define SVM_EXIT_INVPCID 0x0a2 +#define SVM_EXIT_BUS_LOCK 0x0a5 #define SVM_EXIT_NPF 0x400 #define SVM_EXIT_AVIC_INCOMPLETE_IPI 0x401 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS 0x402 @@ -224,6 +225,7 @@ { SVM_EXIT_CR4_WRITE_TRAP, "write_cr4_trap" }, \ { SVM_EXIT_CR8_WRITE_TRAP, "write_cr8_trap" }, \ { SVM_EXIT_INVPCID, "invpcid" }, \
- { SVM_EXIT_BUS_LOCK, "buslock" }, \ { SVM_EXIT_NPF, "npf" }, \ { SVM_EXIT_AVIC_INCOMPLETE_IPI, "avic_incomplete_ipi" }, \ { SVM_EXIT_AVIC_UNACCELERATED_ACCESS, "avic_unaccelerated_access" }, \
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 6f704c1037e5..670092d31f77 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -669,6 +669,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm, vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa; vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
- /*
* The bus lock threshold count should keep running across nested* transitions.* Copied the buslock threshold count from vmcb01 to vmcb02.
No need to start this part of the comment on a separate line. Also, s/Copied/Copy/ (ditto below).
*/- vmcb02->control.bus_lock_counter = vmcb01->control.bus_lock_counter; /* Done at vmrun: asid. */
/* Also overwritten later if necessary. */ @@ -1035,6 +1041,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm) }
- /*
* The bus lock threshold count should keep running across nested* transitions.* Copied the buslock threshold count from vmcb02 to vmcb01.*/- vmcb01->control.bus_lock_counter = vmcb02->control.bus_lock_counter; nested_svm_copy_common_state(svm->nested.vmcb02.ptr, svm->vmcb01.ptr);
svm_switch_vmcb(svm, &svm->vmcb01); diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 5ab2c92c7331..41c773a40f99 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -1372,6 +1372,11 @@ static void init_vmcb(struct kvm_vcpu *vcpu) svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK; }
- if (vcpu->kvm->arch.bus_lock_detection_enabled)
svm_set_intercept(svm, INTERCEPT_BUSLOCK);- else
svm_clr_intercept(svm, INTERCEPT_BUSLOCK);
Is the else path really needed?
Thanks, Tom
- if (sev_guest(vcpu->kvm)) sev_init_vmcb(svm);
@@ -3283,6 +3288,24 @@ static int invpcid_interception(struct kvm_vcpu *vcpu) return kvm_handle_invpcid(vcpu, type, gva); } +static int bus_lock_exit(struct kvm_vcpu *vcpu) +{
- struct vcpu_svm *svm = to_svm(vcpu);
- vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
- vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
- /*
* Reload the counter with value greater than '0'.* The bus lock exit on SVM happens with RIP pointing to the guilty* instruction. So, reloading the value of bus_lock_counter to '0'* results into generating continuous bus lock exits.*/- svm->vmcb->control.bus_lock_counter = 1;
- return 0;
+}
static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_READ_CR0] = cr_interception, [SVM_EXIT_READ_CR3] = cr_interception, @@ -3350,6 +3373,7 @@ static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_CR4_WRITE_TRAP] = cr_trap, [SVM_EXIT_CR8_WRITE_TRAP] = cr_trap, [SVM_EXIT_INVPCID] = invpcid_interception,
- [SVM_EXIT_BUS_LOCK] = bus_lock_exit, [SVM_EXIT_NPF] = npf_interception, [SVM_EXIT_RSM] = rsm_interception, [SVM_EXIT_AVIC_INCOMPLETE_IPI] = avic_incomplete_ipi_interception,
@@ -5212,6 +5236,11 @@ static __init void svm_set_cpu_caps(void) kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK); }
- if (cpu_feature_enabled(X86_VIRT_FEATURE_BUS_LOCK_THRESHOLD)) {
pr_info("Bus Lock Threashold supported\n");kvm_caps.has_bus_lock_exit = true;- }
- /* CPUID 0x80000008 */ if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) || boot_cpu_has(X86_FEATURE_AMD_SSBD))
Hi Tom,
Thank you for reviewing my patches.
On 10/1/2024 7:13 PM, Tom Lendacky wrote:
On 10/1/24 01:34, Manali Shukla wrote:
From: Nikunj A Dadhania nikunj@amd.com
Virtual machines can exploit bus locks to degrade the performance of the system. Bus locks can be caused by Non-WB(Write back) and misaligned locked RMW (Read-modify-Write) instructions and require systemwide synchronization among all processors which can result into significant performance penalties.
To address this issue, the Bus Lock Threshold feature is introduced to provide ability to hypervisor to restrict guests' capability of initiating mulitple buslocks, thereby preventing system slowdowns.
Support for the buslock threshold is indicated via CPUID function 0x8000000A_EDX[29].
On the processors that support the Bus Lock Threshold feature, the VMCB provides a Bus Lock Threshold enable bit and an unsigned 16-bit Bus Lock threshold count.
VMCB intercept bit VMCB Offset Bits Function 14h 5 Intercept bus lock operations (occurs after guest instruction finishes)
Bus lock threshold count VMCB Offset Bits Function 120h 15:0 Bus lock counter
When a VMRUN instruction is executed, the bus lock threshold count is loaded into an internal count register. Before the processor executes a bus lock in the guest, it checks the value of this register:
If the value is greater than '0', the processor successfully executes the bus lock and decrements the count.
If the value is '0', the bus lock is not executed, and a #VMEXIT to the VMM is taken.
The bus lock threshold #VMEXIT is reported to the VMM with the VMEXIT code A5h, SVM_EXIT_BUS_LOCK.
When SVM_EXIT_BUS_LOCK occurs, the value of the current Bus Lock Threshold counter, which is '0', is loaded into the VMCB. This value must be reset to a default greater than '0' in bus_lock_exit handler in hypervisor, because the behavior of SVM_EXIT_BUS_LOCK is fault like, so the bus lock exit to userspace happens with the RIP pointing to the offending instruction (which generates buslocks).
So, if the value of the Bus Lock Threshold counter remains '0', the guest continuously exits with SVM_EXIT_BUS_LOCK.
The generated SVM_EXIT_BUS_LOCK in kvm will exit to user space by setting the KVM_RUN_BUS_LOCK flag in vcpu->run->flags, indicating to the user space that a bus lock has been detected in the guest.
Use the already available KVM capability KVM_CAP_X86_BUS_LOCK_EXIT to enable the feature. This feature is disabled by default, it can be enabled explicitly from user space.
More details about the Bus Lock Threshold feature can be found in AMD APM [1].
You should mention in the commit message that this implementation is set up to intercept every guest bus lock. The initial value of the threshold is 0 in the VMCB, so the very first bus lock will exit, set the threshold to 1 (so that the offending instruction can make forward progress) but then the value is at 0 again so the next bus lock will exit.
Sure. I will add to V3.
[1]: AMD64 Architecture Programmer's Manual Pub. 24593, April 2024, Vol 2, 15.14.5 Bus Lock Threshold. https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Nikunj A Dadhania nikunj@amd.com Co-developed-by: Manali Shukla manali.shukla@amd.com Signed-off-by: Manali Shukla manali.shukla@amd.com
arch/x86/include/asm/svm.h | 5 ++++- arch/x86/include/uapi/asm/svm.h | 2 ++ arch/x86/kvm/svm/nested.c | 12 ++++++++++++ arch/x86/kvm/svm/svm.c | 29 +++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index f0dea3750ca9..bad9723f40e1 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -116,6 +116,7 @@ enum { INTERCEPT_INVPCID, INTERCEPT_MCOMMIT, INTERCEPT_TLBSYNC,
- INTERCEPT_BUSLOCK,
}; @@ -158,7 +159,9 @@ struct __attribute__ ((__packed__)) vmcb_control_area { u64 avic_physical_id; /* Offset 0xf8 */ u8 reserved_7[8]; u64 vmsa_pa; /* Used for an SEV-ES guest */
- u8 reserved_8[720];
- u8 reserved_8[16];
- u16 bus_lock_counter; /* Offset 0x120 */
- u8 reserved_9[702]; /*
- Offset 0x3e0, 32 bytes reserved
- for use by hypervisor/software.
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h index 1814b413fd57..abf6aed88cee 100644 --- a/arch/x86/include/uapi/asm/svm.h +++ b/arch/x86/include/uapi/asm/svm.h @@ -95,6 +95,7 @@ #define SVM_EXIT_CR14_WRITE_TRAP 0x09e #define SVM_EXIT_CR15_WRITE_TRAP 0x09f #define SVM_EXIT_INVPCID 0x0a2 +#define SVM_EXIT_BUS_LOCK 0x0a5 #define SVM_EXIT_NPF 0x400 #define SVM_EXIT_AVIC_INCOMPLETE_IPI 0x401 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS 0x402 @@ -224,6 +225,7 @@ { SVM_EXIT_CR4_WRITE_TRAP, "write_cr4_trap" }, \ { SVM_EXIT_CR8_WRITE_TRAP, "write_cr8_trap" }, \ { SVM_EXIT_INVPCID, "invpcid" }, \
- { SVM_EXIT_BUS_LOCK, "buslock" }, \ { SVM_EXIT_NPF, "npf" }, \ { SVM_EXIT_AVIC_INCOMPLETE_IPI, "avic_incomplete_ipi" }, \ { SVM_EXIT_AVIC_UNACCELERATED_ACCESS, "avic_unaccelerated_access" }, \
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 6f704c1037e5..670092d31f77 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -669,6 +669,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm, vmcb02->control.iopm_base_pa = vmcb01->control.iopm_base_pa; vmcb02->control.msrpm_base_pa = vmcb01->control.msrpm_base_pa;
- /*
* The bus lock threshold count should keep running across nested* transitions.* Copied the buslock threshold count from vmcb01 to vmcb02.No need to start this part of the comment on a separate line. Also, s/Copied/Copy/ (ditto below).
Ack.
*/- vmcb02->control.bus_lock_counter = vmcb01->control.bus_lock_counter; /* Done at vmrun: asid. */
/* Also overwritten later if necessary. */ @@ -1035,6 +1041,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm) }
- /*
* The bus lock threshold count should keep running across nested* transitions.* Copied the buslock threshold count from vmcb02 to vmcb01.*/- vmcb01->control.bus_lock_counter = vmcb02->control.bus_lock_counter; nested_svm_copy_common_state(svm->nested.vmcb02.ptr, svm->vmcb01.ptr);
svm_switch_vmcb(svm, &svm->vmcb01); diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 5ab2c92c7331..41c773a40f99 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -1372,6 +1372,11 @@ static void init_vmcb(struct kvm_vcpu *vcpu) svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK; }
- if (vcpu->kvm->arch.bus_lock_detection_enabled)
svm_set_intercept(svm, INTERCEPT_BUSLOCK);- else
svm_clr_intercept(svm, INTERCEPT_BUSLOCK);Is the else path really needed?
Thanks, Tom
Correct. It is not needed. I will remove from V3.
- if (sev_guest(vcpu->kvm)) sev_init_vmcb(svm);
@@ -3283,6 +3288,24 @@ static int invpcid_interception(struct kvm_vcpu *vcpu) return kvm_handle_invpcid(vcpu, type, gva); } +static int bus_lock_exit(struct kvm_vcpu *vcpu) +{
- struct vcpu_svm *svm = to_svm(vcpu);
- vcpu->run->exit_reason = KVM_EXIT_X86_BUS_LOCK;
- vcpu->run->flags |= KVM_RUN_X86_BUS_LOCK;
- /*
* Reload the counter with value greater than '0'.* The bus lock exit on SVM happens with RIP pointing to the guilty* instruction. So, reloading the value of bus_lock_counter to '0'* results into generating continuous bus lock exits.*/- svm->vmcb->control.bus_lock_counter = 1;
- return 0;
+}
static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_READ_CR0] = cr_interception, [SVM_EXIT_READ_CR3] = cr_interception, @@ -3350,6 +3373,7 @@ static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = { [SVM_EXIT_CR4_WRITE_TRAP] = cr_trap, [SVM_EXIT_CR8_WRITE_TRAP] = cr_trap, [SVM_EXIT_INVPCID] = invpcid_interception,
- [SVM_EXIT_BUS_LOCK] = bus_lock_exit, [SVM_EXIT_NPF] = npf_interception, [SVM_EXIT_RSM] = rsm_interception, [SVM_EXIT_AVIC_INCOMPLETE_IPI] = avic_incomplete_ipi_interception,
@@ -5212,6 +5236,11 @@ static __init void svm_set_cpu_caps(void) kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK); }
- if (cpu_feature_enabled(X86_VIRT_FEATURE_BUS_LOCK_THRESHOLD)) {
pr_info("Bus Lock Threashold supported\n");kvm_caps.has_bus_lock_exit = true;- }
- /* CPUID 0x80000008 */ if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) || boot_cpu_has(X86_FEATURE_AMD_SSBD))
-Manali
Add a note about behavioral difference for KVM_EXIT_X86_BUS_LOCK between AMD CPUs and Intel CPUs in KVM_CAP_X86_BUS_LOCK_EXIT capability documentation.
Signed-off-by: Manali Shukla manali.shukla@amd.com --- Documentation/virt/kvm/api.rst | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index b3be87489108..536010e1df1d 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -7869,6 +7869,11 @@ Note! Detected bus locks may be coincident with other exits to userspace, i.e. KVM_RUN_X86_BUS_LOCK should be checked regardless of the primary exit reason if userspace wants to take action on all detected bus locks.
+Note! In KVM on AMD CPUs, the bus lock exit to user space occurs with RIP +pointing at the offending instruction. In contrast, in KVM on Intel CPUs, the +RIP points to the instruction right after the guilty one after the bus lock +exit to user space. + 7.23 KVM_CAP_PPC_DAWR1 ----------------------
From: Nikunj A Dadhania nikunj@amd.com
Add a test case to verify the Bus Lock exit feature
The main thing that the selftest verifies is that when a Buslock is generated in the guest context, the KVM_EXIT_X86_BUS_LOCK is triggered for SVM or VMX when the KVM capability KVM_CAP_X86_BUS_LOCK_EXIT is enabled.
This test case also verifies the Bus Lock exit in nested scenario.
Signed-off-by: Nikunj A Dadhania nikunj@amd.com Co-developed-by: Manali Shukla manali.shukla@amd.com Signed-off-by: Manali Shukla manali.shukla@amd.com --- tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/x86_64/kvm_buslock_test.c | 130 ++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 tools/testing/selftests/kvm/x86_64/kvm_buslock_test.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 0c4b254ab56b..fd7b43fd2a8b 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -81,6 +81,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test TEST_GEN_PROGS_x86_64 += x86_64/hyperv_tlb_flush TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test +TEST_GEN_PROGS_x86_64 += x86_64/kvm_buslock_test TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test TEST_GEN_PROGS_x86_64 += x86_64/nested_exceptions_test TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test diff --git a/tools/testing/selftests/kvm/x86_64/kvm_buslock_test.c b/tools/testing/selftests/kvm/x86_64/kvm_buslock_test.c new file mode 100644 index 000000000000..82693520485c --- /dev/null +++ b/tools/testing/selftests/kvm/x86_64/kvm_buslock_test.c @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2024 Advanced Micro Devices, Inc. + */ + +#include "test_util.h" +#include "kvm_util.h" +#include "processor.h" +#include "svm_util.h" +#include "vmx.h" + +#define NR_ITERATIONS 100 +#define L2_GUEST_STACK_SIZE 64 + +struct buslock_test { + unsigned char pad[PAGE_SIZE - 2]; + atomic_long_t val; +} __packed; + +struct buslock_test test __aligned(PAGE_SIZE); + +static __always_inline void buslock_atomic_add(int i, atomic_long_t *v) +{ + asm volatile(LOCK_PREFIX "addl %1,%0" + : "+m" (v->counter) + : "ir" (i) : "memory"); +} + +static void buslock_add(void) +{ + /* + * Increment a page unaligned variable atomically. + * This should generate a bus lock exit. + */ + for (int i = 0; i < NR_ITERATIONS; i++) + buslock_atomic_add(2, &test.val); +} + +static void l2_guest_code(void) +{ + buslock_add(); + GUEST_DONE(); +} + +static void l1_svm_code(struct svm_test_data *svm) +{ + unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE]; + struct vmcb *vmcb = svm->vmcb; + + generic_svm_setup(svm, l2_guest_code, + &l2_guest_stack[L2_GUEST_STACK_SIZE]); + run_guest(vmcb, svm->vmcb_gpa); + GUEST_ASSERT(vmcb->control.exit_code == SVM_EXIT_VMMCALL); + GUEST_DONE(); +} + +static void l1_vmx_code(struct vmx_pages *vmx) +{ + unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE]; + + GUEST_ASSERT_EQ(prepare_for_vmx_operation(vmx), true); + GUEST_ASSERT_EQ(load_vmcs(vmx), true); + + prepare_vmcs(vmx, NULL, &l2_guest_stack[L2_GUEST_STACK_SIZE]); + + GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code)); + GUEST_ASSERT(!vmlaunch()); + + GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_VMCALL); + GUEST_DONE(); +} + +static void guest_code(void *test_data) +{ + buslock_add(); + + if (this_cpu_has(X86_FEATURE_SVM)) + l1_svm_code(test_data); + else + l1_vmx_code(test_data); +} + +int main(int argc, char *argv[]) +{ + struct kvm_vcpu *vcpu; + struct kvm_run *run; + struct kvm_vm *vm; + vm_vaddr_t nested_test_data_gva; + + TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM) || kvm_cpu_has(X86_FEATURE_VMX)); + TEST_REQUIRE(kvm_has_cap(KVM_CAP_X86_BUS_LOCK_EXIT)); + + vm = vm_create(1); + vm_enable_cap(vm, KVM_CAP_X86_BUS_LOCK_EXIT, KVM_BUS_LOCK_DETECTION_EXIT); + vcpu = vm_vcpu_add(vm, 0, guest_code); + + if (kvm_cpu_has(X86_FEATURE_SVM)) + vcpu_alloc_svm(vm, &nested_test_data_gva); + else + vcpu_alloc_vmx(vm, &nested_test_data_gva); + + vcpu_args_set(vcpu, 1, nested_test_data_gva); + + run = vcpu->run; + + for (;;) { + struct ucall uc; + + vcpu_run(vcpu); + + if (run->exit_reason == KVM_EXIT_IO) { + switch (get_ucall(vcpu, &uc)) { + case UCALL_ABORT: + REPORT_GUEST_ASSERT(uc); + /* NOT REACHED */ + case UCALL_SYNC: + continue; + case UCALL_DONE: + goto done; + default: + TEST_FAIL("Unknown ucall 0x%lx.", uc.cmd); + } + } + + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_X86_BUS_LOCK); + } +done: + kvm_vm_free(vm); + return 0; +}
linux-kselftest-mirror@lists.linaro.org
-
Manali Shukla -
Sean Christopherson -
Tom Lendacky