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:   Fri, 28 May 2021 16:41:40 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Mike Rapoport <rppt@kernel.org>
Cc:     Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Mike Rapoport <rppt@linux.ibm.com>,
        untaintableangel@hotmail.co.uk, x86@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/Kconfig: decrease maximum of X86_RESERVE_LOW to 512K
Message-ID: <YLEBJM0HbQkuDdqV@zn.tnic>
References: <20210526081100.12239-1-rppt@kernel.org>
 <YK4LGUDWXJWOp7IR@zn.tnic>
 <YK53kWHb4cPeeHsd@kernel.org>
 <YK6QFLUoPZ7btQfH@zn.tnic>
 <YK+gv0vDfLVD7Sqp@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <YK+gv0vDfLVD7Sqp@kernel.org>
Precedence: bulk

On Thu, May 27, 2021 at 04:38:07PM +0300, Mike Rapoport wrote:
> Well 640K is well known memory limit :)

Yah, that I remember - was just wondering why but I guess it was out of
caution to cover all that a BIOS *could* touch, see hpa's reply.

> Another aspect IMHO is that making things explicit would reduce the amount
> of hidden dependencies and in the end make x86::setup_arch() less fragile.

Hohumm.

> I'm looking now also at:
> 
> 5bc653b73182 ("x86/efi: Allocate a trampoline if needed in efi_free_boot_services()")
> 
> that retries the allocation of trampoline when we free EFI services, so
> there is also could be a conflict between reserve_real_mode() and
> reserve_bios_regions() in case EBDA is too low.
> 
> So what we have is
> - BIOSes that corrupt low memory
> - EBDA of unknown size that can be as low as 128k, so we reserve everything
>   from EBDA start to 640k because we don't trust BIOSes to report EBDA size 
>   properly
> - Real mode blob of about 20-30k that must live in the first 640k
> - Build time setting to reserve Xk (4K <= X <= 640k) with the default set
>   to 64k
> - Command line option to reserve Yk (4K <= Y <= 640k), this takes precedence
>   over the build time option.
> - A late fallback that uses memory freed from EFI data to place real mode
>   trampoline there
> 
> It seems to me that we can drop both  build time and run time options
> entirely, reserve 64k early to avoid having trampoline there and then
> always reserve everything below 640k after reserve_real_mode().
> 
> The late fallback for systems that have most of low memory busy with
> BIOS/EFI will remain intact as it does not do memblock allocation anyway.

Yah, I certainly like the simplification. The first 640K seem to be a
minefield anyway and to quote from that bugzilla again:

https://bugzilla.kernel.org/show_bug.cgi?id=16661#c2

"As far as I know, Windows 7 actually reserves all memory below 1 MiB to
avoid BIOS bugs."

so yeah, I think we should do that. But pls put that justification above
in the commit message so that we know why we did it.

Thx.

-- 
Regards/Gruss,
    Boris.

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