Re: [RFC 10/26] subprojects/vm-memory: Patch vm-memory for QEMU memory backend

[Paolo Bonzini <pbonzini@redhat.com>]

On 8/8/25 10:17, Zhao Liu wrote:
> (+Hanna: I would like to align with Hanna on 0002.diff patch :-))
> 
> On Thu, Aug 07, 2025 at 03:59:26PM +0200, Paolo Bonzini wrote:
>> Date: Thu, 7 Aug 2025 15:59:26 +0200
>> From: Paolo Bonzini <pbonzini@redhat.com>
>> Subject: Re: [RFC 10/26] subprojects/vm-memory: Patch vm-memory for QEMU
>>   memory backend
>>
>> On 8/7/25 14:30, Zhao Liu wrote:
>>> Add 2 patches to support QEMU memory backend implementation.
>>>
>>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>>> ---
>>>    .../packagefiles/vm-memory-0.16-rs/0001.diff  |  81 +++++++++++++
>>>    .../packagefiles/vm-memory-0.16-rs/0002.diff  | 111 ++++++++++++++++++
>>>    subprojects/vm-memory-0.16-rs.wrap            |   2 +
>>>    3 files changed, 194 insertions(+)
>>>    create mode 100644 subprojects/packagefiles/vm-memory-0.16-rs/0001.diff
>>>    create mode 100644 subprojects/packagefiles/vm-memory-0.16-rs/0002.diff
>>>
>>> diff --git a/subprojects/packagefiles/vm-memory-0.16-rs/0001.diff b/subprojects/packagefiles/vm-memory-0.16-rs/0001.diff
>>> new file mode 100644
>>> index 000000000000..037193108d45
>>> --- /dev/null
>>> +++ b/subprojects/packagefiles/vm-memory-0.16-rs/0001.diff
>>> @@ -0,0 +1,81 @@
>>> +From 298f8ba019b2fe159fa943e0ae4dfd3c83ee64e0 Mon Sep 17 00:00:00 2001
>>> +From: Zhao Liu <zhao1.liu@intel.com>
>>> +Date: Wed, 6 Aug 2025 11:31:11 +0800
>>> +Subject: [PATCH 1/2] guest_memory: Add a marker tarit to implement
>>> + Bytes<GuestAddress> for GuestMemory
>>
>> This was a bit surprising.  Maybe this is something where GuestMemory needs
>> some extra flexibility.
> 
> At least, the default GuestMemory::try_access() need to re-implement in
> QEMU, and this is because GuestMemory::iter() doesn't fit for QEMU's
> case, and GuestMemory::to_region_addr() also needs adjustment to support
> complete translation.
> 
> For details,
> 
> 1) iter() - QEMU has implemented the two-level "page" walk in
>     `phys_page_find`, which is more efficient than linear iteration.
> 
> 2) to_region_addr() - it's function signature is:
> 
>      fn to_region_addr(
>          &self,
> 	addr: GuestAddress
>      ) -> Option<(&Self::R, MemoryRegionAddress)>;
> 
> but QEMU currentlt wants:
> 
>      fn translate(
>          &self,
>          addr: GuestAddress,
>          len: GuestUsize,
>          is_write: bool,
>      ) -> Option<(&MemoryRegionSection, MemoryRegionAddress, GuestUsize)>
> 
> `is_write` is mainly about IOMMU (and read-only case, but that could be
> workaround I think).
> 
> And the 3rd member `GuestUsize` of (&MemoryRegionSection,
> MemoryRegionAddress, GuestUsize) indicates the remianing size, which is
> used to detect cross-region case. Maybe this `GuestUsize` is not
> necessary in its return, since we can check the size of `MemoryRegionSection`
> later. But this would be a bit repetitive.
> 
> But at least, this marker trait is acceptable, right? :-)
> 
> The marker trait for GuestMemoryRegion is introduced at commit 66ff347
> ("refactor: use matches! instead of to_string() for tests").
> 
>>> @@ -0,0 +1,111 @@
>>> +From 2af7ea12a589fde619690e5060c01710cb6f2e0e Mon Sep 17 00:00:00 2001
>>> +From: Zhao Liu <zhao1.liu@intel.com>
>>> +Date: Wed, 6 Aug 2025 14:27:14 +0800
>>> +Subject: [PATCH 2/2] guest_memory: Add is_write argument for
>>> + GuestMemory::try_access()
>>
>> This should be fine.  But Hanna is also working on IOMMU so maybe this won't
>> be needed!
> 
> I'm not sure what method could align with Hanna's design. If there's
> another interface/method, I can have a try.

For example she already has similar fixes in 
https://github.com/rust-vmm/vm-memory/pull/327:

https://github.com/rust-vmm/vm-memory/pull/327/commits/9bcd5ac9b9ae37d1fb421f86f0aff310411933af
    Bytes: Fix read() and write()

    read() and write() must not ignore the `count` parameter: The
    mappings passed into the `try_access()` closure are only valid for up
    to `count` bytes, not more.

https://github.com/rust-vmm/vm-memory/pull/327/commits/2b83c72be656e5d46b83cb3a66d580e56cf33d5b
     Bytes: Do not use to_region_addr()

     When we switch to a (potentially) virtual memory model [...]
     the one memory-region-referencing part we are going to keep is
     `try_access()` [...] switch `Bytes::load()` and `store()` from using
     `to_region_addr()` to `try_access()`.

With some luck, your custom implementation of Bytes<GuestAddress> is not 
needed once vm-memory supports iommu.

Paolo