On 13.03.2023 13:16, Roger Pau Monne wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1232,9 +1232,8 @@ The usage of gnttab v2 is not security supported on ARM
> platforms.
>
> > Can be modified at runtime
>
> -Specify the maximum number of frames which any domain may use as part
> -of its grant table. This value is an upper boundary of the per-domain
> -value settable via Xen tools.
> +Specify the default maximum number of frames which any domain may use as part
> +of its grant table unless a different value is specified at domain creation.
>
> Dom0 is using this value for sizing its grant table.
dom0less DomU-s do as well, at the very least, also ...
> @@ -1245,9 +1244,10 @@ Dom0 is using this value for sizing its grant table.
>
> > Can be modified at runtime
>
> -Specify the maximum number of frames to use as part of a domains
> -maptrack array. This value is an upper boundary of the per-domain
> -value settable via Xen tools.
> +Specify the default maximum number of frames to use as part of a domains
> +maptrack array unless a different value is specified at domain creation.
> +
> +Dom0 is using this value for sizing its maptrack array.
... here. And even ordinary DomU-s appear to default to that in the
absence of a specific value in the guest config. IOW at the very least
the info you add should not be misleading. Better would be if the pre-
existing info was adjusted at the same time.
I also wonder about the specific wording down here: While the max grant
table size can indeed be queried, this isn't the case for the maptrack
array. A domain also doesn't need to know its size, so maybe "This value
is used to size all domains' maptrack arrays, unless overridden by their
guest config"?
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1956,18 +1956,15 @@ int grant_table_init(struct domain *d, int
> max_grant_frames,
> return -EINVAL;
> }
>
> - /* Default to maximum value if no value was specified */
> + /* Apply defaults if no value was specified */
> if ( max_grant_frames < 0 )
> max_grant_frames = opt_max_grant_frames;
> if ( max_maptrack_frames < 0 )
> max_maptrack_frames = opt_max_maptrack_frames;
>
> - if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES ||
> - max_grant_frames > opt_max_grant_frames ||
> - max_maptrack_frames > opt_max_maptrack_frames )
> + if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES )
> {
> - dprintk(XENLOG_INFO, "Bad grant table sizes: grant %u, maptrack
> %u\n",
> - max_grant_frames, max_maptrack_frames);
> + dprintk(XENLOG_INFO, "Bad grant table size %u\n", max_grant_frames);
> return -EINVAL;
> }
I think I agree with the relaxation done here, but I also think this not
introducing security concerns wants spelling out in the description: My
understanding is that even in disaggregated environments we assume only
fully privileged entities can create domains.
Jan
