Re: [PATCH v3 4/7] memory-attribute-manager: Introduce MemoryAttributeManager to manage RAMBLock with guest_memfd

["Gupta, Pankaj" <pankaj.gupta@amd.com>]

>>>>>> As the commit 852f0048f3 ("RAMBlock: make guest_memfd require
>>>>>> uncoordinated discard") highlighted, some subsystems like VFIO may
>>>>>> disable ram block discard. However, guest_memfd relies on the discard
>>>>>> operation to perform page conversion between private and shared
>>>>>> memory.
>>>>>> This can lead to stale IOMMU mapping issue when assigning a hardware
>>>>>> device to a confidential VM via shared memory. To address this, it is
>>>>>> crucial to ensure systems like VFIO refresh its IOMMU mappings.
>>>>>>
>>>>>> RamDiscardManager is an existing concept (used by virtio-mem) to
>>>>>> adjust
>>>>>> VFIO mappings in relation to VM page assignment. Effectively page
>>>>>> conversion is similar to hot-removing a page in one mode and adding it
>>>>>> back in the other. Therefore, similar actions are required for page
>>>>>> conversion events. Introduce the RamDiscardManager to guest_memfd to
>>>>>> facilitate this process.
>>>>>>
>>>>>> Since guest_memfd is not an object, it cannot directly implement the
>>>>>> RamDiscardManager interface. One potential attempt is to implement
>>>>>> it in
>>>>>> HostMemoryBackend. This is not appropriate because guest_memfd is per
>>>>>> RAMBlock. Some RAMBlocks have a memory backend but others do not. In
>>>>>> particular, the ones like virtual BIOS calling
>>>>>> memory_region_init_ram_guest_memfd() do not.
>>>>>>
>>>>>> To manage the RAMBlocks with guest_memfd, define a new object named
>>>>>> MemoryAttributeManager to implement the RamDiscardManager
>>>>>> interface. The
>>>>>
>>>>> Isn't this should be the other way around. 'MemoryAttributeManager'
>>>>> should be an interface and RamDiscardManager a type of it, an
>>>>> implementation?
>>>>
>>>> We want to use 'MemoryAttributeManager' to represent RAMBlock to
>>>> implement the RamDiscardManager interface callbacks because RAMBlock is
>>>> not an object. It includes some metadata of guest_memfd like
>>>> shared_bitmap at the same time.
>>>>
>>>> I can't get it that make 'MemoryAttributeManager' an interface and
>>>> RamDiscardManager a type of it. Can you elaborate it a little bit? I
>>>> think at least we need someone to implement the RamDiscardManager
>>>> interface.
>>>
>>> shared <-> private is translated (abstracted) to "populated <->
>>> discarded", which makes sense. The other way around would be wrong.
>>>
>>> It's going to be interesting once we have more logical states, for
>>> example supporting virtio-mem for confidential VMs.
>>>
>>> Then we'd have "shared+populated, private+populated, shared+discard,
>>> private+discarded". Not sure if this could simply be achieved by
>>> allowing multiple RamDiscardManager that are effectively chained, or
>>> if we'd want a different interface.
>>
>> Exactly! In any case generic manager (parent class) would make more
>> sense that can work on different operations/states implemented in child
>> classes (can be chained as well).
> 
> Ah, we are talking about the generic state management. Sorry for my slow
> reaction.
> 
> So we need to
> 1. Define a generic manager Interface, e.g.
> MemoryStateManager/GenericStateManager.
> 2. Make RamDiscardManager the child of MemoryStateManager which manages
> the state of populated and discarded.
> 3. Define a new child manager Interface PrivateSharedManager which
> manages the state of private and shared.
> 4. Define a new object ConfidentialMemoryAttribute to implement the
> PrivateSharedManager interface.
> (Welcome to rename the above Interface/Object)
> 
> Is my understanding correct?

Yes, in that direction. Where 'RamDiscardManager' & 
'PrivateSharedManager' are both child of 'GenericStateManager'.

Depending on listeners registered, corresponding handlers can be called.

Best regards,
Pankaj