Received-SPF: pass (google.com: domain of linux-kernel+bounces-189929-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
CC: Baolu Lu <baolu.lu@linux.intel.com>, Joerg Roedel <joro@8bytes.org>, "Will
 Deacon" <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>,
	"Jean-Philippe Brucker" <jean-philippe@linaro.org>, Nicolin Chen
	<nicolinc@nvidia.com>, "Liu, Yi L" <yi.l.liu@intel.com>, Jacob Pan
	<jacob.jun.pan@linux.intel.com>, "Joel Granados" <j.granados@samsung.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v5 7/9] iommufd: Associate fault object with
 iommufd_hw_pgtable
Thread-Topic: [PATCH v5 7/9] iommufd: Associate fault object with
 iommufd_hw_pgtable
Thread-Index: AQHamw8P/6/4oit4lU2Ra006NwpZoLGYE2jAgAduk4CAABV6kIAG/q0AgAPevJA=
Date: Mon, 27 May 2024 01:33:25 +0000
Message-ID: <BN9PR11MB527647FF053E249E27F4B43C8CF02@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240430145710.68112-1-baolu.lu@linux.intel.com>
 <20240430145710.68112-8-baolu.lu@linux.intel.com>
 <BN9PR11MB5276A8E898983310B83C399E8CEC2@BN9PR11MB5276.namprd11.prod.outlook.com>
 <b09f96db-8451-4de9-81c5-312cffdfd4fc@linux.intel.com>
 <BN9PR11MB5276F07A130CD617777A3AAD8CE92@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20240524142446.GP69273@ziepe.ca>
In-Reply-To: <20240524142446.GP69273@ziepe.ca>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?trpj9adZJ3qOkBvVbU60QNM3w/967pmqN1B+7CB1fIDT4vLEa0YtLwxodru1?=
 =?us-ascii?Q?M6QFztDcSmQrp8MxYTCB7gvExDbatwL/jCDOAdU76Cza6xAoutWaGZ/UZQQU?=
 =?us-ascii?Q?r9xJNrFqmYHsh8aepOklP0pvlMDFhlQHn5o+1JgqVuYFUpzUgkjjbMC1c3mU?=
 =?us-ascii?Q?GcIGlMvuVQms/0TOCKA/j2FEbHt3fnbg6v9duFtOWaybCNN21DIvOL7oWDp1?=
 =?us-ascii?Q?xSRjRhfuzn0EY1T9eWvT7QD/BRNyWi4lbsEnId/CQU74jbLPYgBMdUGmt/Si?=
 =?us-ascii?Q?aDobTEJc5ilRxAst6rLCA3R6g7NmqusxtkdS+1xFeK9c29rO9eyzd7l8BKms?=
 =?us-ascii?Q?ZJCCquGSiLs6PiFhD2VjVsznYKmwpnsy89QQgtfNQgUYxKjAww6pJA6uB67J?=
 =?us-ascii?Q?t1R/RSgUY/HHCKzsziF2/S9aXAmvTNnaG0nUsVluZAMvsJSXJBbDb6c1qtew?=
 =?us-ascii?Q?iMw7wngqRi6odfE9GyqQqywc1+87hChJdy6UHpv+EXUmw8T7b0judxJFRPft?=
 =?us-ascii?Q?0V29GuJiRaQq3lFCPAIPvXqSpE3mTxPkD3laBYgGc1oM5nrA52TUuJJEDNam?=
 =?us-ascii?Q?WeTrEsDcbU++CoaW+yrlIO3aRp/A1gxkDcISR6kqAoY6UaMlmGAyoLvpNHq2?=
 =?us-ascii?Q?R+FsEYYSerdesdYVTyXSENEdZdGcxZVeKtF89YD2SiAH92bsB9cXz4tNIBRJ?=
 =?us-ascii?Q?KcP1ZNzqgERLOSZPNiOtzqdZo9h7CtNrRUkVWjMu4U3FLBiWUFXAwnZzsFbw?=
 =?us-ascii?Q?Jb/d3XSRvJYQlQq31fzRuD6iink065hzL7s7jSqQWm+fObXH4ab0V/Daivt2?=
 =?us-ascii?Q?Jlz/dknr5Glb18KYwRIbd9hlLn913G9Sz/cvO00BRxympQti0qeQR8RbL/AY?=
 =?us-ascii?Q?RVpPKRIEVIRE3NJKZPeyS5Y45tupvbdLUMILX+i8rAWexGGdLaKJPNLt1fon?=
 =?us-ascii?Q?pvQa7jiWMMvA2rWOMsunv1wGZKneFmlZGSE5zTRf4jkyf+cBveU6nK3eVCU4?=
 =?us-ascii?Q?0A8WDbVuEqOZvk38TNXR87vBUu3mP050K0Up2vOS6i/IzrkZo+gqYVuv2447?=
 =?us-ascii?Q?VhpC6icXYW2mPUOBmihGhRNtnSjRTvuxluTQ4YwYDZ5Bpwnnot2pXLh0QLrE?=
 =?us-ascii?Q?/1PSuqMa9nvIG92QBj+s++p66OQLj5Jmg2guoEvd+vkuTiAp+thZkAjir93T?=
 =?us-ascii?Q?VTjP0lLspevPnuFUdYOLTnUXvK5QV/ExDyRVT6gLcUEYWgfjXm0FPWzp2Ap7?=
 =?us-ascii?Q?YYx4l91TT7Q5cRpE2znwdEyuHQqWi2x+sxvOg7IjFbxHAA4Cfc3rbi6+XCWN?=
 =?us-ascii?Q?wXrqg3q+6Qv7RKB43u27RuhfQbeTiZGmNJan6XB+rYhKh0HB2z3QLTRBQqWw?=
 =?us-ascii?Q?8qvn4G5buTlvPuhTbPOlxd/UZJM1qoV5lW255M8/qyV3FW1FOCYdXjW52/R8?=
 =?us-ascii?Q?PBOluVd3A0L+zUcHlK3PMolKn+KqdYTN/4N5Vg6SzfAguef2eFZZI3aqKrFI?=
 =?us-ascii?Q?6eL8uUuDP693lsDiBPfbxgtRnYXKACU3BjmOPBR6QjAs4KaFj14wl2LZY4N+?=
 =?us-ascii?Q?Aookeg+kDXFNFdYJnqxs5QaGHxM8lnKxtCjJqhUl?=
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: 713340d9-bc54-4255-c22e-08dc7ded00a6
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 May 2024 01:33:25.1389
 (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: gReQcsJeb1mkurM7pxij3u62asW0MpOe6FZ5uRO8ByfxftujLACL6wxpMwj0ShFHoaONlzSSbN3cQBPkM1hs7A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7797
X-OriginatorOrg: intel.com

> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Friday, May 24, 2024 10:25 PM
>=20
> On Mon, May 20, 2024 at 03:39:54AM +0000, Tian, Kevin wrote:
> > > From: Baolu Lu <baolu.lu@linux.intel.com>
> > > Sent: Monday, May 20, 2024 10:19 AM
> > >
> > > On 5/15/24 4:50 PM, Tian, Kevin wrote:
> > > >> From: Lu Baolu <baolu.lu@linux.intel.com>
> > > >> Sent: Tuesday, April 30, 2024 10:57 PM
> > > >>
> > > >> @@ -308,6 +314,19 @@ int iommufd_hwpt_alloc(struct
> iommufd_ucmd
> > > >> *ucmd)
> > > >>   		goto out_put_pt;
> > > >>   	}
> > > >>
> > > >> +	if (cmd->flags & IOMMU_HWPT_FAULT_ID_VALID) {
> > > >> +		struct iommufd_fault *fault;
> > > >> +
> > > >> +		fault =3D iommufd_get_fault(ucmd, cmd->fault_id);
> > > >> +		if (IS_ERR(fault)) {
> > > >> +			rc =3D PTR_ERR(fault);
> > > >> +			goto out_hwpt;
> > > >> +		}
> > > >> +		hwpt->fault =3D fault;
> > > >> +		hwpt->domain->iopf_handler =3D iommufd_fault_iopf_handler;
> > > >> +		hwpt->domain->fault_data =3D hwpt;
> > > >> +	}
> > > >
> > > > this is nesting specific. why not moving it to the nested_alloc()?
> > >
> > > Nesting is currently a use case for userspace I/O page faults, but th=
is
> > > design should be general enough to support other scenarios as well.
> >
> > Do we allow user page table w/o nesting?
> >
> > What would be a scenario in which the user doesn't manage the
> > page table but still want to handle the I/O page fault? The fault
> > should always be delivered to the owner managing the page table...
>=20
> userspace always manages the page table, either it updates the IOPTE
> directly in a nest or it calls iommufd map operations.
>=20
> Ideally the driver will allow PRI on normal cases, although it will
> probably never be used.
>=20

But now it's done in a half way.

valid_flags in normal cases doesn't accept a fault ID. but we then
handle the fault ID flag generally above.

I'd like to see a consistent message throughout the path.=20