Re: [PATCH 01/13] Nested Virtualization: tools

[Christoph Egger <Christoph.Egger@amd.com>]

On Tuesday 16 November 2010 14:03:40 Tim Deegan wrote:
> At 11:52 +0000 on 16 Nov (1289908371), Christoph Egger wrote:
> > On Tuesday 16 November 2010 12:37:06 Tim Deegan wrote:
> > > Hi,
> > >
> > > At 18:40 +0000 on 12 Nov (1289587225), Christoph Egger wrote:
> > > > +#define SVM_FEATURE_NPT            0x00000001
> > > > +#define SVM_FEATURE_LBRV           0x00000002
> > > > +#define SVM_FEATURE_SVML           0x00000004
> > > > +#define SVM_FEATURE_NRIPS          0x00000008
> > > > +#define SVM_FEATURE_PAUSEFILTER    0x00000400
> > > > +
> > > > +        /* Only passthrough SVM features which are implemented */
> > > > +        edx = 0;
> > > > +        if (regs[3] & SVM_FEATURE_NPT)
> > > > +            edx |= SVM_FEATURE_NPT;
> > > > +        if (regs[3] & SVM_FEATURE_LBRV)
> > > > +            edx |= SVM_FEATURE_LBRV;
> > > > +        if (regs[3] & SVM_FEATURE_NRIPS)
> > > > +            edx |= SVM_FEATURE_NRIPS;
> > > > +        if (regs[3] & SVM_FEATURE_PAUSEFILTER)
> > > > +            edx |= SVM_FEATURE_PAUSEFILTER;
> > > > +
> > > > +        regs[3] = edx;
> > >
> > > Minor niggle - why isn't this just a single &= operation?
> >
> > The l1 guest shouldn't see upcoming svm features yet.
> > They will be added here when support for them is implemented.
>
> I meant: why don't you or together the feature flags you support
> (which should probably be defined in a header file with the other CPUID
> bits, btw) and just 'regs[3] &= SVM_FEAURE_FOO|SVM_FEATURE_BAR|...'
> instead of using ten lines of code?

Thanks for clarification (and the other people who guessed right).
I changed that in my local tree.

Christoph
-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632