Re: [Xen-devel] [PATCH v16 01/10] x86: add generic resource (e.g. MSR) access hypercall

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel