Re: [XEN PATCH v2 12/15] x86/vmx: guard access to cpu_has_vmx_* in common code

[Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>]

16.05.24 10:32, Jan Beulich:
> On 16.05.2024 02:50, Stefano Stabellini wrote:
> > On Wed, 15 May 2024, Sergiy Kibrik wrote:
> > > There're several places in common code, outside of arch/x86/hvm/vmx,
> > > where cpu_has_vmx_* get accessed without checking if VMX present first.
> > > We may want to guard these macros, as they read global variables defined
> > > inside vmx-specific files -- so VMX can be made optional later on.
> > >
> > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> > > CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > CC: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > Here I've tried a different approach from prev.patches [1,2] -- instead of
> > > modifying whole set of cpu_has_{svm/vmx}_* macros, we can:
> > >   1) do not touch SVM part at all, because just as Andrew pointed out they're
> > > used inside arch/x86/hvm/svm only.
> > >   2) track several places in common code where cpu_has_vmx_* features are
> > > checked out and guard them using cpu_has_vmx condition
> > >   3) two of cpu_has_vmx_* macros being used in common code are checked in a bit
> > > more tricky way, so instead of making complex conditionals even more 
> > > complicated,
> > > we can instead integrate cpu_has_vmx condition inside these two macros.
> > >
> > > This patch aims to replace [1,2] from v1 series by doing steps above.
[..]
> > I am missing some of the previous discussions but why can't we just fix
> > all of the cpu_has_vmx_* #defines in vmcs.h to also check for
> > cpu_has_vmx?
> >
> > That seems easier and simpler than to add add-hoc checks at the invocations?
> I'd like to take the question on step further: Following 0b5f149338e3
> ("x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware"),
> is this change needed at all? IOW is there a path left where cpu_has_vmx
> may be false, by any cpu_has_vmx_* may still yield true?
This change is about exec control variables (vmx_secondary_exec_control, 
vmx_pin_based_exec_control etc) not been built, because they're in vmx 
code, but accessed in common code. The description is probably unclear 
about that.
Also build issues related to VMX can be solved differently, without 
touching cpu_has_vmx_* macros and related logic at all.
I can move exec control variables from vmcs.c to common hvm.c, this 
would be simpler change and directly related to problem that I'm having.
  -Sergiy