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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 06/10/17 11:30, Roger Pau Monné wrote:
> 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?

The conversion of -1 to unsigned int is well defined.  I think this is
fine as is.  Wei, as maintainer, whats your say?

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

Done.

~Andrew

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