| Message ID | 731d204f5f556ad61bbaf004b1d984f83c90b4f5.1724748249.git.baruch@tkos.co.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: mm: fix DMA zone when dma-ranges is missing | expand |
On Tue, Aug 27, 2024 at 11:44:09AM +0300, Baruch Siach wrote: > Some platforms, like Rockchip RK3568 based Odroid M1, do not provide DMA > limits information in device-tree dma-ranges property. Still some device > drivers set DMA limit that relies on DMA zone at low 4GB memory area. > Until commit ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by > zone_dma_limit"), zone_sizes_init() restricted DMA zone to low 32-bit > when there is RAM there. > > Restore DMA zone 32-bit limit for platforms that have RAM in this area. > > Fixes: ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit") > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > This should go via the dma-mapping tree that contains the fixed commit. > --- > arch/arm64/mm/init.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index bfb10969cbf0..7fcd0aaa9bb6 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -116,6 +116,9 @@ static void __init arch_reserve_crashkernel(void) > > static phys_addr_t __init max_zone_phys(phys_addr_t zone_limit) > { > + if (memblock_start_of_DRAM() < U32_MAX) > + zone_limit = min(zone_limit, U32_MAX); > + > return min(zone_limit, memblock_end_of_DRAM() - 1) + 1; > } Thanks both Baruch and Marek for digging into this and finding a solution. Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
On 2024-08-27 9:44 am, Baruch Siach wrote: > Some platforms, like Rockchip RK3568 based Odroid M1, do not provide DMA > limits information in device-tree dma-ranges property. Still some device > drivers set DMA limit that relies on DMA zone at low 4GB memory area. > Until commit ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by > zone_dma_limit"), zone_sizes_init() restricted DMA zone to low 32-bit > when there is RAM there. > > Restore DMA zone 32-bit limit for platforms that have RAM in this area. > > Fixes: ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit") > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > This should go via the dma-mapping tree that contains the fixed commit. > --- > arch/arm64/mm/init.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c > index bfb10969cbf0..7fcd0aaa9bb6 100644 > --- a/arch/arm64/mm/init.c > +++ b/arch/arm64/mm/init.c > @@ -116,6 +116,9 @@ static void __init arch_reserve_crashkernel(void) > > static phys_addr_t __init max_zone_phys(phys_addr_t zone_limit) > { > + if (memblock_start_of_DRAM() < U32_MAX) > + zone_limit = min(zone_limit, U32_MAX); This seems like a bit of a bodge, since it now means we either get the old or the new behaviour depending on where DRAM is, so if, say, RAM starts at 3GB but all devices have a 3GB DMA offset, we still can't have a properly-sized DMA zone. Really it comes down to this change: - zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits); + zone_dma_limit = min(dt_zone_dma_limit, acpi_zone_dma_limit); where although the "32" represented the assumption of RAM starting at 0, it also implicitly removed the handling of the case where no DMA ranges are specified. Per the commit message, it's that which we want to account for, not the placement of RAM. I think the more correct version should be: if (zone_limit == PHYS_ADDR_MAX) zone_limit = U32_MAX; Thanks, Robin. > + > return min(zone_limit, memblock_end_of_DRAM() - 1) + 1; > } >
Hi Robin, On Tue, Aug 27 2024, Robin Murphy wrote: > On 2024-08-27 9:44 am, Baruch Siach wrote: >> Some platforms, like Rockchip RK3568 based Odroid M1, do not provide DMA >> limits information in device-tree dma-ranges property. Still some device >> drivers set DMA limit that relies on DMA zone at low 4GB memory area. >> Until commit ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by >> zone_dma_limit"), zone_sizes_init() restricted DMA zone to low 32-bit >> when there is RAM there. >> Restore DMA zone 32-bit limit for platforms that have RAM in this area. >> Fixes: ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit") >> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Baruch Siach <baruch@tkos.co.il> >> --- >> This should go via the dma-mapping tree that contains the fixed commit. >> --- >> arch/arm64/mm/init.c | 3 +++ >> 1 file changed, 3 insertions(+) >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c >> index bfb10969cbf0..7fcd0aaa9bb6 100644 >> --- a/arch/arm64/mm/init.c >> +++ b/arch/arm64/mm/init.c >> @@ -116,6 +116,9 @@ static void __init arch_reserve_crashkernel(void) >> static phys_addr_t __init max_zone_phys(phys_addr_t zone_limit) >> { >> + if (memblock_start_of_DRAM() < U32_MAX) >> + zone_limit = min(zone_limit, U32_MAX); > > This seems like a bit of a bodge, since it now means we either get the old or > the new behaviour depending on where DRAM is, so if, say, RAM starts at 3GB > but all devices have a 3GB DMA offset, we still can't have a properly-sized > DMA zone. I'm not following your example here. What is "3GB DMA offset"? Is it relative to 0? Relative to start of RAM? > Really it comes down to this change: > > - zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_zone_dma_bits); > + zone_dma_limit = min(dt_zone_dma_limit, acpi_zone_dma_limit); > > where although the "32" represented the assumption of RAM starting at 0, it > also implicitly removed the handling of the case where no DMA ranges are > specified. Per the commit message, it's that which we want to account for, not > the placement of RAM. > > I think the more correct version should be: > > if (zone_limit == PHYS_ADDR_MAX) > zone_limit = U32_MAX; I like your proposal. The condition of no dma-ranges data is explicit. I'll post v2 later today. Thanks, baruch
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index bfb10969cbf0..7fcd0aaa9bb6 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -116,6 +116,9 @@ static void __init arch_reserve_crashkernel(void) static phys_addr_t __init max_zone_phys(phys_addr_t zone_limit) { + if (memblock_start_of_DRAM() < U32_MAX) + zone_limit = min(zone_limit, U32_MAX); + return min(zone_limit, memblock_end_of_DRAM() - 1) + 1; }
Some platforms, like Rockchip RK3568 based Odroid M1, do not provide DMA limits information in device-tree dma-ranges property. Still some device drivers set DMA limit that relies on DMA zone at low 4GB memory area. Until commit ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit"), zone_sizes_init() restricted DMA zone to low 32-bit when there is RAM there. Restore DMA zone 32-bit limit for platforms that have RAM in this area. Fixes: ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit") Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- This should go via the dma-mapping tree that contains the fixed commit. --- arch/arm64/mm/init.c | 3 +++ 1 file changed, 3 insertions(+)