Received-SPF: pass (google.com: domain of linux-kernel+bounces-48905-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <20e550af-23b7-4583-af91-0bba4c9529f1@amd.com>
Date: Thu, 1 Feb 2024 16:59:06 -0500
User-Agent: Mozilla Thunderbird
Cc: yazen.ghannam@amd.com, tony.luck@intel.com, linux-edac@vger.kernel.org,
 linux-kernel@vger.kernel.org, avadhut.naik@amd.com, john.allen@amd.com,
 muralidhara.mk@amd.com
Subject: Re: [PATCH] RAS/AMD/ATL: Add MI300 DRAM to Normalized address
 translation support
To: Borislav Petkov <bp@alien8.de>
References: <20240131165732.88297-1-yazen.ghannam@amd.com>
 <20240201092148.GBZbtirOfNAR3SYmht@fat_crate.local>
 <132e22b9-2e42-4d8d-a2ea-14023bd5dae1@amd.com>
 <20240201150543.GHZbuzR4_KJHbCTI1H@fat_crate.local>
Content-Language: en-US
From: Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <20240201150543.GHZbuzR4_KJHbCTI1H@fat_crate.local>
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?eEdraSt4anRod3dQVGw4eDJPdXhHdDNSMmd3Rnc5VzFZVnNXYnVoN2tWU1NY?=
 =?utf-8?B?N3MvVkFuZTBubmxlL2lRa0NRN2YrY2lzNmltMk9qcmhmR1I4aStsOU8wWFdm?=
 =?utf-8?B?Vjh6RUNUWWtCdXBzeEFPU29RR2lmVkd1RFN5ZlVnRWFqZDdsVDJqeEl0Q0wx?=
 =?utf-8?B?SHM5aVd6WWVCRncyYjlkUkVLYUxIMkx1RkV0SGJRaURKUkZmdmEyTzJGcFZS?=
 =?utf-8?B?QnJOUGdSaTdYRlM0OTJHQlJGcjFXUlRSQjNIV0M0OXJJQzUzUytEZEhXR0dZ?=
 =?utf-8?B?bWgwY0RBbmd6aVY1UWtGTFlkc1ZLSmFadGJCa1cveDF5NmVQcHlLMEV2azRk?=
 =?utf-8?B?MFhxYnhZNDFIZkFSWnllU2ZGN0sxZmpBeCtRSVVZODAvWU03aDRDT2JVVWlD?=
 =?utf-8?B?SWNLY3pNbDVjTEVtWTVtTmJSS1o5bys3dnNxbWk5aVpYVGwzQmN3djY5cTJW?=
 =?utf-8?B?d2cvTTlqU3owNWRKYmM0QnRQY2V3MHc4WndtdFFHSDh6aURTcy9CNHdoOEZp?=
 =?utf-8?B?SHJ1N011UXFtQStoUFE2THA5QWhKdVB5Q2R3bGpYbzM1UXQ5V3c3MkNRV1Fu?=
 =?utf-8?B?WEpCMCtmdkFsOVVWb00rc0JNcWNKUTI0OC9MUUFWS2VEYjNoRmlhbFREVith?=
 =?utf-8?B?T3QzbFF4VjZocktuYUlLQWRjVjdUV29ka3o2TmdlWnNPY2FiYW9tY2U4OXhI?=
 =?utf-8?B?NVlQYXl3dEhJNHVhdVpuN2sxdURRaUx4OTdNQ3dCVldlV1h5SWFsemxjM29E?=
 =?utf-8?B?ZHNQV09uK21YbFBkSkh0QlpLWEkzeGdxNGpPZ1VvZ09xYnY4VVplcWU0V3pl?=
 =?utf-8?B?QVhELzhEczlaUWdiUXcrSFZBaXpEOURRTWY1MXRINndySjRmWld6T0h6VExa?=
 =?utf-8?B?NXZQb3ZxT1lML0NZMlpwU1dzcjhEVGhoZUIyek45VUdRS3ozYTAydlRnUGRQ?=
 =?utf-8?B?alFKam44dEJEZHdyZlo0OU9GakgxY0EzRllGMVB0SnVDdUxVRThKcGJlVXBB?=
 =?utf-8?B?MHozN01PcXR0RVVZcUFES1JsY2pKMmxjZFU0bGhIWHAycDh6SDdCYUNsM2VX?=
 =?utf-8?B?dVRHai91aDBJWGlsK1lDUHJGcDZwQVBrMUliczd4R3JOMmZzaHovMFoxQ1Nv?=
 =?utf-8?B?QzBxREFGcm9pYWo3Vnh6U3Y2VjdvSzdZbE01NnFKdzQreW5BdjFMR3FnZEVl?=
 =?utf-8?B?WGV4T3lzc3JoY1dmRHlFajNYZWJRWWsrMVM4TVpnYlVtK1ZvT2I4Q014SGNN?=
 =?utf-8?B?VXRJS0ZuUjdNU1VPVHlYL3pvTHVDMGNOUjc5VmkzcjAxaW12Z08rUE1uMHU2?=
 =?utf-8?B?c2tERXI4NlFKYnpUNmcrR3N5WTVLT0pIQ284djJxTHpLWFdaV1YrRy9ZKzN5?=
 =?utf-8?B?Y05PODZDVnhDS1FtcDUrcXN0a3E5Q1dVcDIwSjJsWGxvaG0wNGsxTW9uaG5t?=
 =?utf-8?B?cGZOUUUyWENvdVFnODYrYUZ0TTM2c2xPOTBmakdiRW5mSWgwaVpwL3NoSVZl?=
 =?utf-8?B?bmxYRGQ5M3h4YU5YOTM0eTFPZzhpUWZTOC9hNVJUZHZMSE0zNzNEZjVBVG0w?=
 =?utf-8?B?cThseWM1Sld3dk81V080ZGE5WHViQ0J3SXFIbXV2UU9tb3NlMVNXK1BQWmwy?=
 =?utf-8?B?U3NyWDN4bTVPNHlGWEkyMjRtSGtYZmVBM2ZESWk4cGJGalRrRGZLS2hlYzBr?=
 =?utf-8?B?UkxuK21EU3dLSXMwMzhZeTFuL2xrUXFlK0U3MEt6ZEJLcGk1QlhBb0hLTHU3?=
 =?utf-8?B?cTRNTFY4b0JKQmVsUkJveWJOd1JPV0FGSGhSVDMyaExJME1oZG53RGxBUXJR?=
 =?utf-8?B?UTd2MmZCdE5vOTJNUTlNWkVYNXBEUjFUeHkrdTFRTmJCS244RldNOERuVWs2?=
 =?utf-8?B?aEdpckh4b24vK09GQ2cvSjl3KzJCeHZNZzFYWUlhbGxKQ2tXcTU1S3JWN2wy?=
 =?utf-8?B?NUt5dS9IYXR0VitpZWJmMWZ0Qnljd2twM0JYRTFLenN2eGhLa29kVjMva3dP?=
 =?utf-8?B?cGM0dTU0NnQzWXMweURjVXprcTI3RFBldUU0cFFDNVA2WHdjVllkN2QxT1F5?=
 =?utf-8?B?eERBVVczMmFtcUFZcHZmMkVsZmtTOUN3L1VaZkJqVWZMVXJqbmNrV1RzQ1Bw?=
 =?utf-8?Q?mlRBi80A0B5bkWjUrLkzRza9y?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e0a24c2d-c1c7-43d0-6d54-08dc237103ee
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Feb 2024 21:59:08.5383
 (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: DoFqIqDhjmQ6mdAzysrIRfU5imW2iZ/+F/1db4hZj/R8mvDfXZFc7DTc1vsXVVIBy/N6crp50T1jQRyBav+kGQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7491

On 2/1/2024 10:05 AM, Borislav Petkov wrote:
> On Thu, Feb 01, 2024 at 09:35:13AM -0500, Yazen Ghannam wrote:
>> It's an operation on the bits within a value rather than between two values.
>>
>> BTW, I looked up "internal" in a thesaurus, and nothing seemed much better to me.
>>
>> Maybe something like "xor_bits_in_value()"? This has the verb-first style too.
> 
> Ah, ok, easy:
> 
> ---
> diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
> index 67dc186a1226..7e310d1dfcfc 100644
> --- a/drivers/ras/amd/atl/umc.c
> +++ b/drivers/ras/amd/atl/umc.c
> @@ -49,7 +49,8 @@ static u8 get_coh_st_inst_id_mi300(struct atl_err *err)
>   	return i;
>   }
>   
> -static u16 internal_bitwise_xor(u16 val)
> +/* XOR the bits in @val. */
> +static u16 bitwise_xor_bits(u16 val)
>   {
>   	u16 tmp = 0;
>   	u8 i;
> @@ -181,8 +182,8 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
>   		if (!addr_hash.bank[i].xor_enable)
>   			continue;
>   
> -		temp  = internal_bitwise_xor(col & addr_hash.bank[i].col_xor);
> -		temp ^= internal_bitwise_xor(row & addr_hash.bank[i].row_xor);
> +		temp  = bitwise_xor_bits(col & addr_hash.bank[i].col_xor);
> +		temp ^= bitwise_xor_bits(row & addr_hash.bank[i].row_xor);
>   		bank ^= temp << i;
>   	}
>   
> @@ -191,9 +192,9 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
>   		/* Bits SID[1:0] act as Bank[6:5] for PC hash, so apply them here. */
>   		bank |= sid << 5;
>   
> -		temp  = internal_bitwise_xor(col  & addr_hash.pc.col_xor);
> -		temp ^= internal_bitwise_xor(row  & addr_hash.pc.row_xor);
> -		temp ^= internal_bitwise_xor(bank & addr_hash.bank_xor);
> +		temp  = bitwise_xor_bits(col  & addr_hash.pc.col_xor);
> +		temp ^= bitwise_xor_bits(row  & addr_hash.pc.row_xor);
> +		temp ^= bitwise_xor_bits(bank & addr_hash.bank_xor);
>   		pc   ^= temp;
>   
>   		/* Drop SID bits for the sake of debug printing later. */
> 
> 

Yep, easy :) Looks good to me.

Thanks,
Yazen