[PATCH v3 00/12] x86/HVM: misc tidying

[PATCH v3 00/12] x86/HVM: misc tidying

[Jan Beulich]

01: VMX: don't run with CR4.VMXE set when VMX could not be enabled
02: x86/HVM: improve CET-IBT pruning of ENDBR
03: VMX: drop vmcs_revision_id
04: VMX: convert vmx_basic_msr
05: VMX: convert vmx_pin_based_exec_control
06: VMX: convert vmx_cpu_based_exec_control
07: VMX: convert vmx_secondary_exec_control
08: VMX: convert vmx_tertiary_exec_control
09: VMX: convert vmx_vmexit_control
10: VMX: convert vmx_vmentry_control
11: VMX: convert vmx_ept_vpid_cap
12: VMX: convert vmx_vmfunc

Jan

[PATCH v3 01/12] VMX: don't run with CR4.VMXE set when VMX could not be enabled

[Jan Beulich]

While generally benign, doing so is still at best misleading.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Using set_in_cr4() seems favorable over updating mmu_cr4_features
despite the resulting redundant CR4 update. But I certainly could be
talked into going the alternative route.
---
v2: Actually clear CR4.VMXE for the BSP on the error path.

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2961,14 +2961,18 @@ static bool __init has_if_pschange_mc(vo
 
 const struct hvm_function_table * __init start_vmx(void)
 {
-    set_in_cr4(X86_CR4_VMXE);
+    write_cr4(read_cr4() | X86_CR4_VMXE);
 
     if ( vmx_vmcs_init() )
     {
+        write_cr4(read_cr4() & ~X86_CR4_VMXE);
         printk("VMX: failed to initialise.\n");
         return NULL;
     }
 
+    /* Arrange for APs to have CR4.VMXE set early on. */
+    set_in_cr4(X86_CR4_VMXE);
+
     vmx_function_table.singlestep_supported = cpu_has_monitor_trap_flag;
 
     if ( cpu_has_vmx_dt_exiting )

Re: [PATCH v3 01/12] VMX: don't run with CR4.VMXE set when VMX could not be enabled

[Roger Pau Monné]

On Mon, Feb 26, 2024 at 05:42:02PM +0100, Jan Beulich wrote:
> While generally benign, doing so is still at best misleading.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

[PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Jan Beulich]

__init{const,data}_cf_clobber can have an effect only for pointers
actually populated in the respective tables. While not the case for SVM
right now, VMX installs a number of pointers only under certain
conditions. Hence the respective functions would have their ENDBR purged
only when those conditions are met. Invoke "pruning" functions after
having copied the respective tables, for them to install any "missing"
pointers.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This is largely cosmetic for present hardware, which when supporting
CET-IBT likely also supports all of the advanced VMX features for which
hook pointers are installed conditionally. The only case this would make
a difference there is when use of respective features was suppressed via
command line option (where available). For future hooks it may end up
relevant even by default, and it also would be if AMD started supporting
CET-IBT; right now it matters only for .pi_update_irte, as iommu_intpost
continues to default to off.

Originally I had meant to put the SVM and VMX functions in presmp-
initcalls, but hvm/{svm,vmx}/built_in.o are linked into hvm/built_in.o
before hvm/hvm.o. And I don't think I want to fiddle with link order
here.
---
v3: Re-base.
v2: Use cpu_has_xen_ibt in prune_{svm,vmx}().

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
     else if ( cpu_has_svm )
         fns = start_svm();
 
+    if ( fns )
+        hvm_funcs = *fns;
+
+    prune_vmx();
+    prune_svm();
+
     if ( fns == NULL )
         return 0;
 
-    hvm_funcs = *fns;
     hvm_enabled = 1;
 
     printk("HVM: %s enabled\n", fns->name);
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2587,6 +2587,19 @@ const struct hvm_function_table * __init
     return &svm_function_table;
 }
 
+void __init prune_svm(void)
+{
+    /*
+     * Now that svm_function_table was copied, populate all function pointers
+     * which may have been left at NULL, for __initdata_cf_clobber to have as
+     * much of an effect as possible.
+     */
+    if ( !cpu_has_xen_ibt )
+        return;
+
+    /* Nothing at present. */
+}
+
 void asmlinkage svm_vmexit_handler(void)
 {
     struct cpu_user_regs *regs = guest_cpu_user_regs();
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3042,6 +3042,30 @@ const struct hvm_function_table * __init
     return &vmx_function_table;
 }
 
+void __init prune_vmx(void)
+{
+    /*
+     * Now that vmx_function_table was copied, populate all function pointers
+     * which may have been left at NULL, for __initdata_cf_clobber to have as
+     * much of an effect as possible.
+     */
+    if ( !cpu_has_xen_ibt )
+        return;
+
+    vmx_function_table.set_descriptor_access_exiting =
+        vmx_set_descriptor_access_exiting;
+
+    vmx_function_table.update_eoi_exit_bitmap = vmx_update_eoi_exit_bitmap;
+    vmx_function_table.process_isr            = vmx_process_isr;
+    vmx_function_table.handle_eoi             = vmx_handle_eoi;
+
+    vmx_function_table.pi_update_irte = vmx_pi_update_irte;
+
+    vmx_function_table.deliver_posted_intr = vmx_deliver_posted_intr;
+    vmx_function_table.sync_pir_to_irr     = vmx_sync_pir_to_irr;
+    vmx_function_table.test_pir            = vmx_test_pir;
+}
+
 /*
  * Not all cases receive valid value in the VM-exit instruction length field.
  * Callers must know what they're doing!
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -250,6 +250,9 @@ extern s8 hvm_port80_allowed;
 extern const struct hvm_function_table *start_svm(void);
 extern const struct hvm_function_table *start_vmx(void);
 
+void prune_svm(void);
+void prune_vmx(void);
+
 int hvm_domain_initialise(struct domain *d,
                           const struct xen_domctl_createdomain *config);
 void hvm_domain_relinquish_resources(struct domain *d);

Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Roger Pau Monné]

On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
> __init{const,data}_cf_clobber can have an effect only for pointers
> actually populated in the respective tables. While not the case for SVM
> right now, VMX installs a number of pointers only under certain
> conditions. Hence the respective functions would have their ENDBR purged
> only when those conditions are met. Invoke "pruning" functions after
> having copied the respective tables, for them to install any "missing"
> pointers.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This is largely cosmetic for present hardware, which when supporting
> CET-IBT likely also supports all of the advanced VMX features for which
> hook pointers are installed conditionally. The only case this would make
> a difference there is when use of respective features was suppressed via
> command line option (where available). For future hooks it may end up
> relevant even by default, and it also would be if AMD started supporting
> CET-IBT; right now it matters only for .pi_update_irte, as iommu_intpost
> continues to default to off.
> 
> Originally I had meant to put the SVM and VMX functions in presmp-
> initcalls, but hvm/{svm,vmx}/built_in.o are linked into hvm/built_in.o
> before hvm/hvm.o. And I don't think I want to fiddle with link order
> here.
> ---
> v3: Re-base.
> v2: Use cpu_has_xen_ibt in prune_{svm,vmx}().
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
>      else if ( cpu_has_svm )
>          fns = start_svm();
>  
> +    if ( fns )
> +        hvm_funcs = *fns;
> +
> +    prune_vmx();
> +    prune_svm();

Isn't it actually the opposite of pruning.  What the helpers do is
fill all the pointers in the structure.  I would rather name them
{vmx,svm}_fill_hvm_funcs() or similar.  And possibly pull the
cpu_has_xen_ibt check outside the functions:

if ( cpu_has_xen_ibt )
{
    /*
     * Now that svm_function_table was copied, populate all function pointers
     * which may have been left at NULL, for __initdata_cf_clobber to have as
     * much of an effect as possible.
     */
    vmx_fill_hvm_funcs();
    svm_fill_hvm_funcs();
}

I would be nice to avoid directly exporting more vmx and smv specific
helpers, as if we ever want to compile out vmx or svm it would be more
churn to deal with those.  I however cannot think of any good way to
do this here, so it's fine to export those functions.

Thanks, Roger.

Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Jan Beulich]

On 23.01.2025 13:41, Roger Pau Monné wrote:
> On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
>>      else if ( cpu_has_svm )
>>          fns = start_svm();
>>  
>> +    if ( fns )
>> +        hvm_funcs = *fns;
>> +
>> +    prune_vmx();
>> +    prune_svm();
> 
> Isn't it actually the opposite of pruning.  What the helpers do is
> fill all the pointers in the structure.

With the goal of their ENDBR to then be pruned. I agree though that the
functions don't do any pruning themselves. Yet
{svm,vmx}_prepare_for_cet_ibt_pruning() is a little awkward for my taste
(although it would properly document the purpose). Plus ...

>  I would rather name them {vmx,svm}_fill_hvm_funcs() or similar.

... while I can use those names (perhaps without the "hvm" infix), the
present names have the advantage that any other pruning that we may
find desirable could also be put there. Hence also why the cpu_has_*
checks live there.

>  And possibly pull the
> cpu_has_xen_ibt check outside the functions:
> 
> if ( cpu_has_xen_ibt )
> {
>     /*
>      * Now that svm_function_table was copied, populate all function pointers
>      * which may have been left at NULL, for __initdata_cf_clobber to have as
>      * much of an effect as possible.
>      */
>     vmx_fill_hvm_funcs();
>     svm_fill_hvm_funcs();
> }

Which would leave the SVM function entirely empty. The intention was for
that to not be the case, and also for the comment you have added above
to also live in the per-vendor functions.

> I would be nice to avoid directly exporting more vmx and smv specific
> helpers, as if we ever want to compile out vmx or svm it would be more
> churn to deal with those.  I however cannot think of any good way to
> do this here, so it's fine to export those functions.

It could be another hook, just that the hook pointer then would point
into .init.text (i.e. become stale once we purge .init.*). We could zap
it of course after invoking it ...

Note that the vendor function invocations have meanwhile, in the course
of re-basing, gained "if ( IS_ENABLED(CONFIG_...) )", so no extra
(future) churn for the (already available) option you talk about.

Jan

Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Roger Pau Monné]

On Thu, Jan 23, 2025 at 02:18:41PM +0100, Jan Beulich wrote:
> On 23.01.2025 13:41, Roger Pau Monné wrote:
> > On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
> >>      else if ( cpu_has_svm )
> >>          fns = start_svm();
> >>  
> >> +    if ( fns )
> >> +        hvm_funcs = *fns;
> >> +
> >> +    prune_vmx();
> >> +    prune_svm();
> > 
> > Isn't it actually the opposite of pruning.  What the helpers do is
> > fill all the pointers in the structure.
> 
> With the goal of their ENDBR to then be pruned. I agree though that the
> functions don't do any pruning themselves. Yet
> {svm,vmx}_prepare_for_cet_ibt_pruning() is a little awkward for my taste
> (although it would properly document the purpose). Plus ...
> 
> >  I would rather name them {vmx,svm}_fill_hvm_funcs() or similar.
> 
> ... while I can use those names (perhaps without the "hvm" infix), the
> present names have the advantage that any other pruning that we may
> find desirable could also be put there. Hence also why the cpu_has_*
> checks live there.

Hm, I'm unsure.  What else do you see becoming part of those
functions?  It's hard for me to suggest a name when it's unclear what
future logic do you think they could contain.

Given the current code I still think something that contains 'fill' or
similar is way more appropriate, the more if the IBT check is pulled
out into the caller.

> >  And possibly pull the
> > cpu_has_xen_ibt check outside the functions:
> > 
> > if ( cpu_has_xen_ibt )
> > {
> >     /*
> >      * Now that svm_function_table was copied, populate all function pointers
> >      * which may have been left at NULL, for __initdata_cf_clobber to have as
> >      * much of an effect as possible.
> >      */
> >     vmx_fill_hvm_funcs();
> >     svm_fill_hvm_funcs();
> > }
> 
> Which would leave the SVM function entirely empty.

You could possible declare it as an static inline in the hvm.h header
for the time being?

> The intention was for
> that to not be the case, and also for the comment you have added above
> to also live in the per-vendor functions.

Isn't that a bit redundant?  I would prefer to not have duplicated
comments over the code, hence my suggestion to place part of the logic
in the caller.

> > I would be nice to avoid directly exporting more vmx and smv specific
> > helpers, as if we ever want to compile out vmx or svm it would be more
> > churn to deal with those.  I however cannot think of any good way to
> > do this here, so it's fine to export those functions.
> 
> It could be another hook, just that the hook pointer then would point
> into .init.text (i.e. become stale once we purge .init.*). We could zap
> it of course after invoking it ...

Yes, I also think this is not great, and prefer your current proposal.

> Note that the vendor function invocations have meanwhile, in the course
> of re-basing, gained "if ( IS_ENABLED(CONFIG_...) )", so no extra
> (future) churn for the (already available) option you talk about.

Oh, OK, that's better then.

Thanks, Roger.

Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Jan Beulich]

On 23.01.2025 15:24, Roger Pau Monné wrote:
> On Thu, Jan 23, 2025 at 02:18:41PM +0100, Jan Beulich wrote:
>> On 23.01.2025 13:41, Roger Pau Monné wrote:
>>> On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
>>>>      else if ( cpu_has_svm )
>>>>          fns = start_svm();
>>>>  
>>>> +    if ( fns )
>>>> +        hvm_funcs = *fns;
>>>> +
>>>> +    prune_vmx();
>>>> +    prune_svm();
>>>
>>> Isn't it actually the opposite of pruning.  What the helpers do is
>>> fill all the pointers in the structure.
>>
>> With the goal of their ENDBR to then be pruned. I agree though that the
>> functions don't do any pruning themselves. Yet
>> {svm,vmx}_prepare_for_cet_ibt_pruning() is a little awkward for my taste
>> (although it would properly document the purpose). Plus ...
>>
>>>  I would rather name them {vmx,svm}_fill_hvm_funcs() or similar.
>>
>> ... while I can use those names (perhaps without the "hvm" infix), the
>> present names have the advantage that any other pruning that we may
>> find desirable could also be put there. Hence also why the cpu_has_*
>> checks live there.
> 
> Hm, I'm unsure.  What else do you see becoming part of those
> functions?  It's hard for me to suggest a name when it's unclear what
> future logic do you think they could contain.

Prior to IBT it wasn't foreseeable any pruning might be needed. We're
in a similar position now: We simply can't know whether anything else
is going to be needed there.

> Given the current code I still think something that contains 'fill' or
> similar is way more appropriate, the more if the IBT check is pulled
> out into the caller.

As indicated, I'd prefer the IBT check to remain in the function. But
yes, I'll see about renaming. If ever other stuff wants adding there,
we can surely rename another time.

>>>  And possibly pull the
>>> cpu_has_xen_ibt check outside the functions:
>>>
>>> if ( cpu_has_xen_ibt )
>>> {
>>>     /*
>>>      * Now that svm_function_table was copied, populate all function pointers
>>>      * which may have been left at NULL, for __initdata_cf_clobber to have as
>>>      * much of an effect as possible.
>>>      */
>>>     vmx_fill_hvm_funcs();
>>>     svm_fill_hvm_funcs();
>>> }
>>
>> Which would leave the SVM function entirely empty.
> 
> You could possible declare it as an static inline in the hvm.h header
> for the time being?
> 
>> The intention was for
>> that to not be the case, and also for the comment you have added above
>> to also live in the per-vendor functions.
> 
> Isn't that a bit redundant?  I would prefer to not have duplicated
> comments over the code, hence my suggestion to place part of the logic
> in the caller.

In this case I view the redundancy as necessary. You want to know what
to add to the functions when you look at them, irrespective of whether
you also look at their caller.

Jan

Re: [PATCH v3 02/12] x86/HVM: improve CET-IBT pruning of ENDBR

[Roger Pau Monné]

On Thu, Jan 23, 2025 at 05:14:39PM +0100, Jan Beulich wrote:
> On 23.01.2025 15:24, Roger Pau Monné wrote:
> > On Thu, Jan 23, 2025 at 02:18:41PM +0100, Jan Beulich wrote:
> >> On 23.01.2025 13:41, Roger Pau Monné wrote:
> >>> On Mon, Feb 26, 2024 at 05:42:20PM +0100, Jan Beulich wrote:
> >>>> --- a/xen/arch/x86/hvm/hvm.c
> >>>> +++ b/xen/arch/x86/hvm/hvm.c
> >>>> @@ -161,10 +161,15 @@ static int __init cf_check hvm_enable(vo
> >>>>      else if ( cpu_has_svm )
> >>>>          fns = start_svm();
> >>>>  
> >>>> +    if ( fns )
> >>>> +        hvm_funcs = *fns;
> >>>> +
> >>>> +    prune_vmx();
> >>>> +    prune_svm();
> >>>
> >>> Isn't it actually the opposite of pruning.  What the helpers do is
> >>> fill all the pointers in the structure.
> >>
> >> With the goal of their ENDBR to then be pruned. I agree though that the
> >> functions don't do any pruning themselves. Yet
> >> {svm,vmx}_prepare_for_cet_ibt_pruning() is a little awkward for my taste
> >> (although it would properly document the purpose). Plus ...
> >>
> >>>  I would rather name them {vmx,svm}_fill_hvm_funcs() or similar.
> >>
> >> ... while I can use those names (perhaps without the "hvm" infix), the
> >> present names have the advantage that any other pruning that we may
> >> find desirable could also be put there. Hence also why the cpu_has_*
> >> checks live there.
> > 
> > Hm, I'm unsure.  What else do you see becoming part of those
> > functions?  It's hard for me to suggest a name when it's unclear what
> > future logic do you think they could contain.
> 
> Prior to IBT it wasn't foreseeable any pruning might be needed. We're
> in a similar position now: We simply can't know whether anything else
> is going to be needed there.
> 
> > Given the current code I still think something that contains 'fill' or
> > similar is way more appropriate, the more if the IBT check is pulled
> > out into the caller.
> 
> As indicated, I'd prefer the IBT check to remain in the function. But
> yes, I'll see about renaming. If ever other stuff wants adding there,
> we can surely rename another time.
> 
> >>>  And possibly pull the
> >>> cpu_has_xen_ibt check outside the functions:
> >>>
> >>> if ( cpu_has_xen_ibt )
> >>> {
> >>>     /*
> >>>      * Now that svm_function_table was copied, populate all function pointers
> >>>      * which may have been left at NULL, for __initdata_cf_clobber to have as
> >>>      * much of an effect as possible.
> >>>      */
> >>>     vmx_fill_hvm_funcs();
> >>>     svm_fill_hvm_funcs();
> >>> }
> >>
> >> Which would leave the SVM function entirely empty.
> > 
> > You could possible declare it as an static inline in the hvm.h header
> > for the time being?
> > 
> >> The intention was for
> >> that to not be the case, and also for the comment you have added above
> >> to also live in the per-vendor functions.
> > 
> > Isn't that a bit redundant?  I would prefer to not have duplicated
> > comments over the code, hence my suggestion to place part of the logic
> > in the caller.
> 
> In this case I view the redundancy as necessary. You want to know what
> to add to the functions when you look at them, irrespective of whether
> you also look at their caller.

OK, I won't oppose to it, but I do think function names need
adjusting.

Thanks, Roger.

[PATCH v3 03/12] VMX: drop vmcs_revision_id

[Jan Beulich]

It's effectively redundant with vmx_basic_msr. For the #define
replacement to work, struct vmcs_struct's respective field name also
needs to change: Drop the not really meaningful "vmcs_" prefix from it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -175,7 +175,7 @@ static DEFINE_PER_CPU(paddr_t, current_v
 static DEFINE_PER_CPU(struct list_head, active_vmcs_list);
 DEFINE_PER_CPU(bool, vmxon);
 
-static u32 vmcs_revision_id __read_mostly;
+#define vmcs_revision_id (vmx_basic_msr & VMX_BASIC_REVISION_MASK)
 u64 __read_mostly vmx_basic_msr;
 
 static void __init vmx_display_features(void)
@@ -498,7 +498,6 @@ static int vmx_init_vmcs_config(bool bsp
     if ( !vmx_pin_based_exec_control )
     {
         /* First time through. */
-        vmcs_revision_id           = vmx_basic_msr_low & VMX_BASIC_REVISION_MASK;
         vmx_pin_based_exec_control = _vmx_pin_based_exec_control;
         vmx_cpu_based_exec_control = _vmx_cpu_based_exec_control;
         vmx_secondary_exec_control = _vmx_secondary_exec_control;
@@ -610,7 +609,7 @@ static paddr_t vmx_alloc_vmcs(void)
 
     vmcs = __map_domain_page(pg);
     clear_page(vmcs);
-    vmcs->vmcs_revision_id = vmcs_revision_id;
+    vmcs->revision_id = vmcs_revision_id;
     unmap_domain_page(vmcs);
 
     return page_to_maddr(pg);
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1166,7 +1166,7 @@ static void nvmx_set_vmcs_pointer(struct
     paddr_t vvmcs_maddr = v->arch.hvm.vmx.vmcs_shadow_maddr;
 
     __vmpclear(vvmcs_maddr);
-    vvmcs->vmcs_revision_id |= VMCS_RID_TYPE_MASK;
+    vvmcs->revision_id |= VMCS_RID_TYPE_MASK;
     v->arch.hvm.vmx.secondary_exec_control |=
         SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
     __vmwrite(SECONDARY_VM_EXEC_CONTROL,
@@ -1181,7 +1181,7 @@ static void nvmx_clear_vmcs_pointer(stru
     paddr_t vvmcs_maddr = v->arch.hvm.vmx.vmcs_shadow_maddr;
 
     __vmpclear(vvmcs_maddr);
-    vvmcs->vmcs_revision_id &= ~VMCS_RID_TYPE_MASK;
+    vvmcs->revision_id &= ~VMCS_RID_TYPE_MASK;
     v->arch.hvm.vmx.secondary_exec_control &=
         ~SECONDARY_EXEC_ENABLE_VMCS_SHADOWING;
     __vmwrite(SECONDARY_VM_EXEC_CONTROL,
@@ -1799,10 +1799,10 @@ static int nvmx_handle_vmptrld(struct cp
             {
                 struct vmcs_struct *vvmcs = vvmcx;
 
-                if ( ((vvmcs->vmcs_revision_id ^ vmx_basic_msr) &
-                                         VMX_BASIC_REVISION_MASK) ||
+                if ( ((vvmcs->revision_id ^ vmx_basic_msr) &
+                      VMX_BASIC_REVISION_MASK) ||
                      (!cpu_has_vmx_vmcs_shadowing &&
-                      (vvmcs->vmcs_revision_id & ~VMX_BASIC_REVISION_MASK)) )
+                      (vvmcs->revision_id & ~VMX_BASIC_REVISION_MASK)) )
                 {
                     hvm_unmap_guest_frame(vvmcx, 1);
                     vmfail(regs, VMX_INSN_VMPTRLD_INCORRECT_VMCS_ID);
@@ -2214,7 +2214,7 @@ int nvmx_msr_read_intercept(unsigned int
             map_domain_page(_mfn(PFN_DOWN(v->arch.hvm.vmx.vmcs_pa)));
 
         data = (host_data & (~0ul << 32)) |
-               (vmcs->vmcs_revision_id & 0x7fffffff);
+               (vmcs->revision_id & 0x7fffffff);
         unmap_domain_page(vmcs);
 
         if ( !cpu_has_vmx_vmcs_shadowing )
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -17,7 +17,7 @@ int cf_check vmx_cpu_up(void);
 void cf_check vmx_cpu_down(void);
 
 struct vmcs_struct {
-    u32 vmcs_revision_id;
+    uint32_t revision_id;
     unsigned char data [0]; /* vmcs size is read from MSR */
 };
 

Re: [PATCH v3 03/12] VMX: drop vmcs_revision_id

[Roger Pau Monné]

On Mon, Feb 26, 2024 at 05:42:50PM +0100, Jan Beulich wrote:
> It's effectively redundant with vmx_basic_msr. For the #define
> replacement to work, struct vmcs_struct's respective field name also
> needs to change: Drop the not really meaningful "vmcs_" prefix from it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: New.
> 
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -175,7 +175,7 @@ static DEFINE_PER_CPU(paddr_t, current_v
>  static DEFINE_PER_CPU(struct list_head, active_vmcs_list);
>  DEFINE_PER_CPU(bool, vmxon);
>  
> -static u32 vmcs_revision_id __read_mostly;
> +#define vmcs_revision_id (vmx_basic_msr & VMX_BASIC_REVISION_MASK)
>  u64 __read_mostly vmx_basic_msr;

__ro_after_init maybe while at it (and then uint64_t also)?

I would place the #define after the definition of vmx_basic_msr, but
that's just my taste.

Thanks, Roger.

Re: [PATCH v3 03/12] VMX: drop vmcs_revision_id

[Roger Pau Monné]

On Fri, Jan 24, 2025 at 11:51:37AM +0100, Roger Pau Monné wrote:
> On Mon, Feb 26, 2024 at 05:42:50PM +0100, Jan Beulich wrote:
> > It's effectively redundant with vmx_basic_msr. For the #define
> > replacement to work, struct vmcs_struct's respective field name also
> > needs to change: Drop the not really meaningful "vmcs_" prefix from it.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > ---
> > v2: New.
> > 
> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> > @@ -175,7 +175,7 @@ static DEFINE_PER_CPU(paddr_t, current_v
> >  static DEFINE_PER_CPU(struct list_head, active_vmcs_list);
> >  DEFINE_PER_CPU(bool, vmxon);
> >  
> > -static u32 vmcs_revision_id __read_mostly;
> > +#define vmcs_revision_id (vmx_basic_msr & VMX_BASIC_REVISION_MASK)
> >  u64 __read_mostly vmx_basic_msr;
> 
> __ro_after_init maybe while at it (and then uint64_t also)?
> 
> I would place the #define after the definition of vmx_basic_msr, but
> that's just my taste.

I see that this gets further adjusted by the next patch, and the
comments I made are no longer relevant.

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

[PATCH v3 04/12] VMX: convert vmx_basic_msr

[Jan Beulich]

... to a struct field, which is then going to be accompanied by other
capability/control data presently living in individual variables. As
this structure isn't supposed to be altered post-boot, put it in
.data.ro_after_init right away.

Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -161,6 +161,7 @@ static int cf_check parse_ept_param_runt
 #endif
 
 /* Dynamic (run-time adjusted) execution control flags. */
+struct vmx_caps __ro_after_init vmx_caps;
 u32 vmx_pin_based_exec_control __read_mostly;
 u32 vmx_cpu_based_exec_control __read_mostly;
 u32 vmx_secondary_exec_control __read_mostly;
@@ -175,8 +176,7 @@ static DEFINE_PER_CPU(paddr_t, current_v
 static DEFINE_PER_CPU(struct list_head, active_vmcs_list);
 DEFINE_PER_CPU(bool, vmxon);
 
-#define vmcs_revision_id (vmx_basic_msr & VMX_BASIC_REVISION_MASK)
-u64 __read_mostly vmx_basic_msr;
+#define vmcs_revision_id (vmx_caps.basic_msr & VMX_BASIC_REVISION_MASK)
 
 static void __init vmx_display_features(void)
 {
@@ -505,8 +505,8 @@ static int vmx_init_vmcs_config(bool bsp
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
         vmx_vmexit_control         = _vmx_vmexit_control;
         vmx_vmentry_control        = _vmx_vmentry_control;
-        vmx_basic_msr              = ((u64)vmx_basic_msr_high << 32) |
-                                     vmx_basic_msr_low;
+        vmx_caps.basic_msr = ((uint64_t)vmx_basic_msr_high << 32) |
+                             vmx_basic_msr_low;
         vmx_vmfunc                 = _vmx_vmfunc;
 
         vmx_display_features();
@@ -560,7 +560,7 @@ static int vmx_init_vmcs_config(bool bsp
             mismatch = 1;
         }
         if ( (vmx_basic_msr_high & (VMX_BASIC_VMCS_SIZE_MASK >> 32)) !=
-             ((vmx_basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
+             ((vmx_caps.basic_msr & VMX_BASIC_VMCS_SIZE_MASK) >> 32) )
         {
             printk("VMX: CPU%d unexpected VMCS size %Lu\n",
                    smp_processor_id(),
@@ -2214,7 +2214,7 @@ int __init vmx_vmcs_init(void)
          * _vmx_vcpu_up() may have made it past feature identification.
          * Make sure all dependent features are off as well.
          */
-        vmx_basic_msr              = 0;
+        memset(&vmx_caps, 0, sizeof(vmx_caps));
         vmx_pin_based_exec_control = 0;
         vmx_cpu_based_exec_control = 0;
         vmx_secondary_exec_control = 0;
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -294,6 +294,12 @@ extern u64 vmx_ept_vpid_cap;
 
 #define VMX_TSC_MULTIPLIER_MAX                  0xffffffffffffffffULL
 
+/* Capabilities and dynamic (run-time adjusted) execution control flags. */
+struct vmx_caps {
+    uint64_t basic_msr;
+};
+extern struct vmx_caps vmx_caps;
+
 #define cpu_has_wbinvd_exiting \
     (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
 #define cpu_has_vmx_virtualize_apic_accesses \
@@ -379,9 +385,8 @@ extern u64 vmx_ept_vpid_cap;
  */
 #define VMX_BASIC_DEFAULT1_ZERO		(1ULL << 55)
 
-extern u64 vmx_basic_msr;
 #define cpu_has_vmx_ins_outs_instr_info \
-    (!!(vmx_basic_msr & VMX_BASIC_INS_OUT_INFO))
+    (!!(vmx_caps.basic_msr & VMX_BASIC_INS_OUT_INFO))
 
 /* Guest interrupt status */
 #define VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK  0x0FF
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1561,7 +1561,7 @@ static int nvmx_handle_vmxon(struct cpu_
     rc = hvm_copy_from_guest_phys(&nvmcs_revid, gpa, sizeof(nvmcs_revid));
     if ( rc != HVMTRANS_okay ||
          (nvmcs_revid & ~VMX_BASIC_REVISION_MASK) ||
-         ((nvmcs_revid ^ vmx_basic_msr) & VMX_BASIC_REVISION_MASK) )
+         ((nvmcs_revid ^ vmx_caps.basic_msr) & VMX_BASIC_REVISION_MASK) )
     {
         vmfail_invalid(regs);
         return X86EMUL_OKAY;
@@ -1799,7 +1799,7 @@ static int nvmx_handle_vmptrld(struct cp
             {
                 struct vmcs_struct *vvmcs = vvmcx;
 
-                if ( ((vvmcs->revision_id ^ vmx_basic_msr) &
+                if ( ((vvmcs->revision_id ^ vmx_caps.basic_msr) &
                       VMX_BASIC_REVISION_MASK) ||
                      (!cpu_has_vmx_vmcs_shadowing &&
                       (vvmcs->revision_id & ~VMX_BASIC_REVISION_MASK)) )
@@ -2192,7 +2192,7 @@ int nvmx_msr_read_intercept(unsigned int
     case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
     case MSR_IA32_VMX_TRUE_EXIT_CTLS:
     case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
-        if ( !(vmx_basic_msr & VMX_BASIC_DEFAULT1_ZERO) )
+        if ( !(vmx_caps.basic_msr & VMX_BASIC_DEFAULT1_ZERO) )
             return 0;
         break;
 

Re: [PATCH v3 04/12] VMX: convert vmx_basic_msr

[Roger Pau Monné]

On Mon, Feb 26, 2024 at 05:43:21PM +0100, Jan Beulich wrote:
> ... to a struct field, which is then going to be accompanied by other
> capability/control data presently living in individual variables. As
> this structure isn't supposed to be altered post-boot, put it in
> .data.ro_after_init right away.
> 
> Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

[PATCH v3 05/12] VMX: convert vmx_pin_based_exec_control

[Jan Beulich]

... to a field in the capability/controls struct. Use an instance of
that struct also in vmx_init_vmcs_config().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: Add initializer to vmx_init_vmcs_config() new local var.
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u32 vmx_pin_based_exec_control __read_mostly;
 u32 vmx_cpu_based_exec_control __read_mostly;
 u32 vmx_secondary_exec_control __read_mostly;
 uint64_t vmx_tertiary_exec_control __read_mostly;
@@ -261,7 +260,7 @@ static bool cap_check(
 static int vmx_init_vmcs_config(bool bsp)
 {
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
-    u32 _vmx_pin_based_exec_control;
+    struct vmx_caps caps = {};
     u32 _vmx_cpu_based_exec_control;
     u32 _vmx_secondary_exec_control = 0;
     uint64_t _vmx_tertiary_exec_control = 0;
@@ -278,7 +277,7 @@ static int vmx_init_vmcs_config(bool bsp
            PIN_BASED_NMI_EXITING);
     opt = (PIN_BASED_VIRTUAL_NMIS |
            PIN_BASED_POSTED_INTERRUPT);
-    _vmx_pin_based_exec_control = adjust_vmx_controls(
+    caps.pin_based_exec_control = adjust_vmx_controls(
         "Pin-Based Exec Control", min, opt,
         MSR_IA32_VMX_PINBASED_CTLS, &mismatch);
 
@@ -440,7 +439,7 @@ static int vmx_init_vmcs_config(bool bsp
     if ( (_vmx_secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING) &&
           ple_gap == 0 )
     {
-        if ( !vmx_pin_based_exec_control )
+        if ( !vmx_caps.pin_based_exec_control )
             printk(XENLOG_INFO "Disable Pause-Loop Exiting.\n");
         _vmx_secondary_exec_control &= ~ SECONDARY_EXEC_PAUSE_LOOP_EXITING;
     }
@@ -458,10 +457,10 @@ static int vmx_init_vmcs_config(bool bsp
      * is a minimal requirement, only check the former, which is optional.
      */
     if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
-        _vmx_pin_based_exec_control &= ~PIN_BASED_POSTED_INTERRUPT;
+        caps.pin_based_exec_control &= ~PIN_BASED_POSTED_INTERRUPT;
 
     if ( iommu_intpost &&
-         !(_vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
+         !(caps.pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
     {
         printk("Intel VT-d Posted Interrupt is disabled for CPU-side Posted "
                "Interrupt is not enabled\n");
@@ -495,10 +494,10 @@ static int vmx_init_vmcs_config(bool bsp
     if ( mismatch )
         return -EINVAL;
 
-    if ( !vmx_pin_based_exec_control )
+    if ( !vmx_caps.pin_based_exec_control )
     {
         /* First time through. */
-        vmx_pin_based_exec_control = _vmx_pin_based_exec_control;
+        vmx_caps = caps;
         vmx_cpu_based_exec_control = _vmx_cpu_based_exec_control;
         vmx_secondary_exec_control = _vmx_secondary_exec_control;
         vmx_tertiary_exec_control  = _vmx_tertiary_exec_control;
@@ -529,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmcs_revision_id, vmx_basic_msr_low & VMX_BASIC_REVISION_MASK);
         mismatch |= cap_check(
             "Pin-Based Exec Control",
-            vmx_pin_based_exec_control, _vmx_pin_based_exec_control);
+            vmx_caps.pin_based_exec_control, caps.pin_based_exec_control);
         mismatch |= cap_check(
             "CPU-Based Exec Control",
             vmx_cpu_based_exec_control, _vmx_cpu_based_exec_control);
@@ -1110,7 +1109,7 @@ static int construct_vmcs(struct vcpu *v
     vmx_vmcs_enter(v);
 
     /* VMCS controls. */
-    __vmwrite(PIN_BASED_VM_EXEC_CONTROL, vmx_pin_based_exec_control);
+    __vmwrite(PIN_BASED_VM_EXEC_CONTROL, vmx_caps.pin_based_exec_control);
 
     v->arch.hvm.vmx.exec_control = vmx_cpu_based_exec_control;
     if ( d->arch.vtsc && !cpu_has_vmx_tsc_scaling )
@@ -2136,7 +2135,7 @@ void vmcs_dump_vcpu(struct vcpu *v)
     printk("TSC Offset = 0x%016lx  TSC Multiplier = 0x%016lx\n",
            vmr(TSC_OFFSET), vmr(TSC_MULTIPLIER));
     if ( (v->arch.hvm.vmx.exec_control & CPU_BASED_TPR_SHADOW) ||
-         (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
+         (vmx_caps.pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT) )
         printk("TPR Threshold = 0x%02x  PostedIntrVec = 0x%02x\n",
                vmr32(TPR_THRESHOLD), vmr16(POSTED_INTR_NOTIFICATION_VECTOR));
     if ( (v->arch.hvm.vmx.secondary_exec_control &
@@ -2215,7 +2214,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_pin_based_exec_control = 0;
         vmx_cpu_based_exec_control = 0;
         vmx_secondary_exec_control = 0;
         vmx_tertiary_exec_control  = 0;
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1057,7 +1057,7 @@ static void load_shadow_control(struct v
      * and EXCEPTION
      * Enforce the removed features
      */
-    nvmx_update_pin_control(v, vmx_pin_based_exec_control);
+    nvmx_update_pin_control(v, vmx_caps.pin_based_exec_control);
     vmx_update_cpu_exec_control(v);
     vmx_update_secondary_exec_control(v);
     nvmx_update_exit_control(v, vmx_vmexit_control);
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -217,7 +217,6 @@ extern u32 vmx_cpu_based_exec_control;
 #define PIN_BASED_VIRTUAL_NMIS          0x00000020
 #define PIN_BASED_PREEMPT_TIMER         0x00000040
 #define PIN_BASED_POSTED_INTERRUPT      0x00000080
-extern u32 vmx_pin_based_exec_control;
 
 #define VM_EXIT_SAVE_DEBUG_CNTRLS       0x00000004
 #define VM_EXIT_IA32E_MODE              0x00000200
@@ -297,6 +296,7 @@ extern u64 vmx_ept_vpid_cap;
 /* Capabilities and dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps {
     uint64_t basic_msr;
+    uint32_t pin_based_exec_control;
 };
 extern struct vmx_caps vmx_caps;
 
@@ -307,7 +307,7 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_tpr_shadow \
     (vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
 #define cpu_has_vmx_vnmi \
-    (vmx_pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
+    (vmx_caps.pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
 #define cpu_has_vmx_msr_bitmap \
     (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
 #define cpu_has_vmx_secondary_exec_control \
@@ -344,7 +344,7 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_virtualize_x2apic_mode \
     (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
 #define cpu_has_vmx_posted_intr_processing \
-    (vmx_pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT)
+    (vmx_caps.pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT)
 #define cpu_has_vmx_vmcs_shadowing \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VMCS_SHADOWING)
 #define cpu_has_vmx_vmfunc \

[PATCH v3 06/12] VMX: convert vmx_cpu_based_exec_control

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u32 vmx_cpu_based_exec_control __read_mostly;
 u32 vmx_secondary_exec_control __read_mostly;
 uint64_t vmx_tertiary_exec_control __read_mostly;
 u32 vmx_vmexit_control __read_mostly;
@@ -261,7 +260,6 @@ static int vmx_init_vmcs_config(bool bsp
 {
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
     struct vmx_caps caps = {};
-    u32 _vmx_cpu_based_exec_control;
     u32 _vmx_secondary_exec_control = 0;
     uint64_t _vmx_tertiary_exec_control = 0;
     u64 _vmx_ept_vpid_cap = 0;
@@ -299,12 +297,12 @@ static int vmx_init_vmcs_config(bool bsp
            CPU_BASED_MONITOR_TRAP_FLAG |
            CPU_BASED_ACTIVATE_SECONDARY_CONTROLS |
            CPU_BASED_ACTIVATE_TERTIARY_CONTROLS);
-    _vmx_cpu_based_exec_control = adjust_vmx_controls(
+    caps.cpu_based_exec_control = adjust_vmx_controls(
         "CPU-Based Exec Control", min, opt,
         MSR_IA32_VMX_PROCBASED_CTLS, &mismatch);
-    _vmx_cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING;
-    if ( _vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW )
-        _vmx_cpu_based_exec_control &=
+    caps.cpu_based_exec_control &= ~CPU_BASED_RDTSC_EXITING;
+    if ( caps.cpu_based_exec_control & CPU_BASED_TPR_SHADOW )
+        caps.cpu_based_exec_control &=
             ~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING);
 
     rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap);
@@ -321,7 +319,7 @@ static int vmx_init_vmcs_config(bool bsp
         return -EINVAL;
     }
 
-    if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS )
+    if ( caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS )
     {
         min = 0;
         opt = (SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
@@ -351,7 +349,7 @@ static int vmx_init_vmcs_config(bool bsp
          * "APIC Register Virtualization" and "Virtual Interrupt Delivery"
          * can be set only when "use TPR shadow" is set
          */
-        if ( (_vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW) &&
+        if ( (caps.cpu_based_exec_control & CPU_BASED_TPR_SHADOW) &&
              opt_apicv_enabled )
             opt |= SECONDARY_EXEC_APIC_REGISTER_VIRT |
                    SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
@@ -362,7 +360,7 @@ static int vmx_init_vmcs_config(bool bsp
             MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);
     }
 
-    if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS )
+    if ( caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS )
     {
         uint64_t opt = 0;
 
@@ -498,7 +496,6 @@ static int vmx_init_vmcs_config(bool bsp
     {
         /* First time through. */
         vmx_caps = caps;
-        vmx_cpu_based_exec_control = _vmx_cpu_based_exec_control;
         vmx_secondary_exec_control = _vmx_secondary_exec_control;
         vmx_tertiary_exec_control  = _vmx_tertiary_exec_control;
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
@@ -531,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmx_caps.pin_based_exec_control, caps.pin_based_exec_control);
         mismatch |= cap_check(
             "CPU-Based Exec Control",
-            vmx_cpu_based_exec_control, _vmx_cpu_based_exec_control);
+            vmx_caps.cpu_based_exec_control, caps.cpu_based_exec_control);
         mismatch |= cap_check(
             "Secondary Exec Control",
             vmx_secondary_exec_control, _vmx_secondary_exec_control);
@@ -1111,7 +1108,7 @@ static int construct_vmcs(struct vcpu *v
     /* VMCS controls. */
     __vmwrite(PIN_BASED_VM_EXEC_CONTROL, vmx_caps.pin_based_exec_control);
 
-    v->arch.hvm.vmx.exec_control = vmx_cpu_based_exec_control;
+    v->arch.hvm.vmx.exec_control = vmx_caps.cpu_based_exec_control;
     if ( d->arch.vtsc && !cpu_has_vmx_tsc_scaling )
         v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
 
@@ -2214,7 +2211,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_cpu_based_exec_control = 0;
         vmx_secondary_exec_control = 0;
         vmx_tertiary_exec_control  = 0;
         vmx_vmexit_control         = 0;
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -210,7 +210,6 @@ void vmx_vmcs_reload(struct vcpu *v);
 #define CPU_BASED_MONITOR_EXITING             0x20000000U
 #define CPU_BASED_PAUSE_EXITING               0x40000000U
 #define CPU_BASED_ACTIVATE_SECONDARY_CONTROLS 0x80000000U
-extern u32 vmx_cpu_based_exec_control;
 
 #define PIN_BASED_EXT_INTR_MASK         0x00000001
 #define PIN_BASED_NMI_EXITING           0x00000008
@@ -297,6 +296,7 @@ extern u64 vmx_ept_vpid_cap;
 struct vmx_caps {
     uint64_t basic_msr;
     uint32_t pin_based_exec_control;
+    uint32_t cpu_based_exec_control;
 };
 extern struct vmx_caps vmx_caps;
 
@@ -305,15 +305,15 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_virtualize_apic_accesses \
     (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
 #define cpu_has_vmx_tpr_shadow \
-    (vmx_cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
+    (vmx_caps.cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
 #define cpu_has_vmx_vnmi \
     (vmx_caps.pin_based_exec_control & PIN_BASED_VIRTUAL_NMIS)
 #define cpu_has_vmx_msr_bitmap \
-    (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
+    (vmx_caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_MSR_BITMAP)
 #define cpu_has_vmx_secondary_exec_control \
-    (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
+    (vmx_caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
 #define cpu_has_vmx_tertiary_exec_control \
-    (vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS)
+    (vmx_caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS)
 #define cpu_has_vmx_ept \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT)
 #define cpu_has_vmx_dt_exiting \
@@ -323,7 +323,7 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_vpid \
     (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
 #define cpu_has_monitor_trap_flag \
-    (vmx_cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
+    (vmx_caps.cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
 #define cpu_has_vmx_pat \
     (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_PAT)
 #define cpu_has_vmx_efer \

[PATCH v3 07/12] VMX: convert vmx_secondary_exec_control

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u32 vmx_secondary_exec_control __read_mostly;
 uint64_t vmx_tertiary_exec_control __read_mostly;
 u32 vmx_vmexit_control __read_mostly;
 u32 vmx_vmentry_control __read_mostly;
@@ -260,7 +259,6 @@ static int vmx_init_vmcs_config(bool bsp
 {
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
     struct vmx_caps caps = {};
-    u32 _vmx_secondary_exec_control = 0;
     uint64_t _vmx_tertiary_exec_control = 0;
     u64 _vmx_ept_vpid_cap = 0;
     u64 _vmx_misc_cap = 0;
@@ -355,7 +353,7 @@ static int vmx_init_vmcs_config(bool bsp
                    SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
                    SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE;
 
-        _vmx_secondary_exec_control = adjust_vmx_controls(
+        caps.secondary_exec_control = adjust_vmx_controls(
             "Secondary Exec Control", min, opt,
             MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch);
     }
@@ -370,7 +368,7 @@ static int vmx_init_vmcs_config(bool bsp
     }
 
     /* The IA32_VMX_EPT_VPID_CAP MSR exists only when EPT or VPID available */
-    if ( _vmx_secondary_exec_control & (SECONDARY_EXEC_ENABLE_EPT |
+    if ( caps.secondary_exec_control & (SECONDARY_EXEC_ENABLE_EPT |
                                         SECONDARY_EXEC_ENABLE_VPID) )
     {
         rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, _vmx_ept_vpid_cap);
@@ -392,7 +390,7 @@ static int vmx_init_vmcs_config(bool bsp
         if ( !(_vmx_ept_vpid_cap & VMX_EPT_MEMORY_TYPE_WB) ||
              !(_vmx_ept_vpid_cap & VMX_EPT_WALK_LENGTH_4_SUPPORTED) ||
              !(_vmx_ept_vpid_cap & VMX_EPT_INVEPT_ALL_CONTEXT) )
-            _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_EPT;
+            caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_EPT;
 
         /*
          * the CPU must support INVVPID all context invalidation, because we
@@ -401,14 +399,14 @@ static int vmx_init_vmcs_config(bool bsp
          * Or we just don't use VPID.
          */
         if ( !(_vmx_ept_vpid_cap & VMX_VPID_INVVPID_ALL_CONTEXT) )
-            _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
+            caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
 
         /* EPT A/D bits is required for PML */
         if ( !(_vmx_ept_vpid_cap & VMX_EPT_AD_BIT) )
-            _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
+            caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
     }
 
-    if ( _vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT )
+    if ( caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT )
     {
         /*
          * To use EPT we expect to be able to clear certain intercepts.
@@ -421,25 +419,25 @@ static int vmx_init_vmcs_config(bool bsp
         if ( must_be_one & (CPU_BASED_INVLPG_EXITING |
                             CPU_BASED_CR3_LOAD_EXITING |
                             CPU_BASED_CR3_STORE_EXITING) )
-            _vmx_secondary_exec_control &=
+            caps.secondary_exec_control &=
                 ~(SECONDARY_EXEC_ENABLE_EPT |
                   SECONDARY_EXEC_UNRESTRICTED_GUEST);
     }
 
     /* PML cannot be supported if EPT is not used */
-    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT) )
-        _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
+    if ( !(caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT) )
+        caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
     /* Turn off opt_ept_pml if PML feature is not present. */
-    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML) )
+    if ( !(caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_PML) )
         opt_ept_pml = false;
 
-    if ( (_vmx_secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING) &&
+    if ( (caps.secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING) &&
           ple_gap == 0 )
     {
         if ( !vmx_caps.pin_based_exec_control )
             printk(XENLOG_INFO "Disable Pause-Loop Exiting.\n");
-        _vmx_secondary_exec_control &= ~ SECONDARY_EXEC_PAUSE_LOOP_EXITING;
+        caps.secondary_exec_control &= ~ SECONDARY_EXEC_PAUSE_LOOP_EXITING;
     }
 
     min = VM_EXIT_ACK_INTR_ON_EXIT;
@@ -454,7 +452,7 @@ static int vmx_init_vmcs_config(bool bsp
      * delivery" and "acknowledge interrupt on exit" is set. For the latter
      * is a minimal requirement, only check the former, which is optional.
      */
-    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
+    if ( !(caps.secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) )
         caps.pin_based_exec_control &= ~PIN_BASED_POSTED_INTERRUPT;
 
     if ( iommu_intpost &&
@@ -466,7 +464,7 @@ static int vmx_init_vmcs_config(bool bsp
     }
 
     /* The IA32_VMX_VMFUNC MSR exists only when VMFUNC is available */
-    if ( _vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
+    if ( caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
     {
         rdmsrl(MSR_IA32_VMX_VMFUNC, _vmx_vmfunc);
 
@@ -476,12 +474,12 @@ static int vmx_init_vmcs_config(bool bsp
          * Or we just don't use VMFUNC.
          */
         if ( !(_vmx_vmfunc & VMX_VMFUNC_EPTP_SWITCHING) )
-            _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VM_FUNCTIONS;
+            caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VM_FUNCTIONS;
     }
 
     /* Virtualization exceptions are only enabled if VMFUNC is enabled */
-    if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS) )
-        _vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS;
+    if ( !(caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS) )
+        caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS;
 
     min = 0;
     opt = (VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_GUEST_EFER |
@@ -496,7 +494,6 @@ static int vmx_init_vmcs_config(bool bsp
     {
         /* First time through. */
         vmx_caps = caps;
-        vmx_secondary_exec_control = _vmx_secondary_exec_control;
         vmx_tertiary_exec_control  = _vmx_tertiary_exec_control;
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
         vmx_vmexit_control         = _vmx_vmexit_control;
@@ -531,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmx_caps.cpu_based_exec_control, caps.cpu_based_exec_control);
         mismatch |= cap_check(
             "Secondary Exec Control",
-            vmx_secondary_exec_control, _vmx_secondary_exec_control);
+            vmx_caps.secondary_exec_control, caps.secondary_exec_control);
         mismatch |= cap_check(
             "Tertiary Exec Control",
             vmx_tertiary_exec_control, _vmx_tertiary_exec_control);
@@ -1112,7 +1109,7 @@ static int construct_vmcs(struct vcpu *v
     if ( d->arch.vtsc && !cpu_has_vmx_tsc_scaling )
         v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
 
-    v->arch.hvm.vmx.secondary_exec_control = vmx_secondary_exec_control;
+    v->arch.hvm.vmx.secondary_exec_control = vmx_caps.secondary_exec_control;
     v->arch.hvm.vmx.tertiary_exec_control  = vmx_tertiary_exec_control;
 
     /*
@@ -2211,7 +2208,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_secondary_exec_control = 0;
         vmx_tertiary_exec_control  = 0;
         vmx_vmexit_control         = 0;
         vmx_vmentry_control        = 0;
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -258,7 +258,6 @@ extern u32 vmx_vmentry_control;
 #define SECONDARY_EXEC_TSC_SCALING              0x02000000U
 #define SECONDARY_EXEC_BUS_LOCK_DETECTION       0x40000000U
 #define SECONDARY_EXEC_NOTIFY_VM_EXITING        0x80000000U
-extern u32 vmx_secondary_exec_control;
 
 #define TERTIARY_EXEC_LOADIWKEY_EXITING         BIT(0, UL)
 #define TERTIARY_EXEC_ENABLE_HLAT               BIT(1, UL)
@@ -297,13 +296,14 @@ struct vmx_caps {
     uint64_t basic_msr;
     uint32_t pin_based_exec_control;
     uint32_t cpu_based_exec_control;
+    uint32_t secondary_exec_control;
 };
 extern struct vmx_caps vmx_caps;
 
 #define cpu_has_wbinvd_exiting \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_WBINVD_EXITING)
 #define cpu_has_vmx_virtualize_apic_accesses \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)
 #define cpu_has_vmx_tpr_shadow \
     (vmx_caps.cpu_based_exec_control & CPU_BASED_TPR_SHADOW)
 #define cpu_has_vmx_vnmi \
@@ -315,13 +315,13 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_tertiary_exec_control \
     (vmx_caps.cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS)
 #define cpu_has_vmx_ept \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT)
 #define cpu_has_vmx_dt_exiting \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING)
 #define cpu_has_vmx_rdtscp \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_RDTSCP)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_RDTSCP)
 #define cpu_has_vmx_vpid \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
 #define cpu_has_monitor_trap_flag \
     (vmx_caps.cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
 #define cpu_has_vmx_pat \
@@ -329,41 +329,41 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_efer \
     (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_EFER)
 #define cpu_has_vmx_unrestricted_guest \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_UNRESTRICTED_GUEST)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_UNRESTRICTED_GUEST)
 #define vmx_unrestricted_guest(v)               \
     ((v)->arch.hvm.vmx.secondary_exec_control & \
      SECONDARY_EXEC_UNRESTRICTED_GUEST)
 #define cpu_has_vmx_ple \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING)
 #define cpu_has_vmx_invpcid \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_INVPCID)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_INVPCID)
 #define cpu_has_vmx_apic_reg_virt \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_APIC_REGISTER_VIRT)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_APIC_REGISTER_VIRT)
 #define cpu_has_vmx_virtual_intr_delivery \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY)
 #define cpu_has_vmx_virtualize_x2apic_mode \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
 #define cpu_has_vmx_posted_intr_processing \
     (vmx_caps.pin_based_exec_control & PIN_BASED_POSTED_INTERRUPT)
 #define cpu_has_vmx_vmcs_shadowing \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VMCS_SHADOWING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VMCS_SHADOWING)
 #define cpu_has_vmx_vmfunc \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS)
 #define cpu_has_vmx_virt_exceptions \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS)
 #define cpu_has_vmx_pml \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
 #define cpu_has_vmx_mpx \
     ((vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
      (vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS))
 #define cpu_has_vmx_xsaves \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_XSAVES)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_XSAVES)
 #define cpu_has_vmx_tsc_scaling \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_TSC_SCALING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_TSC_SCALING)
 #define cpu_has_vmx_bus_lock_detection \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_BUS_LOCK_DETECTION)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_BUS_LOCK_DETECTION)
 #define cpu_has_vmx_notify_vm_exiting \
-    (vmx_secondary_exec_control & SECONDARY_EXEC_NOTIFY_VM_EXITING)
+    (vmx_caps.secondary_exec_control & SECONDARY_EXEC_NOTIFY_VM_EXITING)
 
 #define VMCS_RID_TYPE_MASK              0x80000000U
 

[PATCH v3 08/12] VMX: convert vmx_tertiary_exec_control

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-uint64_t vmx_tertiary_exec_control __read_mostly;
 u32 vmx_vmexit_control __read_mostly;
 u32 vmx_vmentry_control __read_mostly;
 u64 vmx_ept_vpid_cap __read_mostly;
@@ -259,7 +258,6 @@ static int vmx_init_vmcs_config(bool bsp
 {
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
     struct vmx_caps caps = {};
-    uint64_t _vmx_tertiary_exec_control = 0;
     u64 _vmx_ept_vpid_cap = 0;
     u64 _vmx_misc_cap = 0;
     u32 _vmx_vmexit_control;
@@ -362,7 +360,7 @@ static int vmx_init_vmcs_config(bool bsp
     {
         uint64_t opt = 0;
 
-        _vmx_tertiary_exec_control = adjust_vmx_controls2(
+        caps.tertiary_exec_control = adjust_vmx_controls2(
             "Tertiary Exec Control", 0, opt,
             MSR_IA32_VMX_PROCBASED_CTLS3, &mismatch);
     }
@@ -494,7 +492,6 @@ static int vmx_init_vmcs_config(bool bsp
     {
         /* First time through. */
         vmx_caps = caps;
-        vmx_tertiary_exec_control  = _vmx_tertiary_exec_control;
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
         vmx_vmexit_control         = _vmx_vmexit_control;
         vmx_vmentry_control        = _vmx_vmentry_control;
@@ -531,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmx_caps.secondary_exec_control, caps.secondary_exec_control);
         mismatch |= cap_check(
             "Tertiary Exec Control",
-            vmx_tertiary_exec_control, _vmx_tertiary_exec_control);
+            vmx_caps.tertiary_exec_control, caps.tertiary_exec_control);
         mismatch |= cap_check(
             "VMExit Control",
             vmx_vmexit_control, _vmx_vmexit_control);
@@ -1110,7 +1107,7 @@ static int construct_vmcs(struct vcpu *v
         v->arch.hvm.vmx.exec_control |= CPU_BASED_RDTSC_EXITING;
 
     v->arch.hvm.vmx.secondary_exec_control = vmx_caps.secondary_exec_control;
-    v->arch.hvm.vmx.tertiary_exec_control  = vmx_tertiary_exec_control;
+    v->arch.hvm.vmx.tertiary_exec_control  = vmx_caps.tertiary_exec_control;
 
     /*
      * Disable features which we don't want active by default:
@@ -2208,7 +2205,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_tertiary_exec_control  = 0;
         vmx_vmexit_control         = 0;
         vmx_vmentry_control        = 0;
         vmx_ept_vpid_cap           = 0;
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -265,7 +265,6 @@ extern u32 vmx_vmentry_control;
 #define TERTIARY_EXEC_GUEST_PAGING_VERIFY       BIT(3, UL)
 #define TERTIARY_EXEC_IPI_VIRT                  BIT(4, UL)
 #define TERTIARY_EXEC_VIRT_SPEC_CTRL            BIT(7, UL)
-extern uint64_t vmx_tertiary_exec_control;
 
 #define VMX_EPT_EXEC_ONLY_SUPPORTED                         0x00000001
 #define VMX_EPT_WALK_LENGTH_4_SUPPORTED                     0x00000040
@@ -297,6 +296,7 @@ struct vmx_caps {
     uint32_t pin_based_exec_control;
     uint32_t cpu_based_exec_control;
     uint32_t secondary_exec_control;
+    uint64_t tertiary_exec_control;
 };
 extern struct vmx_caps vmx_caps;
 

[PATCH v3 09/12] VMX: convert vmx_vmexit_control

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u32 vmx_vmexit_control __read_mostly;
 u32 vmx_vmentry_control __read_mostly;
 u64 vmx_ept_vpid_cap __read_mostly;
 static uint64_t __read_mostly vmx_vmfunc;
@@ -260,7 +259,6 @@ static int vmx_init_vmcs_config(bool bsp
     struct vmx_caps caps = {};
     u64 _vmx_ept_vpid_cap = 0;
     u64 _vmx_misc_cap = 0;
-    u32 _vmx_vmexit_control;
     u32 _vmx_vmentry_control;
     u64 _vmx_vmfunc = 0;
     bool mismatch = false;
@@ -442,7 +440,7 @@ static int vmx_init_vmcs_config(bool bsp
     opt = (VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT |
            VM_EXIT_LOAD_HOST_EFER | VM_EXIT_CLEAR_BNDCFGS);
     min |= VM_EXIT_IA32E_MODE;
-    _vmx_vmexit_control = adjust_vmx_controls(
+    caps.vmexit_control = adjust_vmx_controls(
         "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, &mismatch);
 
     /*
@@ -493,7 +491,6 @@ static int vmx_init_vmcs_config(bool bsp
         /* First time through. */
         vmx_caps = caps;
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
-        vmx_vmexit_control         = _vmx_vmexit_control;
         vmx_vmentry_control        = _vmx_vmentry_control;
         vmx_caps.basic_msr = ((uint64_t)vmx_basic_msr_high << 32) |
                              vmx_basic_msr_low;
@@ -531,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmx_caps.tertiary_exec_control, caps.tertiary_exec_control);
         mismatch |= cap_check(
             "VMExit Control",
-            vmx_vmexit_control, _vmx_vmexit_control);
+            vmx_caps.vmexit_control, caps.vmexit_control);
         mismatch |= cap_check(
             "VMEntry Control",
             vmx_vmentry_control, _vmx_vmentry_control);
@@ -1093,7 +1090,7 @@ void nocall vmx_asm_vmexit_handler(void)
 static int construct_vmcs(struct vcpu *v)
 {
     struct domain *d = v->domain;
-    u32 vmexit_ctl = vmx_vmexit_control;
+    uint32_t vmexit_ctl = vmx_caps.vmexit_control;
     u32 vmentry_ctl = vmx_vmentry_control;
     int rc = 0;
 
@@ -2205,7 +2202,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_vmexit_control         = 0;
         vmx_vmentry_control        = 0;
         vmx_ept_vpid_cap           = 0;
         vmx_vmfunc                 = 0;
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1060,7 +1060,7 @@ static void load_shadow_control(struct v
     nvmx_update_pin_control(v, vmx_caps.pin_based_exec_control);
     vmx_update_cpu_exec_control(v);
     vmx_update_secondary_exec_control(v);
-    nvmx_update_exit_control(v, vmx_vmexit_control);
+    nvmx_update_exit_control(v, vmx_caps.vmexit_control);
     nvmx_update_entry_control(v);
     vmx_update_exception_bitmap(v);
     nvmx_update_apic_access_address(v);
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -227,7 +227,6 @@ void vmx_vmcs_reload(struct vcpu *v);
 #define VM_EXIT_LOAD_HOST_EFER          0x00200000
 #define VM_EXIT_SAVE_PREEMPT_TIMER      0x00400000
 #define VM_EXIT_CLEAR_BNDCFGS           0x00800000
-extern u32 vmx_vmexit_control;
 
 #define VM_ENTRY_IA32E_MODE             0x00000200
 #define VM_ENTRY_SMM                    0x00000400
@@ -297,6 +296,7 @@ struct vmx_caps {
     uint32_t cpu_based_exec_control;
     uint32_t secondary_exec_control;
     uint64_t tertiary_exec_control;
+    uint32_t vmexit_control;
 };
 extern struct vmx_caps vmx_caps;
 
@@ -354,7 +354,7 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_vmx_pml \
     (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
 #define cpu_has_vmx_mpx \
-    ((vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
+    ((vmx_caps.vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
      (vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS))
 #define cpu_has_vmx_xsaves \
     (vmx_caps.secondary_exec_control & SECONDARY_EXEC_XSAVES)

[PATCH v3 10/12] VMX: convert vmx_vmentry_control

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u32 vmx_vmentry_control __read_mostly;
 u64 vmx_ept_vpid_cap __read_mostly;
 static uint64_t __read_mostly vmx_vmfunc;
 
@@ -259,7 +258,6 @@ static int vmx_init_vmcs_config(bool bsp
     struct vmx_caps caps = {};
     u64 _vmx_ept_vpid_cap = 0;
     u64 _vmx_misc_cap = 0;
-    u32 _vmx_vmentry_control;
     u64 _vmx_vmfunc = 0;
     bool mismatch = false;
 
@@ -480,7 +478,7 @@ static int vmx_init_vmcs_config(bool bsp
     min = 0;
     opt = (VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_GUEST_EFER |
            VM_ENTRY_LOAD_BNDCFGS);
-    _vmx_vmentry_control = adjust_vmx_controls(
+    caps.vmentry_control = adjust_vmx_controls(
         "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS, &mismatch);
 
     if ( mismatch )
@@ -491,7 +489,6 @@ static int vmx_init_vmcs_config(bool bsp
         /* First time through. */
         vmx_caps = caps;
         vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
-        vmx_vmentry_control        = _vmx_vmentry_control;
         vmx_caps.basic_msr = ((uint64_t)vmx_basic_msr_high << 32) |
                              vmx_basic_msr_low;
         vmx_vmfunc                 = _vmx_vmfunc;
@@ -531,7 +528,7 @@ static int vmx_init_vmcs_config(bool bsp
             vmx_caps.vmexit_control, caps.vmexit_control);
         mismatch |= cap_check(
             "VMEntry Control",
-            vmx_vmentry_control, _vmx_vmentry_control);
+            vmx_caps.vmentry_control, caps.vmentry_control);
         mismatch |= cap_check(
             "EPT and VPID Capability",
             vmx_ept_vpid_cap, _vmx_ept_vpid_cap);
@@ -1091,7 +1088,7 @@ static int construct_vmcs(struct vcpu *v
 {
     struct domain *d = v->domain;
     uint32_t vmexit_ctl = vmx_caps.vmexit_control;
-    u32 vmentry_ctl = vmx_vmentry_control;
+    u32 vmentry_ctl = vmx_caps.vmentry_control;
     int rc = 0;
 
     vmx_vmcs_enter(v);
@@ -2202,7 +2199,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_vmentry_control        = 0;
         vmx_ept_vpid_cap           = 0;
         vmx_vmfunc                 = 0;
     }
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -235,7 +235,6 @@ void vmx_vmcs_reload(struct vcpu *v);
 #define VM_ENTRY_LOAD_GUEST_PAT         0x00004000
 #define VM_ENTRY_LOAD_GUEST_EFER        0x00008000
 #define VM_ENTRY_LOAD_BNDCFGS           0x00010000
-extern u32 vmx_vmentry_control;
 
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001U
 #define SECONDARY_EXEC_ENABLE_EPT               0x00000002U
@@ -297,6 +296,7 @@ struct vmx_caps {
     uint32_t secondary_exec_control;
     uint64_t tertiary_exec_control;
     uint32_t vmexit_control;
+    uint32_t vmentry_control;
 };
 extern struct vmx_caps vmx_caps;
 
@@ -325,9 +325,9 @@ extern struct vmx_caps vmx_caps;
 #define cpu_has_monitor_trap_flag \
     (vmx_caps.cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
 #define cpu_has_vmx_pat \
-    (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_PAT)
+    (vmx_caps.vmentry_control & VM_ENTRY_LOAD_GUEST_PAT)
 #define cpu_has_vmx_efer \
-    (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_EFER)
+    (vmx_caps.vmentry_control & VM_ENTRY_LOAD_GUEST_EFER)
 #define cpu_has_vmx_unrestricted_guest \
     (vmx_caps.secondary_exec_control & SECONDARY_EXEC_UNRESTRICTED_GUEST)
 #define vmx_unrestricted_guest(v)               \
@@ -355,7 +355,7 @@ extern struct vmx_caps vmx_caps;
     (vmx_caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_PML)
 #define cpu_has_vmx_mpx \
     ((vmx_caps.vmexit_control & VM_EXIT_CLEAR_BNDCFGS) && \
-     (vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS))
+     (vmx_caps.vmentry_control & VM_ENTRY_LOAD_BNDCFGS))
 #define cpu_has_vmx_xsaves \
     (vmx_caps.secondary_exec_control & SECONDARY_EXEC_XSAVES)
 #define cpu_has_vmx_tsc_scaling \

[PATCH v3 11/12] VMX: convert vmx_ept_vpid_cap

[Jan Beulich]

... to fields in the capability/controls struct: Take the opportunity
and split the two halves into separate EPT and VPID fields.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: Re-base.
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-u64 vmx_ept_vpid_cap __read_mostly;
 static uint64_t __read_mostly vmx_vmfunc;
 
 static DEFINE_PER_CPU_READ_MOSTLY(paddr_t, vmxon_region);
@@ -256,7 +255,6 @@ static int vmx_init_vmcs_config(bool bsp
 {
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
     struct vmx_caps caps = {};
-    u64 _vmx_ept_vpid_cap = 0;
     u64 _vmx_misc_cap = 0;
     u64 _vmx_vmfunc = 0;
     bool mismatch = false;
@@ -365,10 +363,10 @@ static int vmx_init_vmcs_config(bool bsp
     if ( caps.secondary_exec_control & (SECONDARY_EXEC_ENABLE_EPT |
                                         SECONDARY_EXEC_ENABLE_VPID) )
     {
-        rdmsrl(MSR_IA32_VMX_EPT_VPID_CAP, _vmx_ept_vpid_cap);
+        rdmsr(MSR_IA32_VMX_EPT_VPID_CAP, caps.ept, caps.vpid);
 
         if ( !opt_ept_ad )
-            _vmx_ept_vpid_cap &= ~VMX_EPT_AD_BIT;
+            caps.ept &= ~VMX_EPT_AD_BIT;
 
         /*
          * Additional sanity checking before using EPT:
@@ -381,9 +379,9 @@ static int vmx_init_vmcs_config(bool bsp
          *
          * Or we just don't use EPT.
          */
-        if ( !(_vmx_ept_vpid_cap & VMX_EPT_MEMORY_TYPE_WB) ||
-             !(_vmx_ept_vpid_cap & VMX_EPT_WALK_LENGTH_4_SUPPORTED) ||
-             !(_vmx_ept_vpid_cap & VMX_EPT_INVEPT_ALL_CONTEXT) )
+        if ( !(caps.ept & VMX_EPT_MEMORY_TYPE_WB) ||
+             !(caps.ept & VMX_EPT_WALK_LENGTH_4_SUPPORTED) ||
+             !(caps.ept & VMX_EPT_INVEPT_ALL_CONTEXT) )
             caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_EPT;
 
         /*
@@ -392,11 +390,11 @@ static int vmx_init_vmcs_config(bool bsp
          *
          * Or we just don't use VPID.
          */
-        if ( !(_vmx_ept_vpid_cap & VMX_VPID_INVVPID_ALL_CONTEXT) )
+        if ( !(caps.vpid & VMX_VPID_INVVPID_ALL_CONTEXT) )
             caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
 
         /* EPT A/D bits is required for PML */
-        if ( !(_vmx_ept_vpid_cap & VMX_EPT_AD_BIT) )
+        if ( !(caps.ept & VMX_EPT_AD_BIT) )
             caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
     }
 
@@ -488,7 +486,6 @@ static int vmx_init_vmcs_config(bool bsp
     {
         /* First time through. */
         vmx_caps = caps;
-        vmx_ept_vpid_cap           = _vmx_ept_vpid_cap;
         vmx_caps.basic_msr = ((uint64_t)vmx_basic_msr_high << 32) |
                              vmx_basic_msr_low;
         vmx_vmfunc                 = _vmx_vmfunc;
@@ -529,9 +526,8 @@ static int vmx_init_vmcs_config(bool bsp
         mismatch |= cap_check(
             "VMEntry Control",
             vmx_caps.vmentry_control, caps.vmentry_control);
-        mismatch |= cap_check(
-            "EPT and VPID Capability",
-            vmx_ept_vpid_cap, _vmx_ept_vpid_cap);
+        mismatch |= cap_check("EPT Capability", vmx_caps.ept, caps.ept);
+        mismatch |= cap_check("VPID Capability", vmx_caps.vpid, caps.vpid);
         mismatch |= cap_check(
             "VMFUNC Capability",
             vmx_vmfunc, _vmx_vmfunc);
@@ -2199,7 +2195,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_ept_vpid_cap           = 0;
         vmx_vmfunc                 = 0;
     }
 
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -274,12 +274,11 @@ void vmx_vmcs_reload(struct vcpu *v);
 #define VMX_EPT_AD_BIT                                      0x00200000
 #define VMX_EPT_INVEPT_SINGLE_CONTEXT                       0x02000000
 #define VMX_EPT_INVEPT_ALL_CONTEXT                          0x04000000
-#define VMX_VPID_INVVPID_INSTRUCTION                     0x00100000000ULL
-#define VMX_VPID_INVVPID_INDIVIDUAL_ADDR                 0x10000000000ULL
-#define VMX_VPID_INVVPID_SINGLE_CONTEXT                  0x20000000000ULL
-#define VMX_VPID_INVVPID_ALL_CONTEXT                     0x40000000000ULL
-#define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 0x80000000000ULL
-extern u64 vmx_ept_vpid_cap;
+#define VMX_VPID_INVVPID_INSTRUCTION                        0x00000001
+#define VMX_VPID_INVVPID_INDIVIDUAL_ADDR                    0x00000100
+#define VMX_VPID_INVVPID_SINGLE_CONTEXT                     0x00000200
+#define VMX_VPID_INVVPID_ALL_CONTEXT                        0x00000400
+#define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL    0x00000800
 
 #define VMX_MISC_ACTIVITY_MASK                  0x000001c0
 #define VMX_MISC_PROC_TRACE                     0x00004000
@@ -297,6 +296,8 @@ struct vmx_caps {
     uint64_t tertiary_exec_control;
     uint32_t vmexit_control;
     uint32_t vmentry_control;
+    uint32_t ept;
+    uint32_t vpid;
 };
 extern struct vmx_caps vmx_caps;
 
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -279,17 +279,17 @@ typedef union cr_access_qual {
 extern uint8_t posted_intr_vector;
 
 #define cpu_has_vmx_ept_exec_only_supported        \
-    (vmx_ept_vpid_cap & VMX_EPT_EXEC_ONLY_SUPPORTED)
+    (vmx_caps.ept & VMX_EPT_EXEC_ONLY_SUPPORTED)
 
 #define cpu_has_vmx_ept_wl4_supported           \
-    (vmx_ept_vpid_cap & VMX_EPT_WALK_LENGTH_4_SUPPORTED)
-#define cpu_has_vmx_ept_mt_uc (vmx_ept_vpid_cap & VMX_EPT_MEMORY_TYPE_UC)
-#define cpu_has_vmx_ept_mt_wb (vmx_ept_vpid_cap & VMX_EPT_MEMORY_TYPE_WB)
-#define cpu_has_vmx_ept_2mb   (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_2MB)
-#define cpu_has_vmx_ept_1gb   (vmx_ept_vpid_cap & VMX_EPT_SUPERPAGE_1GB)
-#define cpu_has_vmx_ept_ad    (vmx_ept_vpid_cap & VMX_EPT_AD_BIT)
+    (vmx_caps.ept & VMX_EPT_WALK_LENGTH_4_SUPPORTED)
+#define cpu_has_vmx_ept_mt_uc (vmx_caps.ept & VMX_EPT_MEMORY_TYPE_UC)
+#define cpu_has_vmx_ept_mt_wb (vmx_caps.ept & VMX_EPT_MEMORY_TYPE_WB)
+#define cpu_has_vmx_ept_2mb   (vmx_caps.ept & VMX_EPT_SUPERPAGE_2MB)
+#define cpu_has_vmx_ept_1gb   (vmx_caps.ept & VMX_EPT_SUPERPAGE_1GB)
+#define cpu_has_vmx_ept_ad    (vmx_caps.ept & VMX_EPT_AD_BIT)
 #define cpu_has_vmx_ept_invept_single_context   \
-    (vmx_ept_vpid_cap & VMX_EPT_INVEPT_SINGLE_CONTEXT)
+    (vmx_caps.ept & VMX_EPT_INVEPT_SINGLE_CONTEXT)
 
 #define EPT_2MB_SHIFT     16
 #define EPT_1GB_SHIFT     17
@@ -300,11 +300,11 @@ extern uint8_t posted_intr_vector;
 #define INVEPT_ALL_CONTEXT      2
 
 #define cpu_has_vmx_vpid_invvpid_individual_addr                    \
-    (vmx_ept_vpid_cap & VMX_VPID_INVVPID_INDIVIDUAL_ADDR)
+    (vmx_caps.vpid & VMX_VPID_INVVPID_INDIVIDUAL_ADDR)
 #define cpu_has_vmx_vpid_invvpid_single_context                     \
-    (vmx_ept_vpid_cap & VMX_VPID_INVVPID_SINGLE_CONTEXT)
+    (vmx_caps.vpid & VMX_VPID_INVVPID_SINGLE_CONTEXT)
 #define cpu_has_vmx_vpid_invvpid_single_context_retaining_global    \
-    (vmx_ept_vpid_cap & VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL)
+    (vmx_caps.vpid & VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL)
 
 #define INVVPID_INDIVIDUAL_ADDR                 0
 #define INVVPID_SINGLE_CONTEXT                  1

[PATCH v3 12/12] VMX: convert vmx_vmfunc

[Jan Beulich]

... to a field in the capability/controls struct.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.

--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -162,7 +162,6 @@ static int cf_check parse_ept_param_runt
 
 /* Dynamic (run-time adjusted) execution control flags. */
 struct vmx_caps __ro_after_init vmx_caps;
-static uint64_t __read_mostly vmx_vmfunc;
 
 static DEFINE_PER_CPU_READ_MOSTLY(paddr_t, vmxon_region);
 static DEFINE_PER_CPU(paddr_t, current_vmcs);
@@ -256,7 +255,6 @@ static int vmx_init_vmcs_config(bool bsp
     u32 vmx_basic_msr_low, vmx_basic_msr_high, min, opt;
     struct vmx_caps caps = {};
     u64 _vmx_misc_cap = 0;
-    u64 _vmx_vmfunc = 0;
     bool mismatch = false;
 
     rdmsr(MSR_IA32_VMX_BASIC, vmx_basic_msr_low, vmx_basic_msr_high);
@@ -458,14 +456,14 @@ static int vmx_init_vmcs_config(bool bsp
     /* The IA32_VMX_VMFUNC MSR exists only when VMFUNC is available */
     if ( caps.secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
     {
-        rdmsrl(MSR_IA32_VMX_VMFUNC, _vmx_vmfunc);
+        rdmsrl(MSR_IA32_VMX_VMFUNC, caps.vmfunc);
 
         /*
          * VMFUNC leaf 0 (EPTP switching) must be supported.
          *
          * Or we just don't use VMFUNC.
          */
-        if ( !(_vmx_vmfunc & VMX_VMFUNC_EPTP_SWITCHING) )
+        if ( !(caps.vmfunc & VMX_VMFUNC_EPTP_SWITCHING) )
             caps.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VM_FUNCTIONS;
     }
 
@@ -488,7 +486,6 @@ static int vmx_init_vmcs_config(bool bsp
         vmx_caps = caps;
         vmx_caps.basic_msr = ((uint64_t)vmx_basic_msr_high << 32) |
                              vmx_basic_msr_low;
-        vmx_vmfunc                 = _vmx_vmfunc;
 
         vmx_display_features();
 
@@ -530,7 +527,7 @@ static int vmx_init_vmcs_config(bool bsp
         mismatch |= cap_check("VPID Capability", vmx_caps.vpid, caps.vpid);
         mismatch |= cap_check(
             "VMFUNC Capability",
-            vmx_vmfunc, _vmx_vmfunc);
+            vmx_caps.vmfunc, caps.vmfunc);
         if ( cpu_has_vmx_ins_outs_instr_info !=
              !!(vmx_basic_msr_high & (VMX_BASIC_INS_OUT_INFO >> 32)) )
         {
@@ -2195,7 +2192,6 @@ int __init vmx_vmcs_init(void)
          * Make sure all dependent features are off as well.
          */
         memset(&vmx_caps, 0, sizeof(vmx_caps));
-        vmx_vmfunc                 = 0;
     }
 
     return ret;
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -298,6 +298,7 @@ struct vmx_caps {
     uint32_t vmentry_control;
     uint32_t ept;
     uint32_t vpid;
+    uint64_t vmfunc;
 };
 extern struct vmx_caps vmx_caps;
 

Re: [PATCH v3 00/12] x86/HVM: misc tidying

[Roger Pau Monné]

On Mon, Feb 26, 2024 at 05:40:25PM +0100, Jan Beulich wrote:
> 01: VMX: don't run with CR4.VMXE set when VMX could not be enabled
> 02: x86/HVM: improve CET-IBT pruning of ENDBR
> 03: VMX: drop vmcs_revision_id
> 04: VMX: convert vmx_basic_msr
> 05: VMX: convert vmx_pin_based_exec_control
> 06: VMX: convert vmx_cpu_based_exec_control
> 07: VMX: convert vmx_secondary_exec_control
> 08: VMX: convert vmx_tertiary_exec_control
> 09: VMX: convert vmx_vmexit_control
> 10: VMX: convert vmx_vmentry_control
> 11: VMX: convert vmx_ept_vpid_cap
> 12: VMX: convert vmx_vmfunc

 For the "convert ..." ones:

 Acked-by: Roger Pau Monné <roger.pau@citrix.com>

 I've taken a quick look and it all looks like mechanical
 transformations.

 Thanks, Roger.