Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <894b3737-1a0a-4139-9c73-686a95481795@amd.com>
Date:   Mon, 7 Aug 2023 22:17:33 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] platform/x86/amd: Introduce AMD Address Translation
 Library
To:     Yazen Ghannam <yazen.ghannam@amd.com>, bp@alien8.de,
        linux-edac@vger.kernel.org, hdegoede@redhat.com,
        markgross@kernel.org, platform-driver-x86@vger.kernel.org,
        "Luck, Tony" <tony.luck@intel.com>
Cc:     linux-kernel@vger.kernel.org, avadhut.naik@amd.com
References: <20230802185504.606855-1-yazen.ghannam@amd.com>
 <20230802185504.606855-2-yazen.ghannam@amd.com>
 <58934edf-4fad-48e0-bc5d-62712b11e607@amd.com>
Content-Language: en-US
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <58934edf-4fad-48e0-bc5d-62712b11e607@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OHpLdmdUY3dHNkR6SWlqWVVyR2g1SXI0cUJPaEF2U01LazJRK3Uvd0lvOUlI?=
 =?utf-8?B?M1pkbHVoNGZsbkNSUXlsUVhmQ2Vxc3JXMENOLzVweWtoMW95UzBJRHl0MnRE?=
 =?utf-8?B?Ujl6aEhZOEk4RTdUMjdTUzFCME1CdGNQTGthaUNnYUUvTklVdk1tZWZSVVhI?=
 =?utf-8?B?ZDhUZHVTUEM2QTF0R0RGTU1KTEJKNlBvbHJWM0dla2E1TXhiS2tIbzRIb1Mw?=
 =?utf-8?B?SVhvRVhwOEFnTXQydEs4TGYxaitNNkVRSHpsN3MrazhPd0JtRElBMU9uRHhJ?=
 =?utf-8?B?cnFCbEViRXVtRjRrZkZsWE9FL0x2bUVKbE1mVmVzRktTMzVjd2pxSmJldkFF?=
 =?utf-8?B?QlY0dGxpejVMSExnclBJaXM1MGhSYWJWSUkwTVpiSXhiKzlWQnZhU3FqWWE2?=
 =?utf-8?B?SE9QcUV1aVJ2WWJzcU1ISDExMURLeklWSnorZEhTZ0JRUjJhcVBiakgrVUdx?=
 =?utf-8?B?a3IzVUZTSDk3Zk92RUVyaTRvak9INHRNU3hUOS9EWTVYSjJFWDhyaVg0R0o1?=
 =?utf-8?B?REcwaTJpY2kyaHhENGRvMEdPc0NmbStBLzNyM2JpbWhCY0pEdUpVMnlVdzhP?=
 =?utf-8?B?Wk1tYTFLZGlaeDRTTkpobVAxNHF6MldBK2pUeTAvVEpqL1M1Tkt0TkNhNlZB?=
 =?utf-8?B?dHlwU1gyQU83RndQZEp4S2NxeVFwY2FUZ1NBdWJvQXdOWjV5UDN3dUp2M2tW?=
 =?utf-8?B?V3NwZDVTVDgzWldzZXZUUVp3ZWtqcGNDdEgyeFBoLzkvRnY1aDNWY1J0STMv?=
 =?utf-8?B?OU9DemREdkZRK0xvL1dMWi8rNEp2czFKR0U5QlR5VU1FNFNQVk5DaCtnaDZi?=
 =?utf-8?B?ZDY5MStuYUQwV2tCRnJJa2lBWXZFT0xkOGxPL1F0RU1RSmI0S0V1OThocWtl?=
 =?utf-8?B?L0tweEVXTHVvbTBqL3N3N0xCR2owUkdWbFc5ZWxEZkU0ZkEwQUVxZUg4bkIy?=
 =?utf-8?B?NG5yUmpwVnFQOW9ES2oyUCs4SWpsMFJ6UGtYTjVwTWlDenNsMG0vclVXUUtx?=
 =?utf-8?B?dytDU1BvbHB0dzg4dXhxUkRUdlRTLzZ6Y3pLb2hKVWVlQVpUdUVHSkltSkh3?=
 =?utf-8?B?VHU0aFcrTjR6QVMrUnNPTUc4U3ZuR0JHL1RGakxDQWkxNDk4dFNBZjMrRk5y?=
 =?utf-8?B?bmFFT3pqSXlBakd5OWxtUnE3eU9sQVJIU0ZuSTBBRHVlVy81Zyt6RVRLZ1ln?=
 =?utf-8?B?bzgvd2VDaEZLNmpUNTIyWWNNTkljcG01UXptQmFGR0hRd09HWXo5RXR1YmE4?=
 =?utf-8?B?RTYwU1gwZHJranFmekdKWnpBNmFRQk84VWFFcGJ6YjQyTFpBbWMvejI3cXRM?=
 =?utf-8?B?ZnY3SjJyUzFJNW5TbU8xWXRBREJIU0VRLy9UM2pCRWJnVUM2WG9uaGxZQk1K?=
 =?utf-8?B?TEFCaE1QbDA3TDFYV3VxTkNvdTJkUXNFcFdsYW8zZVUwN2U4SUlHbHNSbjFx?=
 =?utf-8?B?NzRWbkVmM2RVOWc2YW9LcDI0am1DUWsrNFZubDRTTHJsMG0wTjE3ams4U0xx?=
 =?utf-8?B?S0pNNmNUU28rdXp1R3lsMU1vZ0lkcHpDK3c0aFBZQ0NRWjZ5dEVCbTJkQjMx?=
 =?utf-8?B?L3NYZGJ5OVlZZ0UrMjZMYk51TUZTQXF4YjU2YjRyUWVIRXhtU0NlMGtqemQw?=
 =?utf-8?B?SVB3MzUwcnBOYUx6QkNoOUtaZEdQdnl6SDFWTTFKdE5YWmxwUlRNLzFFOE5m?=
 =?utf-8?B?YUttdFpXL015UDJTL0NkSDJMdTBkS3dZaWIxcWxmZVl5YkFOWThneHlYeDZ5?=
 =?utf-8?B?aFdVVTlPYkU0MmtBQzQ3WEhac1BncXY3SnRITE5TeVNrcmV0dFR5bFNsZGt2?=
 =?utf-8?B?aHZMQVhKVkplS0NrcXd5eDZqcWhMVnpSSFhqaWQzSkEwcy9hQXhBcXR0TUlS?=
 =?utf-8?B?dllDM1RuODllT1dIQnoreFBOdVFKbVFoMnVQSWVKaHg2NWxqZXRVTkxwbVdL?=
 =?utf-8?B?U0JwNWJQbmdrc0ZQZHZmQklQYlpwVHR5WE1BbFlEMUZFL2hVZkYwdy9OUkVs?=
 =?utf-8?B?Tlh0L0ZNV0ZqcmVsYXZvNUZta1FiaTZCbXUwUTRSM2JQZXhPeU5ZNXA0dlRz?=
 =?utf-8?B?MXZQcEUwaFJ1RFZSQ0Y1SGV1OHJZZGdzS3Q0djZwc3haRlRCRTlBNDNjSlY5?=
 =?utf-8?Q?SdRsjIhL+Lt/IoMmcqkalV2ho?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 692cda97-9f38-492f-8885-08db97be016b
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2023 03:17:32.8628
 (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: J9Cigs4b5HWbyPkErfMgmLcPt1wycp0MvALLyW8rmTPdoZceuPj9pwzO+hu2EtGzpvrnG1T3p7JCNqrR+9Ez+g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB5013
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 8/7/2023 3:44 PM, Yazen Ghannam wrote:
> On 8/2/2023 2:55 PM, Yazen Ghannam wrote:
>> 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.
>>
>> Future development to include:
>> - DF4.5 Non-power-of-2 interleaving modes.
>> - Heterogeneous CPU+GPU system support.
>> - CXL translation support.
>> - Caching of common intermediate values and results.
>> - Leverage UEFI PRM methods as alternate backends to existing native
>>    code.
>>
>> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
>> ---
>>   MAINTAINERS                                |   7 +
>>   drivers/platform/x86/amd/Kconfig           |   1 +
>>   drivers/platform/x86/amd/Makefile          |   1 +
>>   drivers/platform/x86/amd/atl/Kconfig       |  20 +
>>   drivers/platform/x86/amd/atl/Makefile      |  18 +
>>   drivers/platform/x86/amd/atl/access.c      | 107 ++++
>>   drivers/platform/x86/amd/atl/core.c        | 212 +++++++
>>   drivers/platform/x86/amd/atl/dehash.c      | 459 ++++++++++++++
>>   drivers/platform/x86/amd/atl/denormalize.c | 644 ++++++++++++++++++++
>>   drivers/platform/x86/amd/atl/internal.h    | 307 ++++++++++
>>   drivers/platform/x86/amd/atl/map.c         | 659 +++++++++++++++++++++
>>   drivers/platform/x86/amd/atl/reg_fields.h  | 603 +++++++++++++++++++
>>   drivers/platform/x86/amd/atl/system.c      | 282 +++++++++
>>   drivers/platform/x86/amd/atl/umc.c         |  53 ++
>>   include/linux/amd-atl.h                    |  18 +
>>
> 
> Hi all,
> 
> I'd like to get feedback on the most appropriate place for this code.
> 
> I want to move this out of EDAC, since it's not really an EDAC feature. 
> And it needs to be used by subsystems other than EDAC.
> 
> I thought x86 Platform Drivers, because the code is very 
> platform-specific. And there are already some AMD platform drivers. But 
> there isn't any platform control or management for this translation. 
> It's just reading registers and calculating values. So it's not really a 
> "platform driver" in the sense that it manages platform-specific behavior.
> 
> Another option is for this code to be in arch/x86/ras/. But I would like 
> the option for this code to be built as a module, at least for debug and 
> development. And I don't know that modules, nor platform-specific code, 
> should be in arch/.
> 
> Currently, I think this could go in drivers/ras/. This address 
> translation is needed for RAS use cases, so making it a part of "RAS 
> Infrastructure" may make the most sense.
> 
> Boris, Tony, (and others) what do you think?

Given it's 'library code' to be used by a bunch of things and also want 
to be able to use a module, what about putting it in lib/?  There's 
plenty of library code there as tristate.