Skip to content

kernel/platform: Add mmio support for native platform#775

Merged
joergroedel merged 2 commits intococonut-svsm:mainfrom
luigix25:mmio_native
Oct 1, 2025
Merged

kernel/platform: Add mmio support for native platform#775
joergroedel merged 2 commits intococonut-svsm:mainfrom
luigix25:mmio_native

Conversation

@luigix25
Copy link
Collaborator

@luigix25 luigix25 commented Aug 5, 2025

Currently native platform does not support mmio operations. This can be useful when developing or testing drivers that rely on MMIO, like virtio-vsock or virtio-block.

Tested on top of my RFC #766 that introduces Vsock support, using make test-in-svsm

@luigix25
Copy link
Collaborator Author

luigix25 commented Aug 5, 2025

@stefano-garzarella thanks for the review!
I should have addressed all your comments.

@luigix25 luigix25 marked this pull request as ready for review August 5, 2025 15:28
@luigix25
Copy link
Collaborator Author

luigix25 commented Aug 7, 2025

@stefano-garzarella @osteffenrh thank you for the review, I addressed all of your comments :)

@stefano-garzarella
Copy link
Member

@luigix25 I just posted #782 that maybe can simplify a bit also the code in this PR

@luigix25
Copy link
Collaborator Author

luigix25 commented Aug 7, 2025

@stefano-garzarella yes! I'll wait until that PR is merged and then rebase

@stefano-garzarella stefano-garzarella added needs-rebase The PR needs to be rebased to the latest upstream branch in-review PR is under active review and not yet approved labels Aug 7, 2025
@joergroedel joergroedel self-requested a review August 19, 2025 15:47
@luigix25
Copy link
Collaborator Author

I rebased to the latest upstream that is now using MaybeUninit. I had to make some changes to mmio_read.
I used the same approach used in read_buffer_slice.

Currently, `mmio_read` and `mmio_write` take `PhysAddr` as a parameter.
This can be limiting for other platforms that may require a virtual address,
because going back from physical to virtual is not always possible.

Change the parameter from `PhysAddr` to `VirtAddr` and let the platform
handle the conversion if necessary.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
@joergroedel joergroedel removed the needs-rebase The PR needs to be rebased to the latest upstream branch label Sep 10, 2025
@luigix25
Copy link
Collaborator Author

As suggested by @stefano-garzarella, I introduced an helper for mmio_read and mmio_write that uses generics to reduce code duplication.

Ideally it would be nice to keep the generics from the Hal itself, but I'm not sure on how to do it because SvsmPlatform is used with dyn in some places.

Introduce `mmio_write` and `mmio_read` for native platform. This is useful
when developing or testing drivers that rely on MMIO, like virtio.

It was not possible to implement these two function using generics
because SvsmPlatform struct uses the `dyn` trait.

It is also not possible to iterate and read byte by byte as this
would violate the virtio spec, where is stated that all multi-byte
long registers must be read in one operation, therefore making it
impossible to implement a virtio driver.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
@luigix25
Copy link
Collaborator Author

Added a check on data_ptr alignment and use read/write_aligned() or read()/write()

Copy link
Collaborator

@osteffenrh osteffenrh left a comment

Choose a reason for hiding this comment

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

Tested it on top of #823 ✔️

Copy link
Member

@stefano-garzarella stefano-garzarella left a comment

Choose a reason for hiding this comment

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

In the last TSC meeting we discussed this PR and decided to merge, but for long term we should improve our MMIO interface. Of course this is outside the scope of this PR so we can postpone it.

I'm approving and wait @joergroedel to take a final look before merging it.

@stefano-garzarella stefano-garzarella added ready-to-merge PR is ready for merging into main branch and removed in-review PR is under active review and not yet approved labels Oct 1, 2025
@joergroedel joergroedel merged commit 710ace6 into coconut-svsm:main Oct 1, 2025
4 checks passed
@luigix25 luigix25 deleted the mmio_native branch October 3, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge PR is ready for merging into main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants