| Message ID | 20241120-vma-v8-3-eb31425da66b@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Rust support for mm_struct, vm_area_struct, and mmap | expand |
On Wed, Nov 20, 2024 at 02:49:57PM +0000, Alice Ryhl wrote: > The vm_insert_page method is only usable on vmas with the VM_MIXEDMAP > flag, so we introduce a new type to keep track of such vmas. Worth mentioning that it can be used for VMAs without this flag set, but if so we must hold a _write_ lock to be able to do so, so it can update the flag itself, however we intend only to use it with VMAs which already have this flag set. > > The approach used in this patch assumes that we will not need to encode > many flag combinations in the type. I don't think we need to encode more > than VM_MIXEDMAP and VM_PFNMAP as things are now. However, if that > becomes necessary, using generic parameters in a single type would scale > better as the number of flags increases. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > rust/kernel/mm/virt.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 67 insertions(+), 1 deletion(-) > > diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs > index 1e755dca46dd..de7f2338810a 100644 > --- a/rust/kernel/mm/virt.rs > +++ b/rust/kernel/mm/virt.rs > @@ -4,7 +4,14 @@ > > //! Virtual memory. > > -use crate::{bindings, types::Opaque}; > +use crate::{ > + bindings, > + error::{to_result, Result}, > + page::Page, > + types::Opaque, > +}; > + > +use core::ops::Deref; > > /// A wrapper for the kernel's `struct vm_area_struct` with read access. > /// > @@ -80,6 +87,65 @@ pub fn zap_page_range_single(&self, address: usize, size: usize) { > ) > }; > } > + > + /// Check whether the `VM_MIXEDMAP` flag is set. > + #[inline] > + pub fn check_mixedmap(&self) -> Option<&VmAreaMixedMap> { Nitty + a little bikesheddy (this may be my mistake as I am unfamiliar with rust idioms also) but shouldn't this be 'as_mixedmap_vma()' or something? > + if self.flags() & flags::MIXEDMAP != 0 { > + // SAFETY: We just checked that `VM_MIXEDMAP` is set. All other requirements are > + // satisfied by the type invariants of `VmAreaRef`. > + Some(unsafe { VmAreaMixedMap::from_raw(self.as_ptr()) }) > + } else { > + None > + } > + } > +} > + > +/// A wrapper for the kernel's `struct vm_area_struct` with read access and `VM_MIXEDMAP` set. > +/// > +/// It represents an area of virtual memory. > +/// > +/// # Invariants > +/// > +/// The caller must hold the mmap read lock or the vma read lock. The `VM_MIXEDMAP` flag must be > +/// set. > +#[repr(transparent)] > +pub struct VmAreaMixedMap { > + vma: VmAreaRef, > +} > + > +// Make all `VmAreaRef` methods available on `VmAreaMixedMap`. > +impl Deref for VmAreaMixedMap { > + type Target = VmAreaRef; > + > + #[inline] > + fn deref(&self) -> &VmAreaRef { > + &self.vma > + } > +} Ah OK, thinking back to the 'impl Deref' from the other patch, I guess this allows you to deref VmAreaMixedMap as a VmAreaRef, does it all sort of automagically merge methods for you as if they were mix-ins then? > + > +impl VmAreaMixedMap { > + /// Access a virtual memory area given a raw pointer. > + /// > + /// # Safety > + /// > + /// Callers must ensure that `vma` is valid for the duration of 'a, and that the mmap read lock > + /// (or stronger) is held for at least the duration of 'a. The `VM_MIXEDMAP` flag must be set. > + #[inline] > + pub unsafe fn from_raw<'a>(vma: *const bindings::vm_area_struct) -> &'a Self { > + // SAFETY: The caller ensures that the invariants are satisfied for the duration of 'a. > + unsafe { &*vma.cast() } > + } > + > + /// Maps a single page at the given address within the virtual memory area. > + /// > + /// This operation does not take ownership of the page. > + #[inline] > + pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result { I'm guessing this 'Result' type encodes 0 vs. -Exxx error codes? > + // SAFETY: The caller has read access and has verified that `VM_MIXEDMAP` is set. The page > + // is order 0. The address is checked on the C side so it can take any value. > + to_result(unsafe { bindings::vm_insert_page(self.as_ptr(), address as _, page.as_ptr()) }) > + } > } It's really nice to abstract this as a seprate type and then to know its methods only apply in known circumstances... I guess in future we can use clever generic types when it comes to combinations of characteristics that would otherwise result in a combinatorial explosion? > > /// The integer type used for vma flags. > > -- > 2.47.0.371.ga323438b13-goog >
diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs index 1e755dca46dd..de7f2338810a 100644 --- a/rust/kernel/mm/virt.rs +++ b/rust/kernel/mm/virt.rs @@ -4,7 +4,14 @@ //! Virtual memory. -use crate::{bindings, types::Opaque}; +use crate::{ + bindings, + error::{to_result, Result}, + page::Page, + types::Opaque, +}; + +use core::ops::Deref; /// A wrapper for the kernel's `struct vm_area_struct` with read access. /// @@ -80,6 +87,65 @@ pub fn zap_page_range_single(&self, address: usize, size: usize) { ) }; } + + /// Check whether the `VM_MIXEDMAP` flag is set. + #[inline] + pub fn check_mixedmap(&self) -> Option<&VmAreaMixedMap> { + if self.flags() & flags::MIXEDMAP != 0 { + // SAFETY: We just checked that `VM_MIXEDMAP` is set. All other requirements are + // satisfied by the type invariants of `VmAreaRef`. + Some(unsafe { VmAreaMixedMap::from_raw(self.as_ptr()) }) + } else { + None + } + } +} + +/// A wrapper for the kernel's `struct vm_area_struct` with read access and `VM_MIXEDMAP` set. +/// +/// It represents an area of virtual memory. +/// +/// # Invariants +/// +/// The caller must hold the mmap read lock or the vma read lock. The `VM_MIXEDMAP` flag must be +/// set. +#[repr(transparent)] +pub struct VmAreaMixedMap { + vma: VmAreaRef, +} + +// Make all `VmAreaRef` methods available on `VmAreaMixedMap`. +impl Deref for VmAreaMixedMap { + type Target = VmAreaRef; + + #[inline] + fn deref(&self) -> &VmAreaRef { + &self.vma + } +} + +impl VmAreaMixedMap { + /// Access a virtual memory area given a raw pointer. + /// + /// # Safety + /// + /// Callers must ensure that `vma` is valid for the duration of 'a, and that the mmap read lock + /// (or stronger) is held for at least the duration of 'a. The `VM_MIXEDMAP` flag must be set. + #[inline] + pub unsafe fn from_raw<'a>(vma: *const bindings::vm_area_struct) -> &'a Self { + // SAFETY: The caller ensures that the invariants are satisfied for the duration of 'a. + unsafe { &*vma.cast() } + } + + /// Maps a single page at the given address within the virtual memory area. + /// + /// This operation does not take ownership of the page. + #[inline] + pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result { + // SAFETY: The caller has read access and has verified that `VM_MIXEDMAP` is set. The page + // is order 0. The address is checked on the C side so it can take any value. + to_result(unsafe { bindings::vm_insert_page(self.as_ptr(), address as _, page.as_ptr()) }) + } } /// The integer type used for vma flags.
The vm_insert_page method is only usable on vmas with the VM_MIXEDMAP flag, so we introduce a new type to keep track of such vmas. The approach used in this patch assumes that we will not need to encode many flag combinations in the type. I don't think we need to encode more than VM_MIXEDMAP and VM_PFNMAP as things are now. However, if that becomes necessary, using generic parameters in a single type would scale better as the number of flags increases. Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- rust/kernel/mm/virt.rs | 68 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-)