Re: [Xen-devel] [RFC 0 PATCH 3/3] PVH dom0: construct_dom0 changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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?

>> > 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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel