[PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Masayoshi Mizuma]

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

If each node of physical memory layout has huge space for hotplug,
the padding used for the physical memory mapping section is not enough.
For exapmle of the layout:
  SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
  SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
  SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
  SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug

We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
however, the needed padding size depends on the system environment.
The kernel option is better than changing the config.

Change log from v2:
 - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
   but a little different. Remove SGI UV description.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/mm/kaslr.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 61db77b0eda9..aeb62615bd46 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -40,6 +40,7 @@
  */
 static const unsigned long vaddr_end = CPU_ENTRY_AREA_BASE;
 
+static int __initdata rand_mem_physical_padding = CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
 /*
  * Memory regions randomized by KASLR (except modules that use a separate logic
  * earlier during boot). The list is ordered based on virtual addresses. This
@@ -69,6 +70,20 @@ static inline bool kaslr_memory_enabled(void)
 	return kaslr_enabled() && !IS_ENABLED(CONFIG_KASAN);
 }
 
+static int __init rand_mem_physical_padding_setup(char *str)
+{
+	int max_padding = (1 << (MAX_PHYSMEM_BITS - TB_SHIFT)) - 1;
+
+	get_option(&str, &rand_mem_physical_padding);
+	if (rand_mem_physical_padding < 0)
+		rand_mem_physical_padding = 0;
+	else if (rand_mem_physical_padding > max_padding)
+		rand_mem_physical_padding = max_padding;
+
+	return 0;
+}
+early_param("rand_mem_physical_padding", rand_mem_physical_padding_setup);
+
 /* Initialize base and padding for each memory region randomized with KASLR */
 void __init kernel_randomize_memory(void)
 {
@@ -102,7 +117,7 @@ void __init kernel_randomize_memory(void)
 	 */
 	BUG_ON(kaslr_regions[0].base != &page_offset_base);
 	memory_tb = DIV_ROUND_UP(max_pfn << PAGE_SHIFT, 1UL << TB_SHIFT) +
-		CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
+		rand_mem_physical_padding;
 
 	/* Adapt phyiscal memory region size based on available memory */
 	if (memory_tb < kaslr_regions[0].size_tb)
-- 
2.18.0

[PATCH v3 2/2] docs: kernel-parameters.txt: document rand_mem_physical_padding parameter

[Masayoshi Mizuma]

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

This kernel parameter allows to change the padding used for the physical
memory mapping section when KASLR memory is enabled.

For some systems, the default value, CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
is not enough. The option is useful to adjust the padding size to work
around such issue.

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0c883029881a..97f54c910c9c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3477,6 +3477,13 @@
 	ramdisk_size=	[RAM] Sizes of RAM disks in kilobytes
 			See Documentation/blockdev/ramdisk.txt.
 
+	rand_mem_physical_padding=
+			[KNL] Define the padding size in terabytes
+			used for the physical memory mapping section
+			when KASLR memory is enabled.
+			The default value is
+			CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING.
+
 	ras=option[,option,...]	[KNL] RAS-specific options
 
 		cec_disable	[X86]
-- 
2.18.0

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Masayoshi Mizuma]

Ping...
I would appreciate if someone could review it because this patch
fixes the real memory hotplug issue...

On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> 
> If each node of physical memory layout has huge space for hotplug,
> the padding used for the physical memory mapping section is not enough.
> For exapmle of the layout:
>   SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
>   SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
>   SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
>   SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> 
> We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> however, the needed padding size depends on the system environment.
> The kernel option is better than changing the config.
> 
> Change log from v2:
>  - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
>    but a little different. Remove SGI UV description.
> 
> Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> Reviewed-by: Baoquan He <bhe@redhat.com>
> ---
>  arch/x86/mm/kaslr.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 61db77b0eda9..aeb62615bd46 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -40,6 +40,7 @@
>   */
>  static const unsigned long vaddr_end = CPU_ENTRY_AREA_BASE;
>  
> +static int __initdata rand_mem_physical_padding = CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
>  /*
>   * Memory regions randomized by KASLR (except modules that use a separate logic
>   * earlier during boot). The list is ordered based on virtual addresses. This
> @@ -69,6 +70,20 @@ static inline bool kaslr_memory_enabled(void)
>  	return kaslr_enabled() && !IS_ENABLED(CONFIG_KASAN);
>  }
>  
> +static int __init rand_mem_physical_padding_setup(char *str)
> +{
> +	int max_padding = (1 << (MAX_PHYSMEM_BITS - TB_SHIFT)) - 1;
> +
> +	get_option(&str, &rand_mem_physical_padding);
> +	if (rand_mem_physical_padding < 0)
> +		rand_mem_physical_padding = 0;
> +	else if (rand_mem_physical_padding > max_padding)
> +		rand_mem_physical_padding = max_padding;
> +
> +	return 0;
> +}
> +early_param("rand_mem_physical_padding", rand_mem_physical_padding_setup);
> +
>  /* Initialize base and padding for each memory region randomized with KASLR */
>  void __init kernel_randomize_memory(void)
>  {
> @@ -102,7 +117,7 @@ void __init kernel_randomize_memory(void)
>  	 */
>  	BUG_ON(kaslr_regions[0].base != &page_offset_base);
>  	memory_tb = DIV_ROUND_UP(max_pfn << PAGE_SHIFT, 1UL << TB_SHIFT) +
> -		CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
> +		rand_mem_physical_padding;
>  
>  	/* Adapt phyiscal memory region size based on available memory */
>  	if (memory_tb < kaslr_regions[0].size_tb)
> -- 
> 2.18.0
> 

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Ingo Molnar]

* Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:

> Ping...
> I would appreciate if someone could review it because this patch
> fixes the real memory hotplug issue...

Yeah, so I generally try to resist random new boot options that
work around real bugs, so please convince me that this patch
is the best option:

> 
> On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > 
> > If each node of physical memory layout has huge space for hotplug,
> > the padding used for the physical memory mapping section is not enough.
> > For exapmle of the layout:
> >   SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
> >   SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
> >   SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
> >   SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> > 
> > We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> > however, the needed padding size depends on the system environment.
> > The kernel option is better than changing the config.
> > 
> > Change log from v2:
> >  - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
> >    but a little different. Remove SGI UV description.

Could you please explain it a bit better where the higher padding requirement comes from?

'system environment' is very opaque.

Thanks,

	Ingo

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Thomas Gleixner]

On Wed, 19 Sep 2018, Ingo Molnar wrote:
> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
> 
> > Ping...
> > I would appreciate if someone could review it because this patch
> > fixes the real memory hotplug issue...
> 
> Yeah, so I generally try to resist random new boot options that
> work around real bugs, so please convince me that this patch
> is the best option:
> 
> > 
> > On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> > > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > > 
> > > If each node of physical memory layout has huge space for hotplug,
> > > the padding used for the physical memory mapping section is not enough.
> > > For exapmle of the layout:
> > >   SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
> > >   SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
> > >   SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
> > >   SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> > > 
> > > We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> > > however, the needed padding size depends on the system environment.
> > > The kernel option is better than changing the config.
> > > 
> > > Change log from v2:
> > >  - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
> > >    but a little different. Remove SGI UV description.
> 
> Could you please explain it a bit better where the higher padding requirement comes from?
> 
> 'system environment' is very opaque.

As I understand it, it's depending on the actual physical characteristics
of the machine. So setting a fixed value in Kconfig might work for one, but
not for others and having a command line option allows to tweak that at
boot time and having a common kernel image.

Ideally we would calculate that from SRAT, but AFAICT SRAT is not available
at the point where this needs to be done.

Thanks,

	tglx

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Wed, 19 Sep 2018, Ingo Molnar wrote:
> > * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
> > 
> > > Ping...
> > > I would appreciate if someone could review it because this patch
> > > fixes the real memory hotplug issue...
> > 
> > Yeah, so I generally try to resist random new boot options that
> > work around real bugs, so please convince me that this patch
> > is the best option:
> > 
> > > 
> > > On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> > > > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > > > 
> > > > If each node of physical memory layout has huge space for hotplug,
> > > > the padding used for the physical memory mapping section is not enough.
> > > > For exapmle of the layout:
> > > >   SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
> > > >   SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
> > > >   SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
> > > >   SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> > > > 
> > > > We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> > > > however, the needed padding size depends on the system environment.
> > > > The kernel option is better than changing the config.
> > > > 
> > > > Change log from v2:
> > > >  - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
> > > >    but a little different. Remove SGI UV description.
> > 
> > Could you please explain it a bit better where the higher padding requirement comes from?
> > 
> > 'system environment' is very opaque.
> 
> As I understand it, it's depending on the actual physical characteristics
> of the machine. So setting a fixed value in Kconfig might work for one, but
> not for others and having a command line option allows to tweak that at
> boot time and having a common kernel image.
> 
> Ideally we would calculate that from SRAT, but AFAICT SRAT is not available
> at the point where this needs to be done.

Yeah, so could we at least do something like this:

 - See whether using the maximum padding as the new default padding would work for everyone?
   A bit more virtual memory used, or are there other costs as well?

 - Add checking code to the later SRAT case to at least _detect_ bad padding after the fact.
   We don't utilize RAM with bad padding until that, right?

 - Add 'quirk' to the name of the boot parameter, to make it clear that this is really due to 
   suboptimal communication between the firmware and the kernel.

Hm?

Thanks,

	Ingo

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Masayoshi Mizuma]

On Wed, Sep 19, 2018 at 02:48:06PM +0200, Ingo Molnar wrote:
> 
> * Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Wed, 19 Sep 2018, Ingo Molnar wrote:
> > > * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
> > > 
> > > > Ping...
> > > > I would appreciate if someone could review it because this patch
> > > > fixes the real memory hotplug issue...
> > > 
> > > Yeah, so I generally try to resist random new boot options that
> > > work around real bugs, so please convince me that this patch
> > > is the best option:
> > > 
> > > > 
> > > > On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> > > > > From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> > > > > 
> > > > > If each node of physical memory layout has huge space for hotplug,
> > > > > the padding used for the physical memory mapping section is not enough.
> > > > > For exapmle of the layout:
> > > > >   SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
> > > > >   SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
> > > > >   SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
> > > > >   SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> > > > > 
> > > > > We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> > > > > however, the needed padding size depends on the system environment.
> > > > > The kernel option is better than changing the config.
> > > > > 
> > > > > Change log from v2:
> > > > >  - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
> > > > >    but a little different. Remove SGI UV description.
> > > 
> > > Could you please explain it a bit better where the higher padding requirement comes from?
> > > 
> > > 'system environment' is very opaque.
> > 
> > As I understand it, it's depending on the actual physical characteristics
> > of the machine. So setting a fixed value in Kconfig might work for one, but
> > not for others and having a command line option allows to tweak that at
> > boot time and having a common kernel image.
> > 
> > Ideally we would calculate that from SRAT, but AFAICT SRAT is not available
> > at the point where this needs to be done.

Yes, that's right. The KASLR initialization is early boot sequence,
so SRAT is not available at that time.

> 
> Yeah, so could we at least do something like this:
> 
>  - See whether using the maximum padding as the new default padding would work for everyone?
>    A bit more virtual memory used, or are there other costs as well?

The current default padding size if CONFIG_MEMORY_HOTPLUG set is 10TB.
IMO, it should not be increased because it gets the available entropy
decreased...

>  - Add checking code to the later SRAT case to at least _detect_ bad padding after the fact.
>    We don't utilize RAM with bad padding until that, right?

I have an idea as following. Does that make sense?

Add a warning message which shows the padding size is not enough
for the physical memory mapping and tell to the user about
recommended padding size. User can change the padding size in next
reboot to add the boot parameter.

> 
>  - Add 'quirk' to the name of the boot parameter, to make it clear that this is really due to 
>    suboptimal communication between the firmware and the kernel.

I'm ok if 'quirk' is added to the boot parameter.

Thanks,
Masa

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Travis, Mike]

On 9/19/2018 7:10 AM, Masayoshi Mizuma wrote:
> On Wed, Sep 19, 2018 at 02:48:06PM +0200, Ingo Molnar wrote:
>>
>> * Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>>> On Wed, 19 Sep 2018, Ingo Molnar wrote:
>>>> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
>>>>
>>>>> Ping...
>>>>> I would appreciate if someone could review it because this patch
>>>>> fixes the real memory hotplug issue...
>>>>
>>>> Yeah, so I generally try to resist random new boot options that
>>>> work around real bugs, so please convince me that this patch
>>>> is the best option:

I whole hardily concur, that having boot options which are not easily 
understood should be avoided.  The very best is the system should just 
work.  But on very large systems, these boot options are typically 
determined by either automated scripts, or careful instructions to the 
trained onsite customer engineers, who are required to "get it right".

>>>>
>>>>>
>>>>> On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
>>>>>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
>>>>>>
>>>>>> If each node of physical memory layout has huge space for hotplug,
>>>>>> the padding used for the physical memory mapping section is not enough.
>>>>>> For exapmle of the layout:
>>>>>>    SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
>>>>>>    SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
>>>>>>    SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
>>>>>>    SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
>>>>>>
>>>>>> We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
>>>>>> however, the needed padding size depends on the system environment.
>>>>>> The kernel option is better than changing the config.
>>>>>>
>>>>>> Change log from v2:
>>>>>>   - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
>>>>>>     but a little different. Remove SGI UV description.
>>>>
>>>> Could you please explain it a bit better where the higher padding requirement comes from?
>>>>
>>>> 'system environment' is very opaque.
>>>
>>> As I understand it, it's depending on the actual physical characteristics
>>> of the machine. So setting a fixed value in Kconfig might work for one, but
>>> not for others and having a command line option allows to tweak that at
>>> boot time and having a common kernel image.
>>>
>>> Ideally we would calculate that from SRAT, but AFAICT SRAT is not available
>>> at the point where this needs to be done.
> 
> Yes, that's right. The KASLR initialization is early boot sequence,
> so SRAT is not available at that time.

Some facts are available via the x86 boot options structure passed from 
BIOS.  Is there enough info in there to help determine what the optimal 
value of this parameter should be?  Even a safe guess gets the system 
booted and can then be refined for the next reboot.

>>
>> Yeah, so could we at least do something like this:
>>
>>   - See whether using the maximum padding as the new default padding would work for everyone?
>>     A bit more virtual memory used, or are there other costs as well?
> 
> The current default padding size if CONFIG_MEMORY_HOTPLUG set is 10TB.
> IMO, it should not be increased because it gets the available entropy
> decreased...
> 
>>   - Add checking code to the later SRAT case to at least _detect_ bad padding after the fact.
>>     We don't utilize RAM with bad padding until that, right?
> 
> I have an idea as following. Does that make sense?
> 
> Add a warning message which shows the padding size is not enough
> for the physical memory mapping and tell to the user about
> recommended padding size. User can change the padding size in next
> reboot to add the boot parameter.

Again, leaving it solely up to the user is probably not the best 
approach, either for single workstation users who may not understand 
what's up, or large system users which will just generate a customer 
service call, because something went wrong and they can't boot.  Or 
their performance went down the drain.  [Normally upgrades that change 
the system config use an onsite CE, but that's not strictly required.]

So basically a deterministic method of calculating what this padding 
should be works best from a customer support angle.  For an individual 
workstation user, having the kernel determine what's correct for proper 
operation is the best.

Thanks,
Mike

>>
>>   - Add 'quirk' to the name of the boot parameter, to make it clear that this is really due to
>>     suboptimal communication between the firmware and the kernel.
> 
> I'm ok if 'quirk' is added to the boot parameter.
> 
> Thanks,
> Masa
> 

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Thomas Gleixner]

On Wed, 19 Sep 2018, Masayoshi Mizuma wrote:
> On Wed, Sep 19, 2018 at 02:48:06PM +0200, Ingo Molnar wrote:

> > - Add checking code to the later SRAT case to at least _detect_ bad
> >   padding after the fact.
> >
> >   We don't utilize RAM with bad padding until that, right?
> 
> I have an idea as following. Does that make sense?
> 
> Add a warning message which shows the padding size is not enough
> for the physical memory mapping and tell to the user about
> recommended padding size. User can change the padding size in next
> reboot to add the boot parameter.

Yes, that makes sense because silently failing is bad.

Thanks,

	tglx

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Masayoshi Mizuma]

On Wed, Sep 19, 2018 at 11:05:32PM +0000, Travis, Mike wrote:
> 
> 
> On 9/19/2018 7:10 AM, Masayoshi Mizuma wrote:
> > On Wed, Sep 19, 2018 at 02:48:06PM +0200, Ingo Molnar wrote:
> >>
> >> * Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >>> On Wed, 19 Sep 2018, Ingo Molnar wrote:
> >>>> * Masayoshi Mizuma <msys.mizuma@gmail.com> wrote:
> >>>>
> >>>>> Ping...
> >>>>> I would appreciate if someone could review it because this patch
> >>>>> fixes the real memory hotplug issue...
> >>>>
> >>>> Yeah, so I generally try to resist random new boot options that
> >>>> work around real bugs, so please convince me that this patch
> >>>> is the best option:
> 
> I whole hardily concur, that having boot options which are not easily 
> understood should be avoided.  The very best is the system should just 
> work.  But on very large systems, these boot options are typically 
> determined by either automated scripts, or careful instructions to the 
> trained onsite customer engineers, who are required to "get it right".
> 
> >>>>
> >>>>>
> >>>>> On Tue, Sep 04, 2018 at 11:11:40AM -0400, Masayoshi Mizuma wrote:
> >>>>>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
> >>>>>>
> >>>>>> If each node of physical memory layout has huge space for hotplug,
> >>>>>> the padding used for the physical memory mapping section is not enough.
> >>>>>> For exapmle of the layout:
> >>>>>>    SRAT: Node 6 PXM 4 [mem 0x100000000000-0x13ffffffffff] hotplug
> >>>>>>    SRAT: Node 7 PXM 5 [mem 0x140000000000-0x17ffffffffff] hotplug
> >>>>>>    SRAT: Node 2 PXM 6 [mem 0x180000000000-0x1bffffffffff] hotplug
> >>>>>>    SRAT: Node 3 PXM 7 [mem 0x1c0000000000-0x1fffffffffff] hotplug
> >>>>>>
> >>>>>> We can increase the padding by CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING,
> >>>>>> however, the needed padding size depends on the system environment.
> >>>>>> The kernel option is better than changing the config.
> >>>>>>
> >>>>>> Change log from v2:
> >>>>>>   - Simplify the description. As Baoquan said, this is simillar SGI UV issue,
> >>>>>>     but a little different. Remove SGI UV description.
> >>>>
> >>>> Could you please explain it a bit better where the higher padding requirement comes from?
> >>>>
> >>>> 'system environment' is very opaque.
> >>>
> >>> As I understand it, it's depending on the actual physical characteristics
> >>> of the machine. So setting a fixed value in Kconfig might work for one, but
> >>> not for others and having a command line option allows to tweak that at
> >>> boot time and having a common kernel image.
> >>>
> >>> Ideally we would calculate that from SRAT, but AFAICT SRAT is not available
> >>> at the point where this needs to be done.
> > 
> > Yes, that's right. The KASLR initialization is early boot sequence,
> > so SRAT is not available at that time.
> 
> Some facts are available via the x86 boot options structure passed from 
> BIOS.  Is there enough info in there to help determine what the optimal 
> value of this parameter should be?  Even a safe guess gets the system 
> booted and can then be refined for the next reboot.

Unfortunately, the bios information doesn't useful for the padding
size for memory hotplug. ACPI SRAT is needed, but it is not available
at the KASLR initialization time...

> 
> >>
> >> Yeah, so could we at least do something like this:
> >>
> >>   - See whether using the maximum padding as the new default padding would work for everyone?
> >>     A bit more virtual memory used, or are there other costs as well?
> > 
> > The current default padding size if CONFIG_MEMORY_HOTPLUG set is 10TB.
> > IMO, it should not be increased because it gets the available entropy
> > decreased...
> > 
> >>   - Add checking code to the later SRAT case to at least _detect_ bad padding after the fact.
> >>     We don't utilize RAM with bad padding until that, right?
> > 
> > I have an idea as following. Does that make sense?
> > 
> > Add a warning message which shows the padding size is not enough
> > for the physical memory mapping and tell to the user about
> > recommended padding size. User can change the padding size in next
> > reboot to add the boot parameter.
> 
> Again, leaving it solely up to the user is probably not the best 
> approach, either for single workstation users who may not understand 
> what's up, or large system users which will just generate a customer 
> service call, because something went wrong and they can't boot.  Or 
> their performance went down the drain.  [Normally upgrades that change 
> the system config use an onsite CE, but that's not strictly required.]
> 
> So basically a deterministic method of calculating what this padding 
> should be works best from a customer support angle.  For an individual 
> workstation user, having the kernel determine what's correct for proper 
> operation is the best.

I agree it is better to set the padding size automatically for users,
however, we cannot do so at the KASLR initialization timing because
of the above reason.
So, I will try to add warning message to show the recommended padding
size for the memory hotplug.

Thanks,
Masa

Re: [PATCH v3 1/2] x86/mm: Add an option to change the padding used for the physical memory mapping

[Masayoshi Mizuma]

On Thu, Sep 20, 2018 at 11:28:47AM +0200, Thomas Gleixner wrote:
> On Wed, 19 Sep 2018, Masayoshi Mizuma wrote:
> > On Wed, Sep 19, 2018 at 02:48:06PM +0200, Ingo Molnar wrote:
> 
> > > - Add checking code to the later SRAT case to at least _detect_ bad
> > >   padding after the fact.
> > >
> > >   We don't utilize RAM with bad padding until that, right?
> > 
> > I have an idea as following. Does that make sense?
> > 
> > Add a warning message which shows the padding size is not enough
> > for the physical memory mapping and tell to the user about
> > recommended padding size. User can change the padding size in next
> > reboot to add the boot parameter.
> 
> Yes, that makes sense because silently failing is bad.

Thanks. I will try to add it.

- Masa