RE: [PATCH] intel-iommu: ignore SNP bit in scalable mode

[Liu, Yi L]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Wednesday, November 24, 2021 4:29 PM
> 
> On Wed, Nov 24, 2021 at 3:57 PM Peter Xu <peterx@redhat.com> wrote:
> >
> > On Wed, Nov 24, 2021 at 02:03:09PM +0800, Jason Wang wrote:
> > > When booting with scalable mode, I hit this error:
> > >
> > > qemu-system-x86_64: vtd_iova_to_slpte: detected splte reserve non-
> zero iova=0xfffff002, level=0x1slpte=0x102681803)
> > > qemu-system-x86_64: vtd_iommu_translate: detected translation
> failure (dev=01:00:00, iova=0xfffff002)
> > > qemu-system-x86_64: New fault is not recorded due to compression of
> faults
> > >
> > > This is because the SNP bit is set since Linux kernel commit
> > > 6c00612d0cba1 ("iommu/vt-d: Report right snoop capability when using
> > > FL for IOVA") where SNP bit is set if scalable mode is on though this
> > > seems to be an violation on the spec which said the SNP bit is
> > > considered to be reserved if SC is not supported.
> >
> > When I was reading that commit, I was actually confused by this change:
> >
> > ---8<---
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 956a02eb40b4..0ee5f1bd8af2 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -658,7 +658,14 @@ static int
> domain_update_iommu_snooping(struct intel_iommu *skip)
> >         rcu_read_lock();
> >         for_each_active_iommu(iommu, drhd) {
> >                 if (iommu != skip) {
> > -                       if (!ecap_sc_support(iommu->ecap)) {
> > +                       /*
> > +                        * If the hardware is operating in the scalable 
> > mode,
> > +                        * the snooping control is always supported since we
> > +                        * always set PASID-table-entry.PGSNP bit if the 
> > domain
> > +                        * is managed outside (UNMANAGED).
> > +                        */
> > +                       if (!sm_supported(iommu) &&
> > +                           !ecap_sc_support(iommu->ecap)) {
> >                                 ret = 0;
> >                                 break;
> >                         }
> > ---8<---
> >
> > Does it mean that for some hardwares that has sm_supported()==true,
> it'll have
> > SC bit cleared in ecap register?
> 
> I guess not, so it's probably only the problem of vIOMMU.
> 
> > That sounds odd, and not sure why.  Maybe Yi
> > Liu or Yi Sun may know?
> 
> Another interesting point is that, it looks to me after that commit
> SNP is used for the domain that is not UNMANAGED even if PGSNP is not
> set.

Per spec, if the PGSNP is set, it means the final page access is snooped.
If it's not set, then it's up to other bit to decide it. For detail, you may
refer to table 6 of chapter 3.9 in vtd 3.2 spec.

Regards,
Yi Liu