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

[Peter Xu]

On Wed, Aug 21, 2019 at 09:50:43AM +0200, Paolo Bonzini wrote:
> On 21/08/19 07:03, Peter Xu wrote:
> > On Tue, Aug 20, 2019 at 08:24:49AM +0200, Paolo Bonzini wrote:
> >> On 20/08/19 07:22, Peter Xu wrote:
> >>> On Mon, Aug 12, 2019 at 09:45:27AM +0200, Peter Xu wrote:
> >>>> This is a RFC series.
> >>>>
> >>>> 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.
> >>>
> >>> Any more comment on this?
> >>
> >> No problem from me, but I wouldn't mind if someone else merged it. :)
> > 
> > Can I read this as an "acked-by"? :)
> 
> Yes, it shouldn't even need Acked-by since there are other maintainers
> that handle this part of the tree:
> 
> Paolo Bonzini <address@hidden> (maintainer:X86 TCG CPUs)
> Richard Henderson <address@hidden> (maintainer:X86 TCG CPUs)
> Eduardo Habkost <address@hidden> (maintainer:X86 TCG CPUs)
> "Michael S. Tsirkin" <address@hidden> (supporter:PC)
> Marcel Apfelbaum <address@hidden> (supporter:PC)

Michael (or any maintainers listed above):

Do any of you have any further comment on this series?  Do any of you
like to merge this?

Thanks,

-- 
Peter Xu