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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250711055529.1321072-1-yangzh0906@thundersoft.com>
Date: Fri, 11 Jul 2025 13:55:29 +0800
From: Albert Yang <yangzh0906@...ndersoft.com>
To: arnd@...db.de
Cc: adrian.hunter@...el.com,
	ulf.hansson@...aro.org,
	gordon.ge@....ai,
	linux-mmc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/8] mmc: sdhci: add Black Sesame Technologies BST C1200 controller driver

Hi Arnd,

Thank you for your detailed review and suggestions. I have addressed all the 
issues you raised in v2 of the patch series.

On Wed, Jul 2, 2025, at 11:44, Arnd Bergmann wrote:

> 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

Fixed. Updated the Kconfig description to mention "Black Sesame Technologies 
BST C1200 controller" and moved the config entry to the correct alphabetical 
position between MMC_SDHCI_BCM_KONA and MMC_SDHCI_F_SDH30.

> 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.

Agreed. Removed the unused phy_crm_reg_base and phy_crm_reg_size fields from 
dwcmshc_priv structure and replaced the manual platform_get_resource() + 
ioremap() calls with devm_platform_ioremap_resource().

> 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.

Good suggestion. Replaced bst_write_phys_bst() and bst_read_phys_bst() with 
bst_crm_write() and bst_crm_read() that take sdhci_pltfm_host pointer and 
offset parameters for better encapsulation.

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

Fixed the comment to correctly state 32KB.

> 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.

You're absolutely right. I've enhanced the comment with a detailed explanation 
of our specific hardware constraints:

1. System memory uses 64-bit bus, eMMC controller uses 32-bit bus
2. eMMC controller cannot access memory through SMMU due to hardware bug
3. Our SRAM-based bounce buffer solution works within 32-bit address space

> 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

I appreciate your optimism about finding alternative solutions. Unfortunately, 
we have thoroughly investigated these approaches:

Regarding these last two suggestions, I'm not very familiar with the implementation 
details. Could you provide more guidance on:

1. **dma-ranges approach**: We tried adding these properties to the device tree:
   ```
   dma-ranges = <0x00000000 0x00000000 0x100000000>;
   dma-mask = <0xffffffff>;
   ```
   However, we still encounter DMA addressing issues. Are there specific 
   examples in other drivers we could reference for similar hardware constraints?

2. **Moving RAM start position**: Which component would control this configuration? 
   Would this require bootloader parameter changes or SoC-level configuration?

We're certainly interested in exploring these alternatives if they could provide 
a more elegant solution than our current bounce buffer approach.

The v3 patch addresses all your code structure and documentation concerns 
while providing a clear explanation of why this approach is necessary for 
our platform. I have also fixed compilation warnings about unused variables 
that resulted from the refactoring.

**Current DMA Issues**: Despite setting DMA32_ZONE, we still encounter DMA 
addressing warnings. Key error messages include:
```
DMA addr 0x00000008113e2200+512 overflow (mask ffffffff, bus limit 0)
software IO TLB: swiotlb_memblock_alloc: Failed to allocate [various sizes]
```

This confirms our hardware limitation where the eMMC controller cannot access 
memory above 32-bit boundaries, even with ZONE_DMA32 configured.

The complete kernel log with detailed DMA allocation failures and warnings
is available at: https://pastebin.com/eJgtuHDh

Please let me know if you need any additional information or have suggestions
for alternative approaches.

Best regards,
Albert