On 5/9/2025 5:23 PM, Baolu Lu wrote:
> On 4/7/2025 3:49 PM, Chenyi Qiang wrote:
>> In-place page conversion requires operations to follow a specific
>> sequence: unmap-before-conversion-to-private and
>> map-after-conversion-to-shared. Currently, both attribute changes and
>> VFIO DMA map/unmap operations are handled by PrivateSharedListeners,
>> they need to be invoked in a specific order.
>>
>> For private to shared conversion:
>> - Change attribute to shared.
>> - VFIO populates the shared mappings into the IOMMU.
>> - Restore attribute if the operation fails.
>>
>> For shared to private conversion:
>> - VFIO discards shared mapping from the IOMMU.
>> - Change attribute to private.
>>
>> To faciliate this sequence, priority support is added to
>> PrivateSharedListener so that listeners are stored in a determined
>> order based on priority. A tail queue is used to store listeners,
>> allowing traversal in either direction.
>>
>> Signed-off-by: Chenyi Qiang<chenyi.qi...@intel.com>
>> ---
>> Changes in v4:
>> - Newly added.
>> ---
>> accel/kvm/kvm-all.c | 3 ++-
>> hw/vfio/common.c | 3 ++-
>> include/exec/memory.h | 19 +++++++++++++++++--
>> include/exec/ramblock.h | 2 +-
>> system/ram-block-attribute.c | 23 +++++++++++++++++------
>> 5 files changed, 39 insertions(+), 11 deletions(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index aec64d559b..879c61b391 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -1745,7 +1745,8 @@ static void kvm_region_add(MemoryListener
>> *listener,
>> psl = &cpsl->listener;
>> QLIST_INSERT_HEAD(&cgs->cvm_private_shared_list, cpsl, next);
>> private_shared_listener_init(psl,
>> kvm_private_shared_notify_to_shared,
>> - kvm_private_shared_notify_to_private);
>> + kvm_private_shared_notify_to_private,
>> + PRIVATE_SHARED_LISTENER_PRIORITY_MIN);
>> generic_state_manager_register_listener(gsm, &psl->scl, section);
>> }
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 6e49ae597d..a8aacae26c 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -515,7 +515,8 @@ static void
>> vfio_register_private_shared_listener(VFIOContainerBase *bcontainer,
>> psl = &vpsl->listener;
>> private_shared_listener_init(psl,
>> vfio_private_shared_notify_to_shared,
>> - vfio_private_shared_notify_to_private);
>> + vfio_private_shared_notify_to_private,
>> +
>> PRIVATE_SHARED_LISTENER_PRIORITY_COMMON);
>> generic_state_manager_register_listener(gsm, &psl->scl, section);
>> QLIST_INSERT_HEAD(&bcontainer->vpsl_list, vpsl, next);
>> }
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 9472d9e9b4..3d06cc04a0 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -770,11 +770,24 @@ struct RamDiscardManagerClass {
>> GenericStateManagerClass parent_class;
>> };
>> +#define PRIVATE_SHARED_LISTENER_PRIORITY_MIN 0
>> +#define PRIVATE_SHARED_LISTENER_PRIORITY_COMMON 10
>
> For the current implementation with primarily KVM and VFIO needing
> ordered execution, the two priority levels are likely sufficient. Not
> sure whether it needs more priority levels for future development.
For the priority levels, I think they can be classified. Subsystems like
VFIO don't require explicit order and can be put in the same level. The
primary KVM responsible for changing attribute should be classified
separately.
In addition, since this priority support mainly serves the in-place
conversion. I'll drop this patch temporarily since in-place conversion
will likely change the path.
>
> Thanks,
> baolu
