Hi David,
Any thought on patch 10-12, which is to move the change attribute into a
priority listener. A problem is how to handle the error handling of
private_to_shared failure. Previously, we thought it would never be able
to fail, but right now, it is possible in corner cases (e.g. -ENOMEM) in
set_attribute_private(). At present, I simply raise an assert instead of
adding any rollback work (see patch 11).
On 4/7/2025 3:49 PM, Chenyi Qiang wrote:
> So that the caller can check the result of NotifyStateClear() handler if
> the operation fails.
>
> Signed-off-by: Chenyi Qiang <chenyi.qi...@intel.com>
> ---
> Changes in v4:
> - Newly added.
> ---
> hw/vfio/common.c | 18 ++++++++++--------
> include/exec/memory.h | 4 ++--
> 2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 48468a12c3..6e49ae597d 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -335,8 +335,8 @@ out:
> rcu_read_unlock();
> }
>
> -static void vfio_state_change_notify_to_state_clear(VFIOContainerBase
> *bcontainer,
> - MemoryRegionSection
> *section)
> +static int vfio_state_change_notify_to_state_clear(VFIOContainerBase
> *bcontainer,
> + MemoryRegionSection
> *section)
> {
> const hwaddr size = int128_get64(section->size);
> const hwaddr iova = section->offset_within_address_space;
> @@ -348,24 +348,26 @@ static void
> vfio_state_change_notify_to_state_clear(VFIOContainerBase *bcontaine
> error_report("%s: vfio_container_dma_unmap() failed: %s", __func__,
> strerror(-ret));
> }
> +
> + return ret;
> }
>
> -static void vfio_ram_discard_notify_discard(StateChangeListener *scl,
> - MemoryRegionSection *section)
> +static int vfio_ram_discard_notify_discard(StateChangeListener *scl,
> + MemoryRegionSection *section)
> {
> RamDiscardListener *rdl = container_of(scl, RamDiscardListener, scl);
> VFIORamDiscardListener *vrdl = container_of(rdl, VFIORamDiscardListener,
> listener);
> - vfio_state_change_notify_to_state_clear(vrdl->bcontainer, section);
> + return vfio_state_change_notify_to_state_clear(vrdl->bcontainer,
> section);
> }
>
> -static void vfio_private_shared_notify_to_private(StateChangeListener *scl,
> - MemoryRegionSection
> *section)
> +static int vfio_private_shared_notify_to_private(StateChangeListener *scl,
> + MemoryRegionSection
> *section)
> {
> PrivateSharedListener *psl = container_of(scl, PrivateSharedListener,
> scl);
> VFIOPrivateSharedListener *vpsl = container_of(psl,
> VFIOPrivateSharedListener,
> listener);
> - vfio_state_change_notify_to_state_clear(vpsl->bcontainer, section);
> + return vfio_state_change_notify_to_state_clear(vpsl->bcontainer,
> section);
> }
>
> static int vfio_state_change_notify_to_state_set(VFIOContainerBase
> *bcontainer,
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index a61896251c..9472d9e9b4 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -523,8 +523,8 @@ typedef int (*ReplayStateChange)(MemoryRegionSection
> *section, void *opaque);
> typedef struct StateChangeListener StateChangeListener;
> typedef int (*NotifyStateSet)(StateChangeListener *scl,
> MemoryRegionSection *section);
> -typedef void (*NotifyStateClear)(StateChangeListener *scl,
> - MemoryRegionSection *section);
> +typedef int (*NotifyStateClear)(StateChangeListener *scl,
> + MemoryRegionSection *section);
>
> struct StateChangeListener {
> /*
