Re: [PATCH v2 1/5] xen/common: introduce a new framework for save/restore of 'domain' context

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.05.2020 09:34, Paul Durrant wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 07 May 2020 08:22
>>
>> On 06.05.2020 18:44, Paul Durrant wrote:
>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>> Sent: 29 April 2020 12:02
>>>>
>>>> On 07.04.2020 19:38, Paul Durrant wrote:
>>>>> +int domain_load_begin(struct domain_context *c, unsigned int tc,
>>>>> +                      const char *name, const struct vcpu *v, size_t len,
>>>>> +                      bool exact)
>>>>> +{
>>>>> +    if ( c->log )
>>>>> +        gdprintk(XENLOG_INFO, "%pv load: %s (%lu)\n", v, name,
>>>>> +                 (unsigned long)len);
>>>>> +
>>>>> +    BUG_ON(tc != c->desc.typecode);
>>>>> +    BUG_ON(v->vcpu_id != c->desc.vcpu_id);
>>>>> +
>>>>> +    if ( (exact && (len != c->desc.length)) ||
>>>>> +         (len < c->desc.length) )
>>>>> +        return -EINVAL;
>>>>
>>>> How about
>>>>
>>>>     if ( exact ? len != c->desc.length
>>>>                : len < c->desc.length )
>>>>
>>>
>>> Yes, that doesn't look too bad.
>>>
>>>> ? I'm also unsure about the < - don't you mean > instead? Too
>>>> little data would be compensated by zero padding, but too
>>>> much data can't be dealt with. But maybe I'm getting the sense
>>>> of len wrong ...
>>>
>>> I think the < is correct. The caller needs to have at least enough
>>> space to accommodate the context record.
>>
>> But this is load, not save - the caller supplies the data. If
>> there's less data than can be fit, it'll be zero-extended. If
>> there's too much data, the excess you don't know what to do
>> with (it might be okay to tolerate it being all zero).
>>
> 
> But this is a callback. The outer load function iterates over
> the records calling the appropriate hander for each one. Those
> handlers then call this function saying how much data they
> expect and whether they want exactly that amount, or whether
> they can tolerate less (i.e. zero-extend). Hence
> len < c->desc.length is an error, because it means the
> descriptor contains more data than the hander knows how to
> handle.

Oh, I see - "But maybe I'm getting the sense of len wrong ..."
then indeed applies.

Any thoughts on tolerating the excess data being zero?

Jan