On Mon, Jul 29, 2013 at 10:59:31AM +0200, Paolo Bonzini wrote:
> Il 25/07/2013 12:59, Gleb Natapov ha scritto:
> > +#if PTTYPE == PTTYPE_EPT
> > +#define CHECK_BAD_MT_XWR(G) mmu->bad_mt_xwr & (1ull << ((G) & 0x3f));
> > +#else
> > +#define CHECK_BAD_MT_XWR(G) 0;
> > +#endif
> > +
> > static bool FNAME(is_rsvd_bits_set)(struct kvm_mmu *mmu, u64 gpte, int level)
> > {
> > int bit7;
> >
> > bit7 = (gpte >> 7) & 1;
> > - return (gpte & mmu->rsvd_bits_mask[bit7][level-1]) != 0;
> > + return ((gpte & mmu->rsvd_bits_mask[bit7][level-1]) != 0) ||
> > + CHECK_BAD_MT_XWR(gpte);
> > }
> >
> > +#undef CHECK_BAD_MT_XWR
>
> Instead of a macro, you can do
>
> if (...)
> return true;
> #if PTTYPE == PTTYPE_EPT
> if (...)
> return true;
> #endif
> return false;
>
> The compiler should be smart enough to generate the same code for
> non-EPT PTTYPE.
>
The idea behind this rsvd_bits_mask trickery is to produce code that
does not have conditional branches. I don't want to rely on compiler to do
the right things. On the other hand I am not sure that just dropping this
ifdefs here and checking mmu->bad_mt_xwr for non ept case is not a
good idea. The overhead should not be measurable.
> >
> > + /*
> > + * Use PFERR_RSVD_MASK in erorr_code to to tell if EPT
> > + * misconfiguration requires to be injected. The detection is
> > + * done by is_rsvd_bits_set() above.
>
> erorr_code -> error_code
>
> This patch has warnings for unused static functions. You can squash
> them, or split them differently according to file boundaries (i.e. mmu.c
> first, vmx.c second).
>
I prefer to have an in between patch with a warning, but do not divide
code that logically belongs together between patches.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
