Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] [Deepin-Kernel-SIG] [Upstream] Update kernel base to 6.6.63-nouveau #489

Open
wants to merge 4 commits into
base: linux-6.6.y
Choose a base branch
from
Open

[WIP] [Deepin-Kernel-SIG] [Upstream] Update kernel base to 6.6.63-nouveau #489

wants to merge 4 commits into from

Conversation

opsiff
Copy link
Member

@opsiff opsiff commented Nov 23, 2024

opsiff and others added 3 commits November 23, 2024 15:55
@opsiff
Revert "Revert "Revert "Reapply "drm/nouveau/firmware: Fix SG_DEBUG e…
8ce8e98 
…rror with nvkm_firmware_ctor()"""

Revert to apply:
commit 16abd7ce81e4fe "nouveau: fw: sync dma after setup is called."
This reverts commit c51f05b.
need test issue 319 after 6.6.63
Link:#319
@airlied @opsiff
nouveau/firmware: use dma non-coherent allocator
b7d6fbd 
commit 9b340aeb26d50e9a9ec99599e2a39b035fac978e upstream.

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.

Cc: [email protected]
Fixes: 2541626 ("drm/nouveau/acr: use common falcon HS FW code for ACR FWs")
Signed-off-by: Dave Airlie <[email protected]>
Signed-off-by: Danilo Krummrich <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
(cherry picked from commit cc29c5546c6a373648363ac49781f1d74b530707)
@airlied @opsiff
nouveau: fw: sync dma after setup is called.
620ba18 
commit 21ec425eaf2cb7c0371f7683f81ad7d9679b6eb5 upstream.

When this code moved to non-coherent allocator the sync was put too
early for some firmwares which called the setup function, move the
sync down after the setup function.

Reported-by: Diogo Ivo <[email protected]>
Tested-by: Diogo Ivo <[email protected]>
Reviewed-by: Lyude Paul <[email protected]>
Fixes: 9b340aeb26d5 ("nouveau/firmware: use dma non-coherent allocator")
Cc: [email protected]
Signed-off-by: Dave Airlie <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
(cherry picked from commit 16abd7ce81e4fedd058035d4644b3882af16732d)
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from opsiff. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Avenger-285714
Copy link
Collaborator

/ok-to-test

@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见：

  1. 在nvkm_firmware_dtor函数中，dma_unmap_single被替换为dma_free_noncoherent，这可能是为了更清晰地表示内存的释放。但是，dma_free_noncoherent函数的参数顺序与dma_unmap_single不同，需要确认是否正确传递了参数。

  2. 在nvkm_firmware_ctor函数中，fw->img的分配和映射被合并为一步，使用dma_alloc_noncoherent。这简化了代码，但需要确保dma_alloc_noncoherent的返回值fw->img和fw->phys的赋值是正确的。

  3. 在nvkm_falcon_fw_boot函数中，添加了dma_sync_single_for_device调用，以确保在加载固件之前，所有DMA映射的内存都被同步到设备。这是一个好的做法，可以避免潜在的硬件访问问题。

  4. 在nvkm_falcon_fw_boot函数中，FLCNFW_DBG宏被调用两次，但第二次调用没有参数。如果这是故意的，应该确认是否有调试信息需要打印。如果不是，应该移除多余的调用。

  5. 代码中没有明显的语法或逻辑错误，但建议在修改后添加更多的注释，特别是对于新引入的函数dma_alloc_noncoherent和dma_free_noncoherent的使用，以便其他开发者理解这些更改的目的。

  6. 应该检查WARN_ON(1);的使用，确保这是预期的错误处理方式。如果这是一个bug，应该替换为适当的错误处理机制。

  7. 代码中没有明显的性能问题，但建议进行性能测试，以确保新的内存分配和释放机制不会对系统性能产生负面影响。

  8. 代码中没有明显的安全问题，但建议进行安全性审查，确保没有内存泄漏或其他可能导致安全漏洞的问题。

总体来说，代码的修改提高了代码的可读性和效率，但需要进一步的测试和验证以确保这些更改不会引入新的问题。

@Avenger-285714 Avenger-285714 changed the title [Deepin-Kernel-SIG] [Upstream] Update kernel base to 6.6.63-nouveau [WIP] [Deepin-Kernel-SIG] [Upstream] Update kernel base to 6.6.63-nouveau Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ink-Paper Ink-Paper Awaiting requested review from Ink-Paper

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
do-not-merge/work-in-progress ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@opsiff @deepin-ci-robot @Avenger-285714 @airlied