RE: [Xen-devel] [Patch] Enable SMEP CPU feature support for XEN itself

["Li, Xin" <xin.li@xxxxxxxxx>]

> > > +                 setup_clear_cpu_cap(X86_FEATURE_SMEP);
> > > +                 clear_in_cr4(X86_CR4_SMEP);
> > > +         } else
> > > +                 set_in_cr4(X86_CR4_SMEP);
> >
> > Anyway, the whole thing is overkill - {set,clear}_in_cr4() write
> > the updated bits to mmu_cr4_features, and these get loaded
> > on secondary CPUs *before* you have any chance of looking
> > at the CPUID bits. As with everything else, it's assumed that
> > APs don't have less features than the BP, and hence you only
> > need to set_in_cr4() once (on the BP). And then the function
> > can be __init.
> >
> 
> Do you mean?
>         if ( cpu_has(c, X86_FEATURE_SMEP) )
>                 if( likely(!disable_smep) ) {
>                         mmu_cr4_features |= X86_CR4_SMEP;
>                         set_in_cr4(0);
>                 } else
>                         setup_clear_cpu_cap(X86_FEATURE_SMEP);
> 
> Sounds good ... but the code will be harder to read, as it implicitly set 
> smep?
> Also where to put setup_smep thus it's only called in BP?
> 
But it is a good idea to set X86_CR4_SMEP in mmu_cr4_features when SMEP
Is available.  thus

1) for PV, we can make patch like pv_guest_cr4_to_real_cr4
#define pv_guest_cr4_to_real_cr4(v)                         \
    (((v)->arch.pv_vcpu.ctrlreg[4]                          \
      | (mmu_cr4_features & (X86_CR4_PGE | X86_CR4_PSE | X86_CR4_SMEP))    \
      | ((v)->domain->arch.vtsc ? X86_CR4_TSD : 0)          \
      | ((xsave_enabled(v))? X86_CR4_OSXSAVE : 0))          \
      & ~X86_CR4_DE)
when set cr4.

2) For HVM, we don't need to explicitly add SMEP when write to HOST_CR4.

Thanks!
-Xin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel