Received-SPF: pass (google.com: domain of linux-kernel+bounces-32572-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>, "joro@8bytes.org"
	<joro@8bytes.org>, "jgg@nvidia.com" <jgg@nvidia.com>
CC: "Liu, Yi L" <yi.l.liu@intel.com>, "nicolinc@nvidia.com"
	<nicolinc@nvidia.com>, "eric.auger@redhat.com" <eric.auger@redhat.com>,
	"vasant.hegde@amd.com" <vasant.hegde@amd.com>, "jon.grimm@amd.com"
	<jon.grimm@amd.com>, "santosh.shukla@amd.com" <santosh.shukla@amd.com>,
	"Dhaval.Giani@amd.com" <Dhaval.Giani@amd.com>, "pandoh@google.com"
	<pandoh@google.com>, "loganodell@google.com" <loganodell@google.com>
Subject: RE: [RFCv2 PATCH 6/7] iommu/amd: Add nested domain allocation support
Thread-Topic: [RFCv2 PATCH 6/7] iommu/amd: Add nested domain allocation
 support
Thread-Index: AQHaROtTcEDws+vJXUC8MJbA2V9QoLDllRjQ
Date: Mon, 22 Jan 2024 08:52:21 +0000
Message-ID: <BN9PR11MB527638F808D74FA9A6DED43A8C752@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20240112000646.98001-1-suravee.suthikulpanit@amd.com>
 <20240112000646.98001-7-suravee.suthikulpanit@amd.com>
In-Reply-To: <20240112000646.98001-7-suravee.suthikulpanit@amd.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?romYqnbdH0O698bOLlQwQBNvvVwrP8cIb8gdYWPQN8X/1SyXvl4F5jRkEKAW?=
 =?us-ascii?Q?KQuLbtUwVsy1+lA7CE14bgFh3gow+NKaMAdhfwLflwCxrbDY/GaB+CsEmpIu?=
 =?us-ascii?Q?vmBhoTFibLCdQ1nxVEXP062AW8RUa1kCPwhcJPEsoYrCkH6PHrNZRZ5I/07r?=
 =?us-ascii?Q?TL2ILpwXDFhGQ8Z13Hn5nd6F4Ha1OlATRp6fqfi0FjUEgn8vj7sgsgWFJ4Ec?=
 =?us-ascii?Q?A6xloMb46pGrxZTL/St0DAmZRNh9E8fOqrKYI+IuLeyy8EazsPiP/2Lpqk4s?=
 =?us-ascii?Q?4MuNZgn0cwEXcTd0Pa8QpWptLreBgJy86DyHcEDYzlwruHs5oHOsJl+IfZsZ?=
 =?us-ascii?Q?1hutgDq/1068jw9O7fXCMQ9zt6BcQl0wR7ggWzbo8pTBheMBQLn82UxKBFpl?=
 =?us-ascii?Q?sepwmVqVt1o+FuRo6Q/pQDXZRalJ8a7YfUlo1O8QWoMl4sKP/8iHITWbQC2E?=
 =?us-ascii?Q?yrY0NpqbznCeMCWDffREqshb+G5sj+tYOQzeB778yjMBrJ2FnlQ9j8q1WzMP?=
 =?us-ascii?Q?NjKg9H0mS9tA3XsjIDySTKLNjIWvlacr9ml5BE0onmEvyauvy0b4fr+i/1O4?=
 =?us-ascii?Q?UDyUaRjUDf2UIkt+wRvzgan3LhqC5Q2AjUGf8tt4zIRZySrwRxfserZ+ncTE?=
 =?us-ascii?Q?CogrXzENIYH5yr8isr5PlTYvvI26P3OhSbWeg6jpbqNrIr3FChKs83KIvpcx?=
 =?us-ascii?Q?AkWSfw9E5yQkbO7eOMh0qAGZPQzro1xaeoqYDP8ur/NgJ2fL7UbYNKsvyOdj?=
 =?us-ascii?Q?9cb0is81E/raYE+0m21UT/LGWgxNT4KJ2agdXIcMWCECaimQox6AdJoxe+9A?=
 =?us-ascii?Q?48leUNaO8gNlVY+isNlJjOYZZNOxvEBBEDckCIkqys8uXzpu3Fewe1XdWHCA?=
 =?us-ascii?Q?34kZQCZ4q9rkODiFspr9u1C65yGQbnvbiwSc4/4Tj/WYzbVVzyxkjlAIA1Bt?=
 =?us-ascii?Q?Gpe+dqtYAL29bFJ9HmCG5cSJD4ktoYAQYqFQtwQMcCn9xovTFxCJbbWgTog7?=
 =?us-ascii?Q?Jh5ebIMcLFF+IE84Y5S0dCpqZOOSSno5ZqOn6LkwNgBXieF9+yQPOLss4RO7?=
 =?us-ascii?Q?xsbvFqc2iBjWf7dKa94NZ0qmVxs7ruT2p3/LlPgIkCgwtu4qS1vQ5IHYpUR7?=
 =?us-ascii?Q?fjJA9y4n110NnBwBg+OQm8kgB6xPmaqqNV6ya0Q3nq/lEVQGl1wkYR5Y7Yin?=
 =?us-ascii?Q?XviR+OU76Ijxh0nLxo071GoiT568mHJyE7IHEwjbhsJK1paxf+SZrGNkczIW?=
 =?us-ascii?Q?mkazwsqYUgpnIvDvm1gQp2ZOYdr1tizWgOo/a0Ono2P+Ez6Typp3i6YLqFUj?=
 =?us-ascii?Q?9FjmbX4jRio3lwuDmCSx3oYsL/f35HDDG4mi4RnQKXf8JLvVoDBT6VV+MNuz?=
 =?us-ascii?Q?alcwOLVGPgSs1ALOk24PAfHUt0WOMX4w7cQCfNWHLOnTA4fqS5A+jkbKNgrG?=
 =?us-ascii?Q?QwnmURvoAaNXIknHHdFTY8AJOQzbRIiyd4SqKJbQdwEfsJPgAxjN+yjxNPKY?=
 =?us-ascii?Q?RXywLXAbVUw0YV4dX0VMmdZmoDOLHKHVA341TVzmg9ePiIt1NJMV3cuJrInk?=
 =?us-ascii?Q?CVLiRgf1PIy6gSEHVEbZBFZMl/Ob90P/2PbmgZ0a?=
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: 0ef6ed19-c2bd-4fef-fd88-08dc1b277264
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2024 08:52:21.6549
 (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: uyvhhwIsOfmXgtAaxcIvzjLDaWWMYBwAFCnSKIPaie6Zh3eX9dwJAOhch0tg6RN0IMSbr/gz/nTe2lwTs0Undg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5805
X-OriginatorOrg: intel.com

> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Sent: Friday, January 12, 2024 8:07 AM
>=20
> To support nested translation, the parent domain is allocated with flag
> IOMMU_HWPT_ALLOC_NEST_PARENT, and stores information of the v1 page
> table
> for stage 2 (i.e. GPA->SPA), whereas the child domain stores information
> of the GCR3 root pointer table for stage 1 (i.e. GVA->GPA).

put support of NEST_PARENT in a separate patch.

> @@ -569,6 +572,9 @@ struct protection_domain {
>  	bool dirty_tracking;	/* dirty tracking is enabled in the domain */
>  	unsigned dev_cnt;	/* devices assigned to this domain */
>  	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference
> count */
> +	struct protection_domain *parent; /* Nested parent domain */
> +	u16 guest_paging_mode;	/* Guest paging mode */
> +	u16 guest_domain_id;	/* Guest domain ID */

not used

> +struct iommu_domain *do_iommu_domain_alloc(unsigned int type,
>  						  struct device *dev, u32 flags)
>  {
>  	bool dirty_tracking =3D flags &
> IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> @@ -2454,7 +2465,10 @@ static struct iommu_domain
> *do_iommu_domain_alloc(unsigned int type,
>  	if (iommu) {
>  		domain->domain.type =3D type;
>  		domain->domain.pgsize_bitmap =3D iommu->iommu.ops-
> >pgsize_bitmap;
> -		domain->domain.ops =3D iommu->iommu.ops-
> >default_domain_ops;
> +		if (type =3D=3D IOMMU_DOMAIN_NESTED)
> +			domain->domain.ops =3D &nested_domain_ops;
> +		else
> +			domain->domain.ops =3D iommu->iommu.ops-
> >default_domain_ops;
>=20
>  		if (dirty_tracking)
>  			domain->domain.dirty_ops =3D &amd_dirty_ops;

dirty_tracking doesn't apply to nested domain

> +
> +static bool check_nested_support(u32 flags)
> +{
> +	if (!(flags & IOMMU_HWPT_ALLOC_NEST_PARENT))
> +		return true;

it's more readable by putting this check in the caller.

> +
> +	/*
> +	 * When allocated nested parent domain, the device may already
> +	 * have been attached to a domain. For example, a device is already
> +	 * attached to the domain allocated by VFIO, which contains GPA-
> >SPA mapping.
> +	 * In such case, return reference to the same domain.
> +	 */
> +	if (dev_data->domain && nested_parent) {
> +		pr_debug("%s: Found exist: protection domain id=3D%#x\n",
> +			 __func__, dev_data->domain->id);
> +		dom =3D &dev_data->domain->domain;

alloc() shouldn't deal with domain reuse. it's the decision in the
caller. If the caller wants to reuse then it can try to attach to an
existing domain if compatible.

if the caller wants to create a new domain then just follow it.