xen/x86: support setting dom0_mem depending on host size

Quoted 1 line

Quoted 1 line · On 10.12.18 at 12:44, <[email protected]> wrote:

Modify parse_size_and_unit() to support a value followed by a '%' character. In this case ps is required to be non-NULL to ensure the caller can detect that case. The returned value will be the integer value s was pointing to and *ps will point to the '%' character.

Signed-off-by Juergen Gross <[email protected]>

Quoted 1 line

Quoted 1 line · On 10.12.18 at 12:44, <[email protected]> wrote:

Today the memory size of dom0 can be specified only in terms of bytes (either an absolute value or "host-mem - value"). When dom0 shouldn't be auto-ballooned this requires nearly always a manual adaption of the Xen boot parameters to reflect the actual host memory size.

Add more possibilities to specify memory sizes. Today we have:

dom0_mem= List of ( min:<size> | max:<size> | <size> )

with <size> being a positive or negative size value (e.g. 1G).

Modify that to:

dom0_mem= List of ( min:<sz> | max:<sz> | <sz> )
<sz>: <size> | [<size>+]<frac>%
<frac>: integer value < 100

With the following semantics:

<frac>% specifies a fraction of host memory size in percent. <sz> is a percentage of host memory plus an offset.

So <sz> being 1G+25% on a 256G host would result in 65G.

Signed-off-by Juergen Gross <[email protected]>

xen/arch/x86/dom0_build.c +13 -3

-static long __init parse_amt(const char *s, const char **ps)
+static int __init parse_amt(const char *s, const char **ps, struct memsize *sz)
 {
-    long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
-    return (*s == '-') ? -pages : pages;
+    unsigned long val;
+    struct memsize tmp = { };
+    unsigned int items = 0;
+
+    tmp.minus = (*s == '-');
+    if ( tmp.minus )
+        s++;
+
+    do
+    {
+        if ( !isdigit(*s) )
+            return -EINVAL;

Quoted 18 lines

Quoted 1 line

Quoted 1 line · On 10.12.18 at 12:44, <[email protected]> wrote:

Today the memory size of dom0 can be specified only in terms of bytes (either an absolute value or "host-mem - value"). When dom0 shouldn't be auto-ballooned this requires nearly always a manual adaption of the Xen boot parameters to reflect the actual host memory size.

Add more possibilities to specify memory sizes. Today we have:

dom0_mem= List of ( min:<size> | max:<size> | <size> )

with <size> being a positive or negative size value (e.g. 1G).

Modify that to:

dom0_mem= List of ( min:<sz> | max:<sz> | <sz> )
<sz>: <size> | [<size>+]<frac>%
<frac>: integer value < 100

Quoted 8 lines

With the following semantics:

<frac>% specifies a fraction of host memory size in percent. <sz> is a percentage of host memory plus an offset.

So <sz> being 1G+25% on a 256G host would result in 65G.

Signed-off-by Juergen Gross <[email protected]>

Once again

Reviewed-by Jan Beulich <[email protected]>

yet once again with a remark:

Quoted 17 lines

xen/arch/x86/dom0_build.c +13 -3

-static long __init parse_amt(const char *s, const char **ps)
+static int __init parse_amt(const char *s, const char **ps, struct memsize *sz)
 {
-    long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
-    return (*s == '-') ? -pages : pages;
+    unsigned long val;
+    struct memsize tmp = { };
+    unsigned int items = 0;
+
+    tmp.minus = (*s == '-');
+    if ( tmp.minus )
+        s++;
+
+    do
+    {
+        if ( !isdigit(*s) )
+            return -EINVAL;

xen/arch/x86/dom0_build.c static long __initdata dom0_max_nrpages = LONG_MAX; · line 95

Quoted 10 lines

xen/arch/x86/dom0_build.c +10 -0

+
+        val = parse_size_and_unit(s, ps);
+        s = *ps;
+        if ( *s == '%' )
+        {
+            if ( val >= 100 )
+                return -EINVAL;
+            tmp.percent = val;
+            s++;
+            items++; /* No other item allowed. */

Isn't this unnecessary with ...

xen/arch/x86/dom0_build.c static long __initdata dom0_max_nrpages = LONG_MAX; · line 104

Quoted 9 lines

xen/arch/x86/dom0_build.c +9 -0

+        }
+        else
+        {
+            /* <size> item must be first one. */
+            if ( items )
+                return -EINVAL;
+            tmp.nr_pages = val >> PAGE_SHIFT;
+        }
+        items++;

... this? However, allowing <frac>%+<size> would apparently have been easy (and add further flexibility).

With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value.

It will be used only if no dom0_mem parameter was specified in the boot parameters.

Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value.

It will be used only if no dom0_mem parameter was specified in the boot parameters.

Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

Quoted 5 lines · Juergen Gross

With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value.

Quoted 5 lines

It will be used only if no dom0_mem parameter was specified in the boot parameters.

Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

Why was this patch accepted?  We've already got a suitable Kconfig option for this, CONFIG_CMDLINE.

Quoted 6 lines · Andrew Cooper

Quoted 5 lines · Juergen Gross

With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value.

Quoted 7 lines

Quoted 5 lines

It will be used only if no dom0_mem parameter was specified in the boot parameters.

Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

Why was this patch accepted?  We've already got a suitable Kconfig option for this, CONFIG_CMDLINE.

Why do we have a config option for the default scheduler?

And something like:

dom0_mem=max:8G dom0_mem=10%

is different from

CONFIG_DOM0_MEM="max:8G" plus dom0_mem=10%

on a host with say 100G of memory: In the first case this would be 8G, in the second case 10G.

Quoted 10 lines · Juergen Gross

Quoted 9 lines · Andrew Cooper

Quoted 8 lines · Juergen Gross

With being able to specify a dom0_mem value depending on host memory size on x86 make it easy for distros to specify a default dom0 size by adding a CONFIG_DOM0_MEM item which presets the dom0_mem boot parameter value.

It will be used only if no dom0_mem parameter was specified in the boot parameters.

Quoted 5 lines

Quoted 4 lines

Quoted 2 lines

Signed-off-by Juergen Gross <[email protected]>

Reviewed-by Jan Beulich <[email protected]>

Why was this patch accepted?  We've already got a suitable Kconfig option for this, CONFIG_CMDLINE.

Why do we have a config option for the default scheduler?

That pre-dates CONFIG_CMDLINE, but is of dubious use.

Quoted 6 lines

And something like:

dom0_mem=max:8G dom0_mem=10%

Quoted 6 lines

is different from

CONFIG_DOM0_MEM="max:8G" plus dom0_mem=10%

on a host with say 100G of memory: In the first case this would be 8G, in the second case 10G.

And?  There is no way of preventing that.

A user can really put two dom0_mem= on the real command line, and could put a dom0_mem in CONFIG_CMDLINE.

The only solution to this problem is for the final dom0_mem= to be a full specification, and I still see no use for CONFIG_DOM0_MEM