Hi Peter,

On 10/18/22 23:54, Peter Xu wrote:
> All x86 IOMMUs need to fail DEVIOTLB notification registrations if DT is
> not supported.  Otherwise any device (e.g. vhost) can register with
> DEVIOTLB but it'll never receive the notifications, causing device not
> operational.
>
> Fixes: b68ba1ca57 ("memory: Add IOMMU_NOTIFIER_DEVIOTLB_UNMAP 
> IOMMUTLBNotificationType")
> Reported-by: Eric Auger <eric.au...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
>  hw/i386/amd_iommu.c   | 8 ++++++++
>  hw/i386/intel_iommu.c | 7 +++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 725f69095b..42486e790f 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1476,6 +1476,7 @@ static int 
> amdvi_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                                             Error **errp)
>  {
>      AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
> +    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(as->iommu_state);
>  
>      if (new & IOMMU_NOTIFIER_MAP) {
>          error_setg(errp,
> @@ -1484,6 +1485,13 @@ static int 
> amdvi_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>                     PCI_FUNC(as->devfn));
>          return -EINVAL;
>      }
> +
> +    if ((new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) && !x86_iommu->dt_supported) {
> +        error_setg_errno(errp, ENOTSUP,
> +                         "Device-iotlb not declared support for vIOMMU");
with current vhost code, vhost will then silently fallbac to UNMAP
notifier registration and this will succeed. It would be nice to clarify
whether the vIOMMU works with vhost in this downgraded mode (at least
ats=off and device-ioltb=off)?

Thanks

Eric
> +        return -ENOTSUP;
> +    }
> +
>      return 0;
>  }
>  
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 6524c2ee32..a07d9631b0 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3057,6 +3057,7 @@ static int 
> vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>  {
>      VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
>      IntelIOMMUState *s = vtd_as->iommu_state;
> +    X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>  
>      /* TODO: add support for VFIO and vhost users */
>      if (s->snoop_control) {
> @@ -3065,6 +3066,12 @@ static int 
> vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>          return -ENOTSUP;
>      }
>  
> +    if ((new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) && !x86_iommu->dt_supported) {
> +        error_setg_errno(errp, ENOTSUP,
> +                         "Device-iotlb not declared support for vIOMMU");
> +        return -ENOTSUP;
> +    }
> +
>      /* Update per-address-space notifier flags */
>      vtd_as->notifier_flags = new;
>  


Reply via email to