Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <b9beb435-30e4-44a3-b27d-0567a8e873bd@amd.com>
Date:   Wed, 13 Dec 2023 10:35:55 -0500
User-Agent: Mozilla Thunderbird
Cc:     yazen.ghannam@amd.com, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, tony.luck@intel.com, x86@kernel.org,
        avadhut.naik@amd.com, john.allen@amd.com, william.roche@oracle.com,
        muralidhara.mk@amd.com
Subject: Re: [PATCH v3 1/3] RAS: Introduce AMD Address Translation Library
To:     Borislav Petkov <bp@alien8.de>
References: <20231210194932.43992-1-yazen.ghannam@amd.com>
 <20231210194932.43992-2-yazen.ghannam@amd.com>
 <20231211195739.GIZXdps9DNvOgCR5Xs@fat_crate.local>
 <295f3cc9-6140-4813-b107-8c8b60f8aaa1@amd.com>
 <20231212153449.GEZXh9mSJ5epD13Dp7@fat_crate.local>
Content-Language: en-US
From:   Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <20231212153449.GEZXh9mSJ5epD13Dp7@fat_crate.local>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eitVME01RllIS2l3QVM3bmJNcmhGand0NWN0eTZmT1NvWk9NKys3Y1JFeEt0?=
 =?utf-8?B?SkZWcERHVitwQ0FsaS8xN0Zic2lUWXNyaUtQckRXMkwvWnJWK2tGd0t4Qzhz?=
 =?utf-8?B?d1QyaXZabjliRGdWYVNmMjN6RjN1MFNqOUk5K3VzMEN3TjhCRmlvM3ZWd244?=
 =?utf-8?B?YytKNGxOcVFRYm5CcWp4V1hIS3hxOEJBNTV1ZTNZVkwwcVY3Y2JHR3h3M1Ix?=
 =?utf-8?B?ME5JY0N2NUVWREtJdlhtUnFzSzVCS1RuVytPelIxeFBvaWtpcmVyMDFvSVNt?=
 =?utf-8?B?TGtMTVYxZ3VsZmlmMzk5RXFVY0MzVlE3c3p4MUhBanRrMjY5dGQvMnBxVVpr?=
 =?utf-8?B?cUk0NDhMVVZhSVZmdUtUSlZNRnZOWml0U1RrRk9NNlNYeTM3WVdwcEZzZ3lL?=
 =?utf-8?B?SWVEdEU0RlFUQjNUdHhrcFR1Z3JDWG9xdG5RM3FZUVFaTmdzYjlNSjhPbDNz?=
 =?utf-8?B?eTlOL2pWWFRjMkZEaktYbDVnQ0IzM3N1d1VJV1l1RWw2UDh2NC9tdnNTek1Z?=
 =?utf-8?B?QmpJYlZ6NWtsQmtsd3RWTjB5UStlcU5MNFN4THJaeUdWSSs3Y0hpZitNZGpY?=
 =?utf-8?B?alJ2MXZPdWtJQVlIeU5lWDBQbUxyV2RuOWlxVitOS1NTcG1oRVVpM29KNm4r?=
 =?utf-8?B?alVyYk9TVjBSTnNoV1NBVE1DV1VRN0l3ZGloTWJlc0lXS3FwYmJXbjJQOXRS?=
 =?utf-8?B?YWFLU2lmOTdBd0FiaHF4MU0yRGFYMFJIUmtwRmdNUk1qdVJWVzlsMnRSK1Qv?=
 =?utf-8?B?ZDlSdFhDUE9zSFdGRncxeWNRd0VaQU9MWHdXc1JxUHpiUWl3bzlRQ0l0S2NU?=
 =?utf-8?B?b1ZpL01kTkpuLzF2TVRLRHBXRk5aSU9WWFBaTmRrNnlrc3hwS1JyL1dNaDFw?=
 =?utf-8?B?MHVNcWpXSjhIQmVOUE9DQ1huNGZRTlI5UlRSZkU2NnpyWWlaWmY5WUpwTTB2?=
 =?utf-8?B?cThjcENUUFBtejFuMHRmZjdxeTMwV293MHp4Q2Zad2hOUXNUcmE2eHJyZk5H?=
 =?utf-8?B?VC9XdzNHcllYWEJNNTkzUVRQSG0vWlUwc0tUcmhJSi96TDRmVmpzdmRxbnFw?=
 =?utf-8?B?ZkNRUzZkODNldGxTL0MxaGtaOVlPMTc5VkVkQXFHRTYyM01aeWN3aDRGUzBD?=
 =?utf-8?B?TUpDSnhQOUgwS2x6Mys1azBMTjRYdGdLRXAvQVVKdUwwRVhiZGRRWkI2NkQr?=
 =?utf-8?B?dlJURmRVbUQrNWpucmk5Z05oWWcwQWJ2Yk5SN2c3b29LZVlwWXMrME80WnZ1?=
 =?utf-8?B?amJWODFrWTJDams5ZXI4NjBKTUJjRjNuOU5Wc1NPa3dPMFJQaGVCYU1zV0dm?=
 =?utf-8?B?Tm41Tll0WGhxRmpCRDhhVFN0ODJiQWxkc201RWlJbWpPRHAxUThJRWdkcDNh?=
 =?utf-8?B?SU8zTU9XcTROOG1wZGlFdytjKzJ4OVRFSUdsU3NoRWFkUmx0dkY2YTJRNzJ6?=
 =?utf-8?B?aDFDOEZMTUk1REd4aHJkY2dPZWhxLzhRK1E4dGFxTVdxRDNndHJYNHEzWkhv?=
 =?utf-8?B?Z2FjNXVmWWZ5eXIydGVUQTFkRXduWGJaSHArd0hoTFZoaVdDczc5Ymh4Zzh4?=
 =?utf-8?B?MjNFbnNxZUt0Q0hacXZ3elNqckVhMm1PcU01SFNwQTIvS1pzejR4QlJUZzFV?=
 =?utf-8?B?RnBNUCtnWjFKNFdYWEVSZ2x5RWpxeS9qb1VncW5PeUFIbjFVb0FtVTNDbEhq?=
 =?utf-8?B?R0xaeDMvME5lV1o5SWY1NzdDbkZIWDEvbkx0b29heUFXdzVCaG0rS1RybTl2?=
 =?utf-8?B?WDZid0ZEU2FhY005TnpXQUtqYmhxaVYrRkRUNFR5Y1NDckxxcW1xcklpK2F2?=
 =?utf-8?B?YTRYUk9GMndmdytVMHNra3BEV3Ewb0V4dkRQamhrSnpOU3oxZlRxOXNtbVVz?=
 =?utf-8?B?cGZRMWxZNzNXWUYzWnFHcTluTkl4S2lyWHZtKzdmem1WRGxZYmtveGVQbkpq?=
 =?utf-8?B?RjAzOWkzUm9WUGk5dWcxTm45eTR5RVpscUZVdzQrY1BGMHhmNktyVFRzRStN?=
 =?utf-8?B?cjVVUmdZSzE3WDlPYXhMdEtDemMvMDZsdS9BRVoydWhzbmU0SWN3OUlMS21a?=
 =?utf-8?B?bSt6NGlUWFYzSmtITFVvWGRJWlcwamxYeXdEY0c4QThHMDRwM2VZM0Y2TVNN?=
 =?utf-8?Q?Ok9NBPQSfHTQoetHC58+IiZk4?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e98bbea2-5a27-4633-dfff-08dbfbf1334e
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 15:35:57.2236
 (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: Oh3wQJxzgFPjpOtIDZrD8CtdOQE/V08uB0psAuUdIZocbGgEm5BSKV9qohlHv2oqMcDt0iiHh1i4sC969X1SiQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR12MB8855
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 13 Dec 2023 07:36:41 -0800 (PST)

On 12/12/2023 10:34 AM, Borislav Petkov wrote:
> On Tue, Dec 12, 2023 at 09:23:44AM -0500, Yazen Ghannam wrote:
>> I'm thinking that the warning only happens if the "assert" condition above
>> is hit.
> 
> assert usually means "assert - abort the program if assertion is false"
> - from assert(3).
>

Right, agreed. In this context, the program is the translation method. 
But yeah, it doesn't make much sense describing the kernel. I'll change 
the wording if I don't drop the macros completely.

>> In older revisions, I had all these messages as "debug" loglevel. I don't
>> think there's anything a user can do to fix these issues. They're either
>> coding bugs in the library or system configuration.
>>
>> I'd rather go back to the debug messages if you don't mind. It's not
>> difficult to enable dynamic debug messages compared to DEBUG Kconfig
>> options. So I think it'd be okay to work with users on this if they
>> encounter an issue.
> 
> Makes sense.
> 
>>>> +static const struct x86_cpu_id amd_atl_cpuids[] = {
>>>> +	X86_MATCH_FEATURE(X86_FEATURE_SMCA, NULL),
>>>
>>> I'd expect for only this one to be needed, but not those below.
>>>
>>
>> Me too. Those below are to workaround a current module loading issue. I'll
>> add a code comment for that.
> 
> You mean the systemdoofus crap?
> 
> Fget it - we don't fix the kernel because luserspace is nuts.
> 
>>
>>>> +	X86_MATCH_FEATURE(X86_FEATURE_ZEN, NULL),
> 
> ...and those are influx - this is called X86_FEATURE_ZEN1 now and
> X86_FEATURE_ZEN is set on all Zens. So you might as well match on
> X86_FEATURE_ZEN only.
> 
> But you should not need it - if SMCA doesn't match then we have another
> problem. ATL should load on SMCA systems only.
>

I agree in principle. But I don't think it hurts to include an 
additional line to avoid the confusion when the module doesn't load.

Also, the SMCA feature is used here as a short-cut to match on systems 
with a Data Fabric. We could use the Zen feature in the same way.

Thanks,
Yazen