On 2024/5/27 14:04, Duan, Zhenzhong wrote:


-----Original Message-----
From: Jason Wang <jasow...@redhat.com>
Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons defined by
spec

On Fri, May 24, 2024 at 4:41 PM Duan, Zhenzhong
<zhenzhong.d...@intel.com> wrote:



-----Original Message-----
From: Jason Wang <jasow...@redhat.com>
Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons defined by
spec

On Tue, May 21, 2024 at 6:25 PM Duan, Zhenzhong
<zhenzhong.d...@intel.com> wrote:



-----Original Message-----
From: Jason Wang <jasow...@redhat.com>
Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons defined
by
spec

On Mon, May 20, 2024 at 12:15 PM Liu, Yi L <yi.l....@intel.com>
wrote:

From: Duan, Zhenzhong <zhenzhong.d...@intel.com>
Sent: Monday, May 20, 2024 11:41 AM



-----Original Message-----
From: Jason Wang <jasow...@redhat.com>
Sent: Monday, May 20, 2024 8:44 AM
To: Duan, Zhenzhong <zhenzhong.d...@intel.com>
Cc: qemu-devel@nongnu.org; Liu, Yi L <yi.l....@intel.com>; Peng,
Chao
P
<chao.p.p...@intel.com>; Yu Zhang
<yu.c.zh...@linux.intel.com>;
Michael
S. Tsirkin <m...@redhat.com>; Paolo Bonzini
<pbonz...@redhat.com>;
Richard Henderson <richard.hender...@linaro.org>; Eduardo
Habkost
<edua...@habkost.net>; Marcel Apfelbaum
<marcel.apfelb...@gmail.com>
Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons
defined
by
spec

On Fri, May 17, 2024 at 6:26 PM Zhenzhong Duan
<zhenzhong.d...@intel.com> wrote:

From: Yu Zhang <yu.c.zh...@linux.intel.com>

Currently we use only VTD_FR_PASID_TABLE_INV as fault
reason.
Update with more detailed fault reasons listed in VT-d spec
7.2.3.

Signed-off-by: Yu Zhang <yu.c.zh...@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---

I wonder if this could be noticed by the guest or not. If yes should
we consider starting to add thing like version to vtd emulation
code?

Kernel only dumps the reason like below:

DMAR: [DMA Write NO_PASID] Request device [20:00.0] fault addr
0x1234600000
[fault reason 0x71] SM: Present bit in first-level paging entry is
clear

Yes, guest kernel would notice it as the fault would be injected to vm.

Maybe bump 1.0 -> 1.1?
My understanding version number is only informational and is far
from
accurate to mark if a feature supported. Driver should check
cap/ecap
bits instead.

Should the version ID here be aligned with VT-d spec?


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.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

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. :)


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

Reply via email to