Received-SPF: pass (google.com: domain of linux-kernel+bounces-59233-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <324a1c52-359c-44c4-9872-0ad712d9f610@oracle.com>
Date: Fri, 9 Feb 2024 10:36:24 +0000
Subject: Re: [PATCH] iommu/vt-d: Set SSADE when attaching to a parent with
 dirty tracking
Content-Language: en-US
From: Joao Martins <joao.m.martins@oracle.com>
To: Yi Liu <yi.l.liu@intel.com>
Cc: alex.williamson@redhat.com, robin.murphy@arm.com, eric.auger@redhat.com,
        nicolinc@nvidia.com, kvm@vger.kernel.org, chao.p.peng@linux.intel.com,
        yi.y.sun@linux.intel.com, iommu@lists.linux.dev,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        zhenzhong.duan@intel.com, jgg@nvidia.com, joro@8bytes.org,
        kevin.tian@intel.com, baolu.lu@linux.intel.com
References: <20240208091414.28133-1-yi.l.liu@intel.com>
 <a945cd6c-7737-4d66-a319-613e1dbc3798@oracle.com>
In-Reply-To: <a945cd6c-7737-4d66-a319-613e1dbc3798@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?S0h1aUx6TUx6a1o3SVJsOS9mY1h2OTAwdHV5LzAwZWxLNk9EOTV6OGNQbDNm?=
 =?utf-8?B?d3dSbk52TlpxV1AwcXVhTTZFazhmNE5aeTB3bUpRRmxrZlVmOWNaUUhQS0JJ?=
 =?utf-8?B?STBnT3hhS3ZqWnNLVmxIb0ZhL09yeUlJdDI5U01Jdjd4T2FoRkVxQ05PTXA3?=
 =?utf-8?B?eSs4T3FwYWcvVGJsWUV1dHVObU5uTkZpU0JKeU1JbmRIam9keHVnNTd5ZU53?=
 =?utf-8?B?YnZTMmp4L2RlV0RHV0Q5NnZtNCt4M3dVb1lOc1Zkc29NZ1BKVGtMY3dnVUFi?=
 =?utf-8?B?UVhVNlFKMTlDY2psbitPVjdrMDFwdUp1SHlINEdlKzNhYVFML1V5MTM4NVVk?=
 =?utf-8?B?azE0dks2d2Zwc2VHYk10VnRqSWVPODJhUXZUZTBaRTkvT0xOR2VrL3Y4TkZ2?=
 =?utf-8?B?bkw4aG1UVjlraVpzZXhTVndQNnpLOWhSWTU5UE1vczhJajNXa3FLOTBZSGVK?=
 =?utf-8?B?ajNtT1NoVEVQaURKMjFZNHk4RjlPc0FKRFBqME1Yem9Ua014aUdrTmd6bVNE?=
 =?utf-8?B?RElvbHV3Mm50ZEtBb3Yyd0NGLzBzalhxUXlpQ3F6ZWh5Y0FxWWlxNmdYQTgy?=
 =?utf-8?B?V2Y3OVBxOE04M3hTZUJWMU9mY0o0YllLQTFzWlFaSklNdU9jOWV6eS9nUlRH?=
 =?utf-8?B?ZEVmOTZQY0JOTmlaOEYvMklQNU5UVUgwL3A5Tmx0SUJ3WFhGM3BudjMwbmtM?=
 =?utf-8?B?UDk1QmZId05wL0RkSmRBYlQydUdtTjNsdXYvVUk1QWJRVUs0TmVYazBhK3cv?=
 =?utf-8?B?K3ZYK3NwdlJWdlIvTzhMSGU1VEw2NnJWOEJMVjJhODEvOWo1MFRjTE11NUx0?=
 =?utf-8?B?QnhrTjQ1b29Nc2NnTWdSL0dkMHAreGtPY29INHRjTVlZRVJnSENsaHdndGp1?=
 =?utf-8?B?VENXMWhNN2pTeml4Z0N0Q3FhZ2pyZnZkMTZaanAzYU9ybVBCdi95dWZ1ajNR?=
 =?utf-8?B?TFNSM1dtWU1iNmJ4VkdJeG93QlJNVVZWMFNlMU9sQmk0Nld0TkxKRDhiRE1H?=
 =?utf-8?B?ODlycUVJdHdXeFZqUGtPSERneDIvTkkrY1d0bWRNVWJOSlRHYXV2OWhXcUtG?=
 =?utf-8?B?a2UrT1ZRNjRYOVg0eXZRWThsb09YL1ByODBmTjlpOEx6WDZjbHZTdUZKVS8y?=
 =?utf-8?B?em5walBGbTAzblVCTnhXSHBFVGF2Ri9HRjhkRmVKcFN3UURwQUxKLzZVdzVB?=
 =?utf-8?B?bjZXY2hlUHRXaGp3c2hYanFKUjZHVHlpbGFjK2ZYRjJrWlV0U2FzeVR5Q05J?=
 =?utf-8?B?NHlyTDNVdmJ0elFNbWN6WGJQNmJDaEJLcVVWcEp3cEZtanVGWjQ5aDJPTjMv?=
 =?utf-8?B?TnVzclQ5SDU5MVYxM1lxQkVUVzRGdkZrUHIrM2QwWTI5R2tWdHowbnBOZTNK?=
 =?utf-8?B?ckozWEhRK1dqSWN5N0ZGTjU3bWxmNTB1SXFtVnErWUd5STBQQmErSE5XOHI4?=
 =?utf-8?B?M1lCVStqNE8yY3ZHdnNIOEIxbCt2Wloyd0pYaW56SWFtMDBUS2ZFU2tzTzBi?=
 =?utf-8?B?TmxNMGdLdkRYSDg1MkJQeVZGZkhWZU9vMnNnUTFsTUF4aUpCNkxjc0k0bDUw?=
 =?utf-8?B?a290T08vS3RmVCt4eGFOdkdqbk5EbTNKRVZPYmp3ZkZtMTJXNVF3QjZTcTVy?=
 =?utf-8?B?WUJUeVUrUkx3NnA5dFBUc3I0cU1NbDUyd1RnV2QvakZxM3ZhVVhNZWswSjk0?=
 =?utf-8?B?T2g2ZGVSNldtakJDLzh1YVZSN0NYUmlMSkk5R1BnTTlrR3Q0S1ZFTm9pbDZU?=
 =?utf-8?B?SzluSCtyOW1SLzVVSlI2UnQwWWdQR3F1d3hDek5SQkFPRTk4U3ZGNmlmZzdS?=
 =?utf-8?B?NWlOVnp6ay9BUmRIZ1lHUXI0Mmw5eVJLN1hKNEtzY2wrVHkvNFgva1ZETGsy?=
 =?utf-8?B?YWcxMnVZZGVEQWUwdmZkYUIwVC9tMDRMRzVHeFRuVGYvU0xnMEJiamtvVHVE?=
 =?utf-8?B?MDVVUmVWU0xCejJOS3g2b1luQ28rTEZrN3FKdC8wUzA5TWZweC9WWjNlaTJP?=
 =?utf-8?B?cWpqTzZNZEwwekVpZDdGR3krYWtMUVYyWVdOalVPYWtpWWF0cERlaWZwc3Rw?=
 =?utf-8?B?NGczOG9hMU43c3JUZWNOTmI2ejRKdjhaeDJFQTJFeWQ0ZE83MzVhM05vUXFC?=
 =?utf-8?B?aHJVVUptS1hHTk53YTA2WFlKNUV4WGFVZ0lBSTRlaFRtLytCV25oZjFkSjF4?=
 =?utf-8?B?REE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	Po+/+PO4MylqpFCKw6Ag/cu5h6d1IGUTjO0TP65uUwreH3EsSrQwXrUMtzwZKLs0HFVb48gtu+qEZlehN6hsXDhfZ4jjmRm1Ig+xPN9H+bVWib8PGFkO9B0J7W/0RJB33RSLrwCWSiS2nS+BqL86KHoQQN7j5Z0dyEvWlp9hGX9SEIuKeYGUKip+9ETJmUhBK2WAiBRS1Kn8lgkQfRq6o364nQ9hdDFBCOOoT5hybTsC/cCZ7y/yhsIP7MsbdN3SRl7NVRVKEfZuoLM40K+rALyMRXfcAS1EXIbdOPVLuBr0kj3aaJdQ7m010GEi5aV1kUgmEJuotBoLxAWPaeSiehNBnLVTIoaDYZuDdYpEQA4cZwFl5EU1sxLsf31umBp4pQzRiAKN6Gx5EfDMBEWfd4Z1h2tw2E5Vx5QPUHksIng6WAxRH4ns2rSXGA+FUKhVHe7hxc+XuxGs/U2TYPDURAt+GF9oDy2j5UOnR8H275v5ziAWu3/2qqV3d+Y0HKXzfI7NHo7qZouI7K/ib+8LgdfGzoeV3XNqBMPxf/bg9Wim+JDI2xTNq5Rq8hciMcxTdD3ytRQ/Zv+BhiVoHO6FNJPdYZo2OKby2Ee/h0UCn1c=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dd81f68f-0284-4fd2-eb7c-08dc295afaf8
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5893.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 10:36:31.6770
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xC5qc2FhL7U7N2ZyI38/yFWNQEClQfq9/xfiSQ73eIwpI3pZ3Wuaenhdy1nEOH61RgUd+4w7l2Um8jr1F6etdnUZpUjGYStxqzqr2j73XCU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR10MB7402
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-09_08,2024-02-08_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 bulkscore=0
 mlxlogscore=999 phishscore=0 spamscore=0 mlxscore=0 adultscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402090076
X-Proofpoint-ORIG-GUID: UVcMK95bZWIc8VQu4e1QK8P8edw3aTc8
X-Proofpoint-GUID: UVcMK95bZWIc8VQu4e1QK8P8edw3aTc8

On 08/02/2024 10:31, Joao Martins wrote:
> On 08/02/2024 09:14, Yi Liu wrote:
>> Should set the SSADE (Second Stage Access/Dirty bit Enable) bit of the
>> pasid entry when attaching a device to a nested domain if its parent
>> has already enabled dirty tracking.
>>
>> Fixes: 111bf85c68f6 ("iommu/vt-d: Add helper to setup pasid nested translation")
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> 
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
> 
On a second thought, while the patch looks fine if this is what we wanna do,
just a quick clarification on the why (that's also applicable to the last patch
of your other series[0]). I am sure I am missing something :)

Shouldn't the nested domain be subdued to whatever features guest idea of
ecap/cap instead of host's view? Or is this because guest first-stage page table
on Intel is supposed to be always-enabled dirty tracking (per SDM) ? If it's the
latter, it probably should be sprinkled in the commit message(s).

[0] https://lore.kernel.org/linux-iommu/20240208082307.15759-9-yi.l.liu@intel.com/

>> ---
>> base commit: 547ab8fc4cb04a1a6b34377dd8fad34cd2c8a8e3
>> ---
>>  drivers/iommu/intel/pasid.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>> index 3239cefa4c33..9be24bb762cf 100644
>> --- a/drivers/iommu/intel/pasid.c
>> +++ b/drivers/iommu/intel/pasid.c
>> @@ -658,6 +658,8 @@ int intel_pasid_setup_nested(struct intel_iommu *iommu, struct device *dev,
>>  	pasid_set_domain_id(pte, did);
>>  	pasid_set_address_width(pte, s2_domain->agaw);
>>  	pasid_set_page_snoop(pte, !!ecap_smpwc(iommu->ecap));
>> +	if (s2_domain->dirty_tracking)
>> +		pasid_set_ssade(pte);
>>  	pasid_set_translation_type(pte, PASID_ENTRY_PGTT_NESTED);
>>  	pasid_set_present(pte);
>>  	spin_unlock(&iommu->lock);
>