Re: [Xen-devel] [PATCH v2 6/8] xen: add new domctl hypercall to set grant table resource limits

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of
> Juergen Gross
> Sent: 06 September 2017 09:27
> To: xen-devel@xxxxxxxxxxxxx
> Cc: Juergen Gross <jgross@xxxxxxxx>; sstabellini@xxxxxxxxxx; Wei Liu
> <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
> Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
> <Ian.Jackson@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>;
> jbeulich@xxxxxxxx
> Subject: [Xen-devel] [PATCH v2 6/8] xen: add new domctl hypercall to set
> grant table resource limits
> 
> Add a domctl hypercall to set the domain's resource limits regarding
> grant tables. It is accepted only as long as neither
> gnttab_setup_table() has been called for the domain, nor the domain
> has started to run.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

The code LGTM but I wonder, do we want to introduce another contraction of 
'grant table'? The usual one seems to be 'gnttab' rather than 'gnttbl' as you 
have here.

  Paul

> ---
>  tools/flask/policy/modules/dom0.te  |  2 +-
>  xen/common/domctl.c                 |  6 ++++++
>  xen/common/grant_table.c            | 26 ++++++++++++++++++++++++++
>  xen/include/public/domctl.h         |  9 +++++++++
>  xen/include/xen/grant_table.h       |  2 ++
>  xen/xsm/flask/hooks.c               |  3 +++
>  xen/xsm/flask/policy/access_vectors |  2 ++
>  7 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/flask/policy/modules/dom0.te
> b/tools/flask/policy/modules/dom0.te
> index 338caaf41e..fe672a61be 100644
> --- a/tools/flask/policy/modules/dom0.te
> +++ b/tools/flask/policy/modules/dom0.te
> @@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain {
>  };
>  allow dom0_t dom0_t:domain2 {
>       set_cpuid gettsc settsc setscheduler set_max_evtchn
> set_vnumainfo
> -     get_vnumainfo psr_cmt_op psr_cat_op
> +     get_vnumainfo psr_cmt_op psr_cat_op set_gnttbl_limits
>  };
>  allow dom0_t dom0_t:resource { add remove };
> 
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 42658e5744..027501c8ec 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -14,6 +14,7 @@
>  #include <xen/sched-if.h>
>  #include <xen/domain.h>
>  #include <xen/event.h>
> +#include <xen/grant_table.h>
>  #include <xen/domain_page.h>
>  #include <xen/trace.h>
>  #include <xen/console.h>
> @@ -1149,6 +1150,11 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>              copyback = 1;
>          break;
> 
> +    case XEN_DOMCTL_set_gnttbl_limits:
> +        ret = grant_table_set_limits(d, op->u.set_gnttbl_limits.grant_frames,
> +                                     
> op->u.set_gnttbl_limits.maptrack_frames);
> +        break;
> +
>      default:
>          ret = arch_do_domctl(op, d, u_domctl);
>          break;
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 709fd55490..ed57677f1a 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3667,6 +3667,32 @@ void grant_table_init_vcpu(struct vcpu *v)
>      v->maptrack_tail = MAPTRACK_TAIL;
>  }
> 
> +int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
> +                           unsigned int maptrack_frames)
> +{
> +    struct grant_table *gt = d->grant_table;
> +    int ret = -EBUSY;
> +
> +    if ( !gt )
> +        return -EEXIST;
> +
> +    grant_write_lock(gt);
> +
> +    if ( gt->nr_grant_frames )
> +        goto unlock;
> +
> +    ret = 0;
> +    if ( grant_frames )
> +        gt->max_grant_frames = grant_frames;
> +    if ( maptrack_frames )
> +        gt->max_maptrack_frames = maptrack_frames;
> +
> + unlock:
> +    grant_write_unlock(gt);
> +
> +    return ret;
> +}
> +
>  #ifdef CONFIG_HAS_MEM_SHARING
>  int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
>                              gfn_t *gfn, uint16_t *status)
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 50ff58f5b9..05757c06ac 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1163,6 +1163,13 @@ struct xen_domctl_psr_cat_op {
>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
> 
> +struct xen_domctl_set_gnttbl_limits {
> +    uint32_t grant_frames;     /* IN: if 0, dont change */
> +    uint32_t maptrack_frames;  /* IN: if 0, dont change */
> +};
> +typedef struct xen_domctl_set_gnttbl_limits
> xen_domctl_set_gnttbl_limits_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_gnttbl_limits_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> @@ -1240,6 +1247,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_monitor_op                    77
>  #define XEN_DOMCTL_psr_cat_op                    78
>  #define XEN_DOMCTL_soft_reset                    79
> +#define XEN_DOMCTL_set_gnttbl_limits             80
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1302,6 +1310,7 @@ struct xen_domctl {
>          struct xen_domctl_psr_cmt_op        psr_cmt_op;
>          struct xen_domctl_monitor_op        monitor_op;
>          struct xen_domctl_psr_cat_op        psr_cat_op;
> +        struct xen_domctl_set_gnttbl_limits set_gnttbl_limits;
>          uint8_t                             pad[128];
>      } u;
>  };
> diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
> index 84a8d61616..dd9aa3b9ee 100644
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -40,6 +40,8 @@ int grant_table_init(
>  void grant_table_destroy(
>      struct domain *d);
>  void grant_table_init_vcpu(struct vcpu *v);
> +int grant_table_set_limits(struct domain *d, unsigned int grant_frames,
> +                           unsigned int maptrack_frames);
> 
>  /*
>   * Check if domain has active grants and log first 10 of them.
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 56dc5b0ab9..ae5122cfdc 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -749,6 +749,9 @@ static int flask_domctl(struct domain *d, int cmd)
>      case XEN_DOMCTL_soft_reset:
>          return current_has_perm(d, SECCLASS_DOMAIN2,
> DOMAIN2__SOFT_RESET);
> 
> +    case XEN_DOMCTL_set_gnttbl_limits:
> +        return current_has_perm(d, SECCLASS_DOMAIN2,
> DOMAIN2__SET_GNTTBL_LIMITS);
> +
>      default:
>          return avc_unknown_permission("domctl", cmd);
>      }
> diff --git a/xen/xsm/flask/policy/access_vectors
> b/xen/xsm/flask/policy/access_vectors
> index da9f3dfb2e..aa33f1ecb0 100644
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -248,6 +248,8 @@ class domain2
>      mem_sharing
>  # XEN_DOMCTL_psr_cat_op
>      psr_cat_op
> +# XEN_DOMCTL_set_gnttbl_limits
> +    set_gnttbl_limits
>  }
> 
>  # Similar to class domain, but primarily contains domctls related to HVM
> domains
> --
> 2.12.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel