Re: [PATCH 1/1] iommu/vt-d: Decouple igfx_off from graphic identity mapping

[Baolu Lu]

On 2024/4/28 14:45, Tian, Kevin wrote:
> > From: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
> > Sent: Sunday, April 28, 2024 11:20 AM
> >
> > A kernel command called igfx_off was introduced in commit <ba39592764ed>
> > ("Intel IOMMU: Intel IOMMU driver"). This command allows the user to
> > disable the IOMMU dedicated to SOC-integrated graphic devices.
> >
> > Commit <9452618e7462> ("iommu/intel: disable DMAR for g4x integrated
> > gfx")
> > used this mechanism to disable the graphic-dedicated IOMMU for some
> > problematic devices. Later, more problematic graphic devices were added
> > to the list by commit <1f76249cc3beb> ("iommu/vt-d: Declare Broadwell igfx
> > dmar support snafu").
> >
> > On the other hand, commit <19943b0e30b05> ("intel-iommu: Unify
> > hardware
> > and software passthrough support") uses the identity domain for graphic
> > devices if CONFIG_DMAR_BROKEN_GFX_WA is selected.
> >
> > +       if (iommu_pass_through)
> > +               iommu_identity_mapping = 1;
> > +#ifdef CONFIG_DMAR_BROKEN_GFX_WA
> > +       else
> > +               iommu_identity_mapping = 2;
> > +#endif
> > ...
> >
> > static int iommu_should_identity_map(struct pci_dev *pdev, int startup)
> > {
> > +        if (iommu_identity_mapping == 2)
> > +                return IS_GFX_DEVICE(pdev);
> > ...
> >
> > In the following driver evolution, CONFIG_DMAR_BROKEN_GFX_WA and
> > quirk_iommu_igfx() are mixed together, causing confusion in the driver's
> > device_def_domain_type callback. On one hand, dmar_map_gfx is used to
> > turn
> > off the graphic-dedicated IOMMU as a workaround for some buggy
> > hardware;
> > on the other hand, for those graphic devices, IDENTITY mapping is required
> > for the IOMMU core.
> >
> > Commit <4b8d18c0c986> "iommu/vt-d: Remove
> > INTEL_IOMMU_BROKEN_GFX_WA" has
> > removed the CONFIG_DMAR_BROKEN_GFX_WA option, so the
> > IDENTITY_DOMAIN
> > requirement for graphic devices is no longer needed. Therefore, this
> > requirement can be removed from device_def_domain_type() and igfx_off
> > can
> > be made independent.
> >
> > Fixes: 4b8d18c0c986 ("iommu/vt-d: Remove
> > INTEL_IOMMU_BROKEN_GFX_WA")
> > Signed-off-by: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/iommu/intel/iommu.c | 19 ++++++-------------
> >   1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index fbbf8fda22f3..57a986524502 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -217,12 +217,11 @@ int intel_iommu_sm =
> > IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON);
> >   int intel_iommu_enabled = 0;
> >   EXPORT_SYMBOL_GPL(intel_iommu_enabled);
> >
> > -static int dmar_map_gfx = 1;
> >   static int intel_iommu_superpage = 1;
> >   static int iommu_identity_mapping;
> >   static int iommu_skip_te_disable;
> > +static int disable_igfx_dedicated_iommu;
> what about 'no_igfx_iommu"? dedicated is implied for igfx
> so a shorter name might be slightly better.

I like disable_igfx_iommu more. :-)

Best regards,
baolu