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.qi...@intel.com>
>> ---
>> 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)
>
