On 20.10.2021 12:14, Roger Pau Monné wrote:
> On Fri, Oct 15, 2021 at 12:05:06PM +0200, Jan Beulich wrote:
>> On 22.09.2021 10:21, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -750,7 +750,8 @@ static struct domain *__init create_dom0(const module_t
>>> *image,
>>> .max_evtchn_port = -1,
>>> .max_grant_frames = -1,
>>> .max_maptrack_frames = -1,
>>> - .grant_opts = XEN_DOMCTL_GRANT_version_default,
>>> + .grant_opts = XEN_DOMCTL_GRANT_version_default |
>>> + XEN_DOMCTL_GRANT_transitive,
>>> .max_vcpus = dom0_max_vcpus(),
>>> .arch = {
>>> .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
>>
>> While I can see that you make these adjustments for retaining backwards
>> compatibility, I wonder if we need to, at least for Dom0. Dom0 doesn't
>> normally grant anything anyway and hence would even less so use
>> transitive grants. Of course then there's need to be a command line
>> control to re-enable that, just in case.
>
> dom0=gnttab-transitive, or should it be placed somewhere else?
That sounds like the place to have it at; at least that's where I would
have suggested to put it. One question of course is how it ought to
interact with v2 being unavailable.
>>> @@ -1965,6 +1969,8 @@ int grant_table_init(struct domain *d, int
>>> max_grant_frames,
>>> gt->max_grant_frames = max_grant_frames;
>>> gt->max_maptrack_frames = max_maptrack_frames;
>>> gt->max_grant_version = max_grant_version;
>>> + gt->allow_transitive = !!(options & XEN_DOMCTL_GRANT_transitive) &&
>>> + opt_transitive_grants;
>>
>> No need for !! here afaics. Not even if there weren't the &&.
>>
>> As to combining with the global option: I think if an admin requested
>> them for a domain, this should overrule the command line option. Which
>> in turn suggests that the command line option could go away at this
>> point. Otherwise, if you AND both together and the guest is known to
>> not work without this functionality, domain creation would instead
>> better fail (or at the very least it should be logged by the tool
>> stack that the request wasn't satisfied, which would require a means
>> to retrieve the effective setting). IOW I would see the command line
>> turning this off to trump the per-guest enabling request.
>
> How should we go about deprecating it? It would be a bit antisocial
> IMO to just drop the option, since people using it would then be
> exposed to guests using transient grants if they didn't realize it
> should be set in xl.conf or xl.cfg now.
So perhaps for a transitional phase fail if the command line option
says no and the request for the guest says yes? Accompanied by a
log message warning that the command line control is going to go
away, so that people will know to adjust their guest configs?
Jan
