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

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

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

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