Re: [RFC PATCH v4 06/25] KVM: guest_memfd: Support device-backed PFN providers

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Fred Griffoul <griffoul@gmail.com> writes:

> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> writes:
>
>> Device resources such as PCI BARs supply PFNs directly and cannot use
>> the folio allocation path. Store the mapping owner and device ID in the
>> core context during attach(), and use them when preparing device
>> memslots. Keep device memory separate from ordinary PFN-backed RAM.
>
> The device rules you wrote down in api.rst and locking.rst helped a
> lot. If I read v4 right, the device backing differs from the RAM PFN
> backing in these places:
>
>   1. PFN mode calls RAM arch hooks: kvm_arch_gmem_make_shared() at
>      conversion, kvm_arch_gmem_make_private() at fault, and
>      kvm_arch_gmem_reclaim() on revoke and release. The device path
>      has to skip them.
>   2. lookup_pfn_range() only feeds those hooks, so the device path
>      does not need it.
>   3. The host mmap and stage-2 need a device memory type.
>   4. In PFN mode, a conversion to private only updates the attributes,
>      and the guest fault maps the page later. CCA has to map device
>      memory at conversion, because the guest validates the mapping
>      before it touches it.
>   5. The device provider needs lock()/unlock(), the private range
>      summary and prepare_memory_region().
>   6. The provider gets the VM and the vDEVICE binding at attach.
>


Your KVM suggestion below prompted me to take a closer look at
eliminating the distinction between kvm_gmem_ram_pfn_backing_ops and
kvm_gmem_device_backing_ops. This appears to be doable.

> Would these changes to PFN mode be enough to drop the device table?
>
>   1. The arch hooks take the frame attributes. For RAM the arch does
>      the RMP work. For MMIO it skips it, or does the RMM work instead.
>

We discussed making get_pfn() and lookup_pfn_range() return PFN
attributes. We could define:

#define GUEST_MEMFD_PFN_READONLY	(1ULL << 0)
#define GUEST_MEMFD_PFN_MMIO	(1ULL << 1)
#define GUEST_MEMFD_PFN_NOEXEC	(1ULL << 2)

We can then use GUEST_MEMFD_PFN_MMIO to drive this. The attribute
conversion then becomes:

kvm_gmem_pfn_convert_attributes() {
          gmem_provider_lookup_pfn_range()
          ret = kvm_arch_gmem_convert(kvm, slot, gfn, pfn, nr_pages,
				    pfn_attrs, attrs);

}

For RAM PFNs, there should be nothing to convert because
lookup_pfn_range() will not find any pfns before they have been faulted-in?

>      
>   2. lookup_pfn_range() stays required. For vfio-pci it is the BAR
>      start plus the offset.
>   3. get_pfn() and lookup_pfn_range() report a memory type (MMIO) in
>      their attributes, next to read only. The core mmap and the KVM
>      fault path use it.
>   4. A conversion calls an arch hook in both directions, and the hook
>      can fail. CCA maps the private device range there.
>   5. lock()/unlock(), the private range summary and
>      prepare_memory_region() become optional ops for any PFN provider.
>      A provider that moves RAM between guests needs them too: it has
>      to order its moves with conversions and leave private pages
>      alone.
>
> I'm less sure about 6. Could KVM ask the provider for the vDEVICE when
> the memslot is created, passing the VM? The provider would then not
> need to keep a VM pointer.
>

We can pass that as an argument. This is also preferable because it
avoids storing a KVM reference without a clearly documented lifetime
rule.

-aneesh