linux-kernel - RE: [EXT] Re: [PATCH v3 3/3] mmc: xenon: Fix 2G limitation on AC5 SoC

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]

Date:   Thu, 8 Dec 2022 10:19:47 +0000
From:   Elad Nachman <enachman@...vell.com>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Vadym Kochan <vadym.kochan@...ision.eu>
CC:     Hu Ziji <huziji@...vell.com>, Ulf Hansson <ulf.hansson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chris Packham <chris.packham@...iedtelesis.co.nz>
Subject: RE: [EXT] Re: [PATCH v3 3/3] mmc: xenon: Fix 2G limitation on AC5 SoC

Hi,

I would like to explain how this HW mechanism works:

The lower 31-bits of the address placed in the ADMA is passed through the interconnect, and remapped to the base of the DDR.

Hence only addressing of the lower 2GB of the DDR memory is supported for eMMC in this device family (AC5/X).

So the quirk needs to kick in above 2GB of physical memory accessed from the base of the DDR.

Since we cannot control the location of the various buffers passed in the Linux kernel via VFS to the eMMC driver, the only remedy is to kick in the quirk whenever buffer which point to the physical memory above 2GB * can * arrive to the eMMC driver, hence the quirk kicks in whenever a total physical memory size greater than 2GB is detected in the system.

This is why a quirk which only kicks in above 4GB is not sufficient.

Furthermore, SDHCI_QUIRK_32BIT_DMA_ADDR is checked in sdhci_prepare_data() as a way to disable DMA when the offset of the scatter-list DMA address is not 32-bit aligned. If the address is aligned, this quirk does not disable the DMA, and will not solve our problem.

Hopefully this explains the motivation for the way the patch is written.

FYI,

Elad.

-----Original Message-----
From: Linus Walleij <linus.walleij@...aro.org> 
Sent: Wednesday, December 7, 2022 3:40 PM
To: Vadym Kochan <vadym.kochan@...ision.eu>
Cc: Hu Ziji <huziji@...vell.com>; Ulf Hansson <ulf.hansson@...aro.org>; Rob Herring <robh+dt@...nel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>; Adrian Hunter <adrian.hunter@...el.com>; linux-mmc@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Elad Nachman <enachman@...vell.com>; Chris Packham <chris.packham@...iedtelesis.co.nz>
Subject: [EXT] Re: [PATCH v3 3/3] mmc: xenon: Fix 2G limitation on AC5 SoC

External Email

----------------------------------------------------------------------
On Mon, Dec 5, 2022 at 12:00 PM Vadym Kochan <vadym.kochan@...ision.eu> wrote:

> There is a limitation on AC5 SoC that mmc controller can't have DMA 
> access over 2G memory,

That sounds like a pretty common problem when someone uses a 32bit address register in their DMA controller, or the integration engineer not connecting all address lines... :/

>  so use SDMA with a bounce buffer. Swiotlb can't help because on arm64 
> arch it reserves memblock's at the end of the memory.

OK

This:

> Additionally set mask to 34 bit since on AC5 SoC RAM starts at 
> 0x2_00000000.
(...)
> +static int xenon_set_dma_mask(struct sdhci_host *host) {
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +       struct mmc_host *mmc = host->mmc;
> +       struct device *dev = mmc_dev(mmc);
> +
> +       if (priv->hw_version == XENON_AC5) {
> +               host->flags &= ~SDHCI_USE_64_BIT_DMA;
> +
> +               return dma_set_mask_and_coherent(dev, DMA_BIT_MASK(34));
> +       }
> +
> +       return sdhci_set_dma_mask(host); }
(...)
> +       .set_dma_mask           = xenon_set_dma_mask,

I don't know if is so good to assume the size and location of the SoC RAM like you do, that looks really fragile.

Can't you check what physical RAM Linux actually think it has and where? You partly check it with meminfo below.

> +static int xenon_ac5_probe(struct sdhci_host *host) {
> +       struct sysinfo si;
> +
> +       si_meminfo(&si);
> +
> +       if ((si.totalram * si.mem_unit) > SZ_2G)

This looks like a bug since you mention that the RAM does not start at 0x00000000 this means if the memory is 2G it will partly be at physical addresses above 2G....

> +               host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
> +
> +       return 0;
> +}

Here you check how big the RAM is using meminfo (if the bug is fixed).

But is this really a good solution? ADMA still works on the lower 2GB does it not?

What you want is a new quirk that bounces only when you go above SZ_4G.

There *is* SDHCI_QUIRK_32BIT_DMA_ADDR have you tried this? A 32bit DMA address is literally 4GB.
I think all you need to do is set this flag for xenon.

Yours,
Linus Walleij