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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew