Re: [Xen-devel] [PATCH v1 3/8]: PVH startup changes (enlighten.c)

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 24 Sep 2012, Mukesh Rathor wrote:
> On Mon, 24 Sep 2012 13:07:19 +0100
> Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> 
> > On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> > > ---
> > >  arch/x86/xen/enlighten.c |   99
> > > ++++++++++++++++++++++++++++++++++++--------- 1 files changed, 79
> > > insertions(+), 20 deletions(-)
> > > 
> > > +                 "with PVH extensions " : "", pv_info.name);
> > >   printk(KERN_INFO "Xen version: %d.%d%s%s\n",
> > >          version >> 16, version & 0xffff, extra.extraversion,
> > >          xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ?
> > > " (preserve-AD)" : ""); @@ -271,12 +278,15 @@ static void
> > > xen_cpuid(unsigned int *ax, unsigned int *bx, break;
> > >   }
> > >  
> > > - asm(XEN_EMULATE_PREFIX "cpuid"
> > > -         : "=a" (*ax),
> > > -           "=b" (*bx),
> > > -           "=c" (*cx),
> > > -           "=d" (*dx)
> > > -         : "0" (*ax), "2" (*cx));
> > > + if (xen_pvh_domain())
> > > +         native_cpuid(ax, bx, cx, dx);
> > > + else
> > > +         asm(XEN_EMULATE_PREFIX "cpuid"
> > > +                 : "=a" (*ax),
> > > +                 "=b" (*bx),
> > > +                 "=c" (*cx),
> > > +                 "=d" (*dx)
> > > +                 : "0" (*ax), "2" (*cx));
> > 
> > can't we just set the cpuid pvop to native_cpuid?
> 
> We had talked about this a bit at code review. There is some massaging
> before and after that goes on, and so we thought we should just leave 
> it like that.

Ah I see, are you talking about all the masks, correct?

 
> > > + if (xen_pvh_domain() && xen_initial_domain())
> > > +         return;
> > > +
> > >   if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> > >           set_fixmap(FIX_PARAVIRT_BOOTMAP,
> > >                      xen_start_info->shared_info);
> > >
> > > @@ -1044,6 +1058,10 @@ void xen_setup_shared_info(void)
> > >           HYPERVISOR_shared_info =
> > >                   (struct shared_info
> > > *)__va(xen_start_info->shared_info); 
> > > + /* PVH TBD/FIXME: vcpu info placement in phase 2 */
> > > + if (xen_pvh_domain())
> > > +         return;
> > 
> > It seems that if xen_initial_domain we always skip the initialization
> > while if !xen_initial_domain we only initialize
> > HYPERVISOR_shared_info. I don't understand why we have this
> > difference.
> 
> The comment in xen_pvh_guest_init() explains it. For domU the library
> maps the pfn at shared_info, ie, shared_info is pfn. For dom0, it's the
> mfn. Dom0 then allocates a pfn via extend_brk, and maps the mfn to it. This
> happens in the commond hvm code, xen_hvm_init_shared_info().

This difference is really subtle, it would be nice to get rid of it.
Could Xen allocate a pfn for dom0?
Otherwise could we have the tools allocate an mfn instead of a pfn?
In fact looking at xc_dom_x86.c, alloc_magic_pages is explicitly having
a different behavior for xc_dom_feature_translated guests and allocates
pfn instead of an mfn. Maybe we could get rid of that special case: less
code in libxc, a common way of allocating the shared_info page for domU
and dom0 => win.

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