Re: [PATCH 5/6] rust: page: Add physical address conversion functions

[Alice Ryhl]

On Mon, Feb 3, 2025 at 10:54 AM Fiona Behrens <me@xxxxxxxxxx> wrote:
>
> Asahi Lina <lina@xxxxxxxxxxxxx> writes:
>
> > Add methods to allow code using the Page type to obtain the physical
> > address of a page, convert to and from an (owned) physical address, and
> > borrow a Page from a physical address. Most of these operations are, as
> > you might expect, unsafe.
> >
> > These primitives are useful to implement page table structures in Rust,
> > and to implement arbitrary physical memory access (as needed to walk
> > arbitrary page tables and dereference through them). These mechanisms
> > are, of course, fraught with danger, and are only expected to be used
> > for core memory management code (in e.g. drivers with their own device
> > page table implementations) and for debug features such as crash dumps
> > of device memory.
> >
> > Signed-off-by: Asahi Lina <lina@xxxxxxxxxxxxx>
> > ---
> >  rust/helpers/page.c | 26 +++++++++++++++++++++
> >  rust/kernel/page.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 91 insertions(+)
> >
> > diff --git a/rust/helpers/page.c b/rust/helpers/page.c
> > index b3f2b8fbf87fc9aa89cb1636736c52be16411301..1c3bd68818d77f7ce7806329b8f040a7d4205bb3 100644
> > --- a/rust/helpers/page.c
> > +++ b/rust/helpers/page.c
> > @@ -1,5 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >
> > +#include <linux/io.h>
> >  #include <linux/gfp.h>
> >  #include <linux/highmem.h>
> >
> > @@ -17,3 +18,28 @@ void rust_helper_kunmap_local(const void *addr)
> >  {
> >       kunmap_local(addr);
> >  }
> > +
> > +struct page *rust_helper_phys_to_page(phys_addr_t phys)
> > +{
> > +     return phys_to_page(phys);
> > +}
> > +
> > +phys_addr_t rust_helper_page_to_phys(struct page *page)
> > +{
> > +     return page_to_phys(page);
> > +}
> > +
> > +unsigned long rust_helper_phys_to_pfn(phys_addr_t phys)
> > +{
> > +     return __phys_to_pfn(phys);
> > +}
> > +
> > +struct page *rust_helper_pfn_to_page(unsigned long pfn)
> > +{
> > +     return pfn_to_page(pfn);
> > +}
> > +
> > +bool rust_helper_pfn_valid(unsigned long pfn)
> > +{
> > +     return pfn_valid(pfn);
> > +}
> > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> > index fe5f879f9d1a86083fd55c682fad9d52466f79a2..67cd7006fa63ab5aed4c4de2be639ed8e1fbc2ba 100644
> > --- a/rust/kernel/page.rs
> > +++ b/rust/kernel/page.rs
> > @@ -3,6 +3,7 @@
> >  //! Kernel page allocation and management.
> >
> >  use crate::{
> > +    addr::*,
> >      alloc::{AllocError, Flags},
> >      bindings,
> >      error::code::*,
> > @@ -10,6 +11,7 @@
> >      types::{Opaque, Ownable, Owned},
> >      uaccess::UserSliceReader,
> >  };
> > +use core::mem::ManuallyDrop;
> >  use core::ptr::{self, NonNull};
> >
> >  /// A bitwise shift for the page size.
> > @@ -249,6 +251,69 @@ pub unsafe fn copy_from_user_slice_raw(
> >              reader.read_raw(unsafe { core::slice::from_raw_parts_mut(dst.cast(), len) })
> >          })
> >      }
> > +
> > +    /// Returns the physical address of this page.
> > +    pub fn phys(&self) -> PhysicalAddr {
>
> Rust  uses for similar references `as_*` so `as_phys`, would it make
> sense to use the same naming format here?

The as_ prefix is only used when returning a borrow, which is not the
case here. I think not having a prefix is correct in this case.

https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv

Alice