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

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

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

> 
> >  	*bx &= maskebx;
> >  	*cx &= maskecx;
> > @@ -1034,6 +1044,10 @@ static int xen_write_msr_safe(unsigned int
> > msr, unsigned low, unsigned high) 
> >  void xen_setup_shared_info(void)
> >  {
> > +	/* do later in xen_pvh_guest_init() when extend_brk is
> > properly setup*/
> 
> Which one is the function that is going to get called later to do the
> initialization? It cannot be this one because it would still return
> immediately.

xen_pvh_guest_init() just like the comment says :).


> 
> > +	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().