Re: [Xen-devel] [V0 PATCH 1/2] AMD-PVH: set EFER.NX and EFER.SCE for the boot vcpu

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Thu, 21 Aug 2014 21:39:04 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Wed, Aug 20, 2014 at 07:16:39PM -0700, Mukesh Rathor wrote:
> > On AMD, NX feature must be enabled in the efer for NX to be honored
> > in the pte entries, otherwise protection fault. We also set SC for
> > system calls to be enabled.
> 
> How come we don't need to do that for Intel (that is set the NX bit)?
> Could you include the explanation here please?

Intel doesn't have EFER.NX bit. The SC bit is being set in xen, but it
doesn't need to be, and I'm going to submit a patch to undo it.

> 
> > 
> > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > ---
> >  arch/x86/xen/enlighten.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index c0cb11f..4af512d 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1499,6 +1499,17 @@ void __ref xen_pvh_secondary_vcpu_init(int
> > cpu) xen_pvh_set_cr_flags(cpu);
> >  }
> >  
> > +/* This is done in secondary_startup_64 for hvm guests. */
> > +static void __init xen_configure_efer(void)
> > +{
> > +	u64 efer;
> > +
> > +	rdmsrl(MSR_EFER, efer);
> > +	efer |= EFER_SCE;
> > +	efer |= (cpuid_edx(0x80000001) & (1 << 20)) ? EFER_NX : 0;
> 
> Ahem? #defines for these magic values please?

Linux uses these directly all over the code as they are set in stone
pretty much, and I didn't find any #defines. See cpu/common.c for one of
the places. Also see secondary_startup_64, and others...

> Or could you use 'boot_cpu_has'?

Nop, it's not initialized at this point.

thanks,
Mukesh