Received-SPF: pass (google.com: domain of linux-kernel+bounces-4428-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: <2f114c41-5dbb-4019-b0f1-046509521d44@amd.com>
Date: Mon, 18 Dec 2023 16:53:24 -0500
User-Agent: Mozilla Thunderbird
Cc: yazen.ghannam@amd.com, linux-kernel@vger.kernel.org,
 avadhut.naik@amd.com, tony.luck@intel.com, john.allen@amd.com,
 william.roche@oracle.com, muralidhara.mk@amd.com
Subject: Re: [PATCH v4 1/3] RAS: Introduce AMD Address Translation Library
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>, bp@alien8.de,
 linux-edac@vger.kernel.org
References: <20231218190406.27479-1-yazen.ghannam@amd.com>
 <20231218190406.27479-2-yazen.ghannam@amd.com>
 <42c6ed78-75bd-4b4b-8b59-e0562d4968aa@wanadoo.fr>
Content-Language: en-US
From: Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <42c6ed78-75bd-4b4b-8b59-e0562d4968aa@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?Q3ZLS2ZzNDVwbHN4TU10UTlCakNEYW5VdDdKN0JEczk2Y0V5dUZpRW9jRm5M?=
 =?utf-8?B?RXRIQzF6MWZWOHpBRTk5cnBmYWFLUVl6c0ZaazZqRE1yS1RzZUhFK0t5K3pL?=
 =?utf-8?B?T1o0cENxWjZjb20vUDVSWDdmczZzL2NrL1NJUFhCNFIrMkQreE1scGVmaVl4?=
 =?utf-8?B?UHB4eEV3K1JSM05meGNnQVEzbmNpQnFEVUtiTlQ0Mkg1TTQ1Mi9hNWwwNU1U?=
 =?utf-8?B?bVZSSzFzQ1B6MnhUa2tKZFRsV2VvK0cvL1JmQXVSYlh4akd1bmRBSXZQd3ps?=
 =?utf-8?B?enp2ZmpZckdxbE1PTHJRcENFelFIc2phaDhFT3ZHTnZxd29JMVlYb3B1eFBC?=
 =?utf-8?B?ano2UnZ5V0o3Ni9nN0ZDRi9acE9hSmUvMGcxWGVPUk1BN1ZIZFpKWXg2azgy?=
 =?utf-8?B?Wkh3WThpWVJ1akNqKys2TFBpV1NmOWJHMzNrRWkxRHVlY0krbnpub1dmTEZY?=
 =?utf-8?B?SG9qZHpEODFGdDF2S2FsVkxHM0ZYQzdWc1JJdHdabGlwbW1qcGtTZFF6ZkVp?=
 =?utf-8?B?QTY5aFZiNVppaW0yK2tPUlpndmVBQ2dod0ZHZU1Cckxqd2xZeXJFMG4vQWdJ?=
 =?utf-8?B?bHRWTFhQZFErK1VTbmk0ZmFQWVJObDZUWTZyT0VFT2tzYUNldzB5aVBKZWVZ?=
 =?utf-8?B?Um1qd0pxbmJRU3VaMisvMEcyNXNvTVZ6bGdXUjUyejZSaHhtUjZmVUlvb3ZN?=
 =?utf-8?B?U2VvVGROeHdXOGpRakJUZ1ZoR2w1bDhlYWZkNzhhQ1JneGt2YTFTazZCdVEy?=
 =?utf-8?B?Y3FwekoxRnBPaGVxemhBS1dObWFvV2F4Y3hDamkyWUd0VnFYNlkzbGIvUktz?=
 =?utf-8?B?aElSUkFZeForTUdzSWdtdzc4alF5MUR3ZGdZaEV3cXRlS0pyc1ViamlZcmVQ?=
 =?utf-8?B?YktqbW9mbVBlYjRKNGpPZFN5Qm1wZjVXMlhkUlU2Mm0rTmtyYXQ2R3hrcDNP?=
 =?utf-8?B?NTkrVHFjcGlGcEJFTzF6QmpoelBzRFZwTW1icGlSNXBTVUJjTGJDMmMvWHZL?=
 =?utf-8?B?cnVlaURxeDg5bk9LRWhGY3NNMnJqVDFVRUFsTHZGQlBNVWJFR0x3d3dsZEow?=
 =?utf-8?B?bFpjbk9GM25LOFNFSXZOcFc4RExFSWhGOUFHMkZtTFRMV1R2a2dZVnF6T2Zo?=
 =?utf-8?B?TkkwcTB5TmMva0c2cjI0UzdTVFZVVzA0QmtjOU1jaTNiMEkxNGpXT3RiVG44?=
 =?utf-8?B?MzRRbXdHZTA3T29NQVowVGFtK3JPSlI5YjRjNlRzSmRjNWgzUmZucmRWQXhU?=
 =?utf-8?B?K1JvbjNoc2MrbmcrWlkyNVBzSkpHUTkveEgzLzhOUUNNd2ZscGdOMDAzRkJZ?=
 =?utf-8?B?NjZ0MS85UmNlRG00Z2IvVXZGejI5U1I4dkQrN3E5L09obnZDeGtJTlV5eTA2?=
 =?utf-8?B?RjlPS3F6VnVaWlJSYmdqWlFGSkM2ZFZnQ2VOSnZBMVJMNkxEclBUOEUxK0p2?=
 =?utf-8?B?dlhvam1ueFp5ekdhbWZzMGVHRXJOZEkzd3hhRW1GTUJubGZnbjdDZlBMSS9j?=
 =?utf-8?B?OVpvRVlZOHg2NEh0Y2hrUm41RW52eTY3LzRqK0htNnVsTTFINzU4cS9BcEhq?=
 =?utf-8?B?U0VzQStnUmlXTXdTbmJqSVY3clgrREhiazlyaTlISWpVQ0M0d0lIdEh0WVRS?=
 =?utf-8?B?RzFRTWlCdVA1M1RMNFlOLzc3MEJYU20vRWpXSTk5RU94STY4amQ1MEoxTi92?=
 =?utf-8?B?RmdTMEtZQ251dm1kb3d2MkJZc1NvVWwvM0RXL3QzQmtyeHlaaDFaa3VCaVZ3?=
 =?utf-8?B?OTdaMURtaE9ZUTRKck5DRzlYSnFyeG50SkJUcDluTVo5cU41cy9GbExUNXdG?=
 =?utf-8?B?dXZmMHczbGppL1JLMGZLM1FxYkxUanVnQy9BSTBYUHhlb1RwWG5EbEJVNlNp?=
 =?utf-8?B?c0xETFBqZU1qcThJSS85ZEptMHUrMXF4cUtHRm0zTTcyQUZNMHBDY0tXWHZV?=
 =?utf-8?B?Q0dDMnNFNEhGTmM4cm44L2IwZHBDRlR3OEM4VHdhSlpNQWxoMFAzZjN2cFZY?=
 =?utf-8?B?dC9KejFsRUNnWVVjMi9nTngyRU9nRUZVWGNzRGRGTHJGZHBrcFVhZFZDM2U4?=
 =?utf-8?B?aUN6MXdGMzBOdTlVbUs4enFEU1g0YjVxd2pGV3NyU0I0TEZSckswVy82aFRC?=
 =?utf-8?Q?HpIvp9Ob6svd0ripd4M7iKhUa?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d9fcb442-e69a-43a6-2be8-08dc0013c3be
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Dec 2023 21:53:26.9909
 (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: KTf6AGsRBbWIYzB5UneXdIPXXwuQPiKBzmrrNp661WsyarLs/bWV/v/ZD7lqstGRzpUEqIvYrXkXn/Js4L/Rzw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR12MB7942

On 12/18/2023 2:21 PM, Christophe JAILLET wrote:
> Le 18/12/2023 à 20:04, Yazen Ghannam a écrit :
>> AMD Zen-based systems report memory errors through Machine Check banks
>> representing Unified Memory Controllers (UMCs). The address value
>> reported for DRAM ECC errors is a "normalized address" that is relative
>> to the UMC. This normalized address must be converted to a system
>> physical address to be usable by the OS.
>>
>> Support for this address translation was introduced to the MCA subsystem
>> with Zen1 systems. The code was later moved to the AMD64 EDAC module,
>> since this was the only user of the code at the time.
>>
>> However, there are uses for this translation outside of EDAC. The system
>> physical address can be used in MCA for preemptive page offlining as done
>> in some MCA notifier functions. Also, this translation is needed as the
>> basis of similar functionality needed for some CXL configurations on AMD
>> systems.
>>
>> Introduce a common address translation library that can be used for
>> multiple subsystems including MCA, EDAC, and CXL.
>>
>> Include support for UMC normalized to system physical address
>> translation for current CPU systems.
>>
>> The Data Fabric Indirect register access offsets and one of the register
>> fields were changed. Default to the current offsets and register field
>> definition. And fallback to the older values if running on a "legacy"
>> system.
>>
>> Provide built-in code to facilitate the loading and unloading of the
>> library module without affecting other modules or built-in code.
>>
>> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
>> ---
> 
> ...
> 
>> +int get_address_map(struct addr_ctx *ctx)
>> +{
>> +    int ret = 0;
> 
> Nit: unneeded init
> 
>> +
>> +    ret = get_address_map_common(ctx);
>> +    if (ret)
>> +        goto out;
>> +
>> +    ret = get_global_map_data(ctx);
>> +    if (ret)
>> +        goto out;
>> +
>> +    dump_address_map(&ctx->map);
>> +
>> +out:
>> +    return ret;
>> +}
>> diff --git a/drivers/ras/amd/atl/reg_fields.h 
>> b/drivers/ras/amd/atl/reg_fields.h
>> new file mode 100644
>> index 000000000000..6aaa5093f42c
>> --- /dev/null
>> +++ b/drivers/ras/amd/atl/reg_fields.h
>> @@ -0,0 +1,603 @@
> 
> ...
> 
>> +static void get_num_maps(void)
>> +{
>> +    switch (df_cfg.rev) {
>> +    case DF2:
>> +    case DF3:
>> +    case DF3p5:
>> +        df_cfg.num_coh_st_maps    = 2;
>> +        break;
>> +    case DF4:
>> +        df_cfg.num_coh_st_maps    = 4;
>> +        break;
> 
> If 4 is the correct value in both cases, DF4 and DF4p5 cases could be 
> merged.
> 
> CJ
> 
>> +    case DF4p5:
>> +        df_cfg.num_coh_st_maps    = 4;
>> +        break;
>> +    default:
>> +        atl_debug_on_bad_df_rev();
>> +    }
>> +}
> 
> ...
> 

Yep, good points. Thanks for your feedback!

-Yazen