Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <a19031b0-7c30-45e6-b171-c53e3578b867@intel.com>
Date:   Thu, 14 Dec 2023 10:55:25 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 8/8] iommu/vt-d: Add set_dev_pasid callback for nested
 domain
To:     Yi Liu <yi.l.liu@intel.com>, <baolu.lu@linux.intel.com>
CC:     <robin.murphy@arm.com>, <kevin.tian@intel.com>, <jgg@nvidia.com>,
        <alex.williamson@redhat.com>, <joro@8bytes.org>,
        <cohuck@redhat.com>, <eric.auger@redhat.com>,
        <nicolinc@nvidia.com>, <kvm@vger.kernel.org>,
        <mjrosato@linux.ibm.com>, <chao.p.peng@linux.intel.com>,
        <yi.y.sun@linux.intel.com>, <peterx@redhat.com>,
        <jasowang@redhat.com>, <shameerali.kolothum.thodi@huawei.com>,
        <lulu@redhat.com>, <suravee.suthikulpanit@amd.com>,
        <iommu@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
        <linux-kselftest@vger.kernel.org>, <zhenzhong.duan@intel.com>,
        <joao.m.martins@oracle.com>, <xin.zeng@intel.com>,
        <yan.y.zhao@intel.com>
References: <20231127063428.127436-1-yi.l.liu@intel.com>
 <20231127063428.127436-9-yi.l.liu@intel.com>
Content-Language: en-US
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <20231127063428.127436-9-yi.l.liu@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V253UmhpZkdjQ2l4RE9MaDBOamZ5V0ZsUmQ1bFdBaTNhdG9SNlRQWFdWMHJZ?=
 =?utf-8?B?b3h5WXFMRndsN0w2OUE5N053Z2RjMTI3amVadmJmRlBpdndoSkZqaXpZK1pQ?=
 =?utf-8?B?ZnU4V3kxUGk1ZDdwVktZU05CZW9qOVZSQ0dRaUJsUldDSGhQamxPSisvSU13?=
 =?utf-8?B?M3JCcFBJR090MmlrR2tJM1Y1SVJWNUlwT0VHVG1xbW43Z2tUaThNYU85TkNk?=
 =?utf-8?B?VDdZc29KL2NJRFRrWXNXa1NCQ0c3djlJN2VtZEQwazVlQTdiZkJBQjUwY2NW?=
 =?utf-8?B?cis4dUkzYkNpU2xFblk4S0tFV0F5aG9SRnZjcFFpVmVXZUtQOUtlUGhZU0t2?=
 =?utf-8?B?TGFielRCZTlkOE5oTTd1LytQbEZIWDNsYkRDZjgzcmIrTDZPeHB6WWJtN256?=
 =?utf-8?B?dWRHRXlBbzk2TmVjOUJFQkc2UFoyWWxheVpLY2MzcGpYMnFmeW5NWDVOWmQ2?=
 =?utf-8?B?QVJuQWdVTzc3NWtXTVIrR1pqWGJuUmN2SnIwQTkwV05EazNVSVNaTG1QOURJ?=
 =?utf-8?B?SVhucloyN3ZXVUhEQlcvc29BZ0FiaS83bDFmTW5IS3NSQlZMaTl1YXNTWmtl?=
 =?utf-8?B?Ykc1M0hydnUyTzBpK0M4Vk5LVFVXdzlTTkl5UndaZFFrd1lZUlJDdjZFUFZJ?=
 =?utf-8?B?RDBtN2dJVFI5YUpTN0g4a0h3UHFyMHlsNEM0ZCtUdkthVytrZVk5NW01am5E?=
 =?utf-8?B?VHNsSyt1V0F6WFBGeXA4azh5Z1BFejh2QitJWitwQmEzWGRCR0NEeHRNbXcv?=
 =?utf-8?B?OWpidmJiQTVTS1UvbHQyQXBXSW0ydEE5M2ZsSE9PQ3FnQmtSYldjUU80SWhU?=
 =?utf-8?B?cXdtc2Fkb291L2RKR0piQkZyb2FTL2tpdXY1ZUtjMXQyeFBhYWJGdUc5eHVZ?=
 =?utf-8?B?YTAvdXNYSmloMm9HNUx0MENQT25LcUNMM3VkYld2QnFLVnlQVG9Sc0VLc3Jz?=
 =?utf-8?B?bWQrc0thVUhFZ1JSZUJoZ0xaUk0veVJ0dThYMzdvSkgzc3dZeUNpYTVIWnQ0?=
 =?utf-8?B?UWtkZHdWT2IxVFJtUDBtd0lYTWduajFVdFdWeUhrK0tTK215MGdRSm5TdUE2?=
 =?utf-8?B?TzZvbHJubFo3eVB2TVlpbElRUnRWeVdDd3UvQjFBL0xMUDFNWm92d0h3OHFr?=
 =?utf-8?B?RVZMYTRUcUJ2aVd6OWJXMnk4cXZLZnFLU291aTJOUUwza1ZiS0NXUWdIdVdZ?=
 =?utf-8?B?cG1mWWQxdEdJWU9mL1lNYU5DWFhCUVZkUlNlcDRvUEo4N3RxSEVvbUlDWmFO?=
 =?utf-8?B?VkFWQ1QzVDdCTzM3VjZ1a2FLeTljeWF4NDRHVldxVk9VUTVFNStCSXdaQldD?=
 =?utf-8?B?M1V0Um5TT3J4dG1vVVZJbWd2cGRtdUNlTWJNVnVtT3lIaFFhQmgrQmJDRmw1?=
 =?utf-8?B?Nlp4aWE4aDFvNlJQTGNPY09wQ0M1RS9ySDIyWDl1Um96YjViR1ZqeFhEYklR?=
 =?utf-8?B?YUpReG9TNzNnVUZtZnUrZHY2VXVSZkFERXEvWHZZWHB3OXpjVW1ETDQvN0hK?=
 =?utf-8?B?VUQ0aE81Vy8vUUlEN1hUbnE2dUY0NXM5clRJZlk4V1hTSE0zeUltVVNZaWNm?=
 =?utf-8?B?eVE5T3dNVW85bkJDN0ROMUlhQ0x4a0VMaHJpcG5NNXEvMjdtSVhUUkNvb3M0?=
 =?utf-8?B?bXFJUGo2MExBQkZBZ1RMR0JvVm9RMThlQkZZTjF0cUwzODl6cHlSdDI4bUdV?=
 =?utf-8?B?YUhNRWdyNmU4Ky8vOXNmaDRTS1M0UHY3OUZ2anZlQlFnV1I5MUhpOXFhQTZn?=
 =?utf-8?B?amRibE1ma0kyTTQ3MEJjRjh5WFkxRUNqVjVoem4zMlgwV1BRcjB3NCs1d0Fr?=
 =?utf-8?B?UUNDb2E5ZjdBUXB3UEtpR1RlQzZVWmpabkNVM0xZbThnTzRXQloyOXVXZFA0?=
 =?utf-8?B?Wlo0TDdpcTYvbDU2cEUzRE1heU51Y1lEMVdkUDVEc2cydVFjVjRiVFNmcElF?=
 =?utf-8?B?M2N3WDJtbU9sUDNLdkNxUDVVSldOMisvbFFseW1sRkNZQUlFUXNFVHhOd3VU?=
 =?utf-8?B?bEVTeE9jc3lvNnpMK3YrcHJnbUpyQXo3UXV3ZGNwQlVhWWduQ3dYejFlRjNr?=
 =?utf-8?B?MEYwNGg4NVJjbW44T3RmQ014eU9iSEltL3kwWmdHUkZ5WUVtNTBBQjNDRFdK?=
 =?utf-8?B?QjBxNUl3b0RCWVFESnRXV2kwL3dXc0lZbWZhUUZVRm16ekJUZHpQUEtKNGxD?=
 =?utf-8?B?Nmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 51effe6a-3aa3-4f10-c000-08dbfc502857
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Dec 2023 02:55:41.0555
 (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: sDnGzQ+2lQESvBhcA6RBEMmfX0W316b9yNdMmAq8CqwMvBcdmdQFsmk6IKDOBiKm1g+eAquTEDXJvaghkBW34Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4649
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Wed, 13 Dec 2023 18:56:02 -0800 (PST)

On 11/27/2023 2:34 PM, Yi Liu wrote:
> From: Lu Baolu <baolu.lu@linux.intel.com>
>
> This allows the upper layers to set a nested type domain to a PASID of a
> device if the PASID feature is supported by the IOMMU hardware.
>
> The set_dev_pasid callback for non-nest domain has already be there, so
> this only needs to add it for nested domains.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>   drivers/iommu/intel/nested.c | 47 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
> index 44ad48db7ea0..f6f687750104 100644
> --- a/drivers/iommu/intel/nested.c
> +++ b/drivers/iommu/intel/nested.c
> @@ -68,6 +68,52 @@ static int intel_nested_attach_dev(struct iommu_domain *domain,
>   	return 0;
>   }
>   
> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
> +				      struct device *dev, ioasid_t pasid)
> +{
> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> +	struct intel_iommu *iommu = info->iommu;
> +	struct dev_pasid_info *dev_pasid;
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	if (!pasid_supported(iommu))
> +		return -EOPNOTSUPP;
> +
> +	if (iommu->agaw < dmar_domain->s2_domain->agaw)
> +		return -EINVAL;
> +
> +	ret = prepare_domain_attach_device(&dmar_domain->s2_domain->domain, dev);
> +	if (ret)
> +		return ret;
> +
> +	dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
> +	if (!dev_pasid)
> +		return -ENOMEM;
> +
> +	ret = domain_attach_iommu(dmar_domain, iommu);
> +	if (ret)
> +		goto err_free;
> +
> +	ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain);
> +	if (ret)
> +		goto err_detach_iommu;
> +
> +	dev_pasid->dev = dev;
> +	dev_pasid->pasid = pasid;
> +	spin_lock_irqsave(&dmar_domain->lock, flags);
> +	list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);
> +	spin_unlock_irqrestore(&dmar_domain->lock, flags);

---> list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);

dev_pasid is linked at later time, this leads to domain->has_iotlb_device is not correctly set, which finally results into a missing of device iotlb flush in iommu_flush_dev_iotlb()when it's called.
Check this call path:
domain_attach_iommu()->domain_update_iommu_cap()->domain_update_iotlb()->domain->has_iotlb_device = has_iotlb_device; The ugly fixup is to call domain_update_iommu_cap() or domain_update_iotlb() here again before return.
The similar issue is in intel_iommu_set_dev_pasid() and intel_nested_attach_dev().
> +
> +	return 0;
> +err_detach_iommu:
> +	domain_detach_iommu(dmar_domain, iommu);
> +err_free:
> +	kfree(dev_pasid);
> +	return ret;
> +}
> +
>   static void intel_nested_domain_free(struct iommu_domain *domain)
>   {
>   	kfree(to_dmar_domain(domain));
> @@ -128,6 +174,7 @@ static int intel_nested_cache_invalidate_user(struct iommu_domain *domain,
>   
>   static const struct iommu_domain_ops intel_nested_domain_ops = {
>   	.attach_dev		= intel_nested_attach_dev,
> +	.set_dev_pasid		= intel_nested_set_dev_pasid,
>   	.free			= intel_nested_domain_free,
>   	.cache_invalidate_user	= intel_nested_cache_invalidate_user,
>   };