| Message ID | f8cef6845a6141f0277e31a71fe153612daae776.1731436631.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dma-mapping: Save base/size instead of pointer to shared DMA pool | expand |
On 11/12/2024 10:39 AM, Geert Uytterhoeven wrote: > On RZ/Five, which is non-coherent, and uses CONFIG_DMA_GLOBAL_POOL=y: > > Oops - store (or AMO) access fault [#1] > CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-rc1-00015-g8a6e02d0c00e #201 > Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT) > epc : __memset+0x60/0x100 > ra : __dma_alloc_from_coherent+0x150/0x17a > epc : ffffffff8062d2bc ra : ffffffff80053a94 sp : ffffffc60000ba20 > gp : ffffffff812e9938 tp : ffffffd601920000 t0 : ffffffc6000d0000 > t1 : 0000000000000000 t2 : ffffffffe9600000 s0 : ffffffc60000baa0 > s1 : ffffffc6000d0000 a0 : ffffffc6000d0000 a1 : 0000000000000000 > a2 : 0000000000001000 a3 : ffffffc6000d1000 a4 : 0000000000000000 > a5 : 0000000000000000 a6 : ffffffd601adacc0 a7 : ffffffd601a841a8 > s2 : ffffffd6018573c0 s3 : 0000000000001000 s4 : ffffffd6019541e0 > s5 : 0000000200000022 s6 : ffffffd6018f8410 s7 : ffffffd6018573e8 > s8 : 0000000000000001 s9 : 0000000000000001 s10: 0000000000000010 > s11: 0000000000000000 t3 : 0000000000000000 t4 : ffffffffdefe62d1 > t5 : 000000001cd6a3a9 t6 : ffffffd601b2aad6 > status: 0000000200000120 badaddr: ffffffc6000d0000 cause: 0000000000000007 > [<ffffffff8062d2bc>] __memset+0x60/0x100 > [<ffffffff80053e1a>] dma_alloc_from_global_coherent+0x1c/0x28 > [<ffffffff80053056>] dma_direct_alloc+0x98/0x112 > [<ffffffff8005238c>] dma_alloc_attrs+0x78/0x86 > [<ffffffff8035fdb4>] rz_dmac_probe+0x3f6/0x50a > [<ffffffff803a0694>] platform_probe+0x4c/0x8a > > If CONFIG_DMA_GLOBAL_POOL=y, the reserved_mem structure passed to > rmem_dma_setup() is saved for later use, by saving the passed pointer. > However, when dma_init_reserved_memory() is called later, the pointer > has become stale, causing a crash. > > E.g. in the RZ/Five case, the referenced memory now contains the > reserved_mem structure for the "mmode_resv0@30000" node (with base > 0x30000 and size 0x10000), instead of the correct "pma_resv0@58000000" > node (with base 0x58000000 and size 0x8000000). > > Fix this by saving the needed reserved_mem structure's contents instead. > > Fixes: 8a6e02d0c00e7b62 ("of: reserved_mem: Restructure how the reserved memory regions are processed") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Oreoluwa Babatunde <quic_obabatun@quicinc.com> > --- > kernel/dma/coherent.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c > index ff5683a57f77126d..3b2bdca9f1d4b027 100644 > --- a/kernel/dma/coherent.c > +++ b/kernel/dma/coherent.c > @@ -330,7 +330,8 @@ int dma_init_global_coherent(phys_addr_t phys_addr, size_t size) > #include <linux/of_reserved_mem.h> > > #ifdef CONFIG_DMA_GLOBAL_POOL > -static struct reserved_mem *dma_reserved_default_memory __initdata; > +static phys_addr_t dma_reserved_default_memory_base __initdata; > +static phys_addr_t dma_reserved_default_memory_size __initdata; > #endif > > static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev) > @@ -376,9 +377,10 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) > > #ifdef CONFIG_DMA_GLOBAL_POOL > if (of_get_flat_dt_prop(node, "linux,dma-default", NULL)) { > - WARN(dma_reserved_default_memory, > + WARN(dma_reserved_default_memory_size, > "Reserved memory: region for default DMA coherent area is redefined\n"); > - dma_reserved_default_memory = rmem; > + dma_reserved_default_memory_base = rmem->base; > + dma_reserved_default_memory_size = rmem->size; > } > #endif > > @@ -391,10 +393,10 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) > #ifdef CONFIG_DMA_GLOBAL_POOL > static int __init dma_init_reserved_memory(void) > { > - if (!dma_reserved_default_memory) > + if (!dma_reserved_default_memory_size) > return -ENOMEM; > - return dma_init_global_coherent(dma_reserved_default_memory->base, > - dma_reserved_default_memory->size); > + return dma_init_global_coherent(dma_reserved_default_memory_base, > + dma_reserved_default_memory_size); > } > core_initcall(dma_init_reserved_memory); > #endif /* CONFIG_DMA_GLOBAL_POOL */ Hi Geert, Thanks for providing a fix! I also looked around more to see if there are more places that could store a stale reference to rmem but I didn't see any others, so I think we are good! I have given my "reviewed-by" tag as well. Thank you! Oreoluwa
Thanks Geert, I've applied the patch to the dma-mapping for-next tree.
On Tue, Nov 12, 2024 at 6:41 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > > On RZ/Five, which is non-coherent, and uses CONFIG_DMA_GLOBAL_POOL=y: > > Oops - store (or AMO) access fault [#1] > CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-rc1-00015-g8a6e02d0c00e #201 > Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT) > epc : __memset+0x60/0x100 > ra : __dma_alloc_from_coherent+0x150/0x17a > epc : ffffffff8062d2bc ra : ffffffff80053a94 sp : ffffffc60000ba20 > gp : ffffffff812e9938 tp : ffffffd601920000 t0 : ffffffc6000d0000 > t1 : 0000000000000000 t2 : ffffffffe9600000 s0 : ffffffc60000baa0 > s1 : ffffffc6000d0000 a0 : ffffffc6000d0000 a1 : 0000000000000000 > a2 : 0000000000001000 a3 : ffffffc6000d1000 a4 : 0000000000000000 > a5 : 0000000000000000 a6 : ffffffd601adacc0 a7 : ffffffd601a841a8 > s2 : ffffffd6018573c0 s3 : 0000000000001000 s4 : ffffffd6019541e0 > s5 : 0000000200000022 s6 : ffffffd6018f8410 s7 : ffffffd6018573e8 > s8 : 0000000000000001 s9 : 0000000000000001 s10: 0000000000000010 > s11: 0000000000000000 t3 : 0000000000000000 t4 : ffffffffdefe62d1 > t5 : 000000001cd6a3a9 t6 : ffffffd601b2aad6 > status: 0000000200000120 badaddr: ffffffc6000d0000 cause: 0000000000000007 > [<ffffffff8062d2bc>] __memset+0x60/0x100 > [<ffffffff80053e1a>] dma_alloc_from_global_coherent+0x1c/0x28 > [<ffffffff80053056>] dma_direct_alloc+0x98/0x112 > [<ffffffff8005238c>] dma_alloc_attrs+0x78/0x86 > [<ffffffff8035fdb4>] rz_dmac_probe+0x3f6/0x50a > [<ffffffff803a0694>] platform_probe+0x4c/0x8a > > If CONFIG_DMA_GLOBAL_POOL=y, the reserved_mem structure passed to > rmem_dma_setup() is saved for later use, by saving the passed pointer. > However, when dma_init_reserved_memory() is called later, the pointer > has become stale, causing a crash. > > E.g. in the RZ/Five case, the referenced memory now contains the > reserved_mem structure for the "mmode_resv0@30000" node (with base > 0x30000 and size 0x10000), instead of the correct "pma_resv0@58000000" > node (with base 0x58000000 and size 0x8000000). > > Fix this by saving the needed reserved_mem structure's contents instead. > > Fixes: 8a6e02d0c00e7b62 ("of: reserved_mem: Restructure how the reserved memory regions are processed") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > kernel/dma/coherent.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Cheers, Prabhakar
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c index ff5683a57f77126d..3b2bdca9f1d4b027 100644 --- a/kernel/dma/coherent.c +++ b/kernel/dma/coherent.c @@ -330,7 +330,8 @@ int dma_init_global_coherent(phys_addr_t phys_addr, size_t size) #include <linux/of_reserved_mem.h> #ifdef CONFIG_DMA_GLOBAL_POOL -static struct reserved_mem *dma_reserved_default_memory __initdata; +static phys_addr_t dma_reserved_default_memory_base __initdata; +static phys_addr_t dma_reserved_default_memory_size __initdata; #endif static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev) @@ -376,9 +377,10 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) #ifdef CONFIG_DMA_GLOBAL_POOL if (of_get_flat_dt_prop(node, "linux,dma-default", NULL)) { - WARN(dma_reserved_default_memory, + WARN(dma_reserved_default_memory_size, "Reserved memory: region for default DMA coherent area is redefined\n"); - dma_reserved_default_memory = rmem; + dma_reserved_default_memory_base = rmem->base; + dma_reserved_default_memory_size = rmem->size; } #endif @@ -391,10 +393,10 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) #ifdef CONFIG_DMA_GLOBAL_POOL static int __init dma_init_reserved_memory(void) { - if (!dma_reserved_default_memory) + if (!dma_reserved_default_memory_size) return -ENOMEM; - return dma_init_global_coherent(dma_reserved_default_memory->base, - dma_reserved_default_memory->size); + return dma_init_global_coherent(dma_reserved_default_memory_base, + dma_reserved_default_memory_size); } core_initcall(dma_init_reserved_memory); #endif /* CONFIG_DMA_GLOBAL_POOL */
On RZ/Five, which is non-coherent, and uses CONFIG_DMA_GLOBAL_POOL=y: Oops - store (or AMO) access fault [#1] CPU: 0 UID: 0 PID: 1 Comm: swapper Not tainted 6.12.0-rc1-00015-g8a6e02d0c00e #201 Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT) epc : __memset+0x60/0x100 ra : __dma_alloc_from_coherent+0x150/0x17a epc : ffffffff8062d2bc ra : ffffffff80053a94 sp : ffffffc60000ba20 gp : ffffffff812e9938 tp : ffffffd601920000 t0 : ffffffc6000d0000 t1 : 0000000000000000 t2 : ffffffffe9600000 s0 : ffffffc60000baa0 s1 : ffffffc6000d0000 a0 : ffffffc6000d0000 a1 : 0000000000000000 a2 : 0000000000001000 a3 : ffffffc6000d1000 a4 : 0000000000000000 a5 : 0000000000000000 a6 : ffffffd601adacc0 a7 : ffffffd601a841a8 s2 : ffffffd6018573c0 s3 : 0000000000001000 s4 : ffffffd6019541e0 s5 : 0000000200000022 s6 : ffffffd6018f8410 s7 : ffffffd6018573e8 s8 : 0000000000000001 s9 : 0000000000000001 s10: 0000000000000010 s11: 0000000000000000 t3 : 0000000000000000 t4 : ffffffffdefe62d1 t5 : 000000001cd6a3a9 t6 : ffffffd601b2aad6 status: 0000000200000120 badaddr: ffffffc6000d0000 cause: 0000000000000007 [<ffffffff8062d2bc>] __memset+0x60/0x100 [<ffffffff80053e1a>] dma_alloc_from_global_coherent+0x1c/0x28 [<ffffffff80053056>] dma_direct_alloc+0x98/0x112 [<ffffffff8005238c>] dma_alloc_attrs+0x78/0x86 [<ffffffff8035fdb4>] rz_dmac_probe+0x3f6/0x50a [<ffffffff803a0694>] platform_probe+0x4c/0x8a If CONFIG_DMA_GLOBAL_POOL=y, the reserved_mem structure passed to rmem_dma_setup() is saved for later use, by saving the passed pointer. However, when dma_init_reserved_memory() is called later, the pointer has become stale, causing a crash. E.g. in the RZ/Five case, the referenced memory now contains the reserved_mem structure for the "mmode_resv0@30000" node (with base 0x30000 and size 0x10000), instead of the correct "pma_resv0@58000000" node (with base 0x58000000 and size 0x8000000). Fix this by saving the needed reserved_mem structure's contents instead. Fixes: 8a6e02d0c00e7b62 ("of: reserved_mem: Restructure how the reserved memory regions are processed") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- kernel/dma/coherent.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)