[Qemu-devel] [PATCH v2 0/4] intel_iommu: Do sanity check of vfio-pci earlier

Peter Xu posted 4 patches 6 years, 1 month ago
Test docker-clang@ubuntu failed
Test FreeBSD failed
Test checkpatch failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190916075839.390-1-peterx@redhat.com
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Richard Henderson <rth@twiddle.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/core/qdev.c         | 17 +++++++++++++++++
hw/i386/intel_iommu.c  | 40 ++++++++++++++++++++++++++++++++++------
hw/i386/pc.c           | 21 +++++++++++++++++++++
include/hw/boards.h    |  9 +++++++++
include/hw/qdev-core.h |  1 +
qdev-monitor.c         |  7 +++++++
6 files changed, 89 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2 0/4] intel_iommu: Do sanity check of vfio-pci earlier
Posted by Peter Xu 6 years, 1 month ago
v2:
- rebase to master [Eric]
- add r-bs for Eric
- remove RFC tag

The VT-d code has some defects, one of them is that we cannot detect
the misuse of vIOMMU and vfio-pci early enough.

For example, logically this is not allowed:

  -device intel-iommu,caching-mode=off \
  -device vfio-pci,host=05:00.0

Because the caching mode is required to make vfio-pci devices
functional.

Previously we did this sanity check in vtd_iommu_notify_flag_changed()
as when the memory regions change their attributes.  However that's
too late in most cases!  Because the memory region layouts will only
change after IOMMU is enabled, and that's in most cases during the
guest OS boots.  So when the configuration is wrong, we will only bail
out during the guest boots rather than simply telling the user before
QEMU starts.

The same problem happens on device hotplug, say, when we have this:

  -device intel-iommu,caching-mode=off

Then we do something like:

  (HMP) device_add vfio-pci,host=05:00.0,bus=pcie.1

If at that time the vIOMMU is enabled in the guest then the QEMU
process will simply quit directly due to this hotplug event.  This is
a bit insane...

This series tries to solve above two problems by introducing two
sanity checks upon these places separately:

  - machine done
  - hotplug device

This is a bit awkward but I hope this could be better than before.
There is of course other solutions like hard-code the check into
vfio-pci but I feel it even more unpretty.  I didn't think out any
better way to do this, if there is please kindly shout out.

Please have a look to see whether this would be acceptable, thanks.

Peter Xu (4):
  intel_iommu: Sanity check vfio-pci config on machine init done
  qdev/machine: Introduce hotplug_allowed hook
  pc/q35: Disallow vfio-pci hotplug without VT-d caching mode
  intel_iommu: Remove the caching-mode check during flag change

 hw/core/qdev.c         | 17 +++++++++++++++++
 hw/i386/intel_iommu.c  | 40 ++++++++++++++++++++++++++++++++++------
 hw/i386/pc.c           | 21 +++++++++++++++++++++
 include/hw/boards.h    |  9 +++++++++
 include/hw/qdev-core.h |  1 +
 qdev-monitor.c         |  7 +++++++
 6 files changed, 89 insertions(+), 6 deletions(-)

-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 0/4] intel_iommu: Do sanity check of vfio-pci earlier
Posted by Peter Xu 6 years, 1 month ago
On Mon, Sep 16, 2019 at 03:58:35PM +0800, Peter Xu wrote:
> v2:
> - rebase to master [Eric]
> - add r-bs for Eric
> - remove RFC tag

No... this is the new cover letter with the old commits...

I'll post again.  Sorry for the noise.

-- 
Peter Xu