Re: [PATCH v2 2/3] x86/svm: Intercept Bus Locks for HVM guests

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

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