[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 3/9] tools: provide interface for generic MSR access



> -----Original Message-----
> From: Xu, Dongxiao
> Sent: Thursday, July 24, 2014 2:31 PM
> To: Jan Beulich; Ian Campbell
> Cc: andrew.cooper3@xxxxxxxxxx; George.Dunlap@xxxxxxxxxxxxx;
> Ian.Jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx;
> keir@xxxxxxx
> Subject: RE: [PATCH v12 3/9] tools: provide interface for generic MSR access
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: Wednesday, July 23, 2014 3:49 PM
> > To: Ian Campbell
> > Cc: andrew.cooper3@xxxxxxxxxx; George.Dunlap@xxxxxxxxxxxxx;
> > Ian.Jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx; Xu, Dongxiao;
> > xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx;
> > keir@xxxxxxx
> > Subject: Re: [PATCH v12 3/9] tools: provide interface for generic MSR access
> >
> > >>> On 09.07.14 at 18:58, <ian.campbell@xxxxxxxxxx> wrote:
> > > On Fri, 2014-07-04 at 12:42 +0100, Jan Beulich wrote:
> > >> >>> On 04.07.14 at 10:34, <dongxiao.xu@xxxxxxxxx> wrote:
> > >> > Xen added a new sysctl hypercall for generic MSR access, and this is 
> > >> > the
> > >> > tool side change to wrapper the hypercall into xc APIs.
> > >>
> > >> s/sysctl/platform op/
> > >
> > > is platform_op really the right umbrella for this stuff? platform_op.h
> > > says:
> > >  * Hardware platform operations. Intended for use by domain-0 kernel.
> > >
> > > Which in particular I suppose has API stability implications.
> >
> > Yeah, I think Dongxiao earlier also got trapped by this comment. It's
> > origin predates thinking about disaggregation, and hence it's really
> > stale at this point: Hardware operations aren't necessarily limited to
> > Dom0 (they might be limited to hardware_domain, but that in the end
> > is a XSM policy decision).
> 
> Hi Jan,
> 
> Considering many people in the list requires the white-list style to limit the
> capability for resource access (e.g. MSR read/write), so I implement such a
> white-list in my new version patch like following:
> Does it look reasonable to you?

Forget to mention that, the following is hypervisor side code, residing in 
patch 1/9.

Thanks,
Dongxiao

> 
> static unsigned int allowed_msr_list[] = {
>     MSR_IA32_QOSEVTSEL,
>     MSR_IA32_QMC,
> };
> 
> static unsigned int allow_access(unsigned int idx, unsigned int *list, 
> unsigned int
> nr)
> {
>     unsigned int i;
> 
>     for ( i = 0; i < nr; i++ )
>         if ( list[i] == idx )
>             return 1;
> 
>     return 0;
> }
> 
> static void resource_access_one(void *info)
> {
>     ...
>     switch ( ra->type )
>     {
>     case XEN_RESOURCE_TYPE_MSR:
>         if ( !allow_access(ra->data.idx, allowed_msr_list,
>              sizeof(allowed_msr_list)/sizeof(unsigned int)) )
>             ret = -EACCES;
>         ...
>     ...
> }
> 
> Thanks,
> Dongxiao
> 
> >
> > Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.