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.
> : 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.
~Andrew