Re: [PATCH for-4.10 5/5] tools/dombuilder: Prevent failures of xc_dom_gnttab_init()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Oct 05, 2017 at 06:23:44PM +0000, Andrew Cooper wrote:
> Recent changes in grant table configuration have caused calls to
> xc_dom_gnttab_init() to fail if not proceeded with a call to
> xc_domain_set_gnttab_limits().  This is backwards from the point of view of
> 3rd party dombuilder users.
> 
> Add max_{grant,maptrack}_frames parameters to struct xc_dom_image, and require
> them to be set by callers using xc_dom_gnttab_init().  Libxl, which uses
> xc_dom_gnttab_init() itself is updated appropriately.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Some nits with can be fixed while committing IMHO if required.

> ---
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> ---
>  tools/libxc/include/xc_dom.h |  4 ++++
>  tools/libxc/xc_dom_boot.c    | 14 ++++++++++++++
>  tools/libxc/xc_dom_core.c    |  3 +++
>  tools/libxl/libxl_dom.c      | 12 ++++++------
>  4 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 790869b..8e673fb 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -116,6 +116,10 @@ struct xc_dom_image {
>      domid_t console_domid;
>      domid_t xenstore_domid;
>  
> +    /* Grant limit configuration; mandatory if calling xc_dom_gnttab_init(). */
> +    unsigned int max_grant_frames;
> +    unsigned int max_maptrack_frames;
> +
>      /*
>       * initrd parameters as specified in start_info page
>       * Depending on capabilities of the booted kernel this may be a virtual
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index 8d4fefa..7cb9e40 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -419,6 +419,20 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
>  
>  int xc_dom_gnttab_init(struct xc_dom_image *dom)
>  {
> +    int rc;
> +
> +    if ( dom->max_grant_frames == -1 || dom->max_maptrack_frames == -1 )

Not sure if compilers will complain about comparing an unsigned type
against a signed one. Maybe better to use ~0u?

> +    {
> +        xc_dom_panic(dom->xch, XC_INVALID_PARAM,
> +                     "%s: Caller didn't set grant limit information", __func__);

errno = EINVAL;

Thanks, Roger.

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