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

virtqueue: Fix comment on shm_io and fix type #570

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

glneo
Copy link
Contributor

@glneo glneo commented Mar 15, 2024

This should hold a pointer to an metal_io_region, make that the type.

Also fix the comment, this region holds the address of the message buffers, not the vring's descriptor table nor available/used rings. It is only used for virt-to-phys/phys-to-vert on the buffers pointed to by these descriptors.

This comment seems to have cause an issue in virtio_mmio_drv where this region was used to translate the address of the vring descriptors. This may have worked if the vring descriptors where part of the same IO space as the buffers they point to, but this is not guaranteed to always be the case. Fix that here.

Copy link
Collaborator

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

Please split in 2 commits, else LGTM

The metal_io_region in the virtqueue struct is for translating
the buffer address stored in the virtqueue descriptor table, not
the address of the descriptor table itself.

This may have worked previously if the vring descriptors where part
of the same IO space as the buffers they point to, but this is not
guaranteed to always be the case. Fix that here.

Signed-off-by: Andrew Davis <[email protected]>
This should hold a pointer to a metal_io_region, make that the type.

Also fix the comment above this variable. This region holds the address
of the message buffers, not the vring descriptor table nor available/used
ring data. It is only used for virt-to-phys/phys-to-vert translation on
the buffers pointed to by these descriptors.

Signed-off-by: Andrew Davis <[email protected]>
Copy link
Contributor

@edmooring edmooring left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@arnopo arnopo requested a review from danmilea April 9, 2024 09:47
Copy link
Collaborator

@danmilea danmilea left a comment

Choose a reason for hiding this comment

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

OK to merge.

@arnopo arnopo merged commit 4db4a08 into OpenAMP:main Apr 17, 2024
3 checks passed
@arnopo arnopo added this to the Release V2024.04 milestone Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants