From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: RFC: PVH set vcpu info context in vmcs.... Date: Tue, 13 Aug 2013 19:12:03 -0700 Message-ID: <20130813191203.0e2e0d5f@mantra.us.oracle.com> References: <20130812184513.56188d29@mantra.us.oracle.com> <520A2D0402000078000EB7E7@nat28.tlf.novell.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 1V9QZK-0007c5-IF for xen-devel@lists.xenproject.org; Wed, 14 Aug 2013 02:12:14 +0000 In-Reply-To: <520A2D0402000078000EB7E7@nat28.tlf.novell.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: Jan Beulich Cc: xen-devel , Keir Fraser , Tim Deegan List-Id: xen-devel@lists.xenproject.org On Tue, 13 Aug 2013 11:56:36 +0100 "Jan Beulich" wrote: > >>> On 13.08.13 at 03:45, Mukesh Rathor > >>> wrote: > > int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context > > *ctxtp) { > > int rc; > > > > if ( v->vcpu_id == 0 ) > > return 0; > > Bogus/pointless. No, we don't have anything for the boot vcpu. It's totally coming up on the flat address space. For non boot, the vcpu is coming up on the kernel GDT. Recall it's a PV guest (coming up in an HVM container). > > if ( !(ctxtp->flags & VGCF_in_kernel) ) > > return -EINVAL; > > > > if ( ctxtp->ldt_base || ctxtp->ldt_ents || > > (ctxtp->user_regs.cs & 4) || ctxtp->user_regs.ss || > > ctxtp->user_regs.es || ctxtp->user_regs.ds ) > > return -EINVAL; > > How about FS/GS? If you don't enforce these selectors to be zero > too, then loading only base and selector values below isn't > sufficient (and again potentially inconsistent). > > > > > if ( ctxtp->user_regs.cs == 0 ) > > return -EINVAL; > > Perhaps also check RPL == 0? OK. I still think we should just remove the check for VGCF_in_kernel, why do we care for PVH? For 64bit PV, we needed that, but a PVH guest can come up any RPL it chooses to, right? Below is updated version. thanks Mukesh /* * Set vmcs fields in support of vcpu_op -> VCPUOP_initialise hcall used * to initialise a secondary vcpu prior to boot. Called from * arch_set_info_guest() which sets the (PVH relevant) non-vmcs fields. * * In case of linux: * The call comes from cpu_initialize_context(). (boot vcpu 0 context is * set by the tools via do_domctl -> vcpu_initialise). * * PVH 32bitfixme: this function needs to be modified for 32bit guest. */ int vmx_pvh_set_vcpu_info(struct vcpu *v, struct vcpu_guest_context *ctxtp) { int rc = 0; if ( v->vcpu_id == 0 ) return 0; if ( !(ctxtp->flags & VGCF_in_kernel) ) return -EINVAL; if ( ctxtp->ldt_base || ctxtp->ldt_ents || (ctxtp->user_regs.cs & 4) || ctxtp->user_regs.ss || ctxtp->user_regs.es || ctxtp->user_regs.ds || ctxtp->user_regs.fs || ctxtp->user_regs.gs ) return -EINVAL; if ( ctxtp->user_regs.cs == 0 || (ctxtp->user_regs.cs & 3) == 3 ) return -EINVAL; vmx_vmcs_enter(v); __vmwrite(GUEST_GDTR_BASE, ctxtp->gdt.pvh.addr); __vmwrite(GUEST_GDTR_LIMIT, ctxtp->gdt.pvh.limit); __vmwrite(GUEST_FS_BASE, ctxtp->fs_base); __vmwrite(GUEST_GS_BASE, ctxtp->gs_base_kernel); /* IA-32e: ss/es/ds are ignored, we load cs only. */ __vmwrite(GUEST_CS_SELECTOR, ctxtp->user_regs.cs); if ( (rc = hvm_load_segment_selector(v, x86_seg_cs, ctxtp->user_regs.cs)) ) goto out; if ( (rc = vmx_add_guest_msr(MSR_SHADOW_GS_BASE)) ) goto out; vmx_write_guest_msr(MSR_SHADOW_GS_BASE, ctxtp->gs_base_user); out: vmx_vmcs_exit(v); return 0; }