Re: [PATCH v3 1/7] xen/gnttab: Rework resource acquisition

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/2021 11:43, Jan Beulich wrote:
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -4013,6 +4013,59 @@ static int gnttab_get_shared_frame_mfn(struct domain 
>> *d,
>>      return 0;
>>  }
>>  
>> +int gnttab_acquire_resource(
>> +    struct domain *d, unsigned int id, unsigned long frame,
>> +    unsigned int nr_frames, xen_pfn_t mfn_list[])
>> +{
>> +    struct grant_table *gt = d->grant_table;
>> +    unsigned int i = nr_frames, tot_frames;
> It doesn't look like this initializer was needed. The only
> use of i that I can spot is the loop near the end, which
> starts from 0.

Its possibly stale from v1.  I'll adjust.

>> +    mfn_t tmp;
>> +    void **vaddrs;
>> +    int rc;
>> +
>> +    /* Overflow checks */
>> +    if ( frame + nr_frames < frame )
>> +        return -EINVAL;
>> +
>> +    tot_frames = frame + nr_frames;
>> +    if ( tot_frames != frame + nr_frames )
>> +        return -EINVAL;
> Can't these two be folded into
>
>     unsigned int tot_frames = frame + nr_frames;
>
>     if ( tot_frames < frame )
>         return -EINVAL;
>
> ? Both truncation and wrapping look to be taken care of this
> way.

Not when frame is a multiple of 4G (or fractionally over, I think).

This ABI with mismatched widths really is obnoxious to work with.

~Andrew