From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH RFC] pvh: clearly specify used parameters in vcpu_guest_context Date: Mon, 18 Nov 2013 11:19:21 -0500 Message-ID: <20131118161921.GF3364@phenom.dumpdata.com> References: <1384530622-31703-1-git-send-email-roger.pau@citrix.com> <52865AC60200007800103AAE@nat28.tlf.novell.com> <52864E97.9020402@eu.citrix.com> <52865DCB0200007800103AD8@nat28.tlf.novell.com> <20131115135654.52e52bd4@mantra.us.oracle.com> <20131115155652.3201414a@mantra.us.oracle.com> <20131115171227.5c2797d2@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ViRYr-0006qp-Gv for xen-devel@lists.xenproject.org; Mon, 18 Nov 2013 16:20:29 +0000 Content-Disposition: inline In-Reply-To: <20131115171227.5c2797d2@mantra.us.oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Mukesh Rathor Cc: Keir Fraser , George Dunlap , Tim Deegan , Jan Beulich , xen-devel@lists.xenproject.org, Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On Fri, Nov 15, 2013 at 05:12:27PM -0800, Mukesh Rathor wrote: > On Fri, 15 Nov 2013 15:56:52 -0800 > Mukesh Rathor wrote: > > > On Fri, 15 Nov 2013 13:56:54 -0800 > > Mukesh Rathor wrote: > > > > > On Fri, 15 Nov 2013 16:45:47 +0000 > > > "Jan Beulich" 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. I think it is OK. > > 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)); >