On 16.04.25 05:32, Chenyi Qiang wrote:
On 4/10/2025 9:44 AM, Chenyi Qiang wrote:
On 4/10/2025 8:11 AM, Alexey Kardashevskiy wrote:
On 9/4/25 22:57, Chenyi Qiang wrote:
On 4/9/2025 5:56 PM, 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.
OK to me.
Sorry, nah. "Generic" would mean "machine" in QEMU.
OK, anyway, I can rename to RamStateManager if we follow this direction.
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.
Fixed. Thanks.
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.
I think we have two questions here:
1. whether we should define an abstract parent class and distinguish the
RamDiscardManager and PrivateSharedManager?
If it is 1 bit per page with the meaning "1 == populated == shared",
then no, one class will do.
Not restrict to 1 bit per page. As mentioned in questions 2, the parent
class can be more generic, e.g. only including
register/unregister_listener().
Like in this way:
The parent class:
struct StateChangeListener {
MemoryRegionSection *section;
}
struct RamStateManagerClass {
void (*register_listener)();
void (*unregister_listener)();
}
The child class:
1. RamDiscardManager
struct RamDiscardListener {
StateChangeListener scl;
NotifyPopulate notify_populate;
NotifyDiscard notify_discard;
bool double_discard_supported;
QLIST_ENTRY(RamDiscardListener) next;
}
struct RamDiscardManagerClass {
RamStateManagerClass parent_class;
uint64_t (*get_min_granularity)();
bool (*is_populate)();
bool (*replay_populate)();
bool (*replay_discard)();
}
2. PrivateSharedManager (or other name like ConfidentialRamManager?)
struct PrivateSharedListener {
StateChangeListener scl;
NotifyShared notify_shared;
NotifyPrivate notify_private;
int priority;
QLIST_ENTRY(PrivateSharedListener) next;
}
struct PrivateSharedManagerClass {
RamStateManagerClass parent_class;
uint64_t (*get_min_granularity)();
bool (*is_shared)();
// No need to define replay_private/replay_shared as no use case at
present.
}
In the future, if we want to manage three states, we can only extend
PrivateSharedManagerClass/PrivateSharedListener.
Hi Alexey & David,
Any thoughts on this proposal?
Thinking about how to reasonable make virtio-mem and guest_memdfd work
in the future together, I don't think such an abstraction might
necessarily help. (see my other mails)
In the end we populate/discard, how to merge that information from
multiple sources (or maintain it in a single object) is TBD.
virtio-mem has a bitmap that is usually 1 bit per block (e.g., 2 MiB).
guest_memfd has a bitmap that is usually 1 bit per page.
Maybe a GuestRamStateManager would store both separately if requested.
virtio-mem would register itself with it, and guest_memfd would register
itself with that.
GuestRamStateManager would then implement the logic of merging both
information (shared vs. private, plugged vs. unplugged).
But that needs more thought: essentially, the virtio-mem bitmap would
move to the GuestRamStateManager.
OFC, we would only want the bitmaps and the manager if there is an
actual provider for it (e.g., virtio-mem for the plugged part,
guest_memfd for the cc part).
--
Cheers,
David / dhildenb
