Re: [Xen-devel] [PATCH-for-4.13 v4] Rationalize max_grant_frames and max_maptrack_frames handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.11.2019 14:58, Paul Durrant wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -84,11 +84,43 @@ struct grant_table {
>      struct grant_table_arch arch;
>  };
>  
> +static int __init parse_gnttab_limit(const char *param, const char *arg,

No __init please here and below, for this being runtime option
parsing functions.

> +                                     unsigned int *valp)
> +{
> +    const char *e;
> +    unsigned long val;
> +
> +    val = simple_strtoul(arg, &e, 0);
> +    if ( *e )
> +        return -EINVAL;
> +
> +    if ( val <= INT_MAX )
> +        *valp = val;
> +    else
> +        printk("parameter \"%s\" value \"%s\" is out of range; using value 
> \"%u\"\n",
> +               param, arg, *valp);

Better store INT_MAX in this case rather than leaving the value
unchanged? Or otherwise ...

> +    return 0;

... at least don't return success?

> +}
> +
>  unsigned int __read_mostly opt_max_grant_frames = 64;
> -integer_runtime_param("gnttab_max_frames", opt_max_grant_frames);
> +
> +static int __init parse_gnttab_max_frames(const char *arg)
> +{
> +    return parse_gnttab_limit("gnttab_max_frames", arg,
> +                              &opt_max_grant_frames);
> +}
> +custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames);
>  
>  unsigned int __read_mostly opt_max_maptrack_frames = 1024;

As indicated this wants to become static now.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -82,13 +82,15 @@ struct xen_domctl_createdomain {
>      uint32_t iommu_opts;
>  
>      /*
> -     * Various domain limits, which impact the quantity of resources (global
> -     * mapping space, xenheap, etc) a guest may consume.
> +     * Various domain limits, which impact the quantity of resources
> +     * (global mapping space, xenheap, etc) a guest may consume.  For
> +     * max_grant_frames and max_maptrack_frames, < 0 means "use the
> +     * default maximum value in the hypervisor".
>       */
>      uint32_t max_vcpus;
>      uint32_t max_evtchn_port;
> -    uint32_t max_grant_frames;
> -    uint32_t max_maptrack_frames;
> +    int32_t max_grant_frames;
> +    int32_t max_maptrack_frames;

While this may want backporting aiui, we need to be a little
careful with the interface change here.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel