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

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

On Tue, 25 Sep 2012 11:03:13 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> On Mon, 24 Sep 2012, Mukesh Rathor wrote:
> > On Mon, 24 Sep 2012 13:07:19 +0100
> > Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> > 
> > > On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> > > > +		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?

Not easily.

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

Wish it was simple. But for PV and PVH, domU, it's already setup the 
shared page. All we need to do is __va(shared_info). But for HVM domUs 
and PVH dom0, we need to hcall with pfn to get it remapped. Changing the
tool to map pfn, would result in unnecessary hcall for all PV and PVH
domUs. It's only two lines of code, so lets just leave it. I'll make the
comment better.


thx,
Mukesh