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

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Fri, 27 Sep 2013 08:01:16 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 27.09.13 at 03:55, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > On Thu, 26 Sep 2013 09:02:41 +0100 "Jan Beulich"
> > <JBeulich@suse.com> wrote:
> >> > @@ -868,6 +1016,9 @@ int __init construct_dom0(
> >> >                                      L1_PROT : COMPAT_L1_PROT));
> >> >          l1tab++;
> >> >  
> >> > +        if ( is_pvh_domain(d) )
> >> > +            continue;
> >> > +
> >> >          page = mfn_to_page(mfn);
> >> >          if ( (page->u.inuse.type_info == 0) &&
> >> >               !get_page_and_type(page, d, PGT_writable_page) )
> >> 
> >> So why is the remaining part of this loop not applicable to PVH?
> > 
> > My bad, looks like it should be. I'll remove it. BTW, looking at it
> > again I realized we don't really need to set type_info to PGT* for
> > PVH, but it's harmless I guess. Should I just leave it or condition
> > them for PV only?
> 
> No, I'm pretty certain you want them marked writable. If nothing
> else then for forward compatibility with an eventual change
> needing to mark certain pages R/O. But I could also imaging the
> page sharing code to look at this attribute of a page (but I say
> this without knowing that code at all).

Sorry, I meant PGT_l*_page_table settings for type_info. Yes, we do 
need the PGT_writable_page settings.

> >> > @@ -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. vmx_vmenter_helper gets
called on every vmentry, we just need this setting once. 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.

thanks
Mukesh