diff mbox series

[PATCHv7,08/14] x86/mm: Reserve unaccepted memory bitmap

Message ID 20220614120231.48165-9-kirill.shutemov@linux.intel.com
State New
Headers show
Series mm, x86/cc: Implement support for unaccepted memory | expand

Commit Message

Kirill A. Shutemov June 14, 2022, 12:02 p.m. UTC 
A given page of memory can only be accepted once. The kernel has to
accept memory both in the early decompression stage and during normal
runtime.

A bitmap is used to communicate the acceptance state of each page
between the decompression stage and normal runtime.

boot_params is used to communicate location of the bitmap throughout
the boot. The bitmap is allocated and initially populated in EFI stub.
Decompression stage accepts pages required for kernel/initrd and marks
these pages accordingly in the bitmap. The main kernel picks up the
bitmap from the same boot_params and uses it to determine what has to
be accepted on allocation.

In the runtime kernel, reserve the bitmap's memory to ensure nothing
overwrites it.

The size of bitmap is determined with e820__end_of_ram_pfn() which
relies on setup_e820() marking unaccepted memory as E820_TYPE_RAM.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
---
 arch/x86/kernel/e820.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Borislav Petkov July 26, 2022, 9:07 a.m. UTC | #1 
On Tue, Jun 14, 2022 at 03:02:25PM +0300, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> index f267205f2d5a..22d1fe48dcba 100644
> --- a/arch/x86/kernel/e820.c
> +++ b/arch/x86/kernel/e820.c
> @@ -1316,6 +1316,16 @@ void __init e820__memblock_setup(void)
>  	int i;
>  	u64 end;
>  
> +	/* Mark unaccepted memory bitmap reserved */
> +	if (boot_params.unaccepted_memory) {
> +		unsigned long size;
> +
> +		/* One bit per 2MB */
> +		size = DIV_ROUND_UP(e820__end_of_ram_pfn() * PAGE_SIZE,
> +				    PMD_SIZE * BITS_PER_BYTE);
> +		memblock_reserve(boot_params.unaccepted_memory, size);
> +	}
> +

Hmm, I don't like how this is dropped right in the middle of a unrelated
function.

You're adding arch/x86/mm/unaccepted_memory.c later. Why don't you put
that chunk in a function there which is called by early_reserve_memory()
which does exactly what you want - reserve memory early, before memblock
allocations?

Hmm.
Kirill A. Shutemov Nov. 30, 2022, 1:28 a.m. UTC | #2 
On Tue, Jul 26, 2022 at 11:07:14AM +0200, Borislav Petkov wrote:
> On Tue, Jun 14, 2022 at 03:02:25PM +0300, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > index f267205f2d5a..22d1fe48dcba 100644
> > --- a/arch/x86/kernel/e820.c
> > +++ b/arch/x86/kernel/e820.c
> > @@ -1316,6 +1316,16 @@ void __init e820__memblock_setup(void)
> >  	int i;
> >  	u64 end;
> >  
> > +	/* Mark unaccepted memory bitmap reserved */
> > +	if (boot_params.unaccepted_memory) {
> > +		unsigned long size;
> > +
> > +		/* One bit per 2MB */
> > +		size = DIV_ROUND_UP(e820__end_of_ram_pfn() * PAGE_SIZE,
> > +				    PMD_SIZE * BITS_PER_BYTE);
> > +		memblock_reserve(boot_params.unaccepted_memory, size);
> > +	}
> > +
> 
> Hmm, I don't like how this is dropped right in the middle of a unrelated
> function.
> 
> You're adding arch/x86/mm/unaccepted_memory.c later. Why don't you put
> that chunk in a function there which is called by early_reserve_memory()
> which does exactly what you want - reserve memory early, before memblock
> allocations?

early_reserve_memory() specifically called before e820__memory_setup()
(see comment in setup_arch()), so we don't have e820_table finalized and
we need it to get correct RAM size from e820__end_of_ram_pfn().

I guess we can hide the chunk in a function in unaccepted_memory.c and
call it from here, but it would require #ifdeffery in a header file as the
.c is only compiled for CONFIG_UNACCEPTED_MEMORY=y.

Looks like an overkill to me, no?
Mike Rapoport Dec. 1, 2022, 9:37 a.m. UTC | #3 
On Wed, Nov 30, 2022 at 04:28:40AM +0300, Kirill A. Shutemov wrote:
> On Tue, Jul 26, 2022 at 11:07:14AM +0200, Borislav Petkov wrote:
> > On Tue, Jun 14, 2022 at 03:02:25PM +0300, Kirill A. Shutemov wrote:
> > > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > > index f267205f2d5a..22d1fe48dcba 100644
> > > --- a/arch/x86/kernel/e820.c
> > > +++ b/arch/x86/kernel/e820.c
> > > @@ -1316,6 +1316,16 @@ void __init e820__memblock_setup(void)
> > >  	int i;
> > >  	u64 end;
> > >  
> > > +	/* Mark unaccepted memory bitmap reserved */
> > > +	if (boot_params.unaccepted_memory) {
> > > +		unsigned long size;
> > > +
> > > +		/* One bit per 2MB */
> > > +		size = DIV_ROUND_UP(e820__end_of_ram_pfn() * PAGE_SIZE,
> > > +				    PMD_SIZE * BITS_PER_BYTE);
> > > +		memblock_reserve(boot_params.unaccepted_memory, size);
> > > +	}
> > > +
> > 
> > Hmm, I don't like how this is dropped right in the middle of a unrelated
> > function.
> > 
> > You're adding arch/x86/mm/unaccepted_memory.c later. Why don't you put
> > that chunk in a function there which is called by early_reserve_memory()
> > which does exactly what you want - reserve memory early, before memblock
> > allocations?
> 
> early_reserve_memory() specifically called before e820__memory_setup()
> (see comment in setup_arch()), so we don't have e820_table finalized and
> we need it to get correct RAM size from e820__end_of_ram_pfn().
> 
> I guess we can hide the chunk in a function in unaccepted_memory.c and
> call it from here, but it would require #ifdeffery in a header file as the
> .c is only compiled for CONFIG_UNACCEPTED_MEMORY=y.
> 
> Looks like an overkill to me, no?

Agree. Can we just extend the comment to explain why we reserve the bitmap
at e820__memblock_setup() rather than in early_reserve_memory(), pretty
much with the explanation above?
 
> -- 
>   Kiryl Shutsemau / Kirill A. Shutemov
Kirill A. Shutemov Dec. 1, 2022, 1:47 p.m. UTC | #4 
On Thu, Dec 01, 2022 at 11:37:10AM +0200, Mike Rapoport wrote:
> On Wed, Nov 30, 2022 at 04:28:40AM +0300, Kirill A. Shutemov wrote:
> > On Tue, Jul 26, 2022 at 11:07:14AM +0200, Borislav Petkov wrote:
> > > On Tue, Jun 14, 2022 at 03:02:25PM +0300, Kirill A. Shutemov wrote:
> > > > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
> > > > index f267205f2d5a..22d1fe48dcba 100644
> > > > --- a/arch/x86/kernel/e820.c
> > > > +++ b/arch/x86/kernel/e820.c
> > > > @@ -1316,6 +1316,16 @@ void __init e820__memblock_setup(void)
> > > >  	int i;
> > > >  	u64 end;
> > > >  
> > > > +	/* Mark unaccepted memory bitmap reserved */
> > > > +	if (boot_params.unaccepted_memory) {
> > > > +		unsigned long size;
> > > > +
> > > > +		/* One bit per 2MB */
> > > > +		size = DIV_ROUND_UP(e820__end_of_ram_pfn() * PAGE_SIZE,
> > > > +				    PMD_SIZE * BITS_PER_BYTE);
> > > > +		memblock_reserve(boot_params.unaccepted_memory, size);
> > > > +	}
> > > > +
> > > 
> > > Hmm, I don't like how this is dropped right in the middle of a unrelated
> > > function.
> > > 
> > > You're adding arch/x86/mm/unaccepted_memory.c later. Why don't you put
> > > that chunk in a function there which is called by early_reserve_memory()
> > > which does exactly what you want - reserve memory early, before memblock
> > > allocations?
> > 
> > early_reserve_memory() specifically called before e820__memory_setup()
> > (see comment in setup_arch()), so we don't have e820_table finalized and
> > we need it to get correct RAM size from e820__end_of_ram_pfn().
> > 
> > I guess we can hide the chunk in a function in unaccepted_memory.c and
> > call it from here, but it would require #ifdeffery in a header file as the
> > .c is only compiled for CONFIG_UNACCEPTED_MEMORY=y.
> > 
> > Looks like an overkill to me, no?
> 
> Agree. Can we just extend the comment to explain why we reserve the bitmap
> at e820__memblock_setup() rather than in early_reserve_memory(), pretty
> much with the explanation above?

Okay, I will do this:

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 49b5164a4cba..62068956bb76 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1316,7 +1316,14 @@ void __init e820__memblock_setup(void)
 	int i;
 	u64 end;
 
-	/* Mark unaccepted memory bitmap reserved */
+	/*
+	 * Mark unaccepted memory bitmap reserved.
+	 *
+	 * This kind of reservation usually done from early_reserve_memory(),
+	 * but early_reserve_memory() called before e820__memory_setup(), so
+	 * e820_table is not finalized and e820__end_of_ram_pfn() cannot be
+	 * used to get correct RAM size.
+	 */
 	if (boot_params.unaccepted_memory) {
 		unsigned long size;
diff mbox series

Patch

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index f267205f2d5a..22d1fe48dcba 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1316,6 +1316,16 @@  void __init e820__memblock_setup(void)
 	int i;
 	u64 end;
 
+	/* Mark unaccepted memory bitmap reserved */
+	if (boot_params.unaccepted_memory) {
+		unsigned long size;
+
+		/* One bit per 2MB */
+		size = DIV_ROUND_UP(e820__end_of_ram_pfn() * PAGE_SIZE,
+				    PMD_SIZE * BITS_PER_BYTE);
+		memblock_reserve(boot_params.unaccepted_memory, size);
+	}
+
 	/*
 	 * The bootstrap memblock region count maximum is 128 entries
 	 * (INIT_MEMBLOCK_REGIONS), but EFI might pass us more E820 entries