Re: [PATCH 13/13] xen: allow more than 512 GB of RAM for 64 bit pv-domains

[Paul Bolle <pebolle@tiscali.nl>]

On Wed, 2015-02-18 at 10:37 +0100, Juergen Gross wrote:
> On 02/18/2015 10:21 AM, Paul Bolle wrote:
> > On Wed, 2015-02-18 at 07:52 +0100, Juergen Gross wrote:
> >> +choice
> >> +	prompt "Support pv-domains larger than 512GB"
> >> +	default XEN_512GB_NONE
> >> +	help
> >> +	  Support paravirtualized domains with more than 512GB of RAM.
> >> +
> >> +	  The Xen tools and crash dump analysis tools might not support
> >> +	  pv-domains with more than 512 GB of RAM. This option controls the
> >> +	  default setting of the kernel to use only up to 512 GB or more.
> >> +	  It is always possible to change the default via specifying the
> >> +	  boot parameter "xen_512gb_limit".
> >> +
> >> +	config XEN_512GB_NONE
> >> +		bool "neither dom0 nor domUs can be larger than 512GB"
> >> +	config XEN_512GB_DOM0
> >> +		bool "dom0 can be larger than 512GB, domUs not"
> >> +	config XEN_512GB_DOMU
> >> +		bool "domUs can be larger than 512GB, dom0 not"
> >> +	config XEN_512GB_ALL
> >> +		bool "dom0 and domUs can be larger than 512GB"
> >> +endchoice
> >
> > So there are actually two independent limits, configured through a
> > choice with four entries. Would using just two separate Kconfig symbols
> > (XEN_512GB_DOM0 and XEN_512GB_DOMU) without a choice wrapper also work?
> 
> Yes.
> 
> > Because ...
> >
> >> +endif

[...]

> >> @@ -85,6 +87,27 @@ static struct {
> >>    */
> >>   #define EXTRA_MEM_RATIO		(10)
> >>
> >> +static bool xen_dom0_512gb_limit __initdata =
> >> +	IS_ENABLED(CONFIG_XEN_512GB_NONE) || IS_ENABLED(CONFIG_XEN_512GB_DOMU);
> >
> > ... then this could be something like:
> >      static bool xen_dom0_512gb_limit __initdata = !IS_ENABLED(CONFIG_XEN_512GB_DOM0);
> >
> >> +static bool xen_domu_512gb_limit __initdata =
> >> +	IS_ENABLED(CONFIG_XEN_512GB_NONE) || IS_ENABLED(CONFIG_XEN_512GB_DOM0);
> >> +
> >
> > and this likewise:
> >      static bool xen_domu_512gb_limit __initdata = !IS_ENABLED(CONFIG_XEN_512GB_DOMU);
> >
> > Correct?
> 
> Yes.
> 
> That's a matter of taste, I think.

Well, my suggestion does look simpler. Anyhow, I'll be glad to let the
maintainers decide.

> >
> >> +static int __init xen_parse_512gb(char *arg)
> >> +{
> >> +	bool val = false;
> >> +
> >> +	if (!arg)
> >> +		val = true;
> >> +	else if (strtobool(arg, &val))
> >> +		return 1;
> >> +
> >> +	xen_dom0_512gb_limit = val;
> >> +	xen_domu_512gb_limit = val;
> >> +
> >> +	return 0;
> >> +}
> >> +early_param("xen_512gb_limit", xen_parse_512gb);
> >> +
> >>   static void __init xen_add_extra_mem(phys_addr_t start, phys_addr_t size)
> >>   {
> >>   	int i;
> >
> > So one can configure these two limits separately, but the kernel
> > parameter is used for both. Any particular reason?
> 
> Yes. A kernel is running only either as Dom0 or as domU at a given time.
> Having two parameters here would be nonsense, as only one could apply.

I see.

> And being able to configure both limits separately does make sense,
> of course.

Thanks,


Paul Bolle