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

Xen/mmapv3 #241

Merged
merged 7 commits into from
Jun 28, 2023
Merged

Xen/mmapv3 #241

merged 7 commits into from
Jun 28, 2023

Conversation

vireshk
Copy link
Contributor

@vireshk vireshk commented Jun 23, 2023

Add support for Xen specific memory mapping

Xen allows two memory mapping mechanisms:

  • Foreign mapping: entire guest space is mapped at once.
  • Grant mapping: guest controls what backend is allowed to map.

Both these mechanisms required additional mapping support, for example
an ioctl() needs to be issued (with arguments passed from backend),
along with mmap().

Also for grant memory mapping, the mapping may or may not be done in
advance and may be required to be done on-demand.

Add memory mapping support for both these cases, under a new feature
"xen".

We can't use the existing as_ptr() API, as the memory may not be mapped
already. Mark the existing API available to non-xen implementations only.

Also add new implementations, ptr_guard() and ptr_guard_mut(), for both xen
and non-xen platforms, which can be used to map memory and then get a
pointer to it, work on it, and unmap the memory automatically.

This also adds few custom buidkite testcases to test Xen based implementation.

Signed-off-by: Viresh Kumar [email protected]

@jiangliu
Copy link
Member

Really hope GrantTable is available for kvm too:)

used for all tests in this crate.

It was decided by the `rust-vmm` maintainers to keep the interface simple and
build the crate for either standard Unix memory mapping or Xen, and not both.
Copy link
Member

Choose a reason for hiding this comment

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

+1

@vireshk
Copy link
Contributor Author

vireshk commented Jun 27, 2023

Really hope GrantTable is available for kvm too:)

:)

I haven't really followed the pKvm discussions, since it is going to be about secure guests, they will surely need something similar. Either something exactly like Xen, or they may fix the mappable areas somewhere (which may have penalties in form on no ZeroCopy).

Copy link
Collaborator

@roypat roypat 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! I didn't really look too deeply into the mmap_xen.rs code because I'm lacking Xen expertise, but everything else I'm happy with approving! It's just missing a Changelog entry for what's added (Xen support, the new API for accessing pointers to volatile slices is all I think?) and we should be good to go :)

src/volatile_memory.rs Outdated Show resolved Hide resolved
src/volatile_memory.rs Outdated Show resolved Hide resolved
Xen allows two memory mapping mechanisms:
- Foreign mapping: entire guest space is mapped at once.
- Grant mapping: guest controls what backend is allowed to map.

Both these mechanisms required additional mapping support, for example
an ioctl() needs to be issued (with arguments passed from backend),
along with mmap().

Also for grant memory mapping, the mapping may or may not be done in
advance and may be required to be done on-demand. Support for that will
be added by a separate commit later.

Add memory mapping support for both these cases, under a new feature
"xen". Also add support to do generic Unix type mapping via Xen, in
absence of foreign or grant mapping, which will be used by test code for
other modules.

Signed-off-by: Viresh Kumar <[email protected]>
@vireshk
Copy link
Contributor Author

vireshk commented Jun 27, 2023

Looks good! I didn't really look too deeply into the mmap_xen.rs code because I'm lacking Xen expertise, but everything else I'm happy with approving! It's just missing a Changelog entry for what's added (Xen support, the new API for accessing pointers to volatile slices is all I think?) and we should be good to go :)

Thanks. I have updated the changelog and also added two documentation examples (for foreign and grant mappings) in Xen's MmapRegion::from_range() helper.

@roypat
Copy link
Collaborator

roypat commented Jun 27, 2023

Looks good! I didn't really look too deeply into the mmap_xen.rs code because I'm lacking Xen expertise, but everything else I'm happy with approving! It's just missing a Changelog entry for what's added (Xen support, the new API for accessing pointers to volatile slices is all I think?) and we should be good to go :)

Thanks. I have updated the changelog and also added two documentation examples (for foreign and grant mappings) in Xen's MmapRegion::from_range() helper.

I think you might have forgotten to push the changes? 😅

@vireshk
Copy link
Contributor Author

vireshk commented Jun 27, 2023

I think you might have forgotten to push the changes? sweat_smile

Maybe not :)

65f2c8d

@roypat
Copy link
Collaborator

roypat commented Jun 27, 2023

I think you might have forgotten to push the changes? sweat_smile

Maybe not :)

65f2c8d

ah whoops missed that, so sorry! Am still not seeing the changelog though 🤔

@vireshk
Copy link
Contributor Author

vireshk commented Jun 27, 2023

ah whoops missed that, so sorry! Am still not seeing the changelog though thinking

My mistake. I though you were asking me to update the Pull request's commit message :(, only now realized you meant CHANGELOG.md :)

Done now.

For Xen grant memory mapping model, the memory can't always be mapped in
advance and it may be required to map it on the fly.

This commit adds support for the same by introducing a new field for the
various volatile memory structures: `mmap: Option<&'a MmapInfo>`. The
type `MmapInfo` is set to `PhantomData<()>` for all the existing users,
and is set to `MmapXen` for Xen. All the existing users set the new
field to `None` and so the behavior remains unchanged for them.

With Xen grant memory mappings, the address returned by as_ptr() will
simply be an offset into the region, as the real mapping happens at a
later point.

In order not to break existing users of the API, lets keep the existing
API around for non-xen implementations.

Add new implementations, ptr_guard() and ptr_guard_mut(), for both xen
and non-xen platforms, which can be used to map memory and then get a
pointer to it, work on it, and unmap the memory automatically.

Signed-off-by: Viresh Kumar <[email protected]>
Add a documentation examples for Xen specific mappings. They are built
only for UNIX mapping type currently to make sure tests don't fail, but
the documentation does show actual Xen code.

Signed-off-by: Viresh Kumar <[email protected]>
Update README with implementation details of the Xen mapping model.

Signed-off-by: Viresh Kumar <[email protected]>
Some of the custom tests are run with only "backend-mmap" feature
enabled. Run them for "xen" as well.

Some of the standard tests from rust-vmm-ci run with "--all-features"
(which include "xen" too), add custom tests for those without the "xen"
feature.

Since the tests from rust-vmm-ci are named normally (without "xen"
keyword), append non-xen tests with "-no-xen" to make those clear.

Signed-off-by: Viresh Kumar <[email protected]>
Update CHANGELOG.md with latest updates.

Signed-off-by: Viresh Kumar <[email protected]>
Update coverage score, based on new tests.

Signed-off-by: Viresh Kumar <[email protected]>
@Ablu
Copy link
Contributor

Ablu commented Jun 28, 2023

/me is still investigating some test failures with virtiofsd. May very likely be in my setup. Will report once I figured it out.

@Ablu
Copy link
Contributor

Ablu commented Jun 28, 2023

Tested OK with virtiofsd + small patch [1] on Xen [2] with grant mappings [3] on 6.1 series kernel [4].

Errors were my own.

[1] https://gitlab.com/ablu/virtiofsd/-/commit/9466396862dde30f3b692c6e9eacbc89044eb874
[2] vireshk/xen@acb25f0
[3] virtio = [ "type=virtio,device26,transport=mmio,grant_usage=1" ]
[4] 6.1.24-yocto-standard

@vireshk
Copy link
Contributor Author

vireshk commented Jun 28, 2023

Tested OK with virtiofsd + small patch [1] on Xen [2] with grant mappings [3] on 6.1 series kernel [4].

Thanks Erik.

@jiangliu jiangliu merged commit a3cc708 into rust-vmm:main Jun 28, 2023
@vireshk vireshk deleted the xen/mmapv3 branch June 28, 2023 15:45
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