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. >> +#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" David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/