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
>> : 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.
>
> Assuming we can set 0, then yes we could drive SVM like this. However,
> we cannot do the same for PV or VT-x guests, both of which are strictly
> trap behaviour.
>
> So for that reason alone, we probably wouldn't want to drive SVM
> differently to other guest types.
Yet we can't abstract away the fault vs trap behavioral difference. I'd
take a different position: To have behavior as similar as possible, we'd
want it to be uniform how many bus locks we allow to be used without us
noticing. If we trap after the first one for VMX, we ought to fault on
the first one for SVM (provided that's possible in practice).
Jan
