* [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2)
@ 2024-08-15 20:19 Dave Airlie
2024-08-15 20:45 ` Timur Tabi
2024-08-16 0:53 ` Danilo Krummrich
0 siblings, 2 replies; 5+ messages in thread
From: Dave Airlie @ 2024-08-15 20:19 UTC (permalink / raw)
To: dri-devel; +Cc: nouveau, dakr
From: Dave Airlie <airlied@redhat.com>
Currently, enabling SG_DEBUG in the kernel will cause nouveau to hit a
BUG() on startup, when the iommu is enabled:
kernel BUG at include/linux/scatterlist.h:187!
invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
CPU: 7 PID: 930 Comm: (udev-worker) Not tainted 6.9.0-rc3Lyude-Test+ #30
Hardware name: MSI MS-7A39/A320M GAMING PRO (MS-7A39), BIOS 1.I0 01/22/2019
RIP: 0010:sg_init_one+0x85/0xa0
Code: 69 88 32 01 83 e1 03 f6 c3 03 75 20 a8 01 75 1e 48 09 cb 41 89 54
24 08 49 89 1c 24 41 89 6c 24 0c 5b 5d 41 5c e9 7b b9 88 00 <0f> 0b 0f 0b
0f 0b 48 8b 05 5e 46 9a 01 eb b2 66 66 2e 0f 1f 84 00
RSP: 0018:ffffa776017bf6a0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffffa77600d87000 RCX: 000000000000002b
RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa77680d87000
RBP: 000000000000e000 R08: 0000000000000000 R09: 0000000000000000
R10: ffff98f4c46aa508 R11: 0000000000000000 R12: ffff98f4c46aa508
R13: ffff98f4c46aa008 R14: ffffa77600d4a000 R15: ffffa77600d4a018
FS: 00007feeb5aae980(0000) GS:ffff98f5c4dc0000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f22cb9a4520 CR3: 00000001043ba000 CR4: 00000000003506f0
Call Trace:
<TASK>
? die+0x36/0x90
? do_trap+0xdd/0x100
? sg_init_one+0x85/0xa0
? do_error_trap+0x65/0x80
? sg_init_one+0x85/0xa0
? exc_invalid_op+0x50/0x70
? sg_init_one+0x85/0xa0
? asm_exc_invalid_op+0x1a/0x20
? sg_init_one+0x85/0xa0
nvkm_firmware_ctor+0x14a/0x250 [nouveau]
nvkm_falcon_fw_ctor+0x42/0x70 [nouveau]
ga102_gsp_booter_ctor+0xb4/0x1a0 [nouveau]
r535_gsp_oneinit+0xb3/0x15f0 [nouveau]
? srso_return_thunk+0x5/0x5f
? srso_return_thunk+0x5/0x5f
? nvkm_udevice_new+0x95/0x140 [nouveau]
? srso_return_thunk+0x5/0x5f
? srso_return_thunk+0x5/0x5f
? ktime_get+0x47/0xb0
Fix this by using the non-coherent allocator instead, I think there
might be a better answer to this, but it involve ripping up some of
APIs using sg lists.
v2: fix build warning
Signed-off-by: Dave Airlie <airlied@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 9 ++++++---
drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 6 ++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index adc60b25f8e6..0af01a0ec601 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -205,7 +205,8 @@ nvkm_firmware_dtor(struct nvkm_firmware *fw)
break;
case NVKM_FIRMWARE_IMG_DMA:
nvkm_memory_unref(&memory);
- dma_free_coherent(fw->device->dev, sg_dma_len(&fw->mem.sgl), fw->img, fw->phys);
+ dma_free_noncoherent(fw->device->dev, sg_dma_len(&fw->mem.sgl),
+ fw->img, fw->phys, DMA_TO_DEVICE);
break;
case NVKM_FIRMWARE_IMG_SGT:
nvkm_memory_unref(&memory);
@@ -236,10 +237,12 @@ nvkm_firmware_ctor(const struct nvkm_firmware_func *func, const char *name,
break;
case NVKM_FIRMWARE_IMG_DMA: {
dma_addr_t addr;
-
len = ALIGN(fw->len, PAGE_SIZE);
- fw->img = dma_alloc_coherent(fw->device->dev, len, &addr, GFP_KERNEL);
+ fw->img = dma_alloc_noncoherent(fw->device->dev,
+ len, &addr,
+ DMA_TO_DEVICE,
+ GFP_KERNEL);
if (fw->img) {
memcpy(fw->img, src, fw->len);
fw->phys = addr;
diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
index 80a480b12174..a1c8545f1249 100644
--- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
+++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
@@ -89,6 +89,12 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct nvkm_subdev *user,
nvkm_falcon_fw_dtor_sigs(fw);
}
+ /* after last write to the img, sync dma mappings */
+ dma_sync_single_for_device(fw->fw.device->dev,
+ fw->fw.phys,
+ sg_dma_len(&fw->fw.mem.sgl),
+ DMA_TO_DEVICE);
+
FLCNFW_DBG(fw, "resetting");
fw->func->reset(fw);
--
2.46.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2)
2024-08-15 20:19 [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2) Dave Airlie
@ 2024-08-15 20:45 ` Timur Tabi
2024-08-15 21:08 ` Dave Airlie
2024-08-16 0:53 ` Danilo Krummrich
1 sibling, 1 reply; 5+ messages in thread
From: Timur Tabi @ 2024-08-15 20:45 UTC (permalink / raw)
To: dri-devel@lists.freedesktop.org, airlied@gmail.com
Cc: nouveau@lists.freedesktop.org, dakr@redhat.com
On Fri, 2024-08-16 at 06:19 +1000, Dave Airlie wrote:
> Fix this by using the non-coherent allocator instead, I think there
> might be a better answer to this, but it involve ripping up some of
> APIs using sg lists.
Thanks for fixing this, but what is the relationshio between non-coherent
DMA and sg_set_buf()? You're still calling sg_init_one() and hitting this
check:
#ifdef CONFIG_DEBUG_SG
BUG_ON(!virt_addr_valid(buf));
#endif
Does a non-coherent DMA buffer have a different kind of virtual address than
a coherent DMA buffer?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2)
2024-08-15 20:45 ` Timur Tabi
@ 2024-08-15 21:08 ` Dave Airlie
0 siblings, 0 replies; 5+ messages in thread
From: Dave Airlie @ 2024-08-15 21:08 UTC (permalink / raw)
To: Timur Tabi
Cc: dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
dakr@redhat.com
On Fri, 16 Aug 2024 at 06:45, Timur Tabi <ttabi@nvidia.com> wrote:
>
> On Fri, 2024-08-16 at 06:19 +1000, Dave Airlie wrote:
> > Fix this by using the non-coherent allocator instead, I think there
> > might be a better answer to this, but it involve ripping up some of
> > APIs using sg lists.
>
> Thanks for fixing this, but what is the relationshio between non-coherent
> DMA and sg_set_buf()? You're still calling sg_init_one() and hitting this
> check:
>
> #ifdef CONFIG_DEBUG_SG
> BUG_ON(!virt_addr_valid(buf));
> #endif
>
> Does a non-coherent DMA buffer have a different kind of virtual address than
> a coherent DMA buffer?
Yes, it has one that doesn't explode in the check.
We should not be stuffing mapped dma_addr into sg tables at all,
fixing that is a bit messier.
Dave.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2)
2024-08-15 20:19 [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2) Dave Airlie
2024-08-15 20:45 ` Timur Tabi
@ 2024-08-16 0:53 ` Danilo Krummrich
2024-08-16 1:40 ` Dave Airlie
1 sibling, 1 reply; 5+ messages in thread
From: Danilo Krummrich @ 2024-08-16 0:53 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel, nouveau
On Fri, Aug 16, 2024 at 06:19:23AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> Currently, enabling SG_DEBUG in the kernel will cause nouveau to hit a
> BUG() on startup, when the iommu is enabled:
>
> kernel BUG at include/linux/scatterlist.h:187!
> invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> CPU: 7 PID: 930 Comm: (udev-worker) Not tainted 6.9.0-rc3Lyude-Test+ #30
> Hardware name: MSI MS-7A39/A320M GAMING PRO (MS-7A39), BIOS 1.I0 01/22/2019
> RIP: 0010:sg_init_one+0x85/0xa0
> Code: 69 88 32 01 83 e1 03 f6 c3 03 75 20 a8 01 75 1e 48 09 cb 41 89 54
> 24 08 49 89 1c 24 41 89 6c 24 0c 5b 5d 41 5c e9 7b b9 88 00 <0f> 0b 0f 0b
> 0f 0b 48 8b 05 5e 46 9a 01 eb b2 66 66 2e 0f 1f 84 00
> RSP: 0018:ffffa776017bf6a0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffffa77600d87000 RCX: 000000000000002b
> RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa77680d87000
> RBP: 000000000000e000 R08: 0000000000000000 R09: 0000000000000000
> R10: ffff98f4c46aa508 R11: 0000000000000000 R12: ffff98f4c46aa508
> R13: ffff98f4c46aa008 R14: ffffa77600d4a000 R15: ffffa77600d4a018
> FS: 00007feeb5aae980(0000) GS:ffff98f5c4dc0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f22cb9a4520 CR3: 00000001043ba000 CR4: 00000000003506f0
> Call Trace:
> <TASK>
> ? die+0x36/0x90
> ? do_trap+0xdd/0x100
> ? sg_init_one+0x85/0xa0
> ? do_error_trap+0x65/0x80
> ? sg_init_one+0x85/0xa0
> ? exc_invalid_op+0x50/0x70
> ? sg_init_one+0x85/0xa0
> ? asm_exc_invalid_op+0x1a/0x20
> ? sg_init_one+0x85/0xa0
> nvkm_firmware_ctor+0x14a/0x250 [nouveau]
> nvkm_falcon_fw_ctor+0x42/0x70 [nouveau]
> ga102_gsp_booter_ctor+0xb4/0x1a0 [nouveau]
> r535_gsp_oneinit+0xb3/0x15f0 [nouveau]
> ? srso_return_thunk+0x5/0x5f
> ? srso_return_thunk+0x5/0x5f
> ? nvkm_udevice_new+0x95/0x140 [nouveau]
> ? srso_return_thunk+0x5/0x5f
> ? srso_return_thunk+0x5/0x5f
> ? ktime_get+0x47/0xb0
>
> Fix this by using the non-coherent allocator instead, I think there
> might be a better answer to this, but it involve ripping up some of
> APIs using sg lists.
>
> v2: fix build warning
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Cc: stable@vger.kernel.org
Does this need the following fixes tag?
Fixes: 2541626cfb79 ("drm/nouveau/acr: use common falcon HS FW code for ACR FWs")
> ---
> drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 9 ++++++---
> drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 6 ++++++
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> index adc60b25f8e6..0af01a0ec601 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
> @@ -205,7 +205,8 @@ nvkm_firmware_dtor(struct nvkm_firmware *fw)
> break;
> case NVKM_FIRMWARE_IMG_DMA:
> nvkm_memory_unref(&memory);
> - dma_free_coherent(fw->device->dev, sg_dma_len(&fw->mem.sgl), fw->img, fw->phys);
> + dma_free_noncoherent(fw->device->dev, sg_dma_len(&fw->mem.sgl),
> + fw->img, fw->phys, DMA_TO_DEVICE);
> break;
> case NVKM_FIRMWARE_IMG_SGT:
> nvkm_memory_unref(&memory);
> @@ -236,10 +237,12 @@ nvkm_firmware_ctor(const struct nvkm_firmware_func *func, const char *name,
> break;
> case NVKM_FIRMWARE_IMG_DMA: {
> dma_addr_t addr;
> -
> len = ALIGN(fw->len, PAGE_SIZE);
>
> - fw->img = dma_alloc_coherent(fw->device->dev, len, &addr, GFP_KERNEL);
> + fw->img = dma_alloc_noncoherent(fw->device->dev,
> + len, &addr,
> + DMA_TO_DEVICE,
> + GFP_KERNEL);
> if (fw->img) {
> memcpy(fw->img, src, fw->len);
> fw->phys = addr;
> diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> index 80a480b12174..a1c8545f1249 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
> @@ -89,6 +89,12 @@ nvkm_falcon_fw_boot(struct nvkm_falcon_fw *fw, struct nvkm_subdev *user,
> nvkm_falcon_fw_dtor_sigs(fw);
> }
>
> + /* after last write to the img, sync dma mappings */
> + dma_sync_single_for_device(fw->fw.device->dev,
> + fw->fw.phys,
> + sg_dma_len(&fw->fw.mem.sgl),
> + DMA_TO_DEVICE);
> +
> FLCNFW_DBG(fw, "resetting");
> fw->func->reset(fw);
>
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2)
2024-08-16 0:53 ` Danilo Krummrich
@ 2024-08-16 1:40 ` Dave Airlie
0 siblings, 0 replies; 5+ messages in thread
From: Dave Airlie @ 2024-08-16 1:40 UTC (permalink / raw)
To: Danilo Krummrich; +Cc: dri-devel, nouveau
On Fri, 16 Aug 2024 at 10:53, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri, Aug 16, 2024 at 06:19:23AM +1000, Dave Airlie wrote:
> > From: Dave Airlie <airlied@redhat.com>
> >
> > Currently, enabling SG_DEBUG in the kernel will cause nouveau to hit a
> > BUG() on startup, when the iommu is enabled:
> >
> > kernel BUG at include/linux/scatterlist.h:187!
> > invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> > CPU: 7 PID: 930 Comm: (udev-worker) Not tainted 6.9.0-rc3Lyude-Test+ #30
> > Hardware name: MSI MS-7A39/A320M GAMING PRO (MS-7A39), BIOS 1.I0 01/22/2019
> > RIP: 0010:sg_init_one+0x85/0xa0
> > Code: 69 88 32 01 83 e1 03 f6 c3 03 75 20 a8 01 75 1e 48 09 cb 41 89 54
> > 24 08 49 89 1c 24 41 89 6c 24 0c 5b 5d 41 5c e9 7b b9 88 00 <0f> 0b 0f 0b
> > 0f 0b 48 8b 05 5e 46 9a 01 eb b2 66 66 2e 0f 1f 84 00
> > RSP: 0018:ffffa776017bf6a0 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: ffffa77600d87000 RCX: 000000000000002b
> > RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffffa77680d87000
> > RBP: 000000000000e000 R08: 0000000000000000 R09: 0000000000000000
> > R10: ffff98f4c46aa508 R11: 0000000000000000 R12: ffff98f4c46aa508
> > R13: ffff98f4c46aa008 R14: ffffa77600d4a000 R15: ffffa77600d4a018
> > FS: 00007feeb5aae980(0000) GS:ffff98f5c4dc0000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f22cb9a4520 CR3: 00000001043ba000 CR4: 00000000003506f0
> > Call Trace:
> > <TASK>
> > ? die+0x36/0x90
> > ? do_trap+0xdd/0x100
> > ? sg_init_one+0x85/0xa0
> > ? do_error_trap+0x65/0x80
> > ? sg_init_one+0x85/0xa0
> > ? exc_invalid_op+0x50/0x70
> > ? sg_init_one+0x85/0xa0
> > ? asm_exc_invalid_op+0x1a/0x20
> > ? sg_init_one+0x85/0xa0
> > nvkm_firmware_ctor+0x14a/0x250 [nouveau]
> > nvkm_falcon_fw_ctor+0x42/0x70 [nouveau]
> > ga102_gsp_booter_ctor+0xb4/0x1a0 [nouveau]
> > r535_gsp_oneinit+0xb3/0x15f0 [nouveau]
> > ? srso_return_thunk+0x5/0x5f
> > ? srso_return_thunk+0x5/0x5f
> > ? nvkm_udevice_new+0x95/0x140 [nouveau]
> > ? srso_return_thunk+0x5/0x5f
> > ? srso_return_thunk+0x5/0x5f
> > ? ktime_get+0x47/0xb0
> >
> > Fix this by using the non-coherent allocator instead, I think there
> > might be a better answer to this, but it involve ripping up some of
> > APIs using sg lists.
> >
> > v2: fix build warning
> >
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> > Cc: stable@vger.kernel.org
>
> Does this need the following fixes tag?
>
> Fixes: 2541626cfb79 ("drm/nouveau/acr: use common falcon HS FW code for ACR FWs")
Please add that on commit, it's probably not that necessary since I
expect prior to that things are just broken in a different place, but
probably good to have a backport target.
Dave.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-16 1:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 20:19 [PATCH] nouveau/firmware: using dma non-coherent interfaces for fw loading. (v2) Dave Airlie
2024-08-15 20:45 ` Timur Tabi
2024-08-15 21:08 ` Dave Airlie
2024-08-16 0:53 ` Danilo Krummrich
2024-08-16 1:40 ` Dave Airlie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.