Re: [Xen-devel] [V5 PATCH 1/1] x86/xen: Set EFER.NX and EFER.SCE in PVH guests

From: Konrad Rzeszutek Wilk
Date: Mon Sep 15 2014 - 15:28:55 EST


On Mon, Sep 15, 2014 at 03:45:53PM +0100, David Vrabel wrote:
> On 12/09/14 21:42, Konrad Rzeszutek Wilk wrote:
> > On Wed, Sep 10, 2014 at 04:36:06PM -0700, Mukesh Rathor wrote:
> >>
> >> @@ -413,15 +417,18 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
> >> (unsigned long)xen_failsafe_callback;
> >> ctxt->user_regs.cs = __KERNEL_CS;
> >> per_cpu(xen_cr3, cpu) = __pa(swapper_pg_dir);
> >> -#ifdef CONFIG_X86_32
> >> }
> >> -#else
> >> - } else
> >> - /* N.B. The user_regs.eip (cpu_bringup_and_idle) is called with
> >> - * %rdi having the cpu number - which means are passing in
> >> - * as the first parameter the cpu. Subtle!
> >> +#ifdef CONFIG_XEN_PVH
> >> + else {
> >> + /*
> >> + * The vcpu comes on kernel page tables which have the NX pte
> >> + * bit set. This means before DS/SS is touched, NX in
> >> + * EFER must be set. Hence the following assembly glue code.
> >
> > And you ripped out the nice 'N.B' comment I added. Sad :-(
>
> I think I removed that.
>
> I don't think passing parameters to a function is particularly subtle
> and this comment is largely superseded by the comment for
> xen_pvh_early_cpu_init() itself.

Good point.
>
> >> +#ifdef CONFIG_XEN_PVH
> >> +/*
> >> + * xen_pvh_early_cpu_init() - early PVH VCPU initialization
> >> + * @cpu: this cpu number (%rdi)
> >> + * @flag: boolean flag true to indicate this is a secondary vcpu coming up
> >> + * on this entry point or the primary cpu coming back online.
> >
> > Why do we do this? Why not just piggyback on the first parameter - the 'cpu'?
> >
> > If it is zero it is boot CPU.
>
> "Changes in v5 (Mukesh):
> - Jan reminded us that vcpu 0 could go offline/online. So, add flag back"

Right, totally forgot about that.

With that I think I think you can tack on 'Reviewed-by: Konrad Rzeszutek
Wilk <konrad.wilk@xxxxxxxxxx>'.

Granted the PVH ABI doc needs to go in Xen base first as David requested.
Even a draft that explains some of this would be good.

CC-ing Roger - perhaps v1 of the draft could be posted and we can flesh it
out more?
>
> David
--
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/