Re: [PATCH RFC] pvh: clearly specify used parameters in vcpu_guest_context

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

On Fri, 15 Nov 2013 15:56:52 -0800
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Fri, 15 Nov 2013 13:56:54 -0800
> Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
> 
> > On Fri, 15 Nov 2013 16:45:47 +0000
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
........
> > 
> > We had talked about this while ago, but upon boot, the first thing
> > a vcpu needs is access to kernel data structure. (A secondary vcpu
> > is bootstrapped way up into the kernel). It would be possible to
> > get rid of gs_base_kernel, but will take some work on the linux
> > side. I can try and test it out, and let you guys know.
> 
> Ok, looking at this more, I can hack cpu_bringup_and_idle() in
> linux to include a static variable for cpuid, which is the least a
> vcpu needs to know first thing. But, I think that would not work when
> vcpu hotplug support is added. Another option would be to pass cpuid
> in one of the registers, say rdi. Thus, rdi == cpuid will be passed
> to VCPUOP_initialise. In bringup function, the booting vcpu can then
> load it's own gs based on the cpuid. If linux folks, konrad (CCd), is
> OK with this, we can remove gs_base_kernel.  Otherwise, it's such a
> small thing, hopefually it can stay.

Konrad,

Here's the changes needed on linux side to remove gs_base_kernel.
Since, I imagine vcpu hotplug would go thru the same path, it would
work there also. If this looks OK to everybody, we can remove
gs_base_kernel from the VCPUOP_initialise call.

thanks,
Mukesh


diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index aa89bbf..54d1022 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -99,14 +99,8 @@ static void cpu_bringup(void)
 	wmb();			/* make sure everything is out */
 }
 
-/*
- * The vcpu is coming up with only gs_base_kernel set so we can do 
- * smp_processor_id() here. We need to set the rest of the context first thing.
- */
-static void pvh_set_bringup_context(void)
+static void pvh_set_bringup_context(int cpu)
 {
-	int cpu = smp_processor_id();
-
 	load_percpu_segment(cpu);
 	switch_to_new_gdt(smp_processor_id());
 
@@ -121,11 +115,12 @@ static void pvh_set_bringup_context(void)
 		: : "r" (__KERNEL_DS), "a" (__KERNEL_CS) : "memory");
 }
 
-static void cpu_bringup_and_idle(void)
+/* Note: cpu parameter is only relevant for PVH */
+static void cpu_bringup_and_idle(int cpu)
 {
 	if (xen_feature(XENFEAT_auto_translated_physmap) &&
 	    xen_feature(XENFEAT_supervisor_mode_kernel))
-		pvh_set_bringup_context();
+		pvh_set_bringup_context(cpu);
 
 	cpu_bringup();
 	cpu_startup_entry(CPUHP_ONLINE);
@@ -393,8 +388,6 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 	/* Note: PVH is not yet supported on x86_32. */
 	ctxt->user_regs.fs = __KERNEL_PERCPU;
 	ctxt->user_regs.gs = __KERNEL_STACK_CANARY;
-#else
-	ctxt->gs_base_kernel = per_cpu_offset(cpu);
 #endif
 	ctxt->user_regs.eip = (unsigned long)cpu_bringup_and_idle;
 
@@ -426,6 +419,8 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 #ifdef CONFIG_X86_32
 		ctxt->event_callback_cs     = __KERNEL_CS;
 		ctxt->failsafe_callback_cs  = __KERNEL_CS;
+#else
+		ctxt->gs_base_kernel = per_cpu_offset(cpu);
 #endif
 		ctxt->event_callback_eip    =
 					(unsigned long)xen_hypervisor_callback;
@@ -433,7 +428,8 @@ 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);
-	}
+	} else
+		ctxt->user_regs.rdi = cpu;
 
 	ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs);
 	ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_mfn(swapper_pg_dir));