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
| ||
|
Date: Wed, 17 Aug 2022 18:23:02 +0100 From: Robin Murphy <robin.murphy@....com> To: Vadym Kochan <vadym.kochan@...ision.eu>, Adrian Hunter <adrian.hunter@...el.com> Cc: Christoph Hellwig <hch@...radead.org>, Florian Fainelli <f.fainelli@...il.com>, Hu Ziji <huziji@...vell.com>, Ulf Hansson <ulf.hansson@...aro.org>, linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org, Elad Nachman <enachman@...vell.com>, iommu@...ts.linux.dev, Mickey Rachamim <mickeyr@...vell.com> Subject: Re: [PATCH] mmc: sdhci-xenon: Fix 2G limitation on AC5 SoC On 2022-08-17 17:07, Vadym Kochan wrote: > Hi Robin, Adrian, > > On Wed, Aug 17, 2022 at 02:43:46PM +0100, Robin Murphy wrote: >> On 2022-08-16 21:51, Vadym Kochan wrote: >> [...] >>>> The one thing to watch out for is that SWIOTLB doesn't necessarily interact >>>> very well with DMA offsets. Given the intent of >>>> of_dma_get_max_cpu_address(), I think it ought to work out OK now for >>>> current kernels on DT systems if everything is described correctly, but >>>> otherwise it's likely that you end up with ZONE_DMA either being empty or >>>> containing all memory, so the SWIOTLB buffer ends up being allocated >>>> anywhere such that it might not actually work as expected. >>>> >>>> Robin. >>> >>> Hi Robin, >>> >>> Thank you for the reply. >>> >>> My understanding is that swiotlb is allocated (in case of arm64) >>> in the following cases: >>> >>> #1 when it is forced from the kernel cmdline >>> >>> #2 when max_pfn is greater than arm64_dma_phys_limit (and this is used >>> as the end from which to allocate the swiotlb pool in the >>> top-botom direction via memblock API). >>> >>> #3 using restricted dma-pool >>> >>> Of course option #3 works fine because swiotlb is kind of forced to use >>> particulary this range of memory. >>> >>> Both options #1 & #2 causes to use full memory mask even if to specify >>> dma-ranges in the DT: >>> >>> dma-ranges = <0x0 0x0 0x2 0x0 0x0 0x80000000>; >>> >>> or if to specify the opposite: >>> >>> dma-ranges = <0x2 0x0 0x0 0x0 0x0 0x80000000>; >>> >>> just to make it lower than U32 to pass >>> >>> zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits) >>> >>> condition, but then it will be re-set in max_zone_phys() by: >>> >>> if (phys_start > U32_MAX) >>> zone_mask = PHYS_ADDR_MAX; >>> else if (phys_start > zone_mask) >>> zone_mask = U32_MAX; >> >> Ah, indeed I missed that, sorry. It seems that that change to stop assuming >> an offset kind of crossed over with the introduction of >> *_dma_get_max_cpu_address(), but now that that firmware property parsing >> *is* implemented, in principle it should be equally possible to evaluate the >> actual offsets as well, and decide whether an offset ZONE_DMA is appropriate >> or not. Either way, this is definitely the area which needs work if we want >> to to able to support topologies like this properly. >> >>> So, currently I dont see how to pin swiotlb (I see it as a main problem) to some specific range of physical >>> memory (particulary to the first 2G of RAM). >> >> Indeed, if ZONE_DMA and/or ZONE_DMA32 can't be set appropriately, then >> there's no way to guarantee correct allocation of any DMA buffers, short of >> hacking it with explicitly placed reserved-memory carveouts. >> > > I have sent some time ago a solution which binds restricted-dma pool to > the eMMC device, so Adrian, Robin do you think this can be acceptable as > a temporary solution (at least conceptually) ? > > I was also thinking would it be OK to introduce something like > bounced-dma pool (similar to the restricted one) which will reserve > memory for the bounced buffers only ? It should not be hard as looks > like it will re-use existing interface between dma and swiotlb ? In that > case it would allow to map first 2G of memory to eMMC controller. TBH I'd prefer to fix it (or at least work around it) more generally. Putting made-up things in devicetree to work around shortcomings in kernel code tends to be a hole that's hard to dig yourself back out of. As a bodge that would be just about justifiable in its own terms, does the diff below help at all? Thanks, Robin. ----->8----- diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index b9af30be813e..88f7b26f49db 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -451,7 +451,14 @@ void __init bootmem_init(void) */ void __init mem_init(void) { + /* + * Some platforms still manage to elude our attempt to calculate + * ZONE_DMA appropriately, so encourage the SWIOTLB allocation to go + * as low as it can anyway for the best chance of being usable. + */ + memblock_set_bottom_up(true); swiotlb_init(max_pfn > PFN_DOWN(arm64_dma_phys_limit), SWIOTLB_VERBOSE); + memblock_set_bottom_up(false); /* this will put all unused low memory onto the freelists */ memblock_free_all();
Powered by blists - more mailing lists