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-c05a4e090d58@xxxxxxxxxx/ > >> >> 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.gupta@xxxxxxx> Thanks Pankaj for your review! > > > Thanks, > Pankaj >
