linux-kernel - Re: [PATCH v6 3/4] arm64: use both ZONE_DMA and ZONE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191203101249.GC6815@willie-the-truck>
Date:   Tue, 3 Dec 2019 10:12:50 +0000
From:   Will Deacon <will@...nel.org>
To:     John Stultz <john.stultz@...aro.org>
Cc:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>,
        Catalin Marinas <catalin.marinas@....com>,
        Christoph Hellwig <hch@....de>, wahrenst@....net,
        Marc Zyngier <marc.zyngier@....com>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Robin Murphy <robin.murphy@....com>,
        lkml <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>, mbrugger@...e.com,
        linux-rpi-kernel@...ts.infradead.org,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Nicolas Dechense <nicolas.dechesne@...aro.org>
Subject: Re: [PATCH v6 3/4] arm64: use both ZONE_DMA and ZONE_DMA32

Hi John,

On Mon, Dec 02, 2019 at 10:03:17PM -0800, John Stultz wrote:
> Ok, narrowing it down further, it seems its the following bit from the
> patch:
> 
> > @@ -201,13 +212,18 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> >         struct memblock_region *reg;
> >         unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
> >         unsigned long max_dma32 = min;
> > +       unsigned long max_dma = min;
> >
> >         memset(zone_size, 0, sizeof(zone_size));
> >
> > -       /* 4GB maximum for 32-bit only capable devices */
> > +#ifdef CONFIG_ZONE_DMA
> > +       max_dma = PFN_DOWN(arm64_dma_phys_limit);
> > +       zone_size[ZONE_DMA] = max_dma - min;
> > +       max_dma32 = max_dma;
> > +#endif
> >  #ifdef CONFIG_ZONE_DMA32
> >         max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
> > -       zone_size[ZONE_DMA32] = max_dma32 - min;
> > +       zone_size[ZONE_DMA32] = max_dma32 - max_dma;
> >  #endif
> >         zone_size[ZONE_NORMAL] = max - max_dma32;
> >
> > @@ -219,11 +235,17 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> >
> >                 if (start >= max)
> >                         continue;
> > -
> > +#ifdef CONFIG_ZONE_DMA
> > +               if (start < max_dma) {
> > +                       unsigned long dma_end = min_not_zero(end, max_dma);
> > +                       zhole_size[ZONE_DMA] -= dma_end - start;
> > +               }
> > +#endif
> >  #ifdef CONFIG_ZONE_DMA32
> >                 if (start < max_dma32) {
> > -                       unsigned long dma_end = min(end, max_dma32);
> > -                       zhole_size[ZONE_DMA32] -= dma_end - start;
> > +                       unsigned long dma32_end = min(end, max_dma32);
> > +                       unsigned long dma32_start = max(start, max_dma);
> > +                       zhole_size[ZONE_DMA32] -= dma32_end - dma32_start;
> >                 }
> >  #endif
> >                 if (end > max_dma32) {
> 
> The zhole_sizes end up being:
> zhole_size: DMA: 67671, DMA32: 1145664 NORMAL: 0
> 
> This seems to be due to dma32_start being calculated as 786432 each
> time - I'm guessing that's the max_dma value.
> Where dma32_end is around 548800, but changes each iteration (so we
> end up subtracting a negative value each pass, growing the size).

Yeah, this logic looks utterly buggered to me so I'm surprised that nobody
else has seen the problem. In particlar, kernelci is reporting success
on the same SoC :/

https://kernelci.org/boot/sdm845-db845c/

The logs also don't seem to match up with the trees either. For example,
looking at the boot logs for:

https://kernelci.org/boot/id/5de5fc60b1ed6e2d46960f08/

It claims that the kernel is "next-2019-12-30" but the dmesg says:

[    0.000000] Linux version 5.4.0 (KernelCI@...b74fe311d) (gcc version
8.3.0 (Debian 8.3.0-2)) #1 SMP PREEMPT Tue Dec 3 03:14:07 UTC 2019

Which isn't great.

Anyway, I've had a go at fixing it below but it's 100% untested. I think
the issue is that your SoC has memblocks contained entirely within the
ZONE_DMA region and we don't cater for that at all when processing the
ZONE_DMA32 region.

Will

--->8

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index be9481cdf3b9..af365ce59ed6 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -242,19 +242,19 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 		if (start < max_dma) {
 			unsigned long dma_end = min_not_zero(end, max_dma);
 			zhole_size[ZONE_DMA] -= dma_end - start;
+			start = dma_end;
 		}
 #endif
 #ifdef CONFIG_ZONE_DMA32
-		if (start < max_dma32) {
+		if (start >= max_dma && start < max_dma32) {
 			unsigned long dma32_end = min(end, max_dma32);
-			unsigned long dma32_start = max(start, max_dma);
-			zhole_size[ZONE_DMA32] -= dma32_end - dma32_start;
+			zhole_size[ZONE_DMA32] -= dma32_end - start;
+			start = dma32_end;
 		}
 #endif
-		if (end > max_dma32) {
+		if (start >= max_dma32) {
 			unsigned long normal_end = min(end, max);
-			unsigned long normal_start = max(start, max_dma32);
-			zhole_size[ZONE_NORMAL] -= normal_end - normal_start;
+			zhole_size[ZONE_NORMAL] -= normal_end - start;
 		}
 	}