Re: [PATCHv5 06/12] x86/boot/compressed: Handle unaccepted memory

From: Borislav Petkov <bp@alien8.de>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Sean Christopherson <seanjc@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joerg Roedel <jroedel@suse.de>, Ard Biesheuvel <ardb@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	David Rientjes <rientjes@google.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Varad Gautam <varad.gautam@suse.com>,
	Dario Faggioli <dfaggioli@suse.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Mike Rapoport <rppt@kernel.org>,
	David Hildenbrand <david@redhat.com>,
	x86@kernel.org, linux-mm@kvack.org, linux-coco@lists.linux.dev,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv5 06/12] x86/boot/compressed: Handle unaccepted memory
Date: Tue, 3 May 2022 16:12:55 +0200	[thread overview]
Message-ID: <YnE4ZzzVrxUnr3Uv@zn.tnic> (raw)
In-Reply-To: <20220425033934.68551-7-kirill.shutemov@linux.intel.com>

On Mon, Apr 25, 2022 at 06:39:28AM +0300, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 411b268bc0a2..59db90626042 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -725,10 +725,20 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
>  		 * but in practice there's firmware where using that memory leads
>  		 * to crashes.
>  		 *
> -		 * Only EFI_CONVENTIONAL_MEMORY is guaranteed to be free.
> +		 * Only EFI_CONVENTIONAL_MEMORY and EFI_UNACCEPTED_MEMORY (if
> +		 * supported) are guaranteed to be free.
>  		 */
> -		if (md->type != EFI_CONVENTIONAL_MEMORY)
> +
> +		switch (md->type) {
> +		case EFI_CONVENTIONAL_MEMORY:
> +			break;
> +		case EFI_UNACCEPTED_MEMORY:
> +			if (IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
> +				break;
>  			continue;
> +		default:
> +			continue;
> +		}

Is there any special reason for this to be a switch-case or can it
simply be a compound conditional if (bla...) ?

> @@ -66,3 +69,21 @@ void process_unaccepted_memory(struct boot_params *params, u64 start, u64 end)
>  	bitmap_set((unsigned long *)params->unaccepted_memory,
>  		   start / PMD_SIZE, (end - start) / PMD_SIZE);
>  }
> +
> +void accept_memory(phys_addr_t start, phys_addr_t end)
> +{
> +	unsigned long range_start, range_end;
> +	unsigned long *unaccepted_memory;

Please shorten that name so that you don't have to break the lines below.

> +	unsigned long bitmap_size;
> +
> +	unaccepted_memory = (unsigned long *)boot_params->unaccepted_memory;
> +	range_start = start / PMD_SIZE;
> +	bitmap_size = DIV_ROUND_UP(end, PMD_SIZE);
> +
> +	for_each_set_bitrange_from(range_start, range_end,
> +				   unaccepted_memory, bitmap_size) {
> +		__accept_memory(range_start * PMD_SIZE, range_end * PMD_SIZE);
> +		bitmap_clear(unaccepted_memory,
> +			     range_start, range_end - range_start);
> +	}
> +}
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index fa8969fad011..285b37e28074 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -18,6 +18,7 @@
>  #include "../string.h"
>  #include "../voffset.h"
>  #include <asm/bootparam_utils.h>
> +#include <asm/unaccepted_memory.h>
>  
>  /*
>   * WARNING!!
> @@ -451,6 +452,14 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
>  #endif
>  
>  	debug_putstr("\nDecompressing Linux... ");
> +
> +#ifdef CONFIG_UNACCEPTED_MEMORY

IS_ENABLED() perhaps?

> +	if (boot_params->unaccepted_memory) {

Also, that ->unaccepted_memory will be set only when
ACONFIG_UNACCEPTED_MEMORY is set, FAICT.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette