From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH v16 01/10] x86: add generic resource (e.g.
 MSR) access hypercall
Date: Thu, 25 Sep 2014 16:17:28 -0400
Message-ID: <20140925201728.GA25152@laptop.dumpdata.com>
References: <1411640350-26155-1-git-send-email-chao.p.peng@linux.intel.com>
	<1411640350-26155-2-git-send-email-chao.p.peng@linux.intel.com>
	<542473B2.5040504@citrix.com>
	<20140925201218.GB24583@laptop.dumpdata.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <20140925201218.GB24583@laptop.dumpdata.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: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: keir@xen.org, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, George.Dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, JBeulich@suse.com, Chao Peng <chao.p.peng@linux.intel.com>, dgdegra@tycho.nsa.gov
List-Id: xen-devel@lists.xenproject.org

> > > index 053b9fa..e4d9091 100644
> > > --- a/xen/include/public/platform.h
> > > +++ b/xen/include/public/platform.h
> > > @@ -527,6 +527,28 @@ struct xenpf_core_parking {
> > >  typedef struct xenpf_core_parking xenpf_core_parking_t;
> > >  DEFINE_XEN_GUEST_HANDLE(xenpf_core_parking_t);
> > >  
> > > +#define XENPF_resource_op   61
> 
> More details please.
> > > +
> > > +#define XEN_RESOURCE_OP_MSR_READ  0
> > > +#define XEN_RESOURCE_OP_MSR_WRITE 1
> > > +
> > > +struct xenpf_resource_data {
> > > +    uint32_t cmd;       /* XEN_RESOURCE_OP_* */
> > > +    uint32_t rsvd;
> > > +    uint64_t idx;
> > > +    uint64_t val;
> 
> More details please. Pls say what the 'rsvd' is for, what
> the expected values are for 'idx', and 'val'.
> 
> Do also say which ones are IN or OUT.
> 
> Put yourself in the mindset of somebody who wants to use this
> and does not want to dive in the hypervisor to figure this out.
> Give as much information as possible in the headers.
> 
> > > +typedef struct xenpf_resource_data xenpf_resource_data_t;
> > > +DEFINE_XEN_GUEST_HANDLE(xenpf_resource_data_t);
> > > +
> > > +struct xenpf_resource_op {
> > > +    uint32_t nr;    /* number of data entry */
> > > +    uint32_t cpu;   /* which cpu to run */
> > > +    XEN_GUEST_HANDLE(xenpf_resource_data_t) data;
> > > +};
> > > +typedef struct xenpf_resource_op xenpf_resource_op_t;
> > > +DEFINE_XEN_GUEST_HANDLE(xenpf_resource_op_t);
> > > +
> > >  /*
> > >   * ` enum neg_errnoval
> > >   * ` HYPERVISOR_platform_op(const struct xen_platform_op*);
> > > @@ -553,6 +575,7 @@ struct xen_platform_op {
> > >          struct xenpf_cpu_hotadd        cpu_add;
> > >          struct xenpf_mem_hotadd        mem_add;
> > >          struct xenpf_core_parking      core_parking;
> > > +        struct xenpf_resource_op       resource_op;
> 
> resource_op?  I would really call this 'msr' or 'msr_data'

Thought on the other hand - you are trying to make this
generic (so it can be used for 'port I/O' or other).
In which case 'resource' looks like the best name.