linux-kernel - Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver

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: <53ba18c1-4554-4d77-84fd-d921febb7559@app.fastmail.com>
Date: Wed, 02 Jul 2025 12:40:31 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "yangzh0906@...ndersoft.com" <yangzh0906@...ndersoft.com>,
 "Rob Herring" <robh@...nel.org>, krzk+dt@...nel.org,
 "Krzysztof Kozlowski" <krzk@...nel.org>,
 "Conor Dooley" <conor+dt@...nel.org>, "gordon.ge" <gordon.ge@....ai>,
 "Catalin Marinas" <catalin.marinas@....com>,
 "Geert Uytterhoeven" <geert.uytterhoeven@...il.com>,
 "Will Deacon" <will@...nel.org>, "Ulf Hansson" <ulf.hansson@...aro.org>,
 "Adrian Hunter" <adrian.hunter@...el.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org,
 "linux-mmc @ vger . kernel . org" <linux-mmc@...r.kernel.org>,
 soc@...ts.linux.dev, bst-upstream <bst-upstream@...ai.top>,
 "Neil Armstrong" <neil.armstrong@...aro.org>,
 "Jonathan Cameron" <jonathan.cameron@...wei.com>, bigfoot@...ssfun.cn,
 kever.yang@...k-chips.com, "Manivannan Sadhasivam" <mani@...nel.org>,
 "Geert Uytterhoeven" <geert+renesas@...der.be>,
 "Bjorn Andersson" <andersson@...nel.org>, "Nishanth Menon" <nm@...com>,
 Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 "Taniya Das" <quic_tdas@...cinc.com>, "Eric Biggers" <ebiggers@...gle.com>,
 "Victor Shih" <victor.shih@...esyslogic.com.tw>,
 "Shan-Chun Hung" <shanchun1218@...il.com>,
 "Ben Chuang" <ben.chuang@...esyslogic.com.tw>
Subject: Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200
 controller driver

On Wed, Jul 2, 2025, at 11:44, Albert Yang wrote:

> +
> +config MMC_SDHCI_BST
> +	tristate "SDHCI OF support for the BST DWC MSHC"
> +	depends on ARCH_BST || COMPILE_TEST
> +	depends on MMC_SDHCI_PLTFM
> +	depends on OF
> +	help
> +	  This selects Synopsys DesignWare Cores Mobile Storage Controller
> +	  support.

The description does not mention the actual device it's for
but only DesignWare.

Try to keep this sorted alphabetically between the other
CONFIG_MMC_SDHCI_* backends

> +
> +struct dwcmshc_priv {
> +	void __iomem *crm_reg_base;
> +	u32 phy_crm_reg_base;
> +	u32 phy_crm_reg_size;
> +};

You are only using the first member here, the phy_crm_reg_base
and phy_crm_reg_size are assigned during probe but not referenced
later.  devm_platform_ioremap_resource() should help simplify
that code further.

> +
> +static void bst_write_phys_bst(void __iomem *addr, u32 value)
> +{
> +	iowrite32(value, addr);
> +}

You always pass priv->crm_reg_base into this helper, so
it would be simpler to make it take the sdhci_pltfm_host
pointer and the offset instead of the address.

> +static int bst_sdhci_reallocate_bounce_buffer(struct sdhci_host *host)
> +{
> +	struct mmc_host *mmc = host->mmc;
> +	unsigned int max_blocks;
> +	unsigned int bounce_size;
> +	int ret;
> +
> +	/*
> +	 * Cap the bounce buffer at 64KB. Using a bigger bounce buffer
> +	 * has diminishing returns, this is probably because SD/MMC
> +	 * cards are usually optimized to handle this size of requests.
> +	 */
> +	bounce_size = SZ_32K;

The comment says 64K, but the size you use is 32K.

> +	/* Get CRM registers from the second reg entry */
> +	crm_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);

devm_platform_ioremap_resource()

> +	/*
> +	 * Hardware limitation workaround:
> +	 *
> +	 * Our platform supports 64-bit physical addressing, but the eMMC
> +	 * controller's SRAM-based DMA engine is constrained to a 32-bit
> +	 * address space. When using the standard SDHCI interface, which
> +	 * allocates DDR-based DMA buffers with 64-bit addresses, the
> +	 * dma_map_single() operation fails because the DMA engine cannot
> +	 * handle addresses beyond 32 bits.
> +	 *
> +	 * To resolve this hardware limitation, we implement a bounce buffer
> +	 * allocated via dma_alloc_coherent() to satisfy DMA addressing
> +	 * constraints.
> +	 */
> +	err = bst_sdhci_reallocate_bounce_buffer(host);

Having an explanation here makes sense, but I don't think this
captures what is actually going on, in particular:

- dma_alloc_coherent() being backed by an SRAM that is under
  the 4GB boundary
- the problem that the SoC is configured that all of DRAM
  is outside of ZONE_DMA32
- The type of hardware bug that leads to 64-bit DMA being
  broken in this SoC.

I still have some hope that the hardware is not actually
that broken and you can get it working normally, in one
of these ways:
- enabling 64-bit addressing in the parent bus
- enabling SMMU translation for the parent bus
- configuring the parent bus or the sdhci itself to
  access the first 4GB of RAM, and describing the
  offset in dma-ranges
- moving the start of RAM in a global SoC config

It is rather unlikely that the SoC designer chose to
integrate a 32-bit-only device without adding some
way to configure it to access RAM.

      Arnd