From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [RFC PATCH 1/8]: PVH: Basic and preparatory changes
Date: Thu, 16 Aug 2012 11:46:50 -0700
Message-ID: <20120816114650.4db2079f@mantra.us.oracle.com>
References: <20120815175724.3405043a@mantra.us.oracle.com>
	<alpine.DEB.2.02.1208161454430.4850@kaball.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <alpine.DEB.2.02.1208161454430.4850@kaball.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: "Xen-devel@lists.xensource.com" <Xen-devel@lists.xensource.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
List-Id: xen-devel@lists.xenproject.org

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