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

From: Konrad Rzeszutek Wilk
Date: Fri Aug 22 2014 - 14:00:29 EST


On Thu, Aug 21, 2014 at 07:46:56PM -0700, Mukesh Rathor wrote:
> On Thu, 21 Aug 2014 21:39:04 -0400
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 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.

I understand that it does not have an EFER.NX bit. What I was trying
to figure out is _where_ do we set the NX bit for Intel CPUs? Is
it done in the hypervisor? In the Linux code?

Thank you.
>
> >
> > >
> > > Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> > > ---
> > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/