Re: [RFC PATCH 1/8]: PVH: Basic and preparatory changes

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

On Thu, 16 Aug 2012 14:59:14 +0100
Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:

> It's good to see well split patches :)
> But could you please send them inline next time?

Sorry, that was the intent. 

> >  
> > -	if (!xen_pv_domain())
> > +	/* PVH TBD/FIXME: future work */
> > +	if (!xen_pv_domain() || xen_pvh_domain())
> >  		return -ENODEV;
> >  
> >  	register_xenstore_notifier(&xsn_cpu);
> 
> I don't think we should use or have xen_pvh_domain() in non-x86 files,
> like anything under drivers/xen.
> Isn't XENFEAT_auto_translated_physmap enough?

Ah, ok, I'll change it.

> > diff --git a/include/xen/interface/xen.h
> > b/include/xen/interface/xen.h index 0801468..1d5bc36 100644
> > --- a/include/xen/interface/xen.h
> > +++ b/include/xen/interface/xen.h
> > @@ -493,6 +493,7 @@ struct dom0_vga_console_info {
> >  /* These flags are passed in the 'flags' field of start_info_t. */
> >  #define SIF_PRIVILEGED    (1<<0)  /* Is the domain privileged? */
> >  #define SIF_INITDOMAIN    (1<<1)  /* Is this the initial control
> > domain? */ +#define SIF_IS_PVINHVM    (1<<4)  /* Is it a PV running
> > in HVM container? */ #define SIF_PM_MASK       (0xFF<<8) /* reserve
> > 1 byte for xen-pm options */ 
> >  typedef uint64_t cpumap_t;
> 
> I would avoid adding SIF_IS_PVINHVM, an x86 specific concept, into a
> generic xen.h interface file. 

> > +/* xen_pv_domain check is necessary as start_info ptr is null in
> > HVM. Also,
> > + * note, xen PVH domain shares lot of HVM code */
> > +#define xen_pvh_domain()       (xen_pv_domain()
> > &&                     \
> > +				(xen_start_info->flags &
> > SIF_IS_PVINHVM))
>  
> Also here.

Hmm.. I can move '#define xen_pvh_domain()' to x86 header, easy. But,
not sure how to define SIF_IS_PVINHVM then? I could put SIF_IS_RESVD in
include/xen/interface/xen.h, and then do 
#define SIF_IS_PVINHVM SIF_IS_RESVD in an x86 file.

What do you think about that?

thanks
Mukesh