Re: [PATCH] iommu/vt-d: Add RPLS to quirk list to skip TE disabling

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Thu, 2022-02-24 at 13:42 +0800, Lu Baolu wrote:
> On 2/23/22 2:29 PM, Tejas Upadhyay wrote:
> > The VT-d spec requires (10.4.4 Global Command Register, TE
> > field) that:
> > 
> > Hardware implementations supporting DMA draining must drain
> > any in-flight DMA read/write requests queued within the
> > Root-Complex before completing the translation enable
> > command and reflecting the status of the command through
> > the TES field in the Global Status register.
> > 
> > Unfortunately, some integrated graphic devices fail to do
> > so after some kind of power state transition. As the
> > result, the system might stuck in iommu_disable_translati
> > on(), waiting for the completion of TE transition.
> > 
> > This adds RPLS to a quirk list for those devices and skips
> > TE disabling if the qurik hits.
> > 
> > Fixes: https://gitlab.freedesktop.org/drm/intel/-/issues/4898
> > Fixes: LCK-10789
> 
> Remove this please.

good catch. Wrong use of Fixes tag.
"Fixes:" should only be used for patches fixing other patches and
mentioning the commit id.

Baolu,
could you mind if we use

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4898

or maybe

References: https://gitlab.freedesktop.org/drm/intel/-/issues/4898

This last one seems to be the one use in drivers/iommu
and the Closes is what we use in drm-intel, hence the one used
with gitlab.freedesktop links in general.

> 
> > Tested-by: Raviteja Goud Talla <ravitejax.goud.talla@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Tejas Upadhyay
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>

you can keep my rv-b

> > ---
> >   drivers/iommu/intel/iommu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/intel/iommu.c
> > b/drivers/iommu/intel/iommu.c
> > index 92fea3fbbb11..be9487516617 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -5743,7 +5743,7 @@ static void quirk_igfx_skip_te_disable(struct
> > pci_dev *dev)
> >         ver = (dev->device >> 8) & 0xff;
> >         if (ver != 0x45 && ver != 0x46 && ver != 0x4c &&
> >             ver != 0x4e && ver != 0x8a && ver != 0x98 &&
> > -           ver != 0x9a)
> > +           ver != 0x9a && ver != 0xa7)
> >                 return;
> >   
> >         if (risky_device(dev))
> 
> This is a quirk for integrated graphic device. Rodrigo, does this
> hardware needs this quirk as well?

Yes, 0xa7* are RPL-S that are integrated and very similar to 0x46*
(ADL-S)

I would prefer to have a more expanded list instead of group or at
least some defines with the platform names.

Thanks,
Rodrigo.

> 
> Best regards,
> baolu

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu