On 4/9/2025 5:56 PM, Alexey Kardashevskiy wrote: > > > On 7/4/25 17:49, Chenyi Qiang wrote: >> To manage the private and shared RAM states in confidential VMs, >> introduce a new class of PrivateShareManager as a child of > > missing "d" in "PrivateShareManager" Fixed. Thanks! > > >> GenericStateManager, which inherits the six interface callbacks. With a >> different interface type, it can be distinguished from the >> RamDiscardManager object and provide the flexibility for addressing >> specific requirements of confidential VMs in the future. > > This is still one bit per page, right? What does "set" mean here - > private or shared? It is either RamPrivateManager or RamSharedManager imho. This series only allows one bit per page, let's continue the discussion for this in patch 04/13. "Set" just mean the bitmap set. Maybe rename to RamPrivateManager (corresponding to RamDiscardManager) or CVMRamManager (Confidential VM Ram Manager) if we want to introduce more states in the future. > > >> Signed-off-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx> >> --- >> Changes in v4: >> - Newly added. >> --- >> include/exec/memory.h | 44 +++++++++++++++++++++++++++++++++++++++++-- >> system/memory.c | 17 +++++++++++++++++ >> 2 files changed, 59 insertions(+), 2 deletions(-) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 30e5838d02..08f25e5e84 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -55,6 +55,12 @@ typedef struct RamDiscardManager RamDiscardManager; >> DECLARE_OBJ_CHECKERS(RamDiscardManager, RamDiscardManagerClass, >> RAM_DISCARD_MANAGER, TYPE_RAM_DISCARD_MANAGER); >> +#define TYPE_PRIVATE_SHARED_MANAGER "private-shared-manager" >> +typedef struct PrivateSharedManagerClass PrivateSharedManagerClass; >> +typedef struct PrivateSharedManager PrivateSharedManager; >> +DECLARE_OBJ_CHECKERS(PrivateSharedManager, PrivateSharedManagerClass, >> + PRIVATE_SHARED_MANAGER, >> TYPE_PRIVATE_SHARED_MANAGER) >> + >> #ifdef CONFIG_FUZZ >> void fuzz_dma_read_cb(size_t addr, >> size_t len, >> @@ -692,6 +698,14 @@ void >> generic_state_manager_register_listener(GenericStateManager *gsm, >> void generic_state_manager_unregister_listener(GenericStateManager >> *gsm, >> StateChangeListener >> *scl); >> +static inline void state_change_listener_init(StateChangeListener >> *scl, >> + NotifyStateSet >> state_set_fn, >> + NotifyStateClear >> state_clear_fn) > > This belongs to 04/13 as there is nothing about PrivateSharedManager. > Thanks, Will move it. Thanks. > > >> +{ >> + scl->notify_to_state_set = state_set_fn; >> + scl->notify_to_state_clear = state_clear_fn; >> +} >> + >> typedef struct RamDiscardListener RamDiscardListener; >> struct RamDiscardListener { >> @@ -713,8 +727,7 @@ static inline void >> ram_discard_listener_init(RamDiscardListener *rdl, >> NotifyStateClear >> discard_fn, >> bool >> double_discard_supported) >> { >> - rdl->scl.notify_to_state_set = populate_fn; >> - rdl->scl.notify_to_state_clear = discard_fn; >> + state_change_listener_init(&rdl->scl, populate_fn, discard_fn); >> rdl->double_discard_supported = double_discard_supported; >> } >> @@ -757,6 +770,25 @@ struct RamDiscardManagerClass { >> GenericStateManagerClass parent_class; >> }; >> +typedef struct PrivateSharedListener PrivateSharedListener; >> +struct PrivateSharedListener { >> + struct StateChangeListener scl; >> + >> + QLIST_ENTRY(PrivateSharedListener) next; >> +}; >> + >> +struct PrivateSharedManagerClass { >> + /* private */ >> + GenericStateManagerClass parent_class; >> +}; >> + >> +static inline void private_shared_listener_init(PrivateSharedListener >> *psl, >> + NotifyStateSet >> populate_fn, >> + NotifyStateClear >> discard_fn) >> +{ >> + state_change_listener_init(&psl->scl, populate_fn, discard_fn); >> +} >> + >> /** >> * memory_get_xlat_addr: Extract addresses from a TLB entry >> * >> @@ -2521,6 +2553,14 @@ int >> memory_region_set_generic_state_manager(MemoryRegion *mr, >> */ >> bool memory_region_has_ram_discard_manager(MemoryRegion *mr); >> +/** >> + * memory_region_has_private_shared_manager: check whether a >> #MemoryRegion has a >> + * #PrivateSharedManager assigned >> + * >> + * @mr: the #MemoryRegion >> + */ >> +bool memory_region_has_private_shared_manager(MemoryRegion *mr); >> + >> /** >> * memory_region_find: translate an address/size relative to a >> * MemoryRegion into a #MemoryRegionSection. >> diff --git a/system/memory.c b/system/memory.c >> index 7b921c66a6..e6e944d9c0 100644 >> --- a/system/memory.c >> +++ b/system/memory.c >> @@ -2137,6 +2137,16 @@ bool >> memory_region_has_ram_discard_manager(MemoryRegion *mr) >> return true; >> } >> +bool memory_region_has_private_shared_manager(MemoryRegion *mr) >> +{ >> + if (!memory_region_is_ram(mr) || >> + !object_dynamic_cast(OBJECT(mr->gsm), >> TYPE_PRIVATE_SHARED_MANAGER)) { >> + return false; >> + } >> + >> + return true; >> +} >> + >> uint64_t generic_state_manager_get_min_granularity(const >> GenericStateManager *gsm, >> const >> MemoryRegion *mr) >> { >> @@ -3837,12 +3847,19 @@ static const TypeInfo ram_discard_manager_info >> = { >> .class_size = sizeof(RamDiscardManagerClass), >> }; >> +static const TypeInfo private_shared_manager_info = { >> + .parent = TYPE_GENERIC_STATE_MANAGER, >> + .name = TYPE_PRIVATE_SHARED_MANAGER, >> + .class_size = sizeof(PrivateSharedManagerClass), >> +}; >> + >> static void memory_register_types(void) >> { >> type_register_static(&memory_region_info); >> type_register_static(&iommu_memory_region_info); >> type_register_static(&generic_state_manager_info); >> type_register_static(&ram_discard_manager_info); >> + type_register_static(&private_shared_manager_info); >> } >> type_init(memory_register_types) >