linux-kernel - Re: [RFC PATCH 2/6] iommu/amd: Add support for hw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8ecc6de8-825d-4518-af75-6ba590a89024@amd.com>
Date: Fri, 5 Jan 2024 20:39:28 +0700
From: "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev, joro@...tes.org,
 yi.l.liu@...el.com, kevin.tian@...el.com, nicolinc@...dia.com,
 eric.auger@...hat.com, vasant.hegde@....com, jon.grimm@....com,
 santosh.shukla@....com, Dhaval.Giani@....com, pandoh@...gle.com,
 loganodell@...gle.com
Subject: Re: [RFC PATCH 2/6] iommu/amd: Add support for hw_info for iommu
 capability query

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