Bus Locks are very costly and a VM left unchecked spamming instructions that lock the memory bus (e.g: unaligned atomic CAS) makes system perf take a nosedive. This patch is similar to BLD of VMX, but for SVM. It configures all VMRUNs so they automatically exit at the first encounter of a buslock event, effectively rate-limiting them. Cheers, Alejandro Alejandro Vallejo (2): x86/svm: Add infrastructure for Bus Lock Threshold x86/svm: Intercept Bus Locks for HVM guests xen/arch/x86/hvm/svm/svm.c | 5 +++++ xen/arch/x86/hvm/svm/vmcb.c | 6 ++++++ xen/arch/x86/hvm/svm/vmcb.h | 15 +++++++++++++-- xen/arch/x86/include/asm/hvm/svm.h | 2 ++ xen/arch/x86/include/asm/perfc_defn.h | 2 +- 5 files changed, 27 insertions(+), 3 deletions(-) base-commit: 7b3e1b4e848d34c9a5b6634009959a7b9dd42104 -- 2.43.0 Add missing scaffolding to enable BusLock Threshold. That is: * Add general_intercepts_3. * Add missing VMEXIT * Adjust NPF perf counter base to immediately after the buslock counter Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- xen/arch/x86/hvm/svm/svm.c | 1 + xen/arch/x86/hvm/svm/vmcb.h | 15 +++++++++++++-- xen/arch/x86/include/asm/hvm/svm.h | 2 ++ xen/arch/x86/include/asm/perfc_defn.h | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -XXX,XX +XXX,XX @@ const struct hvm_function_table * __init start_svm(void) P(cpu_has_tsc_ratio, "TSC Rate MSR"); P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack"); P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation"); + P(cpu_has_bus_lock_thresh, "BusLock-Intercept Filter"); #undef P if ( !printed ) diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/vmcb.h +++ b/xen/arch/x86/hvm/svm/vmcb.h @@ -XXX,XX +XXX,XX @@ enum GenericIntercept2bits GENERAL2_INTERCEPT_RDPRU = 1 << 14, }; +/* general 2 intercepts */ +enum GenericIntercept3bits +{ + GENERAL3_INTERCEPT_BUS_LOCK_THRESH = 1 << 5, +}; /* control register intercepts */ enum CRInterceptBits @@ -XXX,XX +XXX,XX @@ enum VMEXIT_EXITCODE VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */ VMEXIT_XSETBV = 141, /* 0x8d */ VMEXIT_RDPRU = 142, /* 0x8e */ + VMEXIT_BUSLOCK = 165, /* 0xa5 */ /* Remember to also update VMEXIT_NPF_PERFC! */ VMEXIT_NPF = 1024, /* 0x400, nested paging fault */ /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */ @@ -XXX,XX +XXX,XX @@ struct vmcb_struct { u32 _exception_intercepts; /* offset 0x08 - cleanbit 0 */ u32 _general1_intercepts; /* offset 0x0C - cleanbit 0 */ u32 _general2_intercepts; /* offset 0x10 - cleanbit 0 */ - u32 res01[10]; + u32 _general3_intercepts; /* offset 0x14 - cleanbit 0 */ + u32 res01[9]; u16 _pause_filter_thresh; /* offset 0x3C - cleanbit 0 */ u16 _pause_filter_count; /* offset 0x3E - cleanbit 0 */ u64 _iopm_base_pa; /* offset 0x40 - cleanbit 1 */ @@ -XXX,XX +XXX,XX @@ struct vmcb_struct { u64 nextrip; /* offset 0xC8 */ u8 guest_ins_len; /* offset 0xD0 */ u8 guest_ins[15]; /* offset 0xD1 */ - u64 res10a[100]; /* offset 0xE0 pad to save area */ + u64 res10a[8]; /* offset 0xE0 */ + u16 bus_lock_thresh; /* offset 0x120 */ + u16 res10b[3]; /* offset 0x122 */ + u64 res10c[91]; /* offset 0x128 pad to save area */ union { struct segment_register sreg[6]; @@ -XXX,XX +XXX,XX @@ VMCB_ACCESSORS(dr_intercepts, intercepts) VMCB_ACCESSORS(exception_intercepts, intercepts) VMCB_ACCESSORS(general1_intercepts, intercepts) VMCB_ACCESSORS(general2_intercepts, intercepts) +VMCB_ACCESSORS(general3_intercepts, intercepts) VMCB_ACCESSORS(pause_filter_count, intercepts) VMCB_ACCESSORS(pause_filter_thresh, intercepts) VMCB_ACCESSORS(tsc_offset, intercepts) diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/hvm/svm.h +++ b/xen/arch/x86/include/asm/hvm/svm.h @@ -XXX,XX +XXX,XX @@ extern u32 svm_feature_flags; #define SVM_FEATURE_VGIF 16 /* Virtual GIF */ #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */ #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */ +#define SVM_FEATURE_BUS_LOCK_THRESH 29 /* Bus Lock Threshold */ static inline bool cpu_has_svm_feature(unsigned int feat) { @@ -XXX,XX +XXX,XX @@ static inline bool cpu_has_svm_feature(unsigned int feat) #define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE) #define cpu_has_svm_sss cpu_has_svm_feature(SVM_FEATURE_SSS) #define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL) +#define cpu_has_bus_lock_thresh cpu_has_svm_feature(SVM_FEATURE_BUS_LOCK_THRESH) #define MSR_INTERCEPT_NONE 0 #define MSR_INTERCEPT_READ 1 diff --git a/xen/arch/x86/include/asm/perfc_defn.h b/xen/arch/x86/include/asm/perfc_defn.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/perfc_defn.h +++ b/xen/arch/x86/include/asm/perfc_defn.h @@ -XXX,XX +XXX,XX @@ PERFCOUNTER_ARRAY(exceptions, "exceptions", 32) #ifdef CONFIG_HVM #define VMX_PERF_EXIT_REASON_SIZE 76 -#define VMEXIT_NPF_PERFC 143 +#define VMEXIT_NPF_PERFC 166 #define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1) PERFCOUNTER_ARRAY(vmexits, "vmexits", MAX(VMX_PERF_EXIT_REASON_SIZE, SVM_PERF_EXIT_REASON_SIZE)) -- 2.43.0 With the threshold initialised to 1 the guest exits at the first buslock. Initialising as zero is invalid and causes an immediate exit. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- xen/arch/x86/hvm/svm/svm.c | 4 ++++ xen/arch/x86/hvm/svm/vmcb.c | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -XXX,XX +XXX,XX @@ void asmlinkage svm_vmexit_handler(void) hvm_descriptor_access_intercept(0, 0, desc, write); break; } + case VMEXIT_BUSLOCK: + perfc_incr(buslock); + vmcb->bus_lock_thresh = 1; + break; default: unexpected_exit_type: diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/vmcb.c +++ b/xen/arch/x86/hvm/svm/vmcb.c @@ -XXX,XX +XXX,XX @@ static int construct_vmcb(struct vcpu *v) GENERAL2_INTERCEPT_XSETBV | GENERAL2_INTERCEPT_ICEBP | GENERAL2_INTERCEPT_RDPRU; + if ( cpu_has_bus_lock_thresh ) + { + vmcb->_general3_intercepts = GENERAL3_INTERCEPT_BUS_LOCK_THRESH; + vmcb->bus_lock_thresh = 1; /* trigger immediately */ + } + /* Intercept all debug-register writes. */ vmcb->_dr_intercepts = ~0u; -- 2.43.0 Hi, v1: https://lore.kernel.org/xen-devel/20260120095353.2778-1-alejandro.garciavallejo@amd.com pipeline (in progress): https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/2276726870 Original cover letter: Bus Locks are very costly and a VM left unchecked spamming instructions that lock the memory bus (e.g: unaligned atomic CAS) makes system perf take a nosedive. This patch is similar to BLD of VMX, but for SVM. It configures all VMRUNs so they automatically exit at the first encounter of a buslock event, effectively rate-limiting them. Cheers, Alejandro Alejandro Vallejo (3): x86/svm: Add infrastructure for Bus Lock Threshold x86/svm: Intercept Bus Locks for HVM guests CHANGELOG: Note the new SVM bus-lock intercept CHANGELOG.md | 3 +++ xen/arch/x86/hvm/svm/svm.c | 6 ++++++ xen/arch/x86/hvm/svm/vmcb.c | 3 +++ xen/arch/x86/hvm/svm/vmcb.h | 15 +++++++++++++-- xen/arch/x86/include/asm/hvm/svm.h | 2 ++ xen/arch/x86/include/asm/perfc_defn.h | 2 +- 6 files changed, 28 insertions(+), 3 deletions(-) base-commit: 61204ed24ba4537d6eff56594faa5d23cacb8310 -- 2.43.0 Add missing scaffolding to enable BusLock Threshold. That is: * Add general_intercepts_3. * Add missing VMEXIT * Adjust NPF perf counter base to immediately after the buslock counter Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> Reviewed-by: Teddy Astie <teddy.astie@vates.tech> --- v2: * s/general intercepts 2/general intercepts 3/ * removed _thresh suffix * added missing _svm_ infix in the SVM feature --- xen/arch/x86/hvm/svm/vmcb.h | 15 +++++++++++++-- xen/arch/x86/include/asm/hvm/svm.h | 2 ++ xen/arch/x86/include/asm/perfc_defn.h | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/vmcb.h +++ b/xen/arch/x86/hvm/svm/vmcb.h @@ -XXX,XX +XXX,XX @@ enum GenericIntercept2bits GENERAL2_INTERCEPT_RDPRU = 1 << 14, }; +/* general 3 intercepts */ +enum GenericIntercept3bits +{ + GENERAL3_INTERCEPT_BUS_LOCK_THRESH = 1 << 5, +}; /* control register intercepts */ enum CRInterceptBits @@ -XXX,XX +XXX,XX @@ enum VMEXIT_EXITCODE VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */ VMEXIT_XSETBV = 141, /* 0x8d */ VMEXIT_RDPRU = 142, /* 0x8e */ + VMEXIT_BUS_LOCK = 165, /* 0xa5 */ /* Remember to also update VMEXIT_NPF_PERFC! */ VMEXIT_NPF = 1024, /* 0x400, nested paging fault */ /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */ @@ -XXX,XX +XXX,XX @@ struct vmcb_struct { u32 _exception_intercepts; /* offset 0x08 - cleanbit 0 */ u32 _general1_intercepts; /* offset 0x0C - cleanbit 0 */ u32 _general2_intercepts; /* offset 0x10 - cleanbit 0 */ - u32 res01[10]; + u32 _general3_intercepts; /* offset 0x14 - cleanbit 0 */ + u32 res01[9]; u16 _pause_filter_thresh; /* offset 0x3C - cleanbit 0 */ u16 _pause_filter_count; /* offset 0x3E - cleanbit 0 */ u64 _iopm_base_pa; /* offset 0x40 - cleanbit 1 */ @@ -XXX,XX +XXX,XX @@ struct vmcb_struct { u64 nextrip; /* offset 0xC8 */ u8 guest_ins_len; /* offset 0xD0 */ u8 guest_ins[15]; /* offset 0xD1 */ - u64 res10a[100]; /* offset 0xE0 pad to save area */ + u64 res10a[8]; /* offset 0xE0 */ + u16 bus_lock_thresh; /* offset 0x120 */ + u16 res10b[3]; /* offset 0x122 */ + u64 res10c[91]; /* offset 0x128 pad to save area */ union { struct segment_register sreg[6]; @@ -XXX,XX +XXX,XX @@ VMCB_ACCESSORS(dr_intercepts, intercepts) VMCB_ACCESSORS(exception_intercepts, intercepts) VMCB_ACCESSORS(general1_intercepts, intercepts) VMCB_ACCESSORS(general2_intercepts, intercepts) +VMCB_ACCESSORS(general3_intercepts, intercepts) VMCB_ACCESSORS(pause_filter_count, intercepts) VMCB_ACCESSORS(pause_filter_thresh, intercepts) VMCB_ACCESSORS(tsc_offset, intercepts) diff --git a/xen/arch/x86/include/asm/hvm/svm.h b/xen/arch/x86/include/asm/hvm/svm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/hvm/svm.h +++ b/xen/arch/x86/include/asm/hvm/svm.h @@ -XXX,XX +XXX,XX @@ extern u32 svm_feature_flags; #define SVM_FEATURE_VGIF 16 /* Virtual GIF */ #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */ #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */ +#define SVM_FEATURE_BUS_LOCK 29 /* Bus Lock Threshold */ static inline bool cpu_has_svm_feature(unsigned int feat) { @@ -XXX,XX +XXX,XX @@ static inline bool cpu_has_svm_feature(unsigned int feat) #define cpu_has_svm_vloadsave cpu_has_svm_feature(SVM_FEATURE_VLOADSAVE) #define cpu_has_svm_sss cpu_has_svm_feature(SVM_FEATURE_SSS) #define cpu_has_svm_spec_ctrl cpu_has_svm_feature(SVM_FEATURE_SPEC_CTRL) +#define cpu_has_svm_bus_lock cpu_has_svm_feature(SVM_FEATURE_BUS_LOCK) #define MSR_INTERCEPT_NONE 0 #define MSR_INTERCEPT_READ 1 diff --git a/xen/arch/x86/include/asm/perfc_defn.h b/xen/arch/x86/include/asm/perfc_defn.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/perfc_defn.h +++ b/xen/arch/x86/include/asm/perfc_defn.h @@ -XXX,XX +XXX,XX @@ PERFCOUNTER_ARRAY(exceptions, "exceptions", 32) #ifdef CONFIG_HVM #define VMX_PERF_EXIT_REASON_SIZE 76 -#define VMEXIT_NPF_PERFC 143 +#define VMEXIT_NPF_PERFC 166 #define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1) PERFCOUNTER_ARRAY(vmexits, "vmexits", MAX(VMX_PERF_EXIT_REASON_SIZE, SVM_PERF_EXIT_REASON_SIZE)) -- 2.43.0 Configure the Bus Lock intercept when supported by the host. The VMCB counter is initialised to zero so it fires upon the first instruction that locks the bus. On the #VMEXIT handler that counter is set to 1 because it has fault behaviour and the offending instruction needs to re-execute. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- v2: * Moved the P() call to this patch. We don't want to print until the feature is fully supported. * Removed the initialisation of the counter to 1 in vmcb.c, so it's implicitly zero-initialised. --- xen/arch/x86/hvm/svm/svm.c | 6 ++++++ xen/arch/x86/hvm/svm/vmcb.c | 3 +++ xen/arch/x86/hvm/svm/vmcb.h | 4 ++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -XXX,XX +XXX,XX @@ const struct hvm_function_table * __init start_svm(void) P(cpu_has_tsc_ratio, "TSC Rate MSR"); P(cpu_has_svm_sss, "NPT Supervisor Shadow Stack"); P(cpu_has_svm_spec_ctrl, "MSR_SPEC_CTRL virtualisation"); + P(cpu_has_svm_bus_lock, "BusLock-Intercept Filter"); #undef P if ( !printed ) @@ -XXX,XX +XXX,XX @@ void asmlinkage svm_vmexit_handler(void) break; } + case VMEXIT_BUS_LOCK: + perfc_incr(buslock); + vmcb->bus_lock_count = 1; + break; + default: unexpected_exit_type: gprintk(XENLOG_ERR, "Unexpected vmexit: reason %#"PRIx64", " diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/vmcb.c +++ b/xen/arch/x86/hvm/svm/vmcb.c @@ -XXX,XX +XXX,XX @@ static int construct_vmcb(struct vcpu *v) GENERAL2_INTERCEPT_XSETBV | GENERAL2_INTERCEPT_ICEBP | GENERAL2_INTERCEPT_RDPRU; + if ( cpu_has_svm_bus_lock ) + vmcb->_general3_intercepts |= GENERAL3_INTERCEPT_BUS_LOCK; + /* Intercept all debug-register writes. */ vmcb->_dr_intercepts = ~0u; diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/hvm/svm/vmcb.h +++ b/xen/arch/x86/hvm/svm/vmcb.h @@ -XXX,XX +XXX,XX @@ enum GenericIntercept2bits /* general 3 intercepts */ enum GenericIntercept3bits { - GENERAL3_INTERCEPT_BUS_LOCK_THRESH = 1 << 5, + GENERAL3_INTERCEPT_BUS_LOCK = 1 << 5, }; /* control register intercepts */ @@ -XXX,XX +XXX,XX @@ struct vmcb_struct { u8 guest_ins_len; /* offset 0xD0 */ u8 guest_ins[15]; /* offset 0xD1 */ u64 res10a[8]; /* offset 0xE0 */ - u16 bus_lock_thresh; /* offset 0x120 */ + u16 bus_lock_count; /* offset 0x120 */ u16 res10b[3]; /* offset 0x122 */ u64 res10c[91]; /* offset 0x128 pad to save area */ -- 2.43.0 Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index XXXXXXX..XXXXXXX 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -XXX,XX +XXX,XX @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Changed ### Added + - On x86: + - AMD bus-lock detect, used by Xen to mitigate (by rate-limiting) the + system wide impact of an HVM guest misusing atomic instructions. ### Removed - On x86: -- 2.43.0