Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 1 Jun 2021 17:06:53 +0800
From:   Baoquan He <bhe@redhat.com>
To:     Mike Rapoport <rppt@kernel.org>
Cc:     x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
        Andy Lutomirski <luto@amacapital.net>,
        Andy Shevchenko <andy@infradead.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Borislav Petkov <bp@alien8.de>,
        Darren Hart <dvhart@infradead.org>,
        Dave Young <dyoung@redhat.com>,
        Hugh Dickins <hughd@google.com>,
        Ingo Molnar <mingo@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Lianbo Jiang <lijiang@redhat.com>,
        Mike Rapoport <rppt@linux.ibm.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-doc@vger.kernel.org, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 1/3] x86/setup: always reserve the first 1M of RAM
Message-ID: <20210601090653.GB361405@MiWiFi-R3L-srv>
References: <20210601075354.5149-1-rppt@kernel.org>
 <20210601075354.5149-2-rppt@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210601075354.5149-2-rppt@kernel.org>
Precedence: bulk

On 06/01/21 at 10:53am, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
......  

> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 7850111008a8..b15ebfe40a73 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -450,6 +450,18 @@ void __init efi_free_boot_services(void)
>  			size -= rm_size;
>  		}

Thanks for taking care of the low-1M excluding in
efi_free_boot_services(), Mike. You might want to remove the old real
mode excluding code either since it's been covered by your new code.

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index b15ebfe40a73..be814f2089ff 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -409,7 +409,6 @@ void __init efi_free_boot_services(void)
 	for_each_efi_memory_desc(md) {
 		unsigned long long start = md->phys_addr;
 		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
-		size_t rm_size;
 
 		if (md->type != EFI_BOOT_SERVICES_CODE &&
 		    md->type != EFI_BOOT_SERVICES_DATA) {
@@ -430,26 +429,6 @@ void __init efi_free_boot_services(void)
 		 */
 		efi_unmap_pages(md);
 
-		/*
-		 * Nasty quirk: if all sub-1MB memory is used for boot
-		 * services, we can get here without having allocated the
-		 * real mode trampoline.  It's too late to hand boot services
-		 * memory back to the memblock allocator, so instead
-		 * try to manually allocate the trampoline if needed.
-		 *
-		 * I've seen this on a Dell XPS 13 9350 with firmware
-		 * 1.4.4 with SGX enabled booting Linux via Fedora 24's
-		 * grub2-efi on a hard disk.  (And no, I don't know why
-		 * this happened, but Linux should still try to boot rather
-		 * panicking early.)
-		 */
-		rm_size = real_mode_size_needed();
-		if (rm_size && (start + rm_size) < (1<<20) && size >= rm_size) {
-			set_real_mode_mem(start);
-			start += rm_size;
-			size -= rm_size;
-		}
-
 		/*
 		 * Don't free memory under 1M for two reasons:
 		 * - BIOS might clobber it

>  
> +		/*
> +		 * Don't free memory under 1M for two reasons:
> +		 * - BIOS might clobber it
> +		 * - Crash kernel needs it to be reserved
> +		 */
> +		if (start + size < SZ_1M)
> +			continue;
> +		if (start < SZ_1M) {
> +			size -= (SZ_1M - start);
> +			start = SZ_1M;
> +		}
> +
>  		memblock_free_late(start, size);
>  	}
>  
> diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
> index 2e1c1bec0f9e..8ea285aca827 100644
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -29,14 +29,16 @@ void __init reserve_real_mode(void)
>  
>  	/* Has to be under 1M so we can execute real-mode AP code. */
>  	mem = memblock_find_in_range(0, 1<<20, size, PAGE_SIZE);
> -	if (!mem) {
> +	if (!mem)
>  		pr_info("No sub-1M memory is available for the trampoline\n");
> -		return;
> -	}
> +	else
> +		set_real_mode_mem(mem);
>  
> -	memblock_reserve(mem, size);
> -	set_real_mode_mem(mem);
> -	crash_reserve_low_1M();
> +	/*
> +	 * Unconditionally reserve the entire fisrt 1M, see comment in
> +	 * setup_arch()
> +	 */
> +	memblock_reserve(0, SZ_1M);
>  }
>  
>  static void sme_sev_setup_real_mode(struct trampoline_header *th)
> -- 
> 2.28.0
>