Re: [Qemu-devel] [PATCH v2] intel_iommu: check misordered init when realize

[Marcel Apfelbaum]

On 03/01/2017 06:14 AM, Jason Wang wrote:
> On 2017年03月01日 11:23, Michael S. Tsirkin wrote:
> > On Wed, Mar 01, 2017 at 10:36:35AM +0800, Peter Xu wrote:
> > > On Tue, Feb 28, 2017 at 04:42:25PM +0200, Marcel Apfelbaum wrote:
> > > > On 02/24/2017 06:29 AM, Peter Xu wrote:
> > > > > Intel vIOMMU devices are created with "-device" parameter, while here
> > > > > actually we need to make sure the dmar device be created before other
> > > > > PCI devices (like vfio-pci) so that we know iommu_fn will be setup
> > > > > correctly before realizations of those PCI devices (it is sensible that
> > > > > PCI device fetch these info during its realization). Now this ordering
> > > > > yet cannot be achieved elsewhere, and devices will be created in the
> > > > > order that user specified. That might be dangerous.
> > > > >
> > > > > Here we add one more function to detect this kind of misordering issue,
> > > > > then report to guest. Currently, the only known device that is affected
> > > > > by this VT-d defect is the vfio-pci typed devices. So for now we just
> > > > > check against it to make sure we are safe.
> > > > Hi,
> > > > I can't say that I like it but if we want it for 2.9 maybe we don't have a 
> > > > choice.
> > > >
> > > > I mentioned in another thread other idea:
> > > >      Maybe we should follow the same "template" as disk/drive, nic/netdev ?
> > > >      I mean something like -device iommu,id=i1, -device vfio-pci,iommu=e1 .
> > > > You are able to change the order, I didn't look how it is done.
> > > I suppose that's done by using different keywords. For example:
> > >
> > >    -netdev user,id=net0 -device e1000,netdev=net0
> > >
> > > Here we are using "netdev" for the backend and "device" for the
> > > frontend.
> > >
> > > Since we have this difference, we can just make sure the ordering by
> > > init netdev first (in net_init_clients()), then the devices (in
> > > device_init_func()).
> > >
> > > > A nice side effect is that you can:
> > > >   1. Limit the iommu scope only to the devices you want to protect (tweaking 
> > > > APCI tables)
> > > >   2. In the future we can support multiple iommu devices.
> > Unfortunately AFAIK this needs a bunch of work in practice:
> > host and guest side.
> > So this opens a can of worms: you can all too easily create
> > configurations that we don't support.
> >
> >
> > > Yes. Thanks,
> > >
> > > -- peterx
> > While I agree this fixes the specific problem, we have the ordering
> > issue in many other places.
>
> Yes, just post a fix for virtio-pci with IOMMU. With this fix, we meet the 
> exact issue as this patch, IOMMU needs to be created before any virtio-pci 
> devices.

Hi Jason,

I am not saying we don't need to create the IOMMU before some other device,
I just don't think that the command line order should matter to user.

BTW, are you working on a way to limit the IOMMU scope to a specific set of 
devices?
If yes, this approach will also help you work.

Thanks,
Marcel

> Thanks
>
> >   For example, this is one of the reasons we
> > don't create built-in PC devices using QOM composition.
> >
> > So I think that support for dependencies does make a lot of sense
> > generally.