Hi Xiaoyao,
Thank you for reviewing my patches.
On 4/16/2025 11:30 AM, Xiaoyao Li wrote:
> On 3/24/2025 9:02 PM, Manali Shukla wrote:
>> From: Nikunj A Dadhania <nikunj@xxxxxxx>
>>
>> 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
>>
>> 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.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
>> Co-developed-by: Manali Shukla <manali.shukla@xxxxxxx>
>> Signed-off-by: Manali Shukla <manali.shukla@xxxxxxx>
>> ---
>> arch/x86/include/asm/svm.h | 5 ++++-
>> arch/x86/include/uapi/asm/svm.h | 2 ++
>> arch/x86/kvm/svm/svm.c | 8 ++++++++
>> 3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 9b7fa99ae951..9dc54da1835a 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,
>> INTERCEPT_IDLE_HLT = 166,
>> };
>> @@ -159,7 +160,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 ec1321248dac..9c640a521a67 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_IDLE_HLT 0x0a6
>> #define SVM_EXIT_NPF 0x400
>> #define SVM_EXIT_AVIC_INCOMPLETE_IPI 0x401
>> @@ -225,6 +226,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_IDLE_HLT, "idle-halt" }, \
>> { SVM_EXIT_NPF, "npf" }, \
>> { SVM_EXIT_AVIC_INCOMPLETE_IPI, "avic_incomplete_ipi" }, \
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 8abeab91d329..244e099e7262 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -1375,6 +1375,9 @@ 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);
>> +
>> if (sev_guest(vcpu->kvm))
>> sev_init_vmcb(svm);
>> @@ -5299,6 +5302,11 @@ static __init void svm_set_cpu_caps(void)
>> kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK);
>> }
>> + if (cpu_feature_enabled(X86_FEATURE_BUS_LOCK_THRESHOLD)) {
>> + pr_info("Bus Lock Threshold supported\n");
>
> It will be printed every time kvm-amd.ko module gets loaded.
>
> I think it's for your development and debug purpose. Comparing to the existing features in svm_set_cpu_caps(), nothing makes it special for BUS_LOCK_THRESHOLD to require a kernel message. So I think we can just remove it.
I didn't add this for development and debug purpose. I added this pr_info() to make it easy to find whether BUS Lock threshold is supported or not from dmesg.
I can remove it if you think it is not required.
>
>> + kvm_caps.has_bus_lock_exit = true;
>
> Besides, this patch doesn't ensure the bisectability. It allows userspace to enable KVM_BUS_LOCK_DETECTION_EXIT and set intercept of INTERCEPT_BUSLOCK but without providing the handler.
>
> So either move next patch before it or just merge them.
>
Oh.., my bad, I will move the next patch before this one in v5.
>> + }
>> +
>> /* CPUID 0x80000008 */
>> if (boot_cpu_has(X86_FEATURE_LS_CFG_SSBD) ||
>> boot_cpu_has(X86_FEATURE_AMD_SSBD))
>
-Manali
