Thanks David for your review! On 4/25/2025 8:49 PM, David Hildenbrand wrote: > On 09.04.25 11:56, Alexey Kardashevskiy wrote: >> >> >> On 7/4/25 17:49, Chenyi Qiang wrote: >>> RamDiscardManager is an interface used by virtio-mem to adjust VFIO >>> mappings in relation to VM page assignment. It manages the state of >>> populated and discard for the RAM. To accommodate future scnarios for >>> managing RAM states, such as private and shared states in confidential >>> VMs, the existing RamDiscardManager interface needs to be generalized. >>> >>> Introduce a parent class, GenericStateManager, to manage a pair of >> >> "GenericState" is the same as "State" really. Call it RamStateManager. >> >> >> >>> opposite states with RamDiscardManager as its child. The changes include >>> - Define a new abstract class GenericStateChange. >>> - Extract six callbacks into GenericStateChangeClass and allow the child >>> classes to inherit them. >>> - Modify RamDiscardManager-related helpers to use GenericStateManager >>> ones. >>> - Define a generic StatChangeListener to extract fields from >> >> "e" missing in StateChangeListener. >> >>> RamDiscardManager listener which allows future listeners to embed it >>> and avoid duplication. >>> - Change the users of RamDiscardManager (virtio-mem, migration, etc.) to >>> switch to use GenericStateChange helpers. >>> >>> It can provide a more flexible and resuable framework for RAM state >>> management, facilitating future enhancements and use cases. >> >> I fail to see how new interface helps with this. RamDiscardManager >> manipulates populated/discarded. It would make sense may be if the new >> class had more bits per page, say private/shared/discarded but it does >> not. And PrivateSharedManager cannot coexist with RamDiscard. imho this >> is going in a wrong direction. > > Agreed. > > In the future, we will have virtio-mem co-exist with guest_memfd. > > Both are information sources, and likely we'd have some instance on top, > that merges these sources to identify if anybody needs to be notified.
Yes, that's the problem. My current proposal is not so abstract to fit for this target. In my proposal, it tries to create a center for each target. For example, virtio-mem has a VirtioMemRamStateManager to manage populate/discard states. guest-memfd has a GuestRamStateManager to manage private/shared/discarded states. That is poorly structured and would bring a lot of duplication. > > Until we figure out how that would look like, I would suggest to keep it > as is. OK, I'll revert to the old implementation in next version. Thanks! > > Maybe, in the future we would have a single RamDiscardManager and > multiple RamDiscardSources per RAMBlock. > > The sources notify the manager, and the manager can ask other sources to > merge the information. >
