| Message ID | 1487614926-5383-1-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 20, 2017 at 10:22:06AM -0800, Stefano Stabellini wrote: > The default dom0_mem is 128M which is not sufficient to boot a Ubuntu > based Dom0. It is not clear what a better default value could be. > > Instead, loudly warn the user when dom0_mem is unspecified and wait 3 > secs. Then use 512M. > > Update the docs to specify that dom0_mem is required on ARM. (The > current xen-command-line document does not actually reflect the current > behavior of dom0_mem on ARM correctly.) > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v3: > - update docs > --- > docs/misc/xen-command-line.markdown | 8 +++++++- > xen/arch/arm/domain_build.c | 22 +++++++++++++++++----- > 2 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > index a11fdf9..c0106fb 100644 > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -603,7 +603,13 @@ For example, with `dom0_max_vcpus=4-8`: > > 8 | 8 > > 10 | 8 > > -### dom0\_mem > +### dom0\_mem (ARM) > +> `= <size>` > + > +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> )` > > Set the amount of memory for the initial domain (dom0). If a size is > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index c97a1f5..db7471f 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1,4 +1,5 @@ > #include <xen/config.h> > +#include <xen/delay.h> > #include <xen/init.h> > #include <xen/compile.h> > #include <xen/lib.h> > @@ -31,14 +32,11 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); > > int dom0_11_mapping = 1; > > -#define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ > -static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; > +static u64 __initdata dom0_mem; > > static void __init parse_dom0_mem(const char *s) > { > dom0_mem = parse_size_and_unit(s, &s); > - if ( dom0_mem == 0 ) > - dom0_mem = DOM0_MEM_DEFAULT; > } > custom_param("dom0_mem", parse_dom0_mem); > > @@ -2125,7 +2123,21 @@ int construct_dom0(struct domain *d) > BUG_ON(d->vcpu[0] == NULL); > BUG_ON(v->is_initialised); > > - printk("*** LOADING DOMAIN 0 ***\n"); > + if ( dom0_mem > 0 ) > + printk("*** LOADING DOMAIN 0 ***\n"); > + else > + { > + int i; > + printk("*** PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW ***\n"); > + for (i = 0; i < 3; i++) > + { > + printk("%d...", i+1); > + mdelay(1000); There is infrastructure in Xen to print warning messages, so that we don't add delays unnecessarily. See warning.h:warning_add. Wei. > + } > + printk("\n"); > + dom0_mem = MB(512); > + } > + > > iommu_hwdom_init(d); > > -- > 1.9.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
On Tue, 21 Feb 2017, Wei Liu wrote: > On Mon, Feb 20, 2017 at 10:22:06AM -0800, Stefano Stabellini wrote: > > The default dom0_mem is 128M which is not sufficient to boot a Ubuntu > > based Dom0. It is not clear what a better default value could be. > > > > Instead, loudly warn the user when dom0_mem is unspecified and wait 3 > > secs. Then use 512M. > > > > Update the docs to specify that dom0_mem is required on ARM. (The > > current xen-command-line document does not actually reflect the current > > behavior of dom0_mem on ARM correctly.) > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > Changes in v3: > > - update docs > > --- > > docs/misc/xen-command-line.markdown | 8 +++++++- > > xen/arch/arm/domain_build.c | 22 +++++++++++++++++----- > > 2 files changed, 24 insertions(+), 6 deletions(-) > > > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > > index a11fdf9..c0106fb 100644 > > --- a/docs/misc/xen-command-line.markdown > > +++ b/docs/misc/xen-command-line.markdown > > @@ -603,7 +603,13 @@ For example, with `dom0_max_vcpus=4-8`: > > > 8 | 8 > > > 10 | 8 > > > > -### dom0\_mem > > +### dom0\_mem (ARM) > > +> `= <size>` > > + > > +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> )` > > > > Set the amount of memory for the initial domain (dom0). If a size is > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index c97a1f5..db7471f 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -1,4 +1,5 @@ > > #include <xen/config.h> > > +#include <xen/delay.h> > > #include <xen/init.h> > > #include <xen/compile.h> > > #include <xen/lib.h> > > @@ -31,14 +32,11 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); > > > > int dom0_11_mapping = 1; > > > > -#define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ > > -static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; > > +static u64 __initdata dom0_mem; > > > > static void __init parse_dom0_mem(const char *s) > > { > > dom0_mem = parse_size_and_unit(s, &s); > > - if ( dom0_mem == 0 ) > > - dom0_mem = DOM0_MEM_DEFAULT; > > } > > custom_param("dom0_mem", parse_dom0_mem); > > > > @@ -2125,7 +2123,21 @@ int construct_dom0(struct domain *d) > > BUG_ON(d->vcpu[0] == NULL); > > BUG_ON(v->is_initialised); > > > > - printk("*** LOADING DOMAIN 0 ***\n"); > > + if ( dom0_mem > 0 ) > > + printk("*** LOADING DOMAIN 0 ***\n"); > > + else > > + { > > + int i; > > + printk("*** PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW ***\n"); > > + for (i = 0; i < 3; i++) > > + { > > + printk("%d...", i+1); > > + mdelay(1000); > > There is infrastructure in Xen to print warning messages, so that we > don't add delays unnecessarily. > > See warning.h:warning_add. I added the delay on purpose to force the user to specify dom0_mem, as we cannot find a way to set a sensible default. The alternative is to panic. > > + } > > + printk("\n"); > > + dom0_mem = MB(512); > > + } > > + > > > > iommu_hwdom_init(d); > > > > -- > > 1.9.1 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > https://lists.xen.org/xen-devel >
Hi Stefano, On 21/02/17 17:26, Stefano Stabellini wrote: > On Tue, 21 Feb 2017, Wei Liu wrote: >> On Mon, Feb 20, 2017 at 10:22:06AM -0800, Stefano Stabellini wrote: >>> The default dom0_mem is 128M which is not sufficient to boot a Ubuntu >>> based Dom0. It is not clear what a better default value could be. >>> >>> Instead, loudly warn the user when dom0_mem is unspecified and wait 3 >>> secs. Then use 512M. >>> >>> Update the docs to specify that dom0_mem is required on ARM. (The >>> current xen-command-line document does not actually reflect the current >>> behavior of dom0_mem on ARM correctly.) >>> >>> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> >>> --- >>> Changes in v3: >>> - update docs >>> --- >>> docs/misc/xen-command-line.markdown | 8 +++++++- >>> xen/arch/arm/domain_build.c | 22 +++++++++++++++++----- >>> 2 files changed, 24 insertions(+), 6 deletions(-) >>> >>> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown >>> index a11fdf9..c0106fb 100644 >>> --- a/docs/misc/xen-command-line.markdown >>> +++ b/docs/misc/xen-command-line.markdown >>> @@ -603,7 +603,13 @@ For example, with `dom0_max_vcpus=4-8`: >>> > 8 | 8 >>> > 10 | 8 >>> >>> -### dom0\_mem >>> +### dom0\_mem (ARM) >>> +> `= <size>` >>> + >>> +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> )` >>> >>> Set the amount of memory for the initial domain (dom0). If a size is >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>> index c97a1f5..db7471f 100644 >>> --- a/xen/arch/arm/domain_build.c >>> +++ b/xen/arch/arm/domain_build.c >>> @@ -1,4 +1,5 @@ >>> #include <xen/config.h> >>> +#include <xen/delay.h> >>> #include <xen/init.h> >>> #include <xen/compile.h> >>> #include <xen/lib.h> >>> @@ -31,14 +32,11 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); >>> >>> int dom0_11_mapping = 1; >>> >>> -#define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ >>> -static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; >>> +static u64 __initdata dom0_mem; >>> >>> static void __init parse_dom0_mem(const char *s) >>> { >>> dom0_mem = parse_size_and_unit(s, &s); >>> - if ( dom0_mem == 0 ) >>> - dom0_mem = DOM0_MEM_DEFAULT; >>> } >>> custom_param("dom0_mem", parse_dom0_mem); >>> >>> @@ -2125,7 +2123,21 @@ int construct_dom0(struct domain *d) >>> BUG_ON(d->vcpu[0] == NULL); >>> BUG_ON(v->is_initialised); >>> >>> - printk("*** LOADING DOMAIN 0 ***\n"); >>> + if ( dom0_mem > 0 ) >>> + printk("*** LOADING DOMAIN 0 ***\n"); >>> + else >>> + { >>> + int i; >>> + printk("*** PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW ***\n"); >>> + for (i = 0; i < 3; i++) >>> + { >>> + printk("%d...", i+1); >>> + mdelay(1000); >> >> There is infrastructure in Xen to print warning messages, so that we >> don't add delays unnecessarily. >> >> See warning.h:warning_add. > > I added the delay on purpose to force the user to specify dom0_mem, as > we cannot find a way to set a sensible default. The alternative is to > panic. warning_add will gather the warning message and then will be printed at the end of the boot with a delay. This is what is used for sync_console message and also FEP on x86. I think this is a good idea to gather all the warnings and print them at the end. At least it is not lost in the noise. Cheers,
On Tue, 21 Feb 2017, Julien Grall wrote: > Hi Stefano, > > On 21/02/17 17:26, Stefano Stabellini wrote: > > On Tue, 21 Feb 2017, Wei Liu wrote: > > > On Mon, Feb 20, 2017 at 10:22:06AM -0800, Stefano Stabellini wrote: > > > > The default dom0_mem is 128M which is not sufficient to boot a Ubuntu > > > > based Dom0. It is not clear what a better default value could be. > > > > > > > > Instead, loudly warn the user when dom0_mem is unspecified and wait 3 > > > > secs. Then use 512M. > > > > > > > > Update the docs to specify that dom0_mem is required on ARM. (The > > > > current xen-command-line document does not actually reflect the current > > > > behavior of dom0_mem on ARM correctly.) > > > > > > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> > > > > --- > > > > Changes in v3: > > > > - update docs > > > > --- > > > > docs/misc/xen-command-line.markdown | 8 +++++++- > > > > xen/arch/arm/domain_build.c | 22 +++++++++++++++++----- > > > > 2 files changed, 24 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/docs/misc/xen-command-line.markdown > > > > b/docs/misc/xen-command-line.markdown > > > > index a11fdf9..c0106fb 100644 > > > > --- a/docs/misc/xen-command-line.markdown > > > > +++ b/docs/misc/xen-command-line.markdown > > > > @@ -603,7 +603,13 @@ For example, with `dom0_max_vcpus=4-8`: > > > > > 8 | 8 > > > > > 10 | 8 > > > > > > > > -### dom0\_mem > > > > +### dom0\_mem (ARM) > > > > +> `= <size>` > > > > + > > > > +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> )` > > > > > > > > Set the amount of memory for the initial domain (dom0). If a size is > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > > > index c97a1f5..db7471f 100644 > > > > --- a/xen/arch/arm/domain_build.c > > > > +++ b/xen/arch/arm/domain_build.c > > > > @@ -1,4 +1,5 @@ > > > > #include <xen/config.h> > > > > +#include <xen/delay.h> > > > > #include <xen/init.h> > > > > #include <xen/compile.h> > > > > #include <xen/lib.h> > > > > @@ -31,14 +32,11 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); > > > > > > > > int dom0_11_mapping = 1; > > > > > > > > -#define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ > > > > -static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; > > > > +static u64 __initdata dom0_mem; > > > > > > > > static void __init parse_dom0_mem(const char *s) > > > > { > > > > dom0_mem = parse_size_and_unit(s, &s); > > > > - if ( dom0_mem == 0 ) > > > > - dom0_mem = DOM0_MEM_DEFAULT; > > > > } > > > > custom_param("dom0_mem", parse_dom0_mem); > > > > > > > > @@ -2125,7 +2123,21 @@ int construct_dom0(struct domain *d) > > > > BUG_ON(d->vcpu[0] == NULL); > > > > BUG_ON(v->is_initialised); > > > > > > > > - printk("*** LOADING DOMAIN 0 ***\n"); > > > > + if ( dom0_mem > 0 ) > > > > + printk("*** LOADING DOMAIN 0 ***\n"); > > > > + else > > > > + { > > > > + int i; > > > > + printk("*** PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR > > > > NOW ***\n"); > > > > + for (i = 0; i < 3; i++) > > > > + { > > > > + printk("%d...", i+1); > > > > + mdelay(1000); > > > > > > There is infrastructure in Xen to print warning messages, so that we > > > don't add delays unnecessarily. > > > > > > See warning.h:warning_add. > > > > I added the delay on purpose to force the user to specify dom0_mem, as > > we cannot find a way to set a sensible default. The alternative is to > > panic. > > warning_add will gather the warning message and then will be printed at the > end of the boot with a delay. This is what is used for sync_console message > and also FEP on x86. > > I think this is a good idea to gather all the warnings and print them at the > end. At least it is not lost in the noise. Yes, warning_add works very well actually. I'll use it.
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index a11fdf9..c0106fb 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -603,7 +603,13 @@ For example, with `dom0_max_vcpus=4-8`: > 8 | 8 > 10 | 8 -### dom0\_mem +### dom0\_mem (ARM) +> `= <size>` + +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> )` Set the amount of memory for the initial domain (dom0). If a size is diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index c97a1f5..db7471f 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1,4 +1,5 @@ #include <xen/config.h> +#include <xen/delay.h> #include <xen/init.h> #include <xen/compile.h> #include <xen/lib.h> @@ -31,14 +32,11 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); int dom0_11_mapping = 1; -#define DOM0_MEM_DEFAULT 0x8000000 /* 128 MiB */ -static u64 __initdata dom0_mem = DOM0_MEM_DEFAULT; +static u64 __initdata dom0_mem; static void __init parse_dom0_mem(const char *s) { dom0_mem = parse_size_and_unit(s, &s); - if ( dom0_mem == 0 ) - dom0_mem = DOM0_MEM_DEFAULT; } custom_param("dom0_mem", parse_dom0_mem); @@ -2125,7 +2123,21 @@ int construct_dom0(struct domain *d) BUG_ON(d->vcpu[0] == NULL); BUG_ON(v->is_initialised); - printk("*** LOADING DOMAIN 0 ***\n"); + if ( dom0_mem > 0 ) + printk("*** LOADING DOMAIN 0 ***\n"); + else + { + int i; + printk("*** PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR NOW ***\n"); + for (i = 0; i < 3; i++) + { + printk("%d...", i+1); + mdelay(1000); + } + printk("\n"); + dom0_mem = MB(512); + } + iommu_hwdom_init(d);
The default dom0_mem is 128M which is not sufficient to boot a Ubuntu based Dom0. It is not clear what a better default value could be. Instead, loudly warn the user when dom0_mem is unspecified and wait 3 secs. Then use 512M. Update the docs to specify that dom0_mem is required on ARM. (The current xen-command-line document does not actually reflect the current behavior of dom0_mem on ARM correctly.) Signed-off-by: Stefano Stabellini <sstabellini@kernel.org> --- Changes in v3: - update docs --- docs/misc/xen-command-line.markdown | 8 +++++++- xen/arch/arm/domain_build.c | 22 +++++++++++++++++----- 2 files changed, 24 insertions(+), 6 deletions(-)