Re: [PATCH] intel_iommu: Use the latest fault reasons defined by spec

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 27, 2024 at 02:32:46PM +0800, Yi Liu wrote:
> Folks, looks like it's not necessary to be aligned with spec version.
> e.g. I can see something like below. This is an old machine which is
> not possible to be built according to vt-d spec 4.0. Let me check more
> machines as well to confirm this.

Aligning to a spec version is preferable. We don't *have* to force
users to do it if we don't want to, but you never know what
assumptions will guests make.



>Perhaps virtual VT-d implementation
> can have its own version policy? @Jason, how about your idea?
> 
> cat /sys/class/iommu/dmar0/intel-iommu/version
> 4:0

This is PV, really best avoided if we can.

> > > > > > > Probably, this might be something that could be noticed by the
> > > > > > > management to migration compatibility.
> > > > > > 
> > > > > > Could you elaborate what we need to do for migration compatibility?
> > > > > > I see version is already exported so libvirt can query it, see:
> > > > > > 
> > > > > >      DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0),
> > > > > 
> > > > > It is the Qemu command line parameters not the version of the vmstate.
> > > > > 
> > > > > For example -device intel-iommu,version=3.0
> > > > > 
> > > > > Qemu then knows it should behave as 3.0.
> > > > 
> > > > So you want to bump vtd_vmstate.version?
> > > 
> > > Well, as I said, it's not a direct bumping.
> > > 
> > > > 
> > > > In fact, this series change intel_iommu property from x-scalable-
> > > mode=["on"|"off"]"
> > > > to x-scalable-mode=["legacy"|"modern"|"off"]".
> > > > 
> > > > My understanding management app should use same qemu cmdline
> > > > in source and destination, so compatibility is already guaranteed even if
> > > > we don't bump vtd_vmstate.version.
> > > 
> > > Exactly, so the point is to
> > > 
> > > vtd=3.0, the device works exactly as vtd spec 3.0.
> > > vtd=3.3, the device works exactly as vtd spec 3.3.
> > 
> > Get your point. But I have some concerns about this:
> > 1.Exact version matching will bring vast of version check in the code,
> >     especially when we support more versions.
> > 2. There are some missed features before we can update version number to 3.0,
> >      i.e., nested translation, Accessed/Dirty (A/D) bits, 5 level page table, etc.
> > 3. Some features are removed in future versions, but we still need to
> >     implement them for intermediate version,
> >     i.e., ExecuteRequested (ER), Advanced Fault Logging, etc.
> 
> Even the hw follows vtd spec 3.0, it is not required to implement all of
> them. So it should be fine to implement part of the capabilities. :)

Yes, that's better.
Specifying version # explicitly is really annoying for both
qemu and management.
I think normally we should just start with capabilities and make
the best decision we can wrt guest support etc.

Being concervative is usually a good idea, if a new version gives no
useful functionality adding that is just churn.
So i.e.  we have a set of capabilities we want and select the earliest version
that supports them.

We can let user override that and I am not sure we need to bother
actually checking it's consistent with capabilities if we do
not want to.


> > > 
> > > When migration to the old qemu, mgmt can specify e.g vtd=3.0 for
> > > backward migration compatibility.
> > 
> > Yes, that makes sense for such migration.
> > But I'm not sure if there is a real scenario migrating to old qemu,
> > why not just update qemu on destination?
> > 
> > Thanks
> > Zhenzhong
> > 
> 
> -- 
> Regards,
> Yi Liu