On 6/3/2025 5:45 PM, Gupta, Pankaj wrote:
> On 6/3/2025 9:41 AM, David Hildenbrand wrote:
>> On 03.06.25 09:17, Gupta, Pankaj wrote:
>>> +CC Tony & Kishen
>>>
>>>>>>> In this patch series we are only maintaining the bitmap for Ram
>>>>>>> discard/
>>>>>>> populate state not for regular guest_memfd private/shared?
>>>>>>
>>>>>> As mentioned in changelog, "In the context of RamDiscardManager,
>>>>>> shared
>>>>>> state is analogous to populated, and private state is signified as
>>>>>> discarded." To keep consistent with RamDiscardManager, I used the ram
>>>>>> "populated/discareded" in variable and function names.
>>>>>>
>>>>>> Of course, we can use private/shared if we rename the
>>>>>> RamDiscardManager
>>>>>> to something like RamStateManager. But I haven't done it in this
>>>>>> series.
>>>>>> Because I think we can also view the bitmap as the state of shared
>>>>>> memory (shared discard/shared populate) at present. The VFIO user
>>>>>> only
>>>>>> manipulate the dma map/unmap of shared mapping. (We need to
>>>>>> consider how
>>>>>> to extend the RDM framwork to manage the shared/private/discard
>>>>>> states
>>>>>> in the future when need to distinguish private and discard states.)
>>>>>
>>>>> As function name 'ram_block_attributes_state_change' is generic. Maybe
>>>>> for now metadata update for only two states (shared/private) is enough
>>>>> as it also aligns with discard vs populate states?
>>>>
>>>> Yes, it is enough to treat the shared/private states align with
>>>> populate/discard at present as the only user is VFIO shared mapping.
>>>>
>>>>>
>>>>> As we would also need the shared vs private state metadata for other
>>>>> COCO operations e.g live migration, so wondering having this metadata
>>>>> already there would be helpful. This also will keep the legacy
>>>>> interface
>>>>> (prior to in-place conversion) consistent (As memory-attributes
>>>>> handling
>>>>> is generic operation anyway).
>>>>
>>>> When live migration in CoCo VMs is introduced, I think it needs to
>>>> distinguish the difference between the states of discard and
>>>> private. It
>>>> cannot simply skip the discard parts any more and needs special
>>>> handling
>>>> for private parts. So still, we have to extend the interface if have to
>>>> make it avaiable in advance.
>>>
>>> You mean even the discard and private would need different handling
>>
>> I am pretty sure they would in any case? Shared memory, you can simply
>> copy, private memory has to be extracted + placed differently.
>>
>> If we run into problems with live-migration, we can investigate how to
>> extend the current approach.
>
> Not problems. My understanding was: newly introduced per RAM BLock
> bitmap gets maintained for RAMBlock corresponding shared <-> private
> conversions in addition to VFIO discard <-> populate conversions.
> Since per RAMBlock bitmap set is disjoint for both the above cases,
> so can be reused for live migration use-case as well when deciding which
> page is private vs shared.
>
> Seems it was part of the series till v3 & v4(in a different design), not
> anymore though. Of-course it can be added later :)
Yeah. I think we can consider the extension in a separate series and
view it as the preparation work for CoCo live migration/virtio-mem
support. Since v4 is considered in a wrong direction, maybe David's idea
[1] is worth a try.
[1]
https://lore.kernel.org/qemu-devel/d1a71e00-243b-4751-ab73-c05a4e090...@redhat.com/
>
>>
>> Just like with memory hotplug / virtio-mem, I shared some ideas on how
>> to make it work, but holding up this work when we don't even know what
>> exactly we will exactly need for other future use cases does not sound
>> too plausible.
>>
>
> Of-course we should not hold this series. But Thanks 'Chenyi Qiang' for
> your efforts for trying different implementation based on information we
> had!
>
> With or w/o shared <-> private bitmap update. Feel free to add:
>
> Reviewed-by: Pankaj Gupta <pankaj.gu...@amd.com>
Thanks Pankaj for your review!
>
>
> Thanks,
> Pankaj
>
