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

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

On Tue Jan 20, 2026 at 4:28 PM CET, Alejandro Vallejo wrote:
> On Tue Jan 20, 2026 at 3:32 PM CET, Jan Beulich wrote:
>> On 20.01.2026 15:26, Andrew Cooper wrote:
>>> On 20/01/2026 2:16 pm, Jan Beulich wrote:
>>>> 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
>>> 
>>> First, we need clarity on what "Initialising as zero is invalid and
>>> causes an immediate exit." means.
>>
>> +1
>
> The exit is not an VMEXIT_INVALID, if that's your fear. Let me clarify:
>
> The TL;DR is that the commit message is the unfortunate side effect of trying
> to remember why I did something a while ago and not remembering very well.
>
> Initialy I had zero at both the initialiser and the reset. That doesn't get
> very far until you notice the behaviour is a fault and not a trap, which the 
> APM
> states as:
>
> ```
> If the value is 0, the bus lock is not executed and a #VMEXIT to the VMM is
> taken
> ```
>
> Then in order to go past that boundary the reset must be set at 1, or the 
> guest
> loops. The initialisation may start at either (though zero would be more
> consistent).
>
> I guess over time I just internalised a bit too much "I can't exec vmrun with
> a counter of 0".
>
> I'll send a v2 with the initialiser set to zero and an amended commit message.

... pending a hardware test, because the board I have with me ATM doesn't
support the feature and need to get ahold of one.

>
> Cheers,
> Alejandro