[Qemu-devel] [PATCH v1 5/6] intel_iommu: Let vtd_iommu_notify_flag_changed() fail

Eric Auger posted 6 patches 6 years, 1 month ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eric Auger <eric.auger@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Peter Maydell <peter.maydell@linaro.org>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <rth@twiddle.net>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v1 5/6] intel_iommu: Let vtd_iommu_notify_flag_changed() fail
Posted by Eric Auger 6 years, 1 month ago
In case a MAP notifier is attempted to be registered without
caching mode, let's simply return an error. This latter now is
handled in the VFIO code.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/i386/intel_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 7a89ea9ba1..2f66d6882c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2931,7 +2931,7 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
     if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) {
         error_report("We need to set caching-mode=on for intel-iommu to enable "
                      "device assignment with IOMMU protection.");
-        exit(1);
+        return -EINVAL;
     }
 
     /* Update per-address-space notifier flags */
-- 
2.20.1


Re: [Qemu-devel] [PATCH v1 5/6] intel_iommu: Let vtd_iommu_notify_flag_changed() fail
Posted by Peter Xu 6 years, 1 month ago
On Fri, Sep 13, 2019 at 10:36:14AM +0200, Eric Auger wrote:
> In case a MAP notifier is attempted to be registered without
> caching mode, let's simply return an error. This latter now is
> handled in the VFIO code.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/i386/intel_iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 7a89ea9ba1..2f66d6882c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2931,7 +2931,7 @@ static int vtd_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu,
>      if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) {
>          error_report("We need to set caching-mode=on for intel-iommu to enable "
>                       "device assignment with IOMMU protection.");
> -        exit(1);
> +        return -EINVAL;

This might be conflicting with the other series because that's going
to drop these lines:

https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg01968.html

Though current change looks ok to me, and I don't even know which one
will reach master earlier after all so let's see. :)

The rest patches besides patch 1 all look sane to me. If we're going
to have "Error **" then the rest patches might need trivial touch-ups
of course.  And, IMHO squashing the whole series into patch 1 could be
better, but it's a personal preference only.

Thanks,

-- 
Peter Xu