crypto: ccree: driver does not handle case where cryptlen = authsize =0

Hello

Booting a salvator X with CRYPTO_MANAGER_EXTRA_TESTS lead to:

[   37.510039] ------------[ cut here ]------------
[   37.514712] WARNING: CPU: 7 PID: 616 at /home/clabbe/linux-next/kernel/dma/mapping.c:200 __dma_map_sg_attrs+0xb8/0x108
[   37.525444] Modules linked in: des_generic ofb snd_soc_hdmi_codec dw_hdmi_cec dw_hdmi_i2s_audio rcar_du_drm rcar_lvds crct10dif_ce snd_soc_audio_graph_card simple_bridge snd_soc_simple_card_utils rcar_dw_hdmi dw_hdmi drm_cma_helper cec rcar_csi2 rcar_fdp1 v4l2_mem2mem vsp1 rcar_cmm videobuf2_vmalloc rcar_fcp renesas_usb3 snd_soc_rcar rcar_vin v4l2_fwnode v4l2_async videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common videodev pwm_rcar mc snd_soc_ak4613 max9611 ccree(+) renesas_usbhs phy_rcar_gen3_usb3 libdes usb_dmac gpio_bd9571mwv at24 display_connector drm_kms_helper pwm_bl drm ecb xts cts essiv cmac xcbc ccm
[   37.581446] CPU: 7 PID: 616 Comm: cryptomgr_test Tainted: G        W         5.17.0-rc3-next-20220207-00110-g41bd5fbfc1e4 #5
[   37.592672] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[   37.599555] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   37.606526] pc : __dma_map_sg_attrs+0xb8/0x108
[   37.610978] lr : __dma_map_sg_attrs+0x48/0x108
[   37.615430] sp : ffff80000c1632a0
[   37.618749] x29: ffff80000c1632a0 x28: ffff0004c2997c00 x27: ffff0004c2997e20
[   37.625909] x26: 0000000000000000 x25: 0000000000000008 x24: 0000000000000000
[   37.633068] x23: 0000000000000000 x22: ffff0004cc777840 x21: 0000000000000000
[   37.640226] x20: 0000000000000000 x19: ffff0004c164c010 x18: ffff80000b3a72f0
[   37.647385] x17: 0000000000000000 x16: 0000000000000072 x15: 0000000000000010
[   37.654544] x14: 001fffffffffffff x13: 000000006894887f x12: 818235788794af1b
[   37.661703] x11: 00000000000ef000 x10: ffff0004c164c518 x9 : ffff800009aa6a38
[   37.668862] x8 : ffff80000b2759d8 x7 : ffff80000c1633a0 x6 : 0000000000000000
[   37.676021] x5 : ffff800009aa6a38 x4 : 0000000000000000 x3 : 0000000000000000
[   37.683179] x2 : 74374306a1f1c500 x1 : 0000000000000000 x0 : 0000000000000000
[   37.690338] Call trace:
[   37.692789]  __dma_map_sg_attrs+0xb8/0x108
[   37.696894]  dma_map_sg_attrs+0x10/0x28
[   37.700739]  cc_map_sg+0x64/0xf8 [ccree]
[   37.704711]  cc_map_aead_request+0x158/0x920 [ccree]
[   37.709703]  cc_proc_aead+0x148/0xeb8 [ccree]
[   37.714087]  cc_aead_encrypt+0x44/0x68 [ccree]
[   37.718557]  crypto_aead_encrypt+0x44/0x78
[   37.722669]  test_aead_vec_cfg+0x244/0x900
[   37.726779]  test_aead_vec+0x8c/0x190
[   37.730450]  test_aead+0x78/0xe0
[   37.733687]  alg_test_aead+0xa4/0x3e0
[   37.737358]  alg_test.part.29+0xb4/0x398
[   37.741289]  alg_test+0x48/0x78
[   37.744439]  cryptomgr_test+0x48/0x50
[   37.748109]  kthread+0x11c/0x128
[   37.751347]  ret_from_fork+0x10/0x20
[   37.754939] irq event stamp: 119412
[   37.758431] hardirqs last  enabled at (119411): [<ffff800009188a54>] _raw_spin_unlock_irqrestore+0x8c/0x90
[   37.768105] hardirqs last disabled at (119412): [<ffff800009179e20>] el1_dbg+0x28/0x90
[   37.776040] softirqs last  enabled at (119376): [<ffff80000125ac20>] cc_send_request+0xa0/0x208 [ccree]
[   37.785462] softirqs last disabled at (119374): [<ffff80000125abf4>] cc_send_request+0x74/0x208 [ccree]
[   37.794883] ---[ end trace 0000000000000000 ]---#

When cryptlen and authsize are both 0, the driver try to map a 0 size buffer which display this warning.

Hi,

Quoted 47 lines · Corentin Labbe

Hello

Booting a salvator X with CRYPTO_MANAGER_EXTRA_TESTS lead to:

[   37.510039] ------------[ cut here ]------------
[   37.514712] WARNING: CPU: 7 PID: 616 at /home/clabbe/linux-next/kernel/dma/mapping.c:200 __dma_map_sg_attrs+0xb8/0x108
[   37.525444] Modules linked in: des_generic ofb snd_soc_hdmi_codec dw_hdmi_cec dw_hdmi_i2s_audio rcar_du_drm rcar_lvds crct10dif_ce snd_soc_audio_graph_card simple_bridge snd_soc_simple_card_utils rcar_dw_hdmi dw_hdmi drm_cma_helper cec rcar_csi2 rcar_fdp1 v4l2_mem2mem vsp1 rcar_cmm videobuf2_vmalloc rcar_fcp renesas_usb3 snd_soc_rcar rcar_vin v4l2_fwnode v4l2_async videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common videodev pwm_rcar mc snd_soc_ak4613 max9611 ccree(+) renesas_usbhs phy_rcar_gen3_usb3 libdes usb_dmac gpio_bd9571mwv at24 display_connector drm_kms_helper pwm_bl drm ecb xts cts essiv cmac xcbc ccm
[   37.581446] CPU: 7 PID: 616 Comm: cryptomgr_test Tainted: G        W         5.17.0-rc3-next-20220207-00110-g41bd5fbfc1e4 #5
[   37.592672] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[   37.599555] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   37.606526] pc : __dma_map_sg_attrs+0xb8/0x108
[   37.610978] lr : __dma_map_sg_attrs+0x48/0x108
[   37.615430] sp : ffff80000c1632a0
[   37.618749] x29: ffff80000c1632a0 x28: ffff0004c2997c00 x27: ffff0004c2997e20
[   37.625909] x26: 0000000000000000 x25: 0000000000000008 x24: 0000000000000000
[   37.633068] x23: 0000000000000000 x22: ffff0004cc777840 x21: 0000000000000000
[   37.640226] x20: 0000000000000000 x19: ffff0004c164c010 x18: ffff80000b3a72f0
[   37.647385] x17: 0000000000000000 x16: 0000000000000072 x15: 0000000000000010
[   37.654544] x14: 001fffffffffffff x13: 000000006894887f x12: 818235788794af1b
[   37.661703] x11: 00000000000ef000 x10: ffff0004c164c518 x9 : ffff800009aa6a38
[   37.668862] x8 : ffff80000b2759d8 x7 : ffff80000c1633a0 x6 : 0000000000000000
[   37.676021] x5 : ffff800009aa6a38 x4 : 0000000000000000 x3 : 0000000000000000
[   37.683179] x2 : 74374306a1f1c500 x1 : 0000000000000000 x0 : 0000000000000000
[   37.690338] Call trace:
[   37.692789]  __dma_map_sg_attrs+0xb8/0x108
[   37.696894]  dma_map_sg_attrs+0x10/0x28
[   37.700739]  cc_map_sg+0x64/0xf8 [ccree]
[   37.704711]  cc_map_aead_request+0x158/0x920 [ccree]
[   37.709703]  cc_proc_aead+0x148/0xeb8 [ccree]
[   37.714087]  cc_aead_encrypt+0x44/0x68 [ccree]
[   37.718557]  crypto_aead_encrypt+0x44/0x78
[   37.722669]  test_aead_vec_cfg+0x244/0x900
[   37.726779]  test_aead_vec+0x8c/0x190
[   37.730450]  test_aead+0x78/0xe0
[   37.733687]  alg_test_aead+0xa4/0x3e0
[   37.737358]  alg_test.part.29+0xb4/0x398
[   37.741289]  alg_test+0x48/0x78
[   37.744439]  cryptomgr_test+0x48/0x50
[   37.748109]  kthread+0x11c/0x128
[   37.751347]  ret_from_fork+0x10/0x20
[   37.754939] irq event stamp: 119412
[   37.758431] hardirqs last  enabled at (119411): [<ffff800009188a54>] _raw_spin_unlock_irqrestore+0x8c/0x90
[   37.768105] hardirqs last disabled at (119412): [<ffff800009179e20>] el1_dbg+0x28/0x90
[   37.776040] softirqs last  enabled at (119376): [<ffff80000125ac20>] cc_send_request+0xa0/0x208 [ccree]
[   37.785462] softirqs last disabled at (119374): [<ffff80000125abf4>] cc_send_request+0x74/0x208 [ccree]
[   37.794883] ---[ end trace 0000000000000000 ]---#

Quoted 1 line

When cryptlen and authsize are both 0, the driver try to map a 0 size buffer which display this warning.

Thank you very much, Corentin!

I thought we squashed all these corner case size bugs but it seems this is not the case...

I am away from my test equipment right now for covid related reasons but will dig into this (and your other patch), hopefully early next week.

Thanks,
Gilad

--
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

So, I still do not have access to my Salvator-X board but I attempted a fix nevertheless and it does not seem to trigger new failures on the boards I do have so please give it a shot and let me know.

And thanks again for taking the time to report this!

Cheers,
Gilad

Hello

While testing your patch for this problem, I saw another warning (unrelated with your patch):

[   34.061953] ------------[ cut here ]------------
[   34.066656] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[   34.076555] WARNING: CPU: 3 PID: 475 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[   34.086748] Modules linked in:
[   34.089809] CPU: 3 PID: 475 Comm: cryptomgr_test Tainted: G        W         5.17.0-rc3-next-20220210-00115-g1ca9f29d9780-dirty #25
[   34.101635] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[   34.108508] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   34.115471] pc : add_dma_entry+0x1d0/0x288
[   34.119569] lr : add_dma_entry+0x1d0/0x288
[   34.123664] sp : ffff80000e3236f0
[   34.126976] x29: ffff80000e3236f0 x28: 0000000000000000 x27: 0000000000000000
[   34.134122] x26: 0000000000000000 x25: ffff80000b3c76c0 x24: ffff80000b3c7000
[   34.141267] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a8c1000
[   34.148410] x20: ffff0004c0b93f80 x19: ffff80000b320000 x18: ffffffffffffffff
[   34.155554] x17: 6c7265766f202c54 x16: 534958454520676e x15: 00000000000001fa
[   34.162697] x14: ffff80000e3233f0 x13: 00000000ffffffea x12: ffff80000b3bdda0
[   34.169840] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b3a5db8
[   34.176984] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b3a5d60
[   34.184126] x5 : 0000000000057fa8 x4 : 0000000000000000 x3 : 00000000ffffefff
[   34.191269] x2 : ffff80000b34db38 x1 : 371dfc0f85526100 x0 : 0000000000000000
[   34.198411] Call trace:
[   34.200856]  add_dma_entry+0x1d0/0x288
[   34.204604]  debug_dma_map_sg+0x150/0x398
[   34.208615]  __dma_map_sg_attrs+0x9c/0x108
[   34.212717]  dma_map_sg_attrs+0x10/0x28
[   34.216554]  cc_map_sg+0x80/0x100
[   34.219875]  cc_map_cipher_request+0x170/0x3c8
[   34.224319]  cc_cipher_process+0x210/0xb58
[   34.228415]  cc_cipher_encrypt+0x2c/0x38
[   34.232338]  crypto_skcipher_encrypt+0x44/0x78
[   34.236786]  drbg_kcapi_sym_ctr+0xdc/0x1f8
[   34.240887]  drbg_ctr_generate+0x5c/0x98
[   34.244812]  drbg_kcapi_random+0xb4/0x260
[   34.248823]  alg_test_stdrng+0xf8/0x2b0
[   34.252661]  alg_test.part.29+0xb4/0x398
[   34.256583]  alg_test+0x48/0x78
[   34.259724]  cryptomgr_test+0x48/0x50

[   34.263386]  kthread+0x11c/0x128
[   34.266619]  ret_from_fork+0x10/0x20
[   34.270198] irq event stamp: 318
[   34.273424] hardirqs last  enabled at (317): [<ffff8000081300e0>] __up_console_sem+0x60/0x98
[   34.281869] hardirqs last disabled at (318): [<ffff800009bfbb18>] el1_dbg+0x28/0x90
[   34.289531] softirqs last  enabled at (314): [<ffff800008010550>] __do_softirq+0x4a8/0x5ec
[   34.297796] softirqs last disabled at (305): [<ffff8000080ab644>] irq_exit_rcu+0x18c/0x1b0
[   34.306061] ---[ end trace 0000000000000000 ]---

Hi,

Quoted 1 line · Corentin Labbe

...

Quoted 4 lines

Hello

While testing your patch for this problem, I saw another warning (unrelated with your patch):

Dear Corentin, you are a treasure trove of bug reports. I love it. Thank you! :-)

Quoted 37 lines

[   34.061953] ------------[ cut here ]------------
[   34.066656] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[   34.076555] WARNING: CPU: 3 PID: 475 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[   34.086748] Modules linked in:
[   34.089809] CPU: 3 PID: 475 Comm: cryptomgr_test Tainted: G        W         5.17.0-rc3-next-20220210-00115-g1ca9f29d9780-dirty #25
[   34.101635] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[   34.108508] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   34.115471] pc : add_dma_entry+0x1d0/0x288
[   34.119569] lr : add_dma_entry+0x1d0/0x288
[   34.123664] sp : ffff80000e3236f0
[   34.126976] x29: ffff80000e3236f0 x28: 0000000000000000 x27: 0000000000000000
[   34.134122] x26: 0000000000000000 x25: ffff80000b3c76c0 x24: ffff80000b3c7000
[   34.141267] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a8c1000
[   34.148410] x20: ffff0004c0b93f80 x19: ffff80000b320000 x18: ffffffffffffffff
[   34.155554] x17: 6c7265766f202c54 x16: 534958454520676e x15: 00000000000001fa
[   34.162697] x14: ffff80000e3233f0 x13: 00000000ffffffea x12: ffff80000b3bdda0
[   34.169840] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b3a5db8
[   34.176984] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b3a5d60
[   34.184126] x5 : 0000000000057fa8 x4 : 0000000000000000 x3 : 00000000ffffefff
[   34.191269] x2 : ffff80000b34db38 x1 : 371dfc0f85526100 x0 : 0000000000000000
[   34.198411] Call trace:
[   34.200856]  add_dma_entry+0x1d0/0x288
[   34.204604]  debug_dma_map_sg+0x150/0x398
[   34.208615]  __dma_map_sg_attrs+0x9c/0x108
[   34.212717]  dma_map_sg_attrs+0x10/0x28
[   34.216554]  cc_map_sg+0x80/0x100
[   34.219875]  cc_map_cipher_request+0x170/0x3c8
[   34.224319]  cc_cipher_process+0x210/0xb58
[   34.228415]  cc_cipher_encrypt+0x2c/0x38
[   34.232338]  crypto_skcipher_encrypt+0x44/0x78
[   34.236786]  drbg_kcapi_sym_ctr+0xdc/0x1f8
[   34.240887]  drbg_ctr_generate+0x5c/0x98
[   34.244812]  drbg_kcapi_random+0xb4/0x260
[   34.248823]  alg_test_stdrng+0xf8/0x2b0
[   34.252661]  alg_test.part.29+0xb4/0x398
[   34.256583]  alg_test+0x48/0x78
[   34.259724]  cryptomgr_test+0x48/0x50

Quoted 9 lines

[   34.263386]  kthread+0x11c/0x128
[   34.266619]  ret_from_fork+0x10/0x20
[   34.270198] irq event stamp: 318
[   34.273424] hardirqs last  enabled at (317): [<ffff8000081300e0>] __up_console_sem+0x60/0x98
[   34.281869] hardirqs last disabled at (318): [<ffff800009bfbb18>] el1_dbg+0x28/0x90
[   34.289531] softirqs last  enabled at (314): [<ffff800008010550>] __do_softirq+0x4a8/0x5ec
[   34.297796] softirqs last disabled at (305): [<ffff8000080ab644>] irq_exit_rcu+0x18c/0x1b0
[   34.306061] ---[ end trace 0000000000000000 ]---

So, this is an interesting one.
What I *think* is happening is that the drbg implementation is actually doing something naughty: it is passing the same exact memory buffer, both as source and destination to an encryption operation to the crypto skcipher API, BUT via two different scatter gather lists.

I'm not sure but I believe this is not a legitimate use of the API, but before we even go into this, let's see if this little fix helps at all and this is indeed the root cause.

Can you test this small change for me, please?

crypto/drbg.c +0 -0

index 177983b6ae38..13824fd27627 100644

Quoted 15 lines · Gilad Ben-Yossef

Hi,

Quoted 1 line · Corentin Labbe

...

Quoted 4 lines

Hello

While testing your patch for this problem, I saw another warning (unrelated with your patch):

Dear Corentin, you are a treasure trove of bug reports. I love it. Thank you! :-)

Quoted 1 line

[   34.061953] ------------[ cut here ]------------

Quoted 25 lines

So, this is an interesting one.
What I *think* is happening is that the drbg implementation is actually doing something naughty: it is passing the same exact memory buffer, both as source and destination to an encryption operation to the crypto skcipher API, BUT via two different scatter gather lists.

I'm not sure but I believe this is not a legitimate use of the API, but before we even go into this, let's see if this little fix helps at all and this is indeed the root cause.

Can you test this small change for me, please?

crypto/drbg.c +1 -1

index 177983b6ae38..13824fd27627 100644
@@ -1851,7 +1851,7 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
                /* Use scratchpad for in-place operation */
                inlen = scratchpad_use;
                memset(drbg->outscratchpad, 0, scratchpad_use);
-               sg_set_buf(sg_in, drbg->outscratchpad, scratchpad_use);
+               sg_in = sg_out;
        }
 

crypto/drbg.c static int drbg_kcapi_sym_ctr(struct drbg_state *drbg, · line 1857

Quoted 2 lines

crypto/drbg.c +0 -0

        while (outlen) {

No more stacktrace !

Quoted 17 lines · Corentin Labbe

Quoted 15 lines · Gilad Ben-Yossef

Hi,

Quoted 1 line · Corentin Labbe

...

Quoted 4 lines

Hello

While testing your patch for this problem, I saw another warning (unrelated with your patch):

Dear Corentin, you are a treasure trove of bug reports. I love it. Thank you! :-)

Quoted 1 line

[   34.061953] ------------[ cut here ]------------

...

Quoted 25 lines

Quoted 25 lines

So, this is an interesting one.
What I *think* is happening is that the drbg implementation is actually doing something naughty: it is passing the same exact memory buffer, both as source and destination to an encryption operation to the crypto skcipher API, BUT via two different scatter gather lists.

I'm not sure but I believe this is not a legitimate use of the API, but before we even go into this, let's see if this little fix helps at all and this is indeed the root cause.

Can you test this small change for me, please?

crypto/drbg.c +1 -1

index 177983b6ae38..13824fd27627 100644
@@ -1851,7 +1851,7 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
                /* Use scratchpad for in-place operation */
                inlen = scratchpad_use;
                memset(drbg->outscratchpad, 0, scratchpad_use);
-               sg_set_buf(sg_in, drbg->outscratchpad, scratchpad_use);
+               sg_in = sg_out;
        }
 

Quoted 4 lines

crypto/drbg.c static int drbg_kcapi_sym_ctr(struct drbg_state *drbg, · line 1857

Quoted 2 lines

crypto/drbg.c +0 -0

        while (outlen) {

No more stacktrace !

Thank you. I will send a patch later today.

--
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

Hi,

Quoted 51 lines · Gilad Ben-Yossef

Quoted 17 lines · Corentin Labbe

Quoted 15 lines · Gilad Ben-Yossef

Hi,

Quoted 1 line · Corentin Labbe

...

Quoted 4 lines

Hello

While testing your patch for this problem, I saw another warning (unrelated with your patch):

Dear Corentin, you are a treasure trove of bug reports. I love it. Thank you! :-)

Quoted 1 line

[   34.061953] ------------[ cut here ]------------

...

Quoted 25 lines

Quoted 25 lines

So, this is an interesting one.
What I *think* is happening is that the drbg implementation is actually doing something naughty: it is passing the same exact memory buffer, both as source and destination to an encryption operation to the crypto skcipher API, BUT via two different scatter gather lists.

I'm not sure but I believe this is not a legitimate use of the API, but before we even go into this, let's see if this little fix helps at all and this is indeed the root cause.

Can you test this small change for me, please?

crypto/drbg.c +1 -1

index 177983b6ae38..13824fd27627 100644
@@ -1851,7 +1851,7 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
                /* Use scratchpad for in-place operation */
                inlen = scratchpad_use;
                memset(drbg->outscratchpad, 0, scratchpad_use);
-               sg_set_buf(sg_in, drbg->outscratchpad, scratchpad_use);
+               sg_in = sg_out;
        }
 

Quoted 4 lines

crypto/drbg.c static int drbg_kcapi_sym_ctr(struct drbg_state *drbg, · line 1857

Quoted 2 lines

crypto/drbg.c +0 -0

        while (outlen) {

No more stacktrace !

Quoted 7 lines

Thank you. I will send a patch later today.

--
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!

OK, it seems my direction of fixing the caller site has not been taken kindly by the power that be.
Let's try something else.

Can you please drop the previous patch and test this one instead?

drivers/crypto/ccree/cc_buffer_mgr.c +1 -0

index 11e0278c8631..398843040566 100644
@@ -377,6 +377,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
        u32 dummy = 0;
        int rc = 0;
        u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
        req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

        mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +400,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
        }
 
        /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                       LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
        if (rc)

                goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +417,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                }
        } else {
                /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                               &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                               &dummy, &mapped_nents);

                if (rc)

Thanks!
Gilad

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

Quoted 6 lines · Corentin Labbe

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Cheers,
--
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Quoted 11 lines · Herbert Xu

Quoted 6 lines · Corentin Labbe

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                              req_ctx->mlli_params.mlli_dma_addr);
        }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
        if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
        }
 }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
        u32 dummy = 0;
        int rc = 0;
        u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
        req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

        mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
        }
 
        /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                       LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
        if (rc)

                goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                }
        } else {
                /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                               &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                               &dummy, &mapped_nents);

                if (rc)

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f 
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc 
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f 

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

Quoted 171 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 9 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

Hello, thanks for the explanation.
Does my driver can do something about it ?
Like checking SGs respect this ARCH_DMA_MINALIGN constraints and if not fallback to a software implementation (as it will does non-DMA transfer).

Regards

Hi Corentin,

A bug in the DMA API it is not.

What is the call site that calls into the crypto driver in your case? Is it the drbg like in the cryptocell case or something else?

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

Quoted 12 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 8 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

         /* If the device is not writing memory then we don't have any
          * concerns about the cpu consuming stale data.  This mitigates
          * legitimate usages of overlapping mappings.
          */
         if (entry->direction == DMA_TO_DEVICE)
                 return 0;

Quoted 26 lines · Gilad Ben-Yossef

Quoted 12 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 8 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

Quoted 6 lines

         /* If the device is not writing memory then we don't have any
          * concerns about the cpu consuming stale data.  This mitigates
          * legitimate usages of overlapping mappings.
          */
         if (entry->direction == DMA_TO_DEVICE)
                 return 0;

It's OK to have multiple mappings that are *all* DMA_TO_DEVICE, which looks to be the case that this check was intended to allow. However I think you're right that it should still actually check for conflicting directions between the new entry and any existing ones, otherwise it ends up a bit too lenient.

Cheers,
Robin.

Quoted 41 lines · Robin Murphy

Quoted 26 lines · Gilad Ben-Yossef

Quoted 12 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 8 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

Quoted 6 lines

          /* If the device is not writing memory then we don't have any
           * concerns about the cpu consuming stale data.  This mitigates
           * legitimate usages of overlapping mappings.
           */
          if (entry->direction == DMA_TO_DEVICE)
                  return 0;

It's OK to have multiple mappings that are *all* DMA_TO_DEVICE, which looks to be the case that this check was intended to allow. However I think you're right that it should still actually check for conflicting directions between the new entry and any existing ones, otherwise it ends up a bit too lenient.

Quoted 2 lines

Cheers,
Robin.

I understand what you are saying about why checking for conflicting directions may be a good thing, but given that the code is as it is right now, how are we seeing the warning for two mapping that one of them is DMA_TO_DEVICE?

Quoted 45 lines · Gilad Ben-Yossef

Quoted 43 lines · Robin Murphy

Quoted 26 lines · Gilad Ben-Yossef

Quoted 12 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0 [  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0 [  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 8 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

Quoted 6 lines

          /* If the device is not writing memory then we don't have any
           * concerns about the cpu consuming stale data.  This mitigates
           * legitimate usages of overlapping mappings.
           */
          if (entry->direction == DMA_TO_DEVICE)
                  return 0;

It's OK to have multiple mappings that are *all* DMA_TO_DEVICE, which looks to be the case that this check was intended to allow. However I think you're right that it should still actually check for conflicting directions between the new entry and any existing ones, otherwise it ends up a bit too lenient.

Cheers,
Robin.

Quoted 4 lines

I understand what you are saying about why checking for conflicting directions may be a good thing, but given that the code is as it is right now, how are we seeing the warning for two mapping that one of them is DMA_TO_DEVICE?

Because it's the second one that isn't. The warning is triggered by adding the DMA_FROM_DEVICE entry, which *is* checked, and finds the DMA_TO_DEVICE entry already present. What's not great is that if those two mappings happened to be made in the opposite order then it would be missed entirely.

Quoted 55 lines · Robin Murphy

Quoted 46 lines · Gilad Ben-Yossef

Quoted 45 lines · Robin Murphy

Quoted 29 lines · Gilad Ben-Yossef

Quoted 20 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the
same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable.
ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Quoted 2 lines

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

Quoted 7 lines

/* If the device is not writing memory then we don't have any
* concerns about the cpu consuming stale data. This
mitigates

           * legitimate usages of overlapping mappings.
           */
          if (entry->direction == DMA_TO_DEVICE)
                  return 0;

It's OK to have multiple mappings that are *all* DMA_TO_DEVICE, which looks to be the case that this check was intended to allow. However I think you're right that it should still actually check for conflicting directions between the new entry and any existing ones, otherwise it ends up a bit too lenient.

Quoted 7 lines

Quoted 2 lines

Cheers,
Robin.

I understand what you are saying about why checking for conflicting directions may be a good thing, but given that the code is as it is right now, how are we seeing the warning for two mapping that one of them is DMA_TO_DEVICE?

Quoted 5 lines

Because it's the second one that isn't. The warning is triggered by adding the DMA_FROM_DEVICE entry, which *is* checked, and finds the DMA_TO_DEVICE entry already present. What's not great is that if those two mappings happened to be made in the opposite order then it would be missed entirely.

Urgh, no, sorry, that's some imaginary conflation of the cacheline radix tree with the entry hash bucket...

What's actually happened here is that I've failed to read the log properly and they're both DMA_FROM_DEVICE. But the potential problem of mixed-direction mappings being missed does still stand in general.

Quoted 42 lines · Gilad Ben-Yossef

Quoted 41 lines · Robin Murphy

Quoted 26 lines · Gilad Ben-Yossef

Quoted 12 lines · Robin Murphy

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 8 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

hmm... I know I'm missing something here, but how does this align with the following from active_cacheline_insert() in kernel/dma/debug.c ?

Quoted 6 lines

          /* If the device is not writing memory then we don't have any
           * concerns about the cpu consuming stale data.  This mitigates
           * legitimate usages of overlapping mappings.
           */
          if (entry->direction == DMA_TO_DEVICE)
                  return 0;

It's OK to have multiple mappings that are *all* DMA_TO_DEVICE, which looks to be the case that this check was intended to allow. However I think you're right that it should still actually check for conflicting directions between the new entry and any existing ones, otherwise it ends up a bit too lenient.

Quoted 7 lines

Quoted 2 lines

Cheers,
Robin.

I understand what you are saying about why checking for conflicting directions may be a good thing, but given that the code is as it is right now, how are we seeing the warning for two mapping that one of them is DMA_TO_DEVICE?

Because it's the second one that isn't. The warning is triggered by adding the DMA_FROM_DEVICE entry, which *is* checked, and finds the DMA_TO_DEVICE entry already present. What's not great is that if those two mappings happened to be made in the opposite order then it would be missed entirely.

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f 
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc 
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f 

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

Quoted 177 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Quoted 3 lines

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

Quoted 179 lines · Corentin Labbe

Quoted 177 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Quoted 8 lines

Quoted 3 lines

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

The thing that does the memory allocation of user calls from libkcapi tests is the crypto/af_alg.c code ...

I assume the sglist has two buffers? could it be that somehow they are not DMA aligned? that would be weird indeed...

Quoted 189 lines · Gilad Ben-Yossef

Quoted 187 lines · Corentin Labbe

Quoted 171 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 9 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

Quoted 6 lines

The thing that does the memory allocation of user calls from libkcapi tests is the crypto/af_alg.c code ...

I assume the sglist has two buffers? could it be that somehow they are not DMA aligned? that would be weird indeed...

The SGlist has only one SG, so only one buffer, it is why I dont understand the double call to add_dma_entry().

Quoted 194 lines · Corentin Labbe

Quoted 192 lines · Gilad Ben-Yossef

Quoted 179 lines · Corentin Labbe

Quoted 177 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Quoted 8 lines

Quoted 3 lines

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

The thing that does the memory allocation of user calls from libkcapi tests is the crypto/af_alg.c code ...

Quoted 5 lines

Quoted 3 lines

I assume the sglist has two buffers? could it be that somehow they are not DMA aligned? that would be weird indeed...

The SGlist has only one SG, so only one buffer, it is why I dont understand the double call to add_dma_entry().

But... a scatter gather list by its nature can hold more than one buffer, no? that is the whole purpose of a scatter gather list...?

Quoted 189 lines · Gilad Ben-Yossef

Quoted 187 lines · Corentin Labbe

Quoted 171 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

Quoted 9 lines

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

Quoted 6 lines

The thing that does the memory allocation of user calls from libkcapi tests is the crypto/af_alg.c code ...

I assume the sglist has two buffers? could it be that somehow they are not DMA aligned? that would be weird indeed...

The SGlist has only one SG, so only one buffer, it is why I dont understand the double call to add_dma_entry().

Quoted 197 lines · Corentin Labbe

Quoted 195 lines · Gilad Ben-Yossef

Quoted 179 lines · Corentin Labbe

Quoted 177 lines · Robin Murphy

Quoted 154 lines · Corentin Labbe

Le Mon, Mar 07, 2022 at 09:59:29AM +0200, Gilad Ben-Yossef a �crit :

Quoted 72 lines · Herbert Xu

Quoted 11 lines · Corentin Labbe

Quoted 6 lines

Hello

I got:

[   17.563793] ------------[ cut here ]------------
[   17.568492] DMA-API: ccree e6601000.crypto: device driver frees DMA memory with different direction [device address=0x0000000078fe5800] [size=8 bytes] [mapped with DMA_TO_DEVICE] [unmapped with DMA_BIDIRECTIONAL]

The direction argument during unmap must match whatever direction you used during the original map call.

Yes, of course. I changed one but forgot the other.

Corentin, could you be kind and check that this solves the original problem and does not produce new warnings?

drivers/crypto/ccree/cc_buffer_mgr.c +7 -4

index 11e0278c8631..31cfe014922e 100644
@@ -356,12 +356,14 @@ void cc_unmap_cipher_request(struct device *dev,
 void *ctx,
                               req_ctx->mlli_params.mlli_dma_addr);
         }
 
-       dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
-       dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
-
         if (src != dst) {
-               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_BIDIRECTIONAL);
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_TO_DEVICE);
+               dma_unmap_sg(dev, dst, req_ctx->out_nents, DMA_FROM_DEVICE);
                 dev_dbg(dev, "Unmapped req->dst=%pK\n", sg_virt(dst));
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
+       } else {
+               dma_unmap_sg(dev, src, req_ctx->in_nents, DMA_BIDIRECTIONAL);
+               dev_dbg(dev, "Unmapped req->src=%pK\n", sg_virt(src));
         }
  }

@@ -377,6 +379,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         u32 dummy = 0;
         int rc = 0;
         u32 mapped_nents = 0;
+       int src_direction = (src != dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL);
 
         req_ctx->dma_buf_type = CC_DMA_BUF_DLLI;

         mlli_params->curr_pool = NULL;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -399,7 +402,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
         }
 
         /* Map the src SGL */
-       rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, &req_ctx->in_nents,
+       rc = cc_map_sg(dev, src, nbytes, src_direction, &req_ctx->in_nents,
                        LLI_MAX_NUM_OF_DATA_ENTRIES, &dummy, &mapped_nents);
         if (rc)

                 goto cipher_exit;

drivers/crypto/ccree/cc_buffer_mgr.c +1 -1

@@ -416,7 +419,7 @@ int cc_map_cipher_request(struct cc_drvdata
 *drvdata, void *ctx,
                 }
         } else {
                 /* Map the dst sg */
-               rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+               rc = cc_map_sg(dev, dst, nbytes, DMA_FROM_DEVICE,
                                &req_ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
                                &dummy, &mapped_nents);

Quoted 3 lines

                 if (rc)

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at:

Quoted 3 lines

Yes the mapped at is empty just after.

And the sequence of DMA operations in my driver is simple, so I cannot see how any overlap could occur.

The "overlap" is in the sense of having more than one mapping within the same cacheline:

[  142.458120] DMA-API: add_dma_entry start P=ba79f200 N=ba79f
D=ba79f200 L=10 DMA_FROM_DEVICE attrs=0
[  142.458156] DMA-API: add_dma_entry start P=445dc010 N=445dc
D=445dc010 L=10 DMA_TO_DEVICE attrs=0
[  142.458178] sun8i-ss 1c15000.crypto: SRC 0/1/1 445dc000 len=16 bi=0
[  142.458215] sun8i-ss 1c15000.crypto: DST 0/1/1 ba79f200 len=16 bi=0
[  142.458234] DMA-API: add_dma_entry start P=ba79f210 N=ba79f

D=ba79f210 L=10 DMA_FROM_DEVICE attrs=0

This actually illustrates exactly the reason why this is unsupportable. ba79f200 is mapped for DMA_FROM_DEVICE, therefore subsequently mapping ba79f210 for DMA_TO_DEVICE may cause the cacheline covering the range ba79f200-ba79f23f to be written back over the top of data that the device has already started to write to memory. Hello data corruption.

Quoted 8 lines

Quoted 3 lines

Separate DMA mappings should be from separate memory allocations, respecting ARCH_DMA_MINALIGN.

I just saw something strange, only one SG is involved, and I dont see any DMA_TO_DEVICE for ba79f210. I see 2 DMA_FROM_DEVICE (ba79f200 and ba79f210), but only one should be done.
Why 2 FROM mappings are added with only one sg ?

The thing that does the memory allocation of user calls from libkcapi tests is the crypto/af_alg.c code ...

I assume the sglist has two buffers? could it be that somehow they are not DMA aligned? that would be weird indeed...

Quoted 1 line

The SGlist has only one SG, so only one buffer, it is why I dont understand the double call to add_dma_entry().

Answering myself, the driver can handle 2 requests in parallel, this is the reason of 2 dma call. And both are using the same key, and so first DMA call (for the key) give same address.

This is something hard to prevent at driver level.

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0

[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

Hello

I still get the warning:

[  433.406230] ------------[ cut here ]------------
[  433.406326] DMA-API: ccree e6601000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  433.406386] WARNING: CPU: 7 PID: 31074 at /home/clabbe/linux-next/kernel/dma/debug.c:571 add_dma_entry+0x1d0/0x288
[  433.406434] Modules linked in:
[  433.406458] CPU: 7 PID: 31074 Comm: kcapi Not tainted 5.17.0-rc6-next-20220303-00130-g30042e47ee47-dirty #54
[  433.406473] Hardware name: Renesas Salvator-X board based on r8a77950 (DT)
[  433.406484] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  433.406498] pc : add_dma_entry+0x1d0/0x288
[  433.406510] lr : add_dma_entry+0x1d0/0x288
[  433.406522] sp : ffff800015da3690
[  433.406531] x29: ffff800015da3690 x28: 0000000000000000 x27: 0000000000000000
[  433.406562] x26: 0000000000000000 x25: ffff80000b4c7bc0 x24: ffff80000b4c7000
[  433.406593] x23: 0000000000000000 x22: 00000000ffffffef x21: ffff80000a9b6000
[  433.406623] x20: ffff0004c0af5c00 x19: ffff80000b420000 x18: ffffffffffffffff
[  433.406653] x17: 6c7265766f202c54 x16: 534958454520676e x15: 000000000000022e
[  433.406683] x14: ffff800015da3380 x13: 00000000ffffffea x12: ffff80000b4be010
[  433.406713] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff80000b4a6028
[  433.406743] x8 : c0000000ffffefff x7 : 0000000000017fe8 x6 : ffff80000b4a5fd0
[  433.406773] x5 : ffff0006ff795c48 x4 : 0000000000000000 x3 : 0000000000000027
[  433.406802] x2 : 0000000000000023 x1 : 8ca4e4fbf4b87900 x0 : 0000000000000000
[  433.406833] Call trace:
[  433.406841]  add_dma_entry+0x1d0/0x288
[  433.406854]  debug_dma_map_sg+0x150/0x398
[  433.406869]  __dma_map_sg_attrs+0x9c/0x108
[  433.406889]  dma_map_sg_attrs+0x10/0x28
[  433.406904]  cc_map_sg+0x80/0x100
[  433.406924]  cc_map_cipher_request+0x178/0x3c8
[  433.406939]  cc_cipher_process+0x210/0xb58
[  433.406953]  cc_cipher_encrypt+0x2c/0x38
[  433.406967]  crypto_skcipher_encrypt+0x44/0x78
[  433.406986]  skcipher_recvmsg+0x36c/0x420
[  433.407003]  ____sys_recvmsg+0x90/0x280
[  433.407024]  ___sys_recvmsg+0x88/0xd0
[  433.407038]  __sys_recvmsg+0x6c/0xd0
[  433.407049]  __arm64_sys_recvmsg+0x24/0x30
[  433.407061]  invoke_syscall+0x44/0x100
[  433.407082]  el0_svc_common.constprop.3+0x90/0x120
[  433.407096]  do_el0_svc+0x24/0x88
[  433.407110]  el0_svc+0x4c/0x100
[  433.407131]  el0t_64_sync_handler+0x90/0xb8
[  433.407145]  el0t_64_sync+0x170/0x174
[  433.407160] irq event stamp: 5624
[  433.407168] hardirqs last  enabled at (5623): [<ffff80000812f6a8>] __up_console_sem+0x60/0x98
[  433.407191] hardirqs last disabled at (5624): [<ffff800009c9a060>] el1_dbg+0x28/0x90
[  433.407208] softirqs last  enabled at (5570): [<ffff8000097e62f8>] lock_sock_nested+0x80/0xa0
[  433.407226] softirqs last disabled at (5568): [<ffff8000097e62d8>] lock_sock_nested+0x60/0xa0
[  433.407241] ---[ end trace 0000000000000000 ]---
[  433.407381] DMA-API: Mapped at:
[  433.407396]  debug_dma_map_sg+0x16c/0x398
[  433.407416]  __dma_map_sg_attrs+0x9c/0x108
[  433.407436]  dma_map_sg_attrs+0x10/0x28
[  433.407455]  cc_map_sg+0x80/0x100
[  433.407475]  cc_map_cipher_request+0x178/0x3c8

BUT I start to thing this is a bug in DMA-API debug.

My sun8i-ss driver hit the same warning:

[  142.458351] WARNING: CPU: 1 PID: 90 at kernel/dma/debug.c:597 add_dma_entry+0x2ec/0x4cc
[  142.458429] DMA-API: sun8i-ss 1c15000.crypto: cacheline tracking EEXIST, overlapping mappings aren't supported
[  142.458455] Modules linked in: ccm algif_aead xts cmac
[  142.458563] CPU: 1 PID: 90 Comm: 1c15000.crypto- Not tainted 5.17.0-rc6-next-20220307-00132-g39dad568d20a-dirty #223
[  142.458581] Hardware name: Allwinner A83t board
[  142.458596]  unwind_backtrace from show_stack+0x10/0x14
[  142.458627]  show_stack from 0xf0abdd1c
[  142.458646] irq event stamp: 31747
[  142.458660] hardirqs last  enabled at (31753): [<c019316c>] __up_console_sem+0x50/0x60
[  142.458688] hardirqs last disabled at (31758): [<c0193158>] __up_console_sem+0x3c/0x60
[  142.458710] softirqs last  enabled at (31600): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458738] softirqs last disabled at (31580): [<c06990c8>] sun8i_ss_handle_cipher_request+0x300/0x8b8
[  142.458758] ---[ end trace 0000000000000000 ]---
[  142.458771] DMA-API: Mapped at: