On Wed Jan 21, 2026 at 4:35 PM CET, Andrew Cooper wrote:
> On 21/01/2026 2:28 pm, Alejandro Vallejo wrote:
>> Configure the Bus Lock intercept when supported by the host.
>
> "which is available on Zen4 and later".
>
> (I think ?)
I don't mind the addition, but does it really matter for the purposes of the
commit message?
>
>
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index 5d23603fc1..abda5a9063 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -2524,6 +2524,7 @@ 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");
>
> "Bus Lock Filter". The Intercept part isn't terribly useful.
sure
>
>> #undef P
>>
>> if ( !printed )
>> @@ -3087,6 +3088,11 @@ void asmlinkage svm_vmexit_handler(void)
>> break;
>> }
>>
>> + case VMEXIT_BUS_LOCK:
>> + perfc_incr(buslock);
>> + vmcb->bus_lock_count = 1;
>> + break;
>
> This needs an explanation of the behaviour.
>
> /* This is a fault and blocked the Bus Lock inducing action. We're only
> interested in rate limiting the guest, so credit it one "lock" in order
> to re-execute the instruction. */
fair
>
>> +
>> 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 cbee10d046..15223a693e 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -66,6 +66,9 @@ 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;
>> +
>
> This wants a justification for why it's unconditional. Something like:
>
> /* Well behaved logic shouldn't ever Bus Lock, but we care about rate
> limiting buggy/malicious cases. */
>
>
>> /* 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 231f9b1b06..68cf5eaf0b 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.h
>> +++ b/xen/arch/x86/hvm/svm/vmcb.h
>> @@ -68,7 +68,7 @@ enum GenericIntercept2bits
>> /* general 3 intercepts */
>> enum GenericIntercept3bits
>> {
>> - GENERAL3_INTERCEPT_BUS_LOCK_THRESH = 1 << 5,
>> + GENERAL3_INTERCEPT_BUS_LOCK = 1 << 5,
>> };
>>
>> /* control register intercepts */
>> @@ -497,7 +497,7 @@ 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 */
>>
>
> Both of these hunks want moving into the prior patch, which resolves one
> of my concerns there.
Damn it. Needless to say, this not where I meant them to be.
>
> All can be fixed on commit.
thanks.
>
> ~Andrew
Cheers,
alejandro
