Re: [PATCH v2 0/5] Allow memory_region_register_iommu_notifier() to fail

[Peter Xu <peterx@redhat.com>]

On Thu, Sep 19, 2019 at 02:18:40PM +0200, Eric Auger wrote:
> This series allows the memory_region_register_iommu_notifier()
> to fail. As of now, when a MAP notifier is attempted to be
> registered along with SMMUv3, Intel iommu without caching mode
> or AMD IOMMU, we exit in the IOMMU MR notify_flag_changed()
> callback. In case of VFIO assigned device hotplug, this could be
> handled more nicely directly within the VFIO code, simply rejecting
> the hotplug without exiting. This is what the series achieves
> by handling the memory_region_register_iommu_notifier() returned
> value.

Could I ask what's the code path for ARM when the hot plug failed?  Is
that vfio_realize() then vfio_connect_container() will fail with this?

    if (container->error) {
        ret = container->error;
        error_setg_errno(errp, -ret,
                         "memory listener initialization failed for container");
        goto listener_release_exit;
    }

If so, I would again suggest you to use Error** in patch 1.  IMHO we
can let vfio_listener_region_add() to be the first one to use the
Error** so that instead of this:

    /*
     * On the initfn path, store the first error in the container so we
     * can gracefully fail.  Runtime, there's not much we can do other
     * than throw a hardware error.
     */
    if (!container->initialized) {
        if (!container->error) {
            container->error = ret;
        }
    } else {
        hw_error("vfio: DMA mapping failed, unable to continue");
    }

We can also cache the Error** into container and return to user if the
user is using QMP which should be better than the int number (or
again, maybe return both errors?).  IIUC error_report() will not work
for QMP.

Regards,

-- 
Peter Xu