With the threshold initialised to 1 the guest exits at the first
buslock. Initialising as zero is invalid and causes an immediate exit.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/x86/hvm/svm/svm.c  | 4 ++++
 xen/arch/x86/hvm/svm/vmcb.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9748df87d8..dbb7f99d5e 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -3087,6 +3087,10 @@ void asmlinkage svm_vmexit_handler(void)
         hvm_descriptor_access_intercept(0, 0, desc, write);
         break;
     }
+    case VMEXIT_BUSLOCK:
+        perfc_incr(buslock);
+        vmcb->bus_lock_thresh = 1;
+        break;
 
     default:
     unexpected_exit_type:
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index cbee10d046..7a19b1ab61 100644
--- 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 */
+    }
+
     /* Intercept all debug-register writes. */
     vmcb->_dr_intercepts = ~0u;
 
-- 
2.43.0

On 20/01/2026 9:53 am, Alejandro Vallejo wrote:
> With the threshold initialised to 1 the guest exits at the first
> buslock. Initialising as zero is invalid and causes an immediate exit.

What do you mean by this?  A VMRUN failure, or a livelock?

>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
>  xen/arch/x86/hvm/svm/svm.c  | 4 ++++
>  xen/arch/x86/hvm/svm/vmcb.c | 6 ++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 9748df87d8..dbb7f99d5e 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -3087,6 +3087,10 @@ void asmlinkage svm_vmexit_handler(void)
>          hvm_descriptor_access_intercept(0, 0, desc, write);
>          break;
>      }

Blank line.

> +    case VMEXIT_BUSLOCK:
> +        perfc_incr(buslock);
> +        vmcb->bus_lock_thresh = 1;
> +        break;
>  
>      default:
>      unexpected_exit_type:
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index cbee10d046..7a19b1ab61 100644
> --- 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.

~Andrew

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?

Jan

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.

Simply dropping the comment would be ok, but I'd also like to understand
hardware behaviour if it differs from what the APM says.

~Andrew

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.

Jan

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

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

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

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

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.

Cheers,
Alejandro

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

Hello, some questions

Le 20/01/2026 à 10:56, Alejandro Vallejo a écrit :
> With the threshold initialised to 1 the guest exits at the first
> buslock. Initialising as zero is invalid and causes an immediate exit.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
>   xen/arch/x86/hvm/svm/svm.c  | 4 ++++
>   xen/arch/x86/hvm/svm/vmcb.c | 6 ++++++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 9748df87d8..dbb7f99d5e 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -3087,6 +3087,10 @@ void asmlinkage svm_vmexit_handler(void)
>           hvm_descriptor_access_intercept(0, 0, desc, write);
>           break;
>       }
> +    case VMEXIT_BUSLOCK:
> +        perfc_incr(buslock);
> +        vmcb->bus_lock_thresh = 1;
> +        break;
>   
>       default:
>       unexpected_exit_type:
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index cbee10d046..7a19b1ab61 100644
> --- 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 */
> +    }
> +
>       /* Intercept all debug-register writes. */
>       vmcb->_dr_intercepts = ~0u;
>   

According to APM,

INTERCEPT_BUS_LOCK_THRESH does
 > Intercept bus lock operations when Bus Lock Threshold Counter is 0

I assume that when set to 0, we intercept all bus locks, so if set to 1, 
every 2 bus lock (since we first go from 1 to 0, then at 0 we intercept 
the next one) ?

I think we want that to be tunable, as intercepting all bus locks may be 
too extreme we probably want to intercept every few ones instead.

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

On 20.01.2026 14:11, Teddy Astie wrote:
> Le 20/01/2026 à 10:56, Alejandro Vallejo a écrit :
>> --- 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 */
>> +    }
>> +
>>       /* Intercept all debug-register writes. */
>>       vmcb->_dr_intercepts = ~0u;
>>   
> 
> According to APM,
> 
> INTERCEPT_BUS_LOCK_THRESH does
>  > Intercept bus lock operations when Bus Lock Threshold Counter is 0
> 
> I assume that when set to 0, we intercept all bus locks, so if set to 1, 
> every 2 bus lock (since we first go from 1 to 0, then at 0 we intercept 
> the next one) ?
> 
> I think we want that to be tunable, as intercepting all bus locks may be 
> too extreme we probably want to intercept every few ones instead.

Otoh bus locks (as opposed to cache locks) would better be rare, or else
perhaps such a guest deserves some extra slowing down?

Jan

On Tue Jan 20, 2026 at 2:29 PM CET, Jan Beulich wrote:
> On 20.01.2026 14:11, Teddy Astie wrote:
>> Le 20/01/2026 à 10:56, Alejandro Vallejo a écrit :
>>> --- 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 */
>>> +    }
>>> +
>>>       /* Intercept all debug-register writes. */
>>>       vmcb->_dr_intercepts = ~0u;
>>>   
>> 
>> According to APM,
>> 
>> INTERCEPT_BUS_LOCK_THRESH does
>>  > Intercept bus lock operations when Bus Lock Threshold Counter is 0
>> 
>> I assume that when set to 0, we intercept all bus locks, so if set to 1, 
>> every 2 bus lock (since we first go from 1 to 0, then at 0 we intercept 
>> the next one) ?

Correct

>> 
>> I think we want that to be tunable, as intercepting all bus locks may be 
>> too extreme we probably want to intercept every few ones instead.
>
> Otoh bus locks (as opposed to cache locks) would better be rare, or else
> perhaps such a guest deserves some extra slowing down?
>
> Jan

I agree with Jan. buslock operations are so rare it's not worth being lenient
about them. They ought to be more expensive than they currently are for the
caller, in fact, so they notice and not do it because there's rarely a rationale
for them to exist at all. I'd be happier if the ISA didn't allow them at all.

Cheers,
Alejandro