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.

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

Rptun framework update and related driver update #13526

Merged
merged 22 commits into from
Sep 25, 2024

Conversation

CV-Bowen
Copy link
Contributor

@CV-Bowen CV-Bowen commented Sep 18, 2024

Summary

Author: Bowen Wang [email protected]
Date: Wed Dec 20 10:46:07 2023 +0800

include/nuttx/rptun/rptun.h: change offset type to uint32_t

Sync the offset data type with resource table defined in OpenAMP
and Linux.

Signed-off-by: Bowen Wang <[email protected]>

Author: mazhuang [email protected]
Date: Tue Aug 20 16:32:38 2024 +0800

rptun/rptun_ivshmem:add restart cmd to reboot slave

Master can send restart command to slave to reboot the slave core

Signed-off-by: mazhuang <[email protected]>
Signed-off-by: Yongrong Wang <[email protected]>

commit 2e71792
Author: xuxingliang [email protected]
Date: Fri Aug 23 19:44:11 2024 +0800

rptun: fix memleak on failure

Leak backtrace:
    1 14 2096 9886 0x4318d768 [0x040320744] <romfs_fileconfigure+184> romfs/fs_romfsutil.c:1039
                                                 [0x04031fd3e] <romfs_open+378> romfs/fs_romfs.c:281
                                                 [0x04027fa9e] <file_open+446> vfs/fs_open.c:244
                                                 [0x0402ab986] <rptun_store_open+26> rptun/rptun.c:955
                                                 [0x04034cc88] <remoteproc_load+120> open-amp/lib/remoteproc/remoteproc.c:452
                                                 [0x0402ac8ac] <rptun_dev_start+176> rptun/rptun.c:748
                                                 [0x0402ad038] <rptun_ioctl+416> rptun/rptun.c:618

Signed-off-by: xuxingliang <[email protected]>

commit 09b4804
Author: Bowen Wang [email protected]
Date: Wed Aug 14 15:01:42 2024 +0800

rptun: move rptun cmd definition before the resource table

Because locate the command at the end the resource table is unfriendly
when we want to support multi virtio devices instead only one virtio
rpmsg device.

Signed-off-by: Bowen Wang <[email protected]>

commit 2ec7b89
Author: Yongrong Wang [email protected]
Date: Tue Aug 6 11:32:14 2024 +0800

rptun/rpmsg_virtio: remove chip cmd and reuse the common ones

Add more common command for rptun and rpmsg_virtio frameworks,
also modify the rptun and rpmsg_virtio driver to use the common
commands.

Signed-off-by: Yongrong Wang <[email protected]>

commit 11430f7
Author: Yongrong Wang [email protected]
Date: Mon Aug 5 20:00:12 2024 +0800

sim_rptun.c: remove sim_rptun_panic

Because we can use the common part implemented in rptun

Signed-off-by: Yongrong Wang <[email protected]>

commit 9f4f470
Author: Yongrong Wang [email protected]
Date: Fri Jul 26 16:57:58 2024 +0800

rptun.c/rpmsg_virtio.c: move panic logic from chip to rptun/rpmsg_virtio

Move the panic logic in common places, later we can move more logic to
the framework instead of having the drivers implement it repeatedly.

Signed-off-by: Yongrong Wang <[email protected]>

commit f2278f5
Author: Bowen Wang [email protected]
Date: Tue Jun 11 15:53:15 2024 +0800

sim/sim_rptun: add 64-bit support for sim_rptun

add remote addrenv to make the da is start from 0, so the uint32_t
da in resource table can store the correct address

Signed-off-by: Bowen Wang <[email protected]>

commit 8843960
Author: wangyongrong [email protected]
Date: Wed May 22 16:41:21 2024 +0800

rptun_ivshmem.c: Replace work queue with wdog

wdog has better performance than work queue

Signed-off-by: wangyongrong <[email protected]>

commit 9d2e70e
Author: Bowen Wang [email protected]
Date: Sat Aug 3 18:27:44 2024 +0800

drivers/rptun/rptun.c: move headrx out of CONFIG_RPTUN_PM

headrx is very convient to check weather current core miss interrupt
by comparing the headrx with the rx vring avail.idx for slave side or
rx vring used.idx for master side.

So move headrx out of the CONFIG_RPTUN_PM range.

Signed-off-by: Bowen Wang <[email protected]>

commit 462f7d7
Author: Bowen Wang [email protected]
Date: Fri Feb 23 19:57:25 2024 +0800

rptun: rptun pm and rptun dump support cacheable memory

Should invalidate the memory when the data is located in shared
memory and write by remote core.

Signed-off-by: Bowen Wang <[email protected]>

commit 7aa295c
Author: Xiang Xiao [email protected]
Date: Sun Feb 18 20:38:46 2024 +0800

rptun: Rename rptun_panic_ to rptun_panic

The function name rptun_panic_ is not consistent with other functions

Signed-off-by: Xiang Xiao <[email protected]>

commit 2185309
Author: ligd [email protected]
Date: Wed Apr 27 16:58:02 2022 +0800

rptun: add timeout to wait_tx_buffer callback

Signed-off-by: ligd <[email protected]>

commit 62ca344
Author: Bowen Wang [email protected]
Date: Wed Sep 18 17:26:16 2024 +0800

rptun/rptun_dump: remove unused rptun_dump.c

rptun_dump related code has been moved to rptun.c from rptun_dump.c,
but file rptun_dump.c is not deleted in PR:
https://github.com/apache/nuttx/pull/11712

So delete this file.

Signed-off-by: Bowen Wang <[email protected]>

commit e8e64e6
Author: Bowen Wang [email protected]
Date: Wed Sep 18 17:20:56 2024 +0800

rptun: BUG fix, should not destory the semaphore twice

This BUG is introduced in PR: https://github.com/apache/nuttx/pull/13172

Signed-off-by: Bowen Wang <[email protected]>

commit f79daed
Author: ligd [email protected]
Date: Mon Jun 20 21:47:20 2022 +0800

rptun: use detail name for pm wakelock

So we can distinguish the pm wakelock

Signed-off-by: ligd <[email protected]>

commit e8ad329
Author: Bowen Wang [email protected]
Date: Thu Mar 28 15:16:40 2024 +0800

drivers/rptun: flush the image memory when read from the file system

Flush the image memory to make sure the remote core access the correct
image.

Signed-off-by: Bowen Wang <[email protected]>

commit 29c8a58
Author: ligd [email protected]
Date: Fri Apr 7 23:38:33 2023 +0800

rptun: add RPTUN_PM_AUTORELAX method.

Signed-off-by: ligd <[email protected]>

commit afb0ea8
Author: ligd [email protected]
Date: Thu Jun 16 20:38:51 2022 +0800

rptun/pm: use pm_wakelock

Signed-off-by: ligd <[email protected]>

commit abe9fd4
Author: ligd [email protected]
Date: Thu Jan 5 12:08:04 2023 +0800

rptun/pm: add check to rptun_pm_callback() incase of start early

Signed-off-by: ligd <[email protected]>

commit 95d3611
Author: ligd [email protected]
Date: Sat Apr 30 12:46:27 2022 +0800

rptun: use local rx virtqueue idx to resolve remote low power

Store the rx virtqueue idx to the local headrx index, and only
process the data when the rx virtqueue has data to avoid access
the ram in low power mode.

Signed-off-by: ligd <[email protected]>

commit dab9e66
Author: yanghuatao [email protected]
Date: Thu Aug 10 16:06:57 2023 +0800

nuttx/dirvers: Add secure rptun file

rptun secure is a rptun driver used for the rpmsg communication
between REE and TEE.

Signed-off-by: yanghuatao <[email protected]>

Impact

rptun

Testing

sim:rpserver, rpproxy and qemu

@lupyuen
Copy link
Member

lupyuen commented Sep 18, 2024

[Experimental Bot, please feedback here]

NuttX PR Requirements Review

This PR appears to bundle multiple changes related to the rptun driver, including bug fixes, feature additions, and refactoring. While the individual commit messages provide some details, the overall PR lacks a cohesive summary and impact analysis.

Summary:

  • Why: The PR description should clearly state the overall motivation and goals for the collection of changes.
  • What: A high-level overview of the functional areas impacted by the changes (e.g., data types, commands, memory management, etc.) is missing.
  • How: The PR description should briefly explain how these changes are implemented.

Impact:

  • The "Impact" section is overly brief. Each bullet point should elaborate on how the changes affect the specified area. For instance:
    • Is new feature added? Yes, the PR introduces a "restart" command and support for 64-bit in sim_rptun. A description of these features is needed.
    • Impact on user? The PR mentions "Master can send restart command to slave." This needs further explanation on how a user would utilize this command and any potential adaptations required.
    • Impact on build? If there are no build process changes, state it explicitly. If there are, provide details.
    • Impact on hardware? The testing section lists "sim:rpserver, rpproxy and qemu." Specify the affected architectures and boards.
    • Impact on documentation? Clarify if documentation updates are needed and if they are included in the PR.
    • Impact on security? The PR introduces a rptun_secure file. Explain the security implications and any potential risks or mitigations.
    • Impact on compatibility? Are there any backward or forward compatibility concerns introduced by the changes?

Testing:

  • The provided testing logs are placeholders. Include actual logs demonstrating the functionality before and after the changes.
  • The testing section should be more comprehensive, covering each significant change in the PR.

Overall:

This PR requires significant improvements to meet the NuttX requirements. Provide a clear and detailed summary, elaborate on the impact of each change, and include actual testing logs to ensure the changes are thoroughly validated.

drivers/rptun/Kconfig Outdated Show resolved Hide resolved
drivers/rptun/rptun.c Show resolved Hide resolved
drivers/rptun/rptun.c Outdated Show resolved Hide resolved
drivers/rptun/rptun_secure.c Outdated Show resolved Hide resolved
drivers/rptun/rptun_secure.c Outdated Show resolved Hide resolved
drivers/rptun/rptun_secure.c Outdated Show resolved Hide resolved
drivers/rptun/rptun_secure.c Outdated Show resolved Hide resolved
yanghuatao and others added 22 commits September 24, 2024 10:52
rptun secure is a rptun driver used for the rpmsg communication
between (Non-Secure) REE and (Secure) TEE environments.

Signed-off-by: yanghuatao <[email protected]>
Signed-off-by: Bowen Wang <[email protected]>
Store the rx virtqueue idx to the local headrx index, and only
process the data when the rx virtqueue has data to avoid access
the ram in low power mode.

Signed-off-by: ligd <[email protected]>
Flush the image memory to make sure the remote core access the correct
image.

Signed-off-by: Bowen Wang <[email protected]>
So we can distinguish the pm wakelock

Signed-off-by: ligd <[email protected]>
This BUG is introduced in PR: apache#13172

Signed-off-by: Bowen Wang <[email protected]>
rptun_dump related code has been moved to rptun.c from rptun_dump.c,
but file rptun_dump.c is not deleted in PR:
apache#11712

So delete this file.

Signed-off-by: Bowen Wang <[email protected]>
The function name rptun_panic_ is not consistent with other functions

Signed-off-by: Xiang Xiao <[email protected]>
Should invalidate the memory when the data is located in shared
memory and write by remote core.

Signed-off-by: Bowen Wang <[email protected]>
headrx is very convient to check weather current core miss interrupt
by comparing the headrx with the rx vring avail.idx for slave side or
rx vring used.idx for master side.

So move headrx out of the CONFIG_RPTUN_PM range.

Signed-off-by: Bowen Wang <[email protected]>
wdog has better performance than work queue

Signed-off-by: wangyongrong <[email protected]>
add remote addrenv to make the da is start from 0, so the uint32_t
da in resource table can store the correct address

Signed-off-by: Bowen Wang <[email protected]>
Move the panic logic in common places, later we can move more logic to
the framework instead of having the drivers implement it repeatedly.

Signed-off-by: Yongrong Wang <[email protected]>
Signed-off-by: Bowen Wang <[email protected]>
Because we can use the common part implemented in rptun

Signed-off-by: Yongrong Wang <[email protected]>
Add more common command for rptun and rpmsg_virtio frameworks,
also modify the rptun and rpmsg_virtio driver to use the common
commands.

Signed-off-by: Yongrong Wang <[email protected]>
Because locate the command at the end the resource table is unfriendly
when we want to support multi virtio devices instead only one virtio
rpmsg device.

Signed-off-by: Bowen Wang <[email protected]>
Leak backtrace:
    1 14 2096 9886 0x4318d768 [0x040320744] <romfs_fileconfigure+184> romfs/fs_romfsutil.c:1039
                                                 [0x04031fd3e] <romfs_open+378> romfs/fs_romfs.c:281
                                                 [0x04027fa9e] <file_open+446> vfs/fs_open.c:244
                                                 [0x0402ab986] <rptun_store_open+26> rptun/rptun.c:955
                                                 [0x04034cc88] <remoteproc_load+120> open-amp/lib/remoteproc/remoteproc.c:452
                                                 [0x0402ac8ac] <rptun_dev_start+176> rptun/rptun.c:748
                                                 [0x0402ad038] <rptun_ioctl+416> rptun/rptun.c:618

Signed-off-by: xuxingliang <[email protected]>
Master can send restart command to slave to reboot the slave core

Signed-off-by: mazhuang <[email protected]>
Signed-off-by: Yongrong Wang <[email protected]>
Sync the offset data type with resource table defined in OpenAMP
and Linux.

Signed-off-by: Bowen Wang <[email protected]>
@github-actions github-actions bot added Arch: simulator Issues related to the SIMulator Size: L The size of the change in this PR is large labels Sep 24, 2024
@xiaoxiang781216 xiaoxiang781216 merged commit 4bfa73f into apache:master Sep 25, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: simulator Issues related to the SIMulator Size: L The size of the change in this PR is large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants