Re: [PATCH v5 6/8] xen: add new domctl hypercall to set grant table resource limits

[Juergen Gross <jgross@suse.com>]

On 11/09/17 13:14, Jan Beulich wrote:
>>>> On 11.09.17 at 12:48, <jgross@suse.com> wrote:
>> On 08/09/17 17:55, Jan Beulich wrote:
>>>>>> On 08.09.17 at 08:56, <jgross@suse.com> wrote:
>>>> --- 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;
>>>
>>> How does EEXIST represent the error condition?
>>
>> Yes, this was a bad choice. What about ENOENT?
> 
> Fine with me. Or ENODEV.
> 
>>>> +    ret = 0;
>>>> +    if ( grant_frames )
>>>> +        gt->max_grant_frames = grant_frames;
>>>> +    if ( maptrack_frames )
>>>> +        gt->max_maptrack_frames = maptrack_frames;
>>>
>>> Together with what I have said regarding making the invocation
>>> of this domctl mandatory, I think these two shouldn't be conditional.
>>> In particular for maptrack I also don't see why a domain couldn't
>>> do with a limit of zero, as long as it's not serving as a backend for
>>> another guest.
>>
>> Okay, then I'd need to specify a "take hypervisor default" value (e.g.
>> ~0) in order to handle the case where no value was specified in the
>> domain's config file.
> 
> Well, part of the point I was trying to make in earlier replies on
> other patches of this series is that I think the lack of a guest
> config file setting should not invoke a _hypervisor_ default.
> Instead, the tool stack should establish a sensible one.

Okay, I can add this to the series.

> 
>> The question would be then: do we want to set maptrack to 0 as default
>> and require it to be specified for backend domains (driver domains,
>> stubdoms)?
> 
> I think so, yes. Question is whether there's a way for the tool stack
> to easily recognize a driver domain when it's being created.

I could think of various mechanisms for driver domains:

1. Add an explicit config item (I guess this could be utilized for other
   cases like XSM, too).

2. Do some guess work, e.g. any domain with PCI-passthrough configured
   could be regarded as a potential driver domain.

3. Just require the max_maptrack_frames= config item.

Starting with 3. is the easiest solution for now, it can be switched to
1. or 2. later.


Juergen

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