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

[Juergen Gross <jgross@suse.com>]

On 02/18/2015 10:21 AM, Paul Bolle wrote:
> 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
> ?

The complete directory is made only if CONFIG_XEN is set.

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

Yes.

That's a matter of taste, I think.

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

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


Juergen