From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [RFC 0 PATCH 3/3] PVH dom0: construct_dom0 changes Date: Mon, 7 Oct 2013 17:52:45 -0700 Message-ID: <20131007175245.0ac35f2b@mantra.us.oracle.com> References: <1380142988-9487-1-git-send-email-mukesh.rathor@oracle.com> <1380142988-9487-4-git-send-email-mukesh.rathor@oracle.com> <5244064102000078000F69AF@nat28.tlf.novell.com> <20130926185508.56a3f7b7@mantra.us.oracle.com> <5245495C02000078000F7403@nat28.tlf.novell.com> <20130927160334.220ccd37@mantra.us.oracle.com> <52493CBE02000078000F7E06@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 1VTLXj-0007pA-Mg for xen-devel@lists.xenproject.org; Tue, 08 Oct 2013 00:52:55 +0000 In-Reply-To: <52493CBE02000078000F7E06@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.xen@gmail.com List-Id: xen-devel@lists.xenproject.org On Mon, 30 Sep 2013 07:56:30 +0100 "Jan Beulich" wrote: > >>> On 28.09.13 at 01:03, Mukesh Rathor > >>> wrote: > > On Fri, 27 Sep 2013 08:01:16 +0100 > > "Jan Beulich" wrote: ....... > >> >> > @@ -1089,11 +1262,18 @@ int __init construct_dom0( > >> >> > regs->eip = parms.virt_entry; > >> >> > regs->esp = vstack_end; > >> >> > regs->esi = vstartinfo_start; > >> >> > - regs->eflags = X86_EFLAGS_IF; > >> >> > + regs->eflags = X86_EFLAGS_IF | 0x2; > >> >> > >> >> Unrelated change? > >> > > >> > Nop, we need to make sure the resvd bit is set in eflags > >> > otherwise it won't vmenter (invalid guest state). Should be > >> > harmless for PV, right? Not sure where it does it for PV before > >> > actually scheduling it.. > >> > >> PV doesn't set this anywhere - the hardware doesn't allow the > >> flag to be cleared (writes are ignored). If VMENTER is picky > >> about this, the GUEST_RFLAGS write at the end of > >> vmx_vmenter_helper() should be doing this instead of having to > >> do it here (and obviously in some other place for DomU creation). > > > > For domU we set it in arch_set_info_guest. > > Which is bogus too. 15910:ec3b23d8d544 ("hvm: Always keep > canonical copy of RIP/RSP/RFLAGS in guest_cpu_user_regs()") did > this adjustment without really explaining why it can't be done > centrally in just the two places copying regs->eflags into the > VMCS/VMCB spot. I beg to differ.... such nit picking is equally bogus IMHO. The bit needs to be set once, putting it in vmx_vmenter_helper adds an unnecessary slowdown IMO. > > vmx_vmenter_helper gets > > called on every vmentry, we just need this setting once. > > Would a debugger update guest state via arch_set_info_guest()? > I doubt it. It would imo be a desirable up front cleanup patch to > move this bogus thing out of arch_set_info_guest() into > vmx_vmenter_helper() (and whatever SVM equivalent, should > SVM too be incapable of dealing with the flag being clear). See > how e.g. hvm_load_cpu_ctxt() already sets the flag? It's really > like being done almost at random... The debugger would always read eflags, muck with only the bits it needs to, leaving the resvd bit as is, then send it down. > The only place where it gets legitimately enforced outside of > the vmx_vmenter_helper() is in the x86 emulator code. > > And if we'd have such a cleanup patch, doing away with the literal > 2 in favor of a proper symbolic (e.g. X86_EFLAGS_MBS) should > probably be done at once. Having X86_EFLAGS_MBS makes sense. > > So I think this is the best place. Do you want me to if it: > > > > regs->eflags = X86_EFLAGS_IF; > > if ( pvh ) > > regs->eflags |= 0x2. > > No, that would be pointless. Mukesh