Received-SPF: pass (google.com: domain of linux-kernel+bounces-161290-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Lu Baolu <baolu.lu@linux.intel.com>, Joerg Roedel <joro@8bytes.org>, "Will
 Deacon" <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>, "Jason
 Gunthorpe" <jgg@ziepe.ca>, "Wang, Zhenyu Z" <zhenyu.z.wang@intel.com>
CC: "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/1] iommu/vt-d: Decouple igfx_off from graphic identity
 mapping
Thread-Topic: [PATCH 1/1] iommu/vt-d: Decouple igfx_off from graphic identity
 mapping
Thread-Index: AQHamRs7H6xZ3tB/HkSdsmnXbNR847F9O6wQ
Date: Sun, 28 Apr 2024 06:45:17 +0000
Message-ID: <BN9PR11MB5276AF4C9A57C3B8D3A25E8E8C142@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240428032020.214616-1-baolu.lu@linux.intel.com>
In-Reply-To: <20240428032020.214616-1-baolu.lu@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?YsqHLWLSkH63z7pTesLLu03iPa3yhU089YaqLNZlek2yqBxkFMT9HzIDl2Iq?=
 =?us-ascii?Q?wsnl4UEiaaaqbnGRqnl/zlicMAZexHS2PeQMDPQaIuy+yz5bOoJCtKIO+Ac3?=
 =?us-ascii?Q?CoJlPlgv4dNOLeInFdh854UhtJ2VHTQHw+K+thm7y2Xi6Kz5cOFMpGJ3fIzL?=
 =?us-ascii?Q?wygTqe+6smqNRepHyoIWzvAMqEee8UaVm4TahBRcs4sAC0W0Mjo2l45EnF13?=
 =?us-ascii?Q?WUXzJ7WwWtnsTrx25SSPLhPY6M1unTQhcgMWGAi8pRgXmz+/sHAmecCMInEu?=
 =?us-ascii?Q?rpWvE5CdgRnTjJ+BdO9nqu4sdyamh82EKlgfqP6JOc0Y6+FIU6dIcuQZhP/Y?=
 =?us-ascii?Q?MKOCBuYF/PXtiGiu/gwbdoDiZxuemYg6uJgVgqkz7ULMVZ27lYrQL+R7Nmu/?=
 =?us-ascii?Q?xMoKK1yWaR/4kciqoy3I9kpXE8tG/XSp0l6Isw0carKnZVj6OU8FbNRpJwBF?=
 =?us-ascii?Q?g9r9CMlzZGeujLxWzMaFlgRY8akecYraUq0gTA7Z7iigTrwBkwKfETbDI0lt?=
 =?us-ascii?Q?MR/2T5+sdtswEsr+vJM2ZI9zkNghrmu9mpofIdVjzWehDNPAzlWrLyQRzTlM?=
 =?us-ascii?Q?2+qPAr/C5fcI5R8sZNo8oTuXMXEpGheYvQkX8PD/f1PRQ27aZTSMyTuwd5A2?=
 =?us-ascii?Q?7uyqzg0QB250PzIWPGVI04EsztrLadU51IRmHOR+3cFC1T5KUBEVE0QzhdWh?=
 =?us-ascii?Q?pOLUbWUJWrR3P2waHJEkmHaFfV0HMBK37q1Bj0yzysS58yXPxtVeaIGazXu7?=
 =?us-ascii?Q?j07pgfu/a98u+LMje2w73OonFZhMbd4z4wslGbEyoXOwZassoEWfAAFoXkL8?=
 =?us-ascii?Q?PtSig7TS17mJWIk9XDkmz0/SrgaMj38KWUFj1jhbN4sIeL7Gd79+HR9iQl8l?=
 =?us-ascii?Q?J4IoYsyoKocw0+ATfm30E78/95rtC01g+1Bw6ZH5AlczVjDADPTLreuOJCkl?=
 =?us-ascii?Q?Bq3X5LkQWszQ+uzP4cK0SJDQVxXCgZ7OOG1+bW5pPx6wUr446YD7VqQPAbar?=
 =?us-ascii?Q?8hnQ2RDEN7ZaFRbx8g/2RWV6Fmu+orpNHnWtlY7KyyUTcNO2DFIbW1mMOHIQ?=
 =?us-ascii?Q?Uns7FuRuI7qywV6okk7WmrUOtXHFB7zubHNsXtmsaSQ5rAhVffRlyA6NpDgs?=
 =?us-ascii?Q?viAGP9sjQoPQ0bfgq1xaoBjWPvTpgsxaE1CDUYZFBOZypR6bixEilCbLofzE?=
 =?us-ascii?Q?P6HQ8g4E8RMmZnbbW3h2DaHrJAEL4vxuGV2PKHhFZ657mZODDo34vsYnSCnq?=
 =?us-ascii?Q?44AUgjDzQA3D9GLpDNl2Qum90AuvERLTgVUa3g+mU3AKt2uHed+tFl2rpock?=
 =?us-ascii?Q?n5jsg5zUEm9kVNJCXFgYMQnGV3MsWfAtNzJGNLnS5NvAhUXSSsSte6NHikv+?=
 =?us-ascii?Q?HHwOjTBn/DdftKgkUAJnx1bG4W9jtQzTBXufnLUYUyEIANPAHWTyRJS1gZEv?=
 =?us-ascii?Q?hDBCyipIEkR+TI3IHHDQFyIOlEjOrpt7qaHesQE/bTxXK1gkIX9Njybcrty5?=
 =?us-ascii?Q?UvlN7cfpyZRZlDbci+FJQlAKICwyu7KQf1CG891TmJcTVqzVJfUSQfGsh2Pu?=
 =?us-ascii?Q?MUpiUOKf7Dq0mG3N25wJjok8boCAHuyym9HC5Ws6?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ca50540-55c1-4660-8360-08dc674ec419
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Apr 2024 06:45:17.4272
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: cYwUMaYoza+G3hkY84ZkrHObXI82YO62FFWyEhZqrAZZH1OdVkOdsL4xep/bP9cG0vGICD3jusWUyPLRcK7gMg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB8323
X-OriginatorOrg: intel.com

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Sunday, April 28, 2024 11:20 AM
>=20
> 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.
>=20
> 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 igf=
x
> dmar support snafu").
>=20
> 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.
>=20
> +       if (iommu_pass_through)
> +               iommu_identity_mapping =3D 1;
> +#ifdef CONFIG_DMAR_BROKEN_GFX_WA
> +       else
> +               iommu_identity_mapping =3D 2;
> +#endif
> ...
>=20
> static int iommu_should_identity_map(struct pci_dev *pdev, int startup)
> {
> +        if (iommu_identity_mapping =3D=3D 2)
> +                return IS_GFX_DEVICE(pdev);
> ...
>=20
> 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 require=
d
> for the IOMMU core.
>=20
> 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.
>=20
> Fixes: 4b8d18c0c986 ("iommu/vt-d: Remove
> INTEL_IOMMU_BROKEN_GFX_WA")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>=20
> 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 =3D
> IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON);
>  int intel_iommu_enabled =3D 0;
>  EXPORT_SYMBOL_GPL(intel_iommu_enabled);
>=20
> -static int dmar_map_gfx =3D 1;
>  static int intel_iommu_superpage =3D 1;
>  static int iommu_identity_mapping;
>  static int iommu_skip_te_disable;
> +static int disable_igfx_dedicated_iommu;
>=20

what about 'no_igfx_iommu"? dedicated is implied for igfx
so a shorter name might be slightly better.

otherwise it looks good:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>