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

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/22/24 07:58, David Airlie wrote:
> On Fri, Nov 22, 2024 at 11:24 AM Matthew Wilcox <willy@infradead.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