Re: [PATCH v4 12/13] ram-block-attribute: Add priority listener support for PrivateSharedListener

[Baolu Lu <baolu.lu@linux.intel.com>]

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.qiang@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.

Thanks,
baolu