From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH 18/18] PVH xen: introduce vmx_pvh.c Date: Fri, 28 Jun 2013 10:44:08 +0100 Message-ID: <51CD770802000078000E1806@nat28.tlf.novell.com> References: <1372118507-16864-1-git-send-email-mukesh.rathor@oracle.com> <1372118507-16864-19-git-send-email-mukesh.rathor@oracle.com> <51C991F502000078000E04E5@nat28.tlf.novell.com> <20130627192818.2ba03853@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130627192818.2ba03853@mantra.us.oracle.com> Content-Disposition: inline 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: xen-devel List-Id: xen-devel@lists.xenproject.org >>> On 28.06.13 at 04:28, Mukesh Rathor wrote: > On Tue, 25 Jun 2013 11:49:57 +0100 "Jan Beulich" wrote: >> > +int vmx_pvh_set_vcpu_info(struct vcpu *v, struct >> > vcpu_guest_context *ctxtp) +{ >> > + if ( v->vcpu_id == 0 ) >> > + return 0; >> > + >> > + vmx_vmcs_enter(v); >> > + __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr); >> > + __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit); >> > + __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_user); >> > + >> > + __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs); >> > + __vmwrite(GUEST_DS_SELECTOR, ctxtp->user_regs.ds); >> > + __vmwrite(GUEST_ES_SELECTOR, ctxtp->user_regs.es); >> > + __vmwrite(GUEST_SS_SELECTOR, ctxtp->user_regs.ss); >> > + __vmwrite(GUEST_GS_SELECTOR, ctxtp->user_regs.gs); >> >> How does this work without also writing the "hidden" register >> fields? > > This is for bringing up SMP CPUs by the guest, which already has set GDT > up so it just needs selectors to be loaded to start the target vcpu. That makes no sense to me: Once you VMLAUNCH that vCPU, it'll get the hidden register fields loaded from the VMCS, without accessing the GDT. If that understanding of mine is wrong, please explain how you see things working in more detail. >> > + if ( vmx_add_guest_msr(MSR_SHADOW_GS_BASE) ) >> > + { >> > + vmx_vmcs_exit(v); >> > + return -EINVAL; >> > + } >> > + vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_kernel); >> >> So you write both GS bases, but not the base of FS (and above >> its selector is being skipped too)? > > Right, for 32bit PVH we'd need to do that. It needs PVH 32bitfixme tag. > Or I can just do it for both, 64bit would be null anyways. This again is a Linux assumption. Please stop this building in of Linux-isms into the hypervisor. >> And there are other parts of struct vcpu_guest_context that >> I don't see getting mirrored - are all of them getting handled >> elsewhere? > > The call comes from VCPUOP_initialise -> arch_set_info_guest() which handles > some of the other fields. There's lot less to load for PVH compared to > PV. So just as an example - where would non-zero ldt_base/ldt_ents get dealt with? Just to repeat the above - don't make any implications on which fields may be unused by your Linux patch set (or add fixme notes identifying those places). And btw., looking at that patch again I'm also getting the impression that the GS base handling in that function is lacking consideration of VGCF_in_kernel. Jan