[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC 0 PATCH 3/3] PVH dom0: construct_dom0 changes
On Tue, 08 Oct 2013 08:43:43 +0100 "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >>> On 08.10.13 at 02:52, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > >>> wrote: > > On Mon, 30 Sep 2013 07:56:30 +0100 > > "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > > > >> >>> On 28.09.13 at 01:03, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> > >> >>> wrote: > >> > On Fri, 27 Sep 2013 08:01:16 +0100 > >> > "Jan Beulich" <JBeulich@xxxxxxxx> 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. > > An "or" being a measurable slowdown? Well, unnecessary dirtying of cache line. But, I'll make the change just to be done with this. > >> > 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. > > So you'd expect every debugger to be responsible for setting this > bit? Pretty odd a requirement, when it can be done centrally in a > single place, covering all cases. Like I said, the debuggers are expected to read existing eflags, change the bits (leaving resvd bit set). But, ir-relevant now after the above change. Mukesh _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |