|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v16 01/10] x86: add generic resource (e.g. MSR) access hypercall
On Thu, Sep 25, 2014 at 08:57:38PM +0100, Andrew Cooper wrote:
> On 25/09/14 11:19, Chao Peng wrote:
> > Add a generic resource access hypercall for tool stack or other
> > components, e.g., accessing MSR, port I/O, etc.
> >
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> > Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> > ---
> > xen/arch/x86/platform_hypercall.c | 90
> > ++++++++++++++++++++++++++++++
> > xen/arch/x86/x86_64/platform_hypercall.c | 4 ++
> > xen/include/public/platform.h | 23 ++++++++
> > xen/include/xlat.lst | 2 +
> > 4 files changed, 119 insertions(+)
> >
> > diff --git a/xen/arch/x86/platform_hypercall.c
> > b/xen/arch/x86/platform_hypercall.c
> > index 2162811..081d9f5 100644
> > --- a/xen/arch/x86/platform_hypercall.c
> > +++ b/xen/arch/x86/platform_hypercall.c
> > @@ -61,6 +61,68 @@ long cpu_down_helper(void *data);
> > long core_parking_helper(void *data);
> > uint32_t get_cur_idle_nums(void);
> >
> > +struct xen_resource_access {
> > + int32_t ret;
> > + uint32_t nr;
> > + XEN_GUEST_HANDLE(xenpf_resource_data_t) data;
> > +};
> > +
> > +static bool_t allow_access_msr(unsigned int msr)
> > +{
> > + return 0;
> > +}
> > +
> > +static void resource_access(void *info)
> > +{
> > + struct xen_resource_access *ra = info;
> > + xenpf_resource_data_t data;
> > + int ret = 0;
> > + unsigned int i;
> > +
> > + for ( i = 0; i < ra->nr; i++ )
> > + {
> > + if ( copy_from_guest_offset(&data, ra->data, i, 1) )
>
> You cannot use copy_{to,from}_guest() here. You are almost certainly on
> the wrong cpu, and running on the wrong set of pagetables.
>
> At best, you will copy bogus data from the wrong process/domain, and
> likely corrupt it when copying back.
>
> > + {
> > + ret = -EFAULT;
> > + break;
> > + }
> > +
> > + if ( data.rsvd ) {
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + switch ( data.cmd )
> > + {
> > + case XEN_RESOURCE_OP_MSR_READ:
> > + case XEN_RESOURCE_OP_MSR_WRITE:
> > + if ( data.idx >> 32 )
> > + ret = -EINVAL;
> > + else if ( !allow_access_msr(data.idx) )
> > + ret = -EACCES;
> > + else if ( data.cmd == XEN_RESOURCE_OP_MSR_READ )
> > + ret = rdmsr_safe(data.idx, data.val);
> > + else
> > + ret = wrmsr_safe(data.idx, data.val);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + if ( ret )
> > + break;
> > +
> > + if ( copy_to_guest_offset(ra->data, i, &data, 1) )
> > + {
> > + ret = -EFAULT;
> > + break;
> > + }
> > + }
> > +
> > + ra->ret = ret;
> > +}
> > +
> > ret_t do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
> > {
> > ret_t ret = 0;
> > @@ -601,6 +663,34 @@ ret_t
> > do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
> > }
> > break;
> >
> > + case XENPF_resource_op:
> > + {
> > + struct xen_resource_access ra;
> > + struct xenpf_resource_op *rsc_op = &op->u.resource_op;
> > + unsigned int cpu = smp_processor_id();
> > +
> > + ra.nr = rsc_op->nr;
> > + ra.data = rsc_op->data;
>
> You must do all copy_{from,to}_user() here, and strictly only pass Xen
> pointers to resource_access().
>
> This means you will need to xmalloc() yourself some space for the
> xenpf_resource_data_t array.
>
>
> On a different note, you need to enforce a maximum resource_op.nr of
> something rather low to (16/32 perhaps?) to prevent a toolstack asking
> for 0xffffffff non-preemptible operations.
>
Correct, thanks Andrew.
Chao
>
> > +
> > + if ( rsc_op->cpu == cpu )
> > + resource_access(&ra);
> > + else if ( cpu_online(rsc_op->cpu) )
> > + on_selected_cpus(cpumask_of(rsc_op->cpu),
> > + resource_access, &ra, 1);
> > + else
> > + {
> > + ret = -ENODEV;
> > + break;
> > + }
> > +
> > + if ( ra.ret )
> > + {
> > + ret = ra.ret;
> > + break;
> > + }
> > + }
> > + break;
> > +
> > default:
> > ret = -ENOSYS;
> > break;
> > diff --git a/xen/arch/x86/x86_64/platform_hypercall.c
> > b/xen/arch/x86/x86_64/platform_hypercall.c
> > index b6f380e..4db6622 100644
> > --- a/xen/arch/x86/x86_64/platform_hypercall.c
> > +++ b/xen/arch/x86/x86_64/platform_hypercall.c
> > @@ -32,6 +32,10 @@ CHECK_pf_pcpu_version;
> > CHECK_pf_enter_acpi_sleep;
> > #undef xen_pf_enter_acpi_sleep
> >
> > +#define xen_pf_resource_data xenpf_resource_data
> > +CHECK_pf_resource_data;
> > +#undef xen_pf_resource_data
> > +
> > #define COMPAT
> > #define _XEN_GUEST_HANDLE(t) XEN_GUEST_HANDLE(t)
> > #define _XEN_GUEST_HANDLE_PARAM(t) XEN_GUEST_HANDLE_PARAM(t)
> > diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
> > 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
> > +
> > +#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;
> > +};
> > +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;
> > uint8_t pad[128];
> > } u;
> > };
> > diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
> > index 9a35dd7..100fcf5 100644
> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -88,6 +88,8 @@
> > ? xenpf_enter_acpi_sleep platform.h
> > ? xenpf_pcpuinfo platform.h
> > ? xenpf_pcpu_version platform.h
> > +? xenpf_resource_op platform.h
> > +? xenpf_resource_data platform.h
> > ! sched_poll sched.h
> > ? sched_remote_shutdown sched.h
> > ? sched_shutdown sched.h
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |