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

qemu-armv7a Ivshmem config and document update #15818

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CV-Bowen
Copy link
Contributor

@CV-Bowen CV-Bowen commented Feb 12, 2025

Summary

commit bfd8f01
Author: Bowen Wang [email protected]
Date: Wed Feb 12 15:46:59 2025 +0800

arm/qemu-armv7a: optimize the rpproxy and rpserver configs and doc

add discard-data=on in proxy launch command, so the proxy side
will clean the share memory to make sure that the shared memory is clean
every time server and proxy are started.

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

commit 7cd275b
Author: Bowen Wang [email protected]
Date: Wed Feb 12 15:46:15 2025 +0800

rpmsgblk_server: fix the syslog format warning

misc/rpmsgblk_server.c:135:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'int32_t' {aka 'long int'} [-Wformat=]
  135 |           ferr("block device open failed, ret=%d\n", msg->header.result);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~~~
      |                                                                 |
      |                                                                 int32_t {aka long int}
misc/rpmsgblk_server.c:135:48: note: format string is defined here
  135 |           ferr("block device open failed, ret=%d\n", msg->header.result);
      |                                               ~^
      |                                                |
      |                                                int
      |                                               %ld
misc/rpmsgblk_server.c: In function 'rpmsgblk_close_handler':
misc/rpmsgblk_server.c:170:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'int32_t' {aka 'long int'} [-Wformat=]
  170 |           ferr("block device close failed, ret=%d\n", msg->header.result);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~~~
      |                                                                  |
      |                                                                  int32_t {aka long int}
misc/rpmsgblk_server.c:170:49: note: format string is defined here
  170 |           ferr("block device close failed, ret=%d\n", msg->header.result);
      |                                                ~^
      |                                                 |
      |                                                 int
      |                                                %ld

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

Impact

ivshmem configs

Testing

testing with qemu-armv7a:rpserver_ivshmem qemu-armv7a:rpproxy_ivshmem with rpmsg ping

qemu-armv7a test result

server> rptun ping all 1 1 1 1
[   17.809022] [ 5] [ EMERG] [server] ping times: 1
[   17.809780] [ 5] [ EMERG] [server] buffer_len: 2032, send_len: 17
[   17.811340] [ 5] [ EMERG] [server] avg: 0 s, 54224016 ns
[   17.812204] [ 5] [ EMERG] [server] min: 0 s, 54224016 ns
[   17.813158] [ 5] [ EMERG] [server] max: 0 s, 54224016 ns
[   17.814339] [ 5] [ EMERG] [server] rate: 0.002508 Mbits/sec
server> [   68.740115] [ 5] [ EMERG] [proxy] ping times: 1
[   68.741721] [ 5] [ EMERG] [proxy] buffer_len: 2032, send_len: 17
[   68.743624] [ 5] [ EMERG] [proxy] avg: 0 s, 46184560 ns
[   68.744854] [ 5] [ EMERG] [proxy] min: 0 s, 46184560 ns
[   68.746030] [ 5] [ EMERG] [proxy] max: 0 s, 46184560 ns
[   68.747437] [ 5] [ EMERG] [proxy] rate: 0.002944 Mbits/sec

server> 
server> uname -a
NuttX server 12.7.0 381d3fe64f4-dirty Feb 12 2025 15:44:48 arm qemu-armv7a
server> 

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Area: Drivers Drivers issues Board: arm Board: arm64 Size: S The size of the change in this PR is small labels Feb 12, 2025
@CV-Bowen
Copy link
Contributor Author

@anchao @acassis @xiaoxiang781216 @GUIDINGLI Could you help review this PR?

@anchao
Copy link
Contributor

anchao commented Feb 12, 2025

please discard roles changes:
#15810

@CV-Bowen
Copy link
Contributor Author

CV-Bowen commented Feb 12, 2025

@anchao OK, I missed #15810, let's merge #15810 first and I will rebase this PR.

Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move the driver chanegs to a separate PR

please rename commit
rpmsgblk_server: fix the syslog format warning title to

drivers/rpmsgblk_server: fix the syslog format warning

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CV-Bowen please include a Documentation/ to rpproxy_ivshmem and rpserver_ivshmem board profiles explaining what it does and how to test/validate it.

misc/rpmsgblk_server.c:135:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'int32_t' {aka 'long int'} [-Wformat=]
  135 |           ferr("block device open failed, ret=%d\n", msg->header.result);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~~~
      |                                                                 |
      |                                                                 int32_t {aka long int}
misc/rpmsgblk_server.c:135:48: note: format string is defined here
  135 |           ferr("block device open failed, ret=%d\n", msg->header.result);
      |                                               ~^
      |                                                |
      |                                                int
      |                                               %ld
misc/rpmsgblk_server.c: In function 'rpmsgblk_close_handler':
misc/rpmsgblk_server.c:170:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'int32_t' {aka 'long int'} [-Wformat=]
  170 |           ferr("block device close failed, ret=%d\n", msg->header.result);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~~~
      |                                                                  |
      |                                                                  int32_t {aka long int}
misc/rpmsgblk_server.c:170:49: note: format string is defined here
  170 |           ferr("block device close failed, ret=%d\n", msg->header.result);
      |                                                ~^
      |                                                 |
      |                                                 int
      |                                                %ld

Signed-off-by: Bowen Wang <[email protected]>
add discard-data=on in proxy launch command, so the proxy side
will clean the share memory to make sure that the shared memory is clean
every time server and proxy are started.

Signed-off-by: Bowen Wang <[email protected]>
@CV-Bowen
Copy link
Contributor Author

CV-Bowen commented Feb 12, 2025

@anchao Done
@jerpelea Update commit messages done, and I found this warning when I enable RPMSGBLK_SERVER, so I think place this patch in this PR is reasonable.
@acassis There is alreay a document about the rpserver_ivshmem and rpproxy_ivshmem

@acassis
Copy link
Contributor

acassis commented Feb 12, 2025

@anchao
Copy link
Contributor

anchao commented Feb 13, 2025

@CV-Bowen I cannot find it:

https://nuttx.apache.org/docs/latest/search.html?q=rpserver_ivshmem&check_keywords=yes&area=default

https://nuttx.apache.org/docs/latest/search.html?q=rpproxy_ivshmem&check_keywords=yes&area=default#

All new board profiles need to be documented to let users know what they are suppose to do

@acassis @CV-Bowen @yf13

I added the missing qemu-armv8a/ivshmem chapter

#15827

@CV-Bowen
Copy link
Contributor Author

Documentation/platforms/arm/qemu/boards/qemu-armv7a/index.rst
@acassis This has documented the qemu armv7a rpproxy_ivshmem and rpserver_ivshmem configs usages.

@CV-Bowen CV-Bowen changed the title qemu-armv7a and armv8a Ivshmem configs update qemu-armv7a Ivshmem configs update Feb 13, 2025
@CV-Bowen CV-Bowen changed the title qemu-armv7a Ivshmem configs update qemu-armv7a Ivshmem config and document update Feb 13, 2025
@jerpelea
Copy link
Contributor

@anchao Done @jerpelea Update commit messages done, and I found this warning when I enable RPMSGBLK_SERVER, so I think place this patch in this PR is reasonable. @acassis There is alreay a document about the rpserver_ivshmem and rpproxy_ivshmem

I think that the driver commit should be in a separate PR since the fix is independent from the other commit

@CV-Bowen
Copy link
Contributor Author

CV-Bowen commented Feb 13, 2025

@jerpelea I enable the CONFIG_BLK_RPMSG_SERVER in rpproxy_server and found this compile warning, so I think this fix is related with other commit in this PR?

@anchao
Copy link
Contributor

anchao commented Feb 14, 2025

These two commits are actually interdependent

  1. Solving the compiler warning alone will not be reflected in CI, it depends on the corresponding kconfig being enabled
  2. After enabling config, there will be compilation warnings and CI will not pass normally
    I think we cannot split PRs completely according to the directory structure, the granularity of the feature set may be better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Documentation Improvements or additions to documentation Area: Drivers Drivers issues Board: arm Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants