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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.09.13 at 03:55, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Thu, 26 Sep 2013 09:02:41 +0100 "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>> >>> On 25.09.13 at 23:03, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
>> > +    sav_guest_l4 = *l4tab;
>> > +
>> > +    /* Give guest a clean slate to start with */
>> > +    clear_page(l4start);
>> > +    *l4tab = sav_guest_l4;
>> > +    BUG_ON(!l4e_get_pfn(sav_guest_l4));
>> 
>> This assumes that the initial mapping is covered by a single L4
>> entry. While true for Linux, this is not generic, and hence needs
>> fixing (the problem continues further down).
> 
> True. How about I just create the opposite of init_guest_l4_table,
> ie, clear_guest_l4_table and just clear out xen entries?

I'd prefer you clearing everything below and above the range you
need mapped, instead of using clear_page().

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

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

Jan


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