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 07:52 +0100, Juergen Gross wrote:
> 64 bit pv-domains under Xen are limited to 512 GB of RAM today. The
> main reason has been the 3 level p2m tree, which was replaced by the
> virtual mapped linear p2m list. Parallel to the p2m list which is
> being used by the kernel itself there is a 3 level mfn tree for usage
> by the Xen tools and eventually for crash dump analysis. For this tree
> the linear p2m list can serve as a replacement, too. As the kernel
> can't know whether the tools are capable of dealing with the p2m list
> instead of the mfn tree, the limit of 512 GB can't be dropped in all
> cases.
> 
> This patch replaces the hard limit by a kernel parameter which tells
> the kernel to obey the 512 GB limit or not. The default is selected by
> a configuration parameter which specifies whether the 512 GB limit
> should be active per default for dom0 (only crash dump analysis is
> affected) and/or for domUs (additionally domain save/restore/migration
> are affected).
> 
> Memory above the domain limit is returned to the hypervisor instead of
> being identity mapped, which was wrong anyways.
> 
> The kernel configuration parameter to specify the maximum size of a
> domain can be deleted, as it is not relevant any more.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  Documentation/kernel-parameters.txt |  7 ++++
>  arch/x86/include/asm/xen/page.h     |  4 ---
>  arch/x86/xen/Kconfig                | 31 +++++++++++-----
>  arch/x86/xen/p2m.c                  | 10 +++---
>  arch/x86/xen/setup.c                | 72 ++++++++++++++++++++++++++++++-------
>  5 files changed, 93 insertions(+), 31 deletions(-)

[...]

> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -23,14 +23,29 @@ config XEN_PVHVM
>  	def_bool y
>  	depends on XEN && PCI && X86_LOCAL_APIC
>  
> -config XEN_MAX_DOMAIN_MEMORY
> -       int
> -       default 500 if X86_64
> -       default 64 if X86_32
> -       depends on XEN
> -       help
> -         This only affects the sizing of some bss arrays, the unused
> -         portions of which are freed.
> +if X86_64

Not
    && XEN
?

> +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?
Because ...

> +endif
>  
>  config XEN_SAVE_RESTORE
>         bool

[...]
 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 84a6473..16d94de 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -32,6 +32,8 @@
>  #include "p2m.h"
>  #include "mmu.h"
>  
> +#define GB(x) ((uint64_t)(x) * 1024 * 1024 * 1024)
> +
>  /* Amount of extra memory space we add to the e820 ranges */
>  struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>  
> @@ -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?

> +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?

Thanks,


Paul Bolle