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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.01.2026 15:11, Andrew Cooper wrote:
> On 20/01/2026 1:34 pm, Jan Beulich wrote:
>> On 20.01.2026 14:29, Andrew Cooper wrote:
>>> On 20/01/2026 1:27 pm, Jan Beulich wrote:
>>>> On 20.01.2026 14:18, Andrew Cooper wrote:
>>>>> On 20/01/2026 9:53 am, Alejandro Vallejo wrote:
>>>>>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>>>>>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>>>>>> @@ -66,6 +66,12 @@ 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 */
>>>>> Really?  The APM states:
>>>>>
>>>>> On processors that support Bus Lock Threshold (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. On
>>>>> VMRUN, this value 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 executes the bus
>>>>> lock successfully and decrements the count. If the value is 0, the bus
>>>>> lock is not executed and a #VMEXIT to the VMM is taken.
>>>>>
>>>>> So according to the APM, setting the count to 1 will permit one bus lock
>>>>> then exit (fault style) immediately before the next.  This also says
>>>>> that a count of 0 is a legal state.
>>>> But then you'd livelock the guest as soon as it uses a bus lock. Are you
>>>> suggesting to set to 1 in response to a bus lock exit, and keep at 0 at
>>>> all other times?
>>> I should have been clearer.  I'm complaining at the "trigger
>>> immediately" comment, because I don't think that's a correct statement
>>> of how hardware behaves.
>> In turn I should have looked at the patch itself before commenting. The
>> other setting to 1 is what makes sense, and what ought to prevent a
>> livelock. The one here indeed raises questions.
> 
> Setting it to 1 here is fine.  This is the constructor for VMCBs, and
> *something* needs to make the state consistent with the setting we chose
> at runtime.

But the setting at runtime is generally going to be 0: When the guest exits
for an intercepted bus lock, we'll set the threshold to 1, re-enter the
guest, it'll retry the bus-locking insn, the counter will be decremented to
0, and the guest will continue to run with that value being zero. Until the
next insn taking a bus lock. So starting with 0 would overall be more
consistent.

Jan