|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 5/8] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl.
On 27/08/15 12:02, Konrad Rzeszutek Wilk wrote:
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1808,25 +1808,25 @@ static PyObject *pyxc_tmem_control(XcObject *self,
> &pool_id, &subop, &cli_id, &arg1, &arg2, &buf) )
> return NULL;
>
> - if ( (subop == TMEMC_LIST) && (arg1 > 32768) )
> + if ( (subop == XEN_SYSCTL_TMEM_OP_LIST) && (arg1 > 32768) )
> arg1 = 32768;
>
> if ( (rc = xc_tmem_control(self->xc_handle, pool_id, subop, cli_id,
> arg1, arg2, buffer)) < 0 )
> return Py_BuildValue("i", rc);
>
> switch (subop) {
> - case TMEMC_LIST:
> + case XEN_SYSCTL_TMEM_OP_LIST:
> return Py_BuildValue("s", buffer);
> - case TMEMC_FLUSH:
> + case XEN_SYSCTL_TMEM_OP_FLUSH:
> return Py_BuildValue("i", rc);
> - case TMEMC_QUERY_FREEABLE_MB:
> + case XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB:
> return Py_BuildValue("i", rc);
> - case TMEMC_THAW:
> - case TMEMC_FREEZE:
> - case TMEMC_DESTROY:
> - case TMEMC_SET_WEIGHT:
> - case TMEMC_SET_CAP:
> - case TMEMC_SET_COMPRESS:
> + case XEN_SYSCTL_TMEM_OP_THAW:
> + case XEN_SYSCTL_TMEM_OP_FREEZE:
> + case XEN_SYSCTL_TMEM_OP_DESTROY:
> + case XEN_SYSCTL_TMEM_OP_SET_WEIGHT:
> + case XEN_SYSCTL_TMEM_OP_SET_CAP:
> + case XEN_SYSCTL_TMEM_OP_SET_COMPRESS:
Any case statements falling through into default like this can simply be
dropped.
> @@ -2555,77 +2556,72 @@ static int tmemc_restore_flush_page(int cli_id,
> uint32_t pool_id, struct oid *oi
> return do_tmem_flush_page(pool,oidp,index);
> }
>
> -static int do_tmem_control(struct tmem_op *op)
> +int tmem_control(struct xen_sysctl_tmem_op *op)
> {
> int ret;
> uint32_t pool_id = op->pool_id;
> - uint32_t subop = op->u.ctrl.subop;
> - struct oid *oidp = (struct oid *)(&op->u.ctrl.oid[0]);
> + uint32_t cmd = op->cmd;
> + struct oid *oidp = (struct oid *)(&op->oid[0]);
Please put a struct xen_sysctl_tmem_oid definition in sysctl.h and avoid
this typesystem abuse.
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -710,6 +710,48 @@ struct xen_sysctl_psr_cat_op {
> typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
> DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>
> +#define XEN_SYSCTL_TMEM_OP_ALL_CLIENTS 0xFFFFU
> +
> +#define XEN_SYSCTL_TMEM_OP_THAW 0
> +#define XEN_SYSCTL_TMEM_OP_FREEZE 1
> +#define XEN_SYSCTL_TMEM_OP_FLUSH 2
> +#define XEN_SYSCTL_TMEM_OP_DESTROY 3
> +#define XEN_SYSCTL_TMEM_OP_LIST 4
> +#define XEN_SYSCTL_TMEM_OP_SET_WEIGHT 5
> +#define XEN_SYSCTL_TMEM_OP_SET_CAP 6
> +#define XEN_SYSCTL_TMEM_OP_SET_COMPRESS 7
> +#define XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB 8
> +#define XEN_SYSCTL_TMEM_OP_SAVE_BEGIN 10
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION 11
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS 12
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT 13
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP 14
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS 15
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS 16
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES 17
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID 18
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE 19
> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV 20
> +#define XEN_SYSCTL_TMEM_OP_SAVE_END 21
> +#define XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN 30
> +#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
> +#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
> +
> +struct xen_sysctl_tmem_op {
> + uint32_t cmd; /* IN: XEN_SYSCTL_TMEM_OP_* . */
> + int32_t pool_id; /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
> + uint32_t cli_id; /* IN: client id, 0 for
> XEN_SYSCTL_TMEM_QUERY_FREEABLE_MB
> + for all others can be the domain id or
> + XEN_SYSCTL_TMEM_OP_ALL_CLIENTS for all. */
> + uint32_t arg1; /* IN: If not applicable to command use 0. */
> + uint32_t arg2; /* IN: If not applicable to command use 0. */
Please can this interface be fixed as part of the move, even if it is in
subsequent patches for clarity.
Part of the issue with the XSA-17 security audit was that these args are
completely opaque.
> + uint8_t pad[4]; /* Padding so structure is the same under 32 and 64.
> */
probably better to use a uint32_t here.
> + uint64_t oid[3]; /* IN: If not applicable to command use 0. */
> + XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore
> ops. */
> +};
> +typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_tmem_op_t);
> +
> struct xen_sysctl {
> uint32_t cmd;
> #define XEN_SYSCTL_readconsole 1
> @@ -734,6 +776,7 @@ struct xen_sysctl {
> #define XEN_SYSCTL_psr_cmt_op 21
> #define XEN_SYSCTL_pcitopoinfo 22
> #define XEN_SYSCTL_psr_cat_op 23
> +#define XEN_SYSCTL_tmem_op 24
> uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
> union {
> struct xen_sysctl_readconsole readconsole;
> @@ -758,6 +801,7 @@ struct xen_sysctl {
> struct xen_sysctl_coverage_op coverage_op;
> struct xen_sysctl_psr_cmt_op psr_cmt_op;
> struct xen_sysctl_psr_cat_op psr_cat_op;
> + struct xen_sysctl_tmem_op tmem_op;
> uint8_t pad[128];
> } u;
> };
> diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
> index 4fd2fc6..208f5a6 100644
> --- a/xen/include/public/tmem.h
> +++ b/xen/include/public/tmem.h
> @@ -33,7 +33,7 @@
> #define TMEM_SPEC_VERSION 1
>
> /* Commands to HYPERVISOR_tmem_op() */
> -#define TMEM_CONTROL 0
> +#define TMEM_CONTROL_MOVED 0
If anything, this should be deleted as opposed to being renamed. The
_MOVED suffix is confusing, as TMEM_CONTROL_MOVED sounds like it could
be a plausible TMEM operation.
> #define TMEM_NEW_POOL 1
> #define TMEM_DESTROY_POOL 2
> #define TMEM_PUT_PAGE 4
> @@ -48,35 +48,9 @@
> #endif
>
> /* Privileged commands to HYPERVISOR_tmem_op() */
> -#define TMEM_AUTH 101
> +#define TMEM_AUTH 101
> #define TMEM_RESTORE_NEW 102
>
> -/* Subops for HYPERVISOR_tmem_op(TMEM_CONTROL) */
> -#define TMEMC_THAW 0
> -#define TMEMC_FREEZE 1
> -#define TMEMC_FLUSH 2
> -#define TMEMC_DESTROY 3
> -#define TMEMC_LIST 4
> -#define TMEMC_SET_WEIGHT 5
> -#define TMEMC_SET_CAP 6
> -#define TMEMC_SET_COMPRESS 7
> -#define TMEMC_QUERY_FREEABLE_MB 8
> -#define TMEMC_SAVE_BEGIN 10
> -#define TMEMC_SAVE_GET_VERSION 11
> -#define TMEMC_SAVE_GET_MAXPOOLS 12
> -#define TMEMC_SAVE_GET_CLIENT_WEIGHT 13
> -#define TMEMC_SAVE_GET_CLIENT_CAP 14
> -#define TMEMC_SAVE_GET_CLIENT_FLAGS 15
> -#define TMEMC_SAVE_GET_POOL_FLAGS 16
> -#define TMEMC_SAVE_GET_POOL_NPAGES 17
> -#define TMEMC_SAVE_GET_POOL_UUID 18
> -#define TMEMC_SAVE_GET_NEXT_PAGE 19
> -#define TMEMC_SAVE_GET_NEXT_INV 20
> -#define TMEMC_SAVE_END 21
> -#define TMEMC_RESTORE_BEGIN 30
> -#define TMEMC_RESTORE_PUT_PAGE 32
> -#define TMEMC_RESTORE_FLUSH_PAGE 33
> -
> /* Bits for HYPERVISOR_tmem_op(TMEM_NEW_POOL) */
> #define TMEM_POOL_PERSIST 1
> #define TMEM_POOL_SHARED 2
> @@ -110,16 +84,7 @@ struct tmem_op {
> uint32_t flags;
> uint32_t arg1;
> } creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
> - struct {
> - uint32_t subop;
> - uint32_t cli_id;
> - uint32_t arg1;
> - uint32_t arg2;
> - uint64_t oid[3];
> - tmem_cli_va_t buf;
> - } ctrl; /* for cmd == TMEM_CONTROL */
> struct {
> -
> uint64_t oid[3];
> uint32_t index;
> uint32_t tmem_offset;
> diff --git a/xen/include/xen/tmem.h b/xen/include/xen/tmem.h
> index 5dbf9d5..32a542a 100644
> --- a/xen/include/xen/tmem.h
> +++ b/xen/include/xen/tmem.h
> @@ -9,6 +9,9 @@
> #ifndef __XEN_TMEM_H__
> #define __XEN_TMEM_H__
>
> +struct xen_sysctl_tmem_op;
#include<public/sysctl.h> please.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |