Re: [PATCH v1] x86/svm: Fix VMLOAD/VMSAVE state handling when using nested virt

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.09.2026 16:37, Ross Lagerwall wrote:
> On 9/21/26 12:21 PM, Jan Beulich wrote:
>> On 11.09.2026 17:12, Ross Lagerwall wrote:
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -444,30 +444,30 @@ static int svm_vmcb_restore(struct vcpu *v, struct 
>>> hvm_hw_cpu *c)
>>>   
>>>   static void svm_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
>>>   {
>>> -    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
>>> +    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
>>
>> Here and elsewhere you assume that vcpu_nestedhvm() is legitimate to use
>> even in the non-nested case. I think that's heading in the wrong direction;
>> I think that a hypothetical mode with CONFIG_NESTED=n and the entire
>> struct nestedvcpu wrapped in #ifdef CONFIG_NESTED should still be possible
>> to put in place, without meaningful rearrangements or renaming besides the
>> adding of the respective #if{,n}def.
> 
> This assumption already exists, e.g. see svm_create_vmcb(), 
> svm_destroy_vmcb(),
> svm_dr_access().

Hmm, I see. That's not very nice, but then of course you're okay to follow
this model.

> If you would prefer not to introduce any new cases like this, I could 
> introduce
> a new macro, e.g.
> 
> #define vcpu_n1vmcx(v) (vcpu_nestedhvm((v)).nv_n1vmcb)
> 
> and then for the hypothetical CONFIG_NESTED=n, it would resolve to:
> 
> #define vcpu_n1vmcx(v) ((v)->arch.hvm.svm.vmcb)

May be worthwhile independently, but as per above I wouldn't insist on you
doing this right here.

Jan