Re: [PATCH V2 1/3] amd-iommu: Add address space notifier and replay support

[Peter Xu <peterx@redhat.com>]

On Fri, Oct 02, 2020 at 09:59:05AM -0500, Wei Huang wrote:
> +static void amdvi_address_space_unmap(AMDVIAddressSpace *as, IOMMUNotifier *n)
> +{
> +    IOMMUTLBEntry entry;
> +    hwaddr start = n->start;
> +    hwaddr end = n->end;
> +    hwaddr size = end - start + 1;
> +
> +    entry.target_as = &address_space_memory;
> +    entry.iova = start;
> +    entry.translated_addr = 0;
> +    entry.perm = IOMMU_NONE;
> +    entry.addr_mask = size - 1;

This may race with Eugenio's series:

  https://mail.gnu.org/archive/html/qemu-ppc/2020-09/msg00131.html

IMHO that series should be acceptable for merging already.  Anyway, there's
probably a trivial conflict to solve.

> +
> +    memory_region_notify_one(n, &entry);
> +}
> +
>  static gboolean amdvi_iotlb_remove_by_domid(gpointer key, gpointer value,
>                                              gpointer user_data)
>  {
> @@ -1473,14 +1491,17 @@ static int amdvi_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                                             Error **errp)
>  {
>      AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
> +    AMDVIState *s = as->iommu_state;
>  
> -    if (new & IOMMU_NOTIFIER_MAP) {
> -        error_setg(errp,
> -                   "device %02x.%02x.%x requires iommu notifier which is not "
> -                   "currently supported", as->bus_num, PCI_SLOT(as->devfn),
> -                   PCI_FUNC(as->devfn));
> -        return -EINVAL;

Ideally, we shouldn't remove this error message until the functionality is
fully implemented.  Otherwise an user could potentially boot such a vm with a
broken assigned device.

Thanks,

> +    /* Update address space notifier flags */
> +    as->notifier_flags = new;
> +
> +    if (old == IOMMU_NOTIFIER_NONE) {
> +        QLIST_INSERT_HEAD(&s->amdvi_as_with_notifiers, as, next);
> +    } else if (new == IOMMU_NOTIFIER_NONE) {
> +        QLIST_REMOVE(as, next);
>      }
> +
>      return 0;
>  }

-- 
Peter Xu