linux-kernel - Re: [PATCH v3 0/2] rust: page: Add support for existing struct page mappings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f5d11fd7-6909-4a9f-b041-e6e59c221e05@redhat.com>
Date: Fri, 22 Nov 2024 13:37:43 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: David Airlie <airlied@...hat.com>, Matthew Wilcox <willy@...radead.org>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>,
 Boqun Feng <boqun.feng@...il.com>, Alice Ryhl <aliceryhl@...gle.com>,
 rust-for-linux@...r.kernel.org, Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>, Wedson Almeida Filho
 <wedsonaf@...il.com>, Valentin Obst <kernel@...entinobst.de>,
 open list <linux-kernel@...r.kernel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
 Kairui Song <ryncsn@...il.com>
Subject: Re: [PATCH v3 0/2] rust: page: Add support for existing struct page
 mappings

On 11/22/24 07:58, David Airlie wrote:
> On Fri, Nov 22, 2024 at 11:24 AM Matthew Wilcox <willy@...radead.org> wrote:
>>
>> On Fri, Nov 22, 2024 at 01:18:28AM +0200, Abdiel Janulgue wrote:
>>> We need an abstraction of struct page to construct a scatterlist which is
>>> needed for an internal firmware structure. Now most of pages needed there
>>> come from vmalloc_to_page() which, unlike the current rust Page abstraction,
>>> not allocated on demand but is an existing mapping.
>>>
>>> Hope that clears things up!
>>
>> That's very helpful!  So the lifetime of the scatterllist must not
>> outlive the lifetime of the vmalloc allocation.  That means you can call
>> kmap_local_page() on the page in the scatterlist without worrying about
>> the refcount of the struct page.  BTW, you can't call page_address() on
>> vmalloc memory because vmalloc can allocate pages from HIGHMEM.  Unless
>> you're willing to disable support for 32-bit systems with highmem ...
>>
> 
> https://elixir.bootlin.com/linux/v6.11.5/source/drivers/gpu/drm/nouveau/nvkm/core/firmware.c#L266
> 
> This is the C code we want to rustify.

I don't think you want to increase/decrease the refcount there.  Instead 
you tie the lifetime of the returned page to the lifetime of the thing 
that provides the page, which would be some kind of NvkmFirmware struct.

pub enum NvkmFirmwareData {
     Ram(KBox<[PageSlice]>,
     Dma(CoherentAllocation<PageSlice>,
     Sgt(VBox<[PageSlice]>,
}

pub struct NvkmFirmware {
     ...,
     img: NvkmFirmwareData,
}

pub struct NvkmFirmwarePages<'a> {
     fw: &'a NvkmFirmware,
     sgt: SgTable,
}

impl NvkmFirmware {
     fn get_sgl(&self) -> NvkmFirmwarePages { ... }
}


Perhaps a trait that is implemented by both {K,V,KV}Vec<PageSlice> and 
{K,V,KV}Box<[PageSlice]>, like

trait ToComponentPage {
     fn to_component_page(&self, i: usize) -> &Page;
}

impl ToComponentPage for KVec<PageSlice> { // same for KBox<[PageSlice]>
     fn to_component_page(&self, i: usize) -> &Page {
         let base = &self[i << PAGE_SHIFT..];
         bindings::virt_to_page(base.as_ptr())
     }
}

impl ToComponentPage for VVec<PageSlice> { // same for VBox<[PageSlice]>
     fn to_component_page(&self, i: usize) -> &Page {
         let base = &self[i << PAGE_SHIFT..];
         bindings::vmalloc_to_page(base.as_ptr())
     }
}

?  And possibly also

trait ToComponentPageMut {
     fn to_component_page_mut(&mut self, i: usize) -> &Page;
}

which would be implemented by the Box types, but not by the Vec types 
because their data is not pinned.

Paolo