-
Notifications
You must be signed in to change notification settings - Fork 171
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
libmetal/nuttx/io.c: width matched access when read/write size = 1,2,4,8 #279
Conversation
51e1585
to
5124f24
Compare
If constraint is generic what about using macros metal_io_write/read ( x = 8,16,32 or 64) in open-amp? |
@arnopo Yes, we used libmetal implemented |
Thanks for the link! Something seems missing in you patch. You should check the alignement of the address to respect the virtio specification |
This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity. |
Follow the virtio spec v1.2: The driver MUST only use 32 bit wide and aligned reads and writes to access the control registers described in table 4.1. For the device-specific configuration space, the driver MUST use 8 bit wide accesses for 8 bit wide fields, 16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide and aligned accesses for 32 and 64 bit wide fields. Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com> Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
@arnopo How about leaving virtio devices and drivers to ensure that the address is aligned? |
@arnopo And could you check why the CI failed, seems not related to this PR? |
|
yes that can make sense a part of the spec. And checking only the MMIO register base address should ensure the alignement. |
*(uint32_t *)dst = *(uint32_t *)va; | ||
else if (len == 8) { | ||
*(uint32_t *)dst = *(uint32_t *)va; | ||
*((uint32_t *)dst + 1) = *((uint32_t *)va + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using uint64_t ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec says:
The driver MUST use 8 bit wide accesses for 8 bit wide fields, 16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide and aligned accesses for 32 and 64 bit wide fields.
libmetal/nuttx/io.c: width matched access when read/write size = 1,2,4,8
Follow the virtio spec v1.2
So change the nuttx implemented io read/write operation.