On Mon, Feb 19, 2024 at 11:24 PM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 06.02.2024 02:20, George Dunlap wrote:
> > --- a/xen/arch/x86/include/asm/hvm/svm/svm.h
> > +++ b/xen/arch/x86/include/asm/hvm/svm/svm.h
> > @@ -38,7 +38,10 @@ extern u32 svm_feature_flags;
> > #define SVM_FEATURE_SSS 19 /* NPT Supervisor Shadow Stacks */
> > #define SVM_FEATURE_SPEC_CTRL 20 /* MSR_SPEC_CTRL virtualisation */
> >
> > -#define cpu_has_svm_feature(f) (svm_feature_flags & (1u << (f)))
> > +static inline bool cpu_has_svm_feature(unsigned int feat)
> > +{
> > + return svm_feature_flags & (1u << (feat));
> > +}
> > #define cpu_has_svm_npt cpu_has_svm_feature(SVM_FEATURE_NPT)
> > #define cpu_has_svm_lbrv cpu_has_svm_feature(SVM_FEATURE_LBRV)
> > #define cpu_has_svm_svml cpu_has_svm_feature(SVM_FEATURE_SVML)
>
> Having seen patch 4 now, I have to raise the question here as well: Why
> do we need a separate variable (svm_feature_flags) when we could use
> the host policy (provided it isn't abused; see comments on patch 4)?
We certainly don't need an extra variable; in fact, moving all of
these into the host cpuid policy thing would make it easier, for
example, to test some of the guest creation restrictions: One could
use the Xen command-line parameters to disable some of the bits, then
try to create a nested SVM guest and verify that it fails as expected.
I would like to do that eventually, but this patch is already done and
improves the code, so I just kept it.
Let me know if you'd like me to simply drop this patch instead.
-George
