[PATCH v3 0/3] xen/x86: support setting dom0_mem depending on host size

[PATCH v3 0/3] xen/x86: support setting dom0_mem depending on host size

[Juergen Gross]

Setting the memory size of dom0 on a server for the non autoballooning
case requires always specification of a boot parameter today. The value
to set will depend mostly on the host memory size.

In order to support that scenario add the possibility to set dom0_mem
depending on the amount of physical memory by allowing to specify a
percentage of host memory (e.g. 10%) with an offset (like 1G+10%).

To make it easy for a distributor to use such a setting as the default
make the standard setting for dom0_mem configurable via Kconfig.

Changes since V2:
- patch 1: let parse_size_and_unit() accept '%' instead of adding a
  new parsing function for that (Jan Beulich)
- patch 2: make dom0mem parsing function more robust against parameter
  format errors (Jan Beulich)

Changes since V1:
- replaced old patch 1 by new one
- rewritten patch 2 according to remarks by Jan Beulich
- changed patch 3 to allow config item on arm, too

Juergen Gross (3):
  xen: modify parse_size_and_unit() to support percentage
  xen/x86: add dom0 memory sizing variants
  xen: add CONFIG item for default dom0 memory size

 docs/misc/xen-command-line.markdown |  19 ++++--
 xen/arch/arm/domain_build.c         |   7 +++
 xen/arch/x86/dom0_build.c           | 118 ++++++++++++++++++++++++++++--------
 xen/common/Kconfig                  |  13 ++++
 xen/common/lib.c                    |   4 ++
 5 files changed, 129 insertions(+), 32 deletions(-)

-- 
2.16.4


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

[PATCH v3 1/3] xen: modify parse_size_and_unit() to support percentage

[Juergen Gross]

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 <jgross@suse.com>
---
 xen/common/lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/common/lib.c b/xen/common/lib.c
index 62330205fe..8ebec811b3 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -476,6 +476,10 @@ unsigned long long parse_size_and_unit(const char *s, const char **ps)
     case 'B': case 'b':
         s1++;
         break;
+    case '%':
+        if ( ps )
+            break;
+        /* fallthrough */
     default:
         ret <<= 10; /* default to kB */
         break;
-- 
2.16.4


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

Re: [PATCH v3 1/3] xen: modify parse_size_and_unit() to support percentage

[Jan Beulich]

>>> On 10.12.18 at 12:44, <jgross@suse.com> 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 <jgross@suse.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

[PATCH v3 2/3] xen/x86: add dom0 memory sizing variants

[Juergen Gross]

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 <jgross@suse.com>
---
 docs/misc/xen-command-line.markdown |  19 ++++--
 xen/arch/x86/dom0_build.c           | 112 +++++++++++++++++++++++++++---------
 2 files changed, 99 insertions(+), 32 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 6f671d3219..4722eede87 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -725,17 +725,17 @@ Set the amount of memory for the initial domain (dom0). It must be
 greater than zero. This parameter is required.
 
 ### dom0\_mem (x86)
-> `= List of ( min:<size> | max:<size> | <size> )`
+> `= List of ( min:<sz> | max:<sz> | <sz> )`
 
 Set the amount of memory for the initial domain (dom0). If a size is
 positive, it represents an absolute value.  If a size is negative, it
 is subtracted from the total available memory.
 
-* `<size>` specifies the exact amount of memory.
-* `min:<size>` specifies the minimum amount of memory.
-* `max:<size>` specifies the maximum amount of memory.
+* `<sz>` specifies the exact amount of memory.
+* `min:<sz>` specifies the minimum amount of memory.
+* `max:<sz>` specifies the maximum amount of memory.
 
-If `<size>` is not specified, the default is all the available memory
+If `<sz>` is not specified, the default is all the available memory
 minus some reserve.  The reserve is 1/16 of the available memory or
 128 MB (whichever is smaller).
 
@@ -743,13 +743,20 @@ The amount of memory will be at least the minimum but never more than
 the maximum (i.e., `max` overrides the `min` option).  If there isn't
 enough memory then as much as possible is allocated.
 
-`max:<size>` also sets the maximum reservation (the maximum amount of
+`max:<sz>` also sets the maximum reservation (the maximum amount of
 memory dom0 can balloon up to).  If this is omitted then the maximum
 reservation is unlimited.
 
 For example, to set dom0's initial memory allocation to 512MB but
 allow it to balloon up as far as 1GB use `dom0_mem=512M,max:1G`
 
+> `<sz>` is: `<size> | [<size>+]<frac>%`
+> `<frac>` is an integer < 100
+
+* `<frac>` specifies a fraction of host memory size in percent.
+
+So `<sz>` being `1G+25%` on a 256 GB host would result in 65 GB.
+
 If you use this option then it is highly recommended that you disable
 any dom0 autoballooning feature present in your toolstack. See the
 _xl.conf(5)_ man page or [Xen Best
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 5e2ad4bd56..673b3ee4e6 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -20,17 +20,42 @@
 #include <asm/p2m.h>
 #include <asm/setup.h>
 
-static long __initdata dom0_nrpages;
-static long __initdata dom0_min_nrpages;
-static long __initdata dom0_max_nrpages = LONG_MAX;
+struct memsize {
+    long nr_pages;
+    unsigned int percent;
+    bool minus;
+};
+
+static struct memsize __initdata dom0_size;
+static struct memsize __initdata dom0_min_size;
+static struct memsize __initdata dom0_max_size = { .nr_pages = LONG_MAX };
+
+static bool __init memsize_gt_zero(const struct memsize *sz)
+{
+    return !sz->minus && sz->nr_pages;
+}
+
+static unsigned long __init get_memsize(const struct memsize *sz,
+                                        unsigned long avail)
+{
+    unsigned long pages;
+
+    pages = sz->nr_pages + sz->percent * avail / 100;
+    return sz->minus ? avail - pages : pages;
+}
 
 /*
  * dom0_mem=[min:<min_amt>,][max:<max_amt>,][<amt>]
- * 
+ *
  * <min_amt>: The minimum amount of memory which should be allocated for dom0.
  * <max_amt>: The maximum amount of memory which should be allocated for dom0.
  * <amt>:     The precise amount of memory to allocate for dom0.
- * 
+ *
+ * The format of <min_amt>, <max_amt> and <amt> is as follows:
+ * <size> | <frac>% | <size>+<frac>%
+ * <size> is a size value like 1G (1 GByte), <frac> is percentage of host
+ * memory (so 1G+10% means 10 percent of host memory + 1 GByte).
+ *
  * Notes:
  *  1. <amt> is clamped from below by <min_amt> and from above by available
  *     memory and <max_amt>
@@ -39,19 +64,59 @@ static long __initdata dom0_max_nrpages = LONG_MAX;
  *  4. If <amt> is not specified, it is calculated as follows:
  *     "All of memory is allocated to domain 0, minus 1/16th which is reserved
  *      for uses such as DMA buffers (the reservation is clamped to 128MB)."
- * 
+ *
  * Each value can be specified as positive or negative:
  *  If +ve: The specified amount is an absolute value.
  *  If -ve: The specified amount is subtracted from total available memory.
  */
-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;
+
+        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. */
+        }
+        else
+        {
+            /* <size> item must be first one. */
+            if ( items )
+                return -EINVAL;
+            tmp.nr_pages = val >> PAGE_SHIFT;
+        }
+        items++;
+    } while ( *s++ == '+' && items < 2 );
+
+    *ps = --s;
+    if ( *s && *s != ',' )
+        return -EINVAL;
+
+    *sz = tmp;
+
+    return 0;
 }
 
 static int __init parse_dom0_mem(const char *s)
 {
+    int ret;
+
     /* xen-shim uses shim_mem parameter instead of dom0_mem */
     if ( pv_shim )
     {
@@ -61,14 +126,14 @@ static int __init parse_dom0_mem(const char *s)
 
     do {
         if ( !strncmp(s, "min:", 4) )
-            dom0_min_nrpages = parse_amt(s+4, &s);
+            ret = parse_amt(s + 4, &s, &dom0_min_size);
         else if ( !strncmp(s, "max:", 4) )
-            dom0_max_nrpages = parse_amt(s+4, &s);
+            ret = parse_amt(s + 4, &s, &dom0_max_size);
         else
-            dom0_nrpages = parse_amt(s, &s);
-    } while ( *s++ == ',' );
+            ret = parse_amt(s, &s, &dom0_size);
+    } while ( *s++ == ',' && !ret );
 
-    return s[-1] ? -EINVAL : 0;
+    return s[-1] ? -EINVAL : ret;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
@@ -298,9 +363,9 @@ unsigned long __init dom0_compute_nr_pages(
         (!iommu_hap_pt_share || !paging_mode_hap(d));
     for ( ; ; need_paging = false )
     {
-        nr_pages = dom0_nrpages;
-        min_pages = dom0_min_nrpages;
-        max_pages = dom0_max_nrpages;
+        nr_pages = get_memsize(&dom0_size, avail);
+        min_pages = get_memsize(&dom0_min_size, avail);
+        max_pages = get_memsize(&dom0_max_size, avail);
 
         /*
          * If allocation isn't specified, reserve 1/16th of available memory
@@ -308,14 +373,9 @@ unsigned long __init dom0_compute_nr_pages(
          * maximum of 128MB.
          */
         if ( !nr_pages )
-            nr_pages = -(pv_shim ? pv_shim_mem(avail)
+            nr_pages = avail - (pv_shim ? pv_shim_mem(avail)
                                  : min(avail / 16, 128UL << (20 - PAGE_SHIFT)));
 
-        /* Negative specification means "all memory - specified amount". */
-        if ( (long)nr_pages  < 0 ) nr_pages  += avail;
-        if ( (long)min_pages < 0 ) min_pages += avail;
-        if ( (long)max_pages < 0 ) max_pages += avail;
-
         /* Clamp according to min/max limits and available memory. */
         nr_pages = max(nr_pages, min_pages);
         nr_pages = min(nr_pages, max_pages);
@@ -329,8 +389,8 @@ unsigned long __init dom0_compute_nr_pages(
     }
 
     if ( is_pv_domain(d) &&
-         (parms->p2m_base == UNSET_ADDR) && (dom0_nrpages <= 0) &&
-         ((dom0_min_nrpages <= 0) || (nr_pages > min_pages)) )
+         (parms->p2m_base == UNSET_ADDR) && !memsize_gt_zero(&dom0_size) &&
+         (!memsize_gt_zero(&dom0_min_size) || (nr_pages > min_pages)) )
     {
         /*
          * Legacy Linux kernels (i.e. such without a XEN_ELFNOTE_INIT_P2M
@@ -356,7 +416,7 @@ unsigned long __init dom0_compute_nr_pages(
         {
             end = sizeof_long >= sizeof(end) ? 0 : 1UL << (8 * sizeof_long);
             nr_pages = (end - vend) / (2 * sizeof_long);
-            if ( dom0_min_nrpages > 0 && nr_pages < min_pages )
+            if ( memsize_gt_zero(&dom0_min_size) && nr_pages < min_pages )
                 nr_pages = min_pages;
             printk("Dom0 memory clipped to %lu pages\n", nr_pages);
         }
-- 
2.16.4


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

Re: [PATCH v3 2/3] xen/x86: add dom0 memory sizing variants

[Jan Beulich]

>>> On 10.12.18 at 12:44, <jgross@suse.com> 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 <jgross@suse.com>

Once again
Reviewed-by: Jan Beulich <jbeulich@suse.com>
yet once again with a remark:

> -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;
> +
> +        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 ...

> +        }
> +        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).

Jan



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

Re: [PATCH v3 2/3] xen/x86: add dom0 memory sizing variants

[Juergen Gross]

On 10/12/2018 15:45, Jan Beulich wrote:
>>>> On 10.12.18 at 12:44, <jgross@suse.com> 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 <jgross@suse.com>
> 
> Once again
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> yet once again with a remark:
> 
>> -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;
>> +
>> +        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 ...
> 
>> +        }
>> +        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).

That would require two booleans instead. Otherwise <frac>%+<frac>%
wouldn't be rejected. In case anyone else thinks this would be better
I can change it, of course.


Juergen

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

[PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[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.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/arm/domain_build.c |  7 +++++++
 xen/arch/x86/dom0_build.c   |  6 ++++++
 xen/common/Kconfig          | 13 +++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b0ec3f0b72..d2c63a89ca 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -32,9 +32,12 @@ static unsigned int __initdata opt_dom0_max_vcpus;
 integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
 
 static u64 __initdata dom0_mem;
+static bool __initdata dom0_mem_set;
 
 static int __init parse_dom0_mem(const char *s)
 {
+    dom0_mem_set = true;
+
     dom0_mem = parse_size_and_unit(s, &s);
 
     return *s ? -EINVAL : 0;
@@ -2114,6 +2117,10 @@ int __init construct_dom0(struct domain *d)
     BUG_ON(d->domain_id != 0);
 
     printk("*** LOADING DOMAIN 0 ***\n");
+
+    if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+        parse_dom0_mem(CONFIG_DOM0_MEM);
+
     if ( dom0_mem <= 0 )
     {
         warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW\n");
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 673b3ee4e6..54737daf6a 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -29,6 +29,7 @@ struct memsize {
 static struct memsize __initdata dom0_size;
 static struct memsize __initdata dom0_min_size;
 static struct memsize __initdata dom0_max_size = { .nr_pages = LONG_MAX };
+static bool __initdata dom0_mem_set;
 
 static bool __init memsize_gt_zero(const struct memsize *sz)
 {
@@ -117,6 +118,8 @@ static int __init parse_dom0_mem(const char *s)
 {
     int ret;
 
+    dom0_mem_set = true;
+
     /* xen-shim uses shim_mem parameter instead of dom0_mem */
     if ( pv_shim )
     {
@@ -339,6 +342,9 @@ unsigned long __init dom0_compute_nr_pages(
     unsigned long avail = 0, nr_pages, min_pages, max_pages;
     bool need_paging;
 
+    if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+        parse_dom0_mem(CONFIG_DOM0_MEM);
+
     for_each_node_mask ( node, dom0_nodes )
         avail += avail_domheap_pages_region(node, 0, 0) +
                  initial_images_nrpages(node);
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 68132a3a10..155a9a45e8 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -323,4 +323,17 @@ config CMDLINE_OVERRIDE
 
 	  This is used to work around broken bootloaders. This should
 	  be set to 'N' under normal conditions.
+
+config DOM0_MEM
+	string "Default value for dom0_mem boot parameter"
+	default ""
+	---help---
+	  Sets a default value for dom0_mem, e.g. "512M".
+	  The specified string will be used for the dom0_mem parameter in
+	  case it was not specified on the command line.
+
+	  See docs/misc/xen-command-line.markdown for the supported syntax.
+
+	  Leave empty if you are not sure what to specify.
+	
 endmenu
-- 
2.16.4


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

Re: [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[Julien Grall]

Hi Juergen,

On 12/10/18 11:44 AM, Juergen Gross wrote:
> 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 <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Julien Grall <julien.grall@arm.com>

It looks like the rest of the series has been merged. So I will commit 
that one as all the tags required should be there.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[Andrew Cooper]

On 10/12/2018 11:44, Juergen Gross wrote:
> 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 <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

~Andrew

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

Re: [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[Juergen Gross]

On 18/01/2019 14:13, Andrew Cooper wrote:
> On 10/12/2018 11:44, Juergen Gross wrote:
>> 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 <jgross@suse.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> 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.


Juergen

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

Re: [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[Andrew Cooper]

On 18/01/2019 13:19, Juergen Gross wrote:
> On 18/01/2019 14:13, Andrew Cooper wrote:
>> On 10/12/2018 11:44, Juergen Gross wrote:
>>> 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 <jgross@suse.com>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> 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.

>
> 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.

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

~Andrew

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

Re: [PATCH v3 3/3] xen: add CONFIG item for default dom0 memory size

[Juergen Gross]

On 18/01/2019 14:44, Andrew Cooper wrote:
> On 18/01/2019 13:19, Juergen Gross wrote:
>> On 18/01/2019 14:13, Andrew Cooper wrote:
>>> On 10/12/2018 11:44, Juergen Gross wrote:
>>>> 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 <jgross@suse.com>
>>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>> 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.

I don't agree with you here. I think this is a very sensible choice.

> 
>>
>> 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.
> 
> And?  There is no way of preventing that.

I wanted to say: using CONFIG_DOM0_MEM and CONFIG_CMDLINE are different.
The former is evaluated only if the user doesn't specify dom0_mem as a
parameter, while the latter _is_ cumulative.

The advantage of CONFIG_DOM0_MEM is that is really only the default, not
an additional setting the user has to take into acount.

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

Right, and then he'd have three cumulative settings.

> 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

So a user specifying dom0_mem in a 4.11 setup would eventually need to
modify his setting. With CONFIG_DOM0_MEM he doesn't have to do that.

Additionally CONFIG_CMDLINE is available in expert mode only, while
CONFIG_DOM0_MEM can be set more easily.


Juergen

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