Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 9 Dec 2022 14:10:13 -0600
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Hans de Goede <hdegoede@redhat.com>
Cc:     linux-pci@vger.kernel.org, Florent DELAHAYE <kernelorg@undead.fr>,
        Konrad J Hambrick <kjhambrick@gmail.com>,
        Matt Hansen <2lprbe78@duck.com>,
        Benoit =?iso-8859-1?Q?Gr=E9goire?= <benoitg@coeus.ca>,
        Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Werner Sembach <wse@tuxedocomputers.com>,
        mumblingdrunkard@protonmail.com, linux-kernel@vger.kernel.org,
        Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH v2 1/4] efi/x86: Remove EfiMemoryMappedIO from E820 map
Message-ID: <20221209201013.GA1719699@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c7418ea8-6b1a-f648-da5f-bf6ae412e359@redhat.com>
Precedence: bulk

On Fri, Dec 09, 2022 at 12:04:53PM +0100, Hans de Goede wrote:
> On 12/9/22 09:06, Hans de Goede wrote:
> > One comment (logging bug in patch) below:
> > ...

> > The logging in this else is re-using the start and end from the previous section which was actually removed.
> > 
> > E.g. Matt's latest log from:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1868899
> > has:
> > 
> > [    0.000000] e820: remove [mem 0xfc800000-0xfe7fffff] reserved
> > [    0.000000] efi: Not removing mem46: MMIO range=[0xfc800000-0xfe7fffff] (4KB) from e820 map
> > [    0.000000] efi: Not removing mem47: MMIO range=[0xfc800000-0xfe7fffff] (32KB) from e820 map
> > [    0.000000] efi: Not removing mem49: MMIO range=[0xfc800000-0xfe7fffff] (8KB) from e820 map
> > [    0.000000] efi: Not removing mem50: MMIO range=[0xfc800000-0xfe7fffff] (4KB) from e820 map
> > 
> > Notice how all the "Not removing ..." lines log the same range as
> > the actually removed map entry above them.
> 
> I realize the fix is very obvious, but since I just fixed this in my
> local tree anyways, here is my fix for this:

Thank you!  Incorporated.

> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -331,9 +331,9 @@ static void __init efi_remove_e820_mmio(void)
>  	for_each_efi_memory_desc(md) {
>  		if (md->type == EFI_MEMORY_MAPPED_IO) {
>  			size = md->num_pages << EFI_PAGE_SHIFT;
> +			start = md->phys_addr;
> +			end = start + size - 1;
>  			if (size >= 256*1024) {
> -				start = md->phys_addr;
> -				end = start + size - 1;
>  				pr_info("Remove mem%02u: MMIO range=[0x%08llx-0x%08llx] (%lluMB) from e820 map\n",
>  					i, start, end, size >> 20);
>  				e820__range_remove(start, size,
>