+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 and
we cannot use a common per RAMBlock object metadata store? That was the
reason I suggested in v4 to go with a base abstract class with common
bits and implementation can be based on specific derived class. As that
seemed cleaner and future extensible, otherwise we would need a major
overhaul in future to this code.
Thanks,
Pankaj
