Received-SPF: pass (google.com: domain of linux-kernel+bounces-17884-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: <8ecc6de8-825d-4518-af75-6ba590a89024@amd.com>
Date: Fri, 5 Jan 2024 20:39:28 +0700
User-Agent: Mozilla Thunderbird
From: "Suthikulpanit, Suravee" <suravee.suthikulpanit@amd.com>
Subject: Re: [RFC PATCH 2/6] iommu/amd: Add support for hw_info for iommu
 capability query
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: linux-kernel@vger.kernel.org, iommu@lists.linux.dev, joro@8bytes.org,
 yi.l.liu@intel.com, kevin.tian@intel.com, nicolinc@nvidia.com,
 eric.auger@redhat.com, vasant.hegde@amd.com, jon.grimm@amd.com,
 santosh.shukla@amd.com, Dhaval.Giani@amd.com, pandoh@google.com,
 loganodell@google.com
References: <20231212160139.174229-1-suravee.suthikulpanit@amd.com>
 <20231212160139.174229-3-suravee.suthikulpanit@amd.com>
 <20231213132721.GT3014157@nvidia.com>
Content-Language: en-US
In-Reply-To: <20231213132721.GT3014157@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
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?blZVTzdyTzVjcytNY3p2eWlQK1lGa2hKWmE5UWFYalF6WVZnZ2pMMWxka1ho?=
 =?utf-8?B?VjlaWEw3YnUvSzFIeStHZ0RnYURDWGR3SzdrQ0Rybi9tWnl4bmMwNjhSS3Nz?=
 =?utf-8?B?ZTFBMTNCNG9ZTXAyRDZHR0NSQWJHdVhCZGVKR0ZUTm9HTGN0TG96bTBVRHk5?=
 =?utf-8?B?SHozenhNRSs0eVl1Uk9MVHdocXlMVmtLK05rNThvdkNjaXkyZERnOWk1eXhx?=
 =?utf-8?B?SGwyYm5YVTBhNlpGcHl4MzdCcmxlQTdmMFZnWkN6L2dRRWUwUTRvZ1JxN2F2?=
 =?utf-8?B?U0JoRWloZVNZNmxpOGdBck9uVG9KLzdFc040cnppbis5Wld2Y0Y5ZS94bkVq?=
 =?utf-8?B?VUs0dy9CaEdsTk5hWTFrdExIRTNUTzhDZWdDRWZVdnpyT0RZZ3dPeCs0aE4y?=
 =?utf-8?B?VnplbUcwenpPSUlGLzU4TkN4Mjh3c2RxNTlIMVpUQzJPL1M3WWtCMFlsbStT?=
 =?utf-8?B?b0Vza1FyQ3JONnQ3QlJWV3VwcHh5WUZKQWhITTA4WkN5TWlONmNuQW52NHN4?=
 =?utf-8?B?N1R6UWNoNC85eXlLSHptOGk0SUZ6K2o0clRaVDViUkp4cEtzRi80ajZCaFZR?=
 =?utf-8?B?ZngxUnpXRWkrVkdDWGc1VExaTXV6VFliZmhKK2ZmN210RlkxRXVHbk54Y1Nn?=
 =?utf-8?B?VG5tSW9JY3NIK21sYjRjYS9aNElqN3VHcld3d25zVkFxZVVuLzRJNW91S2tZ?=
 =?utf-8?B?REVZUlBRSTN0VGJFUE44QWRJbmFTanA1MWhzMzVFbDVpcE9IQWNKRWxpdktm?=
 =?utf-8?B?Q2Joem4xS0hFa1NmbmpuQTRpRnRqZ1hpSFY3Uy9IblVMVHg1UnE5OE52WmRY?=
 =?utf-8?B?OVlmTk5QQmZhT1hkdzR5WTNHa0pjVzkxd25PNk53RW5ObUUvZnpLZWhhcGZO?=
 =?utf-8?B?MHltd0VkYTdHenVSUnh5YU5WZHRNbnZnNXYwajFLanBnVVI2WHprempwUDR6?=
 =?utf-8?B?VXJ2OVhUMy83dFRXbmNUVkVSd2ZIU2JxaWRjQm15d0VTNlFJSWNoTUV0RFFp?=
 =?utf-8?B?QWZ5K0lmWHl2VlA0a2I1QTZjUVJ2LzBrdmdSYTl0V0k2MFNJWG1YZEVsVklK?=
 =?utf-8?B?bkZlMEN2RndjclpqUlN5bkQySlBZd05VSG96YWVhcW5HS0FYOWloMlBqSWpN?=
 =?utf-8?B?M2FpSlEvek9PUkFNSERWemVROVpYdTBqdm9ld21aQWplejlJNFVMM0crTmwr?=
 =?utf-8?B?c3NvaldEZURYMWxYaFFTRnd6MUJ2NXU5MTdvNlB5VFlnTHlld1dudFFDbUZL?=
 =?utf-8?B?WDZabVE5SHUzVm9mSmR1L2hWbWVaQ21PZVNGRlV6WHY0U09LNTJqTTJpRkNC?=
 =?utf-8?B?WG54amdybEcrWU5hd2h5VXhiUi9OaDRrMUdIK1ZrRlRqa2g3QjJrajc3OG4x?=
 =?utf-8?B?a01wdFIzNEs5bHlXVkU3aFh5RzQwWTVHUy9nK2gwSGcvcUVJRDBXYVpsWGVC?=
 =?utf-8?B?SGRwN2hheU5OSmpUVHI4dGRXSDRjWkI1K3JWVC80SXRQak83eXBNWVh3V0hK?=
 =?utf-8?B?ajlWd0Q4WDNWR25FN0MrR0swVUg0MGc4OWNQTUlJYVluU2lkMHB1L1JaTWd6?=
 =?utf-8?B?UklIV0hJYnVyY1dKSUhaWVV3Uk5ObkZtRWVSRzRIZU9HeXZZN1cxZG1LV3E5?=
 =?utf-8?B?SjQrK2hTRjg1bEZ3a1A4Y0tTRk5yTm9SNklDTS9lQzJheDVETzRta0tVRmRx?=
 =?utf-8?B?OGRtSVJUVCtyS3Q5cFBGNGtxNDRMNmtuVjRsTEttMEVpZTQrNnZuM0lJRVVX?=
 =?utf-8?B?MTI5OUxDU0RjU0IwcjVtMlVxQkZUZkFCL1JlWG5aeE96WHIxbFFmZi9hb3kr?=
 =?utf-8?B?SXZaMkNucGJiWUpSUHNES1o0bVlrRUtyaGRzVnFSZzFSQkZXNkxVWjgrOVlX?=
 =?utf-8?B?aHBKWkw2Y1lxcGJsN0o2amIva1hMRW91QldMWGdGczFRU2twMGpCVXdFTUpV?=
 =?utf-8?B?ZkpvRVlIYXpxZVBNcndJMmZ1MUNSSXp5alRIdldKUHR3eDQ2ZGMyR09zOWtD?=
 =?utf-8?B?R2dRL01FNTRBRUpoNVFYM1VaTmVETmpJNHd3clBGaWR3cEdyWWFJakNpM1Rq?=
 =?utf-8?B?Wk1xcHhSZGhsL2x4WSt1TmtwQmlLbSswY2tkMVpSSnJsck92SEkvdTczRlps?=
 =?utf-8?Q?RF+NKHdoibd2a9auCUEb7l3UA?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e011a95b-d40f-4a3e-60b4-08dc0df3bfbf
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5445.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jan 2024 13:39:32.8176
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: VXLtZcz5Yo0Hsq5UhkyxYd0iMCJuKocDGArMJ2UsU3JOcRnDKrpsuOhWLxdz93KBbVmYT/HkVRnu+0l49L98Ew==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6233

Hi Jason

On 12/13/2023 8:27 PM, Jason Gunthorpe wrote:
> On Tue, Dec 12, 2023 at 10:01:35AM -0600, Suravee Suthikulpanit wrote:
>> AMD IOMMU Extended Feature (EFR) and Extended Feature 2 (EFR2) registers
>>
>> ...
>>
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 4e4ff1550cf3..c41932e9f16a 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2822,8 +2822,46 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
>>   	.read_and_clear_dirty = amd_iommu_read_and_clear_dirty,
>>   };
>>   
>> +void amd_iommu_build_efr(u64 *efr, u64 *efr2)
>> +{
>> +	if (efr) {
>> +		*efr = (FEATURE_GT | FEATURE_GIOSUP | FEATURE_PPR);
>> +
>> +		/* 5-level v2 page table support */
>> +		*efr |= ((FEATURE_GATS_5LEVEL << FEATURE_GATS_SHIFT) &
>> +			 FEATURE_GATS_MASK);
>> +
>> +		/* 3-level GCR3 table support */
>> +		*efr |= ((FEATURE_GLX_3LEVEL << FEATURE_GLX_SHIFT) &
>> +			 FEATURE_GLX_MASK);
>> +
>> +		/* 16-bit PASMAX support */
>> +		*efr |= ((FEATURE_PASMAX_16 << FEATURE_PASMAX_SHIFT) &
>> +			 FEATURE_PASMAX_MASK);
>> +	}
>> +
>> +	if (efr2)
>> +		*efr2 = 0;
> 
> Why are you checking for null here? It is never called with null

In subsequent patches of the part 2, this helper function will be used 
to help populate the EFR and/or EFR2 in different call paths, which can 
pass only efr or efr2 parameter individually.

>> +/**
>> + * struct iommu_hw_info_amd - AMD IOMMU device info
>> + *
>> + * @efr : Value of AMD IOMMU Extended Feature Register (EFR)
>> + * @efr2: Value of AMD IOMMU Extended Feature 2 Register (EFR2)
> 
> Please reference a section in the spec for what these are just for
> clarity

Sure

>> + */
>> +struct iommu_hw_info_amd {
>> +	__u64 efr;
>> +	__u64 efr2;
>> +};
> 
> __aligned_u64

Okey.

Thanks,
Suravee