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: <ef3e63b2-d73b-b03f-2f5e-df456d756a63@intel.com>
Date:   Fri, 28 Jul 2023 16:57:54 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [patch v2 19/38] x86/cpu: Provide debug interface
Content-Language: en-US
To:     Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>
CC:     <x86@kernel.org>, Tom Lendacky <thomas.lendacky@amd.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>,
        Arjan van de Ven <arjan@linux.intel.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Dick Kennedy <dick.kennedy@broadcom.com>,
        James Smart <james.smart@broadcom.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        <linux-scsi@vger.kernel.org>, Guenter Roeck <linux@roeck-us.net>,
        <linux-hwmon@vger.kernel.org>, "Jean Delvare" <jdelvare@suse.com>,
        Huang Rui <ray.huang@amd.com>, Juergen Gross <jgross@suse.com>,
        Steve Wahl <steve.wahl@hpe.com>,
        Mike Travis <mike.travis@hpe.com>,
        Dimitri Sivanich <dimitri.sivanich@hpe.com>,
        "Russ Anderson" <russ.anderson@hpe.com>
References: <20230728105650.565799744@linutronix.de>
 <20230728120930.727298737@linutronix.de>
From:   Sohil Mehta <sohil.mehta@intel.com>
In-Reply-To: <20230728120930.727298737@linutronix.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cnE2aml3TG9jaUlPSERUNnB2UUgwRk9OZTUxWWcvaEUwV0Q3K0dib0Y0Rnk5?=
 =?utf-8?B?R3hnaEV3Um1VMWZISWlGblI0ckJxN1RrZkVxaHNTUGpyc1V5MGZ1VDMzbk5I?=
 =?utf-8?B?SlNsVW1paWNDM29TM2xHZFJXazF1aHdQMUw2b05DdTh1UzZwVkwvcE5DdGRi?=
 =?utf-8?B?NmJQdWtFa2thL3FnR0l1MlRvcFpLK1hpYTdRUWN5RkJrWGd0bXlCNy9tT1Na?=
 =?utf-8?B?QWw0Z1JNaDZPNTY0RTBiWlJ0ditkc0crTHRORm5qaFdUcS9kTGFNSUdqdS9S?=
 =?utf-8?B?aFpRdU1sSE9ZQW9lSjU1YnNOSm5hakszMWNsWDlNZEE1R2NtbERkSHJXK2hD?=
 =?utf-8?B?MGJ3cnNWUWVham5uY3JLdXVVYmNjcWk0Z1kzVEdyWXNKUC9USjJadUFVaEZN?=
 =?utf-8?B?dCtMYWtOMWw2SXBmRE92UGkvcVF0MnB1b3VmS0FFQ01xSHdXZmRvZGVmcEVq?=
 =?utf-8?B?VjlXZy9UK2NoU1FRMC9QdXVia0FodE1CNjNhdkkwb2hPOTJpTHlBUldHOThx?=
 =?utf-8?B?T1VzUnBXcEI0VzZHR1FUak1jM1pmKzRxK1hWbjRaekJvaHoyVWk2YUtTbnVO?=
 =?utf-8?B?N3l1bDkrTGlQc0FoRDVhSVFrSzBiL0RNS0puUEljbHUvUFNaZ3E3SFluNHR4?=
 =?utf-8?B?aW42Y09XRCtQY2VqbGgxRjFpd1pSYUtLZWlWV3ZlM2tFRXVzTlRFRTI2MHpV?=
 =?utf-8?B?ZXpkSjljSmQwMm42alQyMWc0R1pPOGpYSkVLSEpLVmxyeGFVRnY4WFduK1Rz?=
 =?utf-8?B?UmtHTHViVVRGaStKSUVqaWhHLzhhcGZGaDR4RSttMVFoc0hOZVQ0bmhFSy9C?=
 =?utf-8?B?d25VNmc0dmRhMUtNRzN1MmIzSHcyNjByRk56K3duN0ozcXVralk3ODJmOWZk?=
 =?utf-8?B?R21BcXhwR2tGTStPdFFTMTFyOVdaTmlaSFdPM243RG9zd1VMUHNhNStvU1ZK?=
 =?utf-8?B?VjNleUtrMVVjU0RpM0hVdGlEL2RFazRWbEVRWXVndWtFYURYZk9ldkt1U3pO?=
 =?utf-8?B?T3ZmUzhBcGltY1pYM0ZLQ1NkZHBMMXRoREdGTmVRcVgzWlhkbEczREZZcnpW?=
 =?utf-8?B?b0wrWnlZLzloQUIyT1AwUVZoUS9vOG9kbkpPQW5JZGNaZ2VsUVdndVU5NVlU?=
 =?utf-8?B?Q0hIUU54MStudFRBZnA4S2ZheVhNNUN6MC9VN0ZkQW5odlQ0MkMzczlwelNt?=
 =?utf-8?B?WVp4MktkczZDOG4yeXFCTy9aMExYdDNTU3R2WW0rZW4xK2NkSjZsVjI5WUFz?=
 =?utf-8?B?NUhuT3RqU1l3WlpEeTFaMnU1M2gyWHlTbm00eWxWM3VucThPZ3haODJqd21k?=
 =?utf-8?B?V0hwTmRtMnBTa1A1SndWckJSL2IzWnhDck1JMVdqQ2xvVkpHTzVVcDVGTzdi?=
 =?utf-8?B?dWxpS3pjMXd6MXFyd2hWSWNRK1ZaWklEYXVHQUQ2dVdsaGtGcFk2VXF5djRI?=
 =?utf-8?B?eTVJa00yNUdId24rZWVqU3o4RmZqcmJNQ2FMWEhEckZZU2t2dG5oMGZlL0th?=
 =?utf-8?B?N1hKTEdrc1NoUmgzOGhuMkZ6bXNISktsVEV5UHJpdjc2UWhQNTBjNkVxM0RQ?=
 =?utf-8?B?OVBuenhBQVlyNUN6MUNmRHlYankyQmtqMGQ0ZjBVOWdPM1NEUXlUMHMxMGVH?=
 =?utf-8?B?dzlNUmJQRnlwb0Z2MGhndlNkeTJoM0VWc0NLQmIyaDJFdGcrdENqQmFyc1Mz?=
 =?utf-8?B?YVdvdURpMFM5U1NuVVRtZEZzajB2alNhaG5pamdGNkZCa0ZXTS9vdjdSSE1Y?=
 =?utf-8?B?NzhZLzBUdVRyVURyMGVxR3F1a1pWZFo5aStuNkVjTHJVUEZ1V2lPbTJEUXAv?=
 =?utf-8?B?d0l2NkFpMDVJcDhta1JQM3R3a2puZWRXc2s4MFRMNnJvaFFrQ3gwQ1Z1WkQ1?=
 =?utf-8?B?azI4ZVNYL3FXdWp1bDlvNGI1U1RuS2FuNGp2bTcyR1N6akViK0RnQmt3ZmtO?=
 =?utf-8?B?dDc5akp0ZnJ5RW5wL2ZqUHdweFlWeGtKdmdsMDFtWituWEdMc21WYXUrZGdv?=
 =?utf-8?B?RE9RalFxeE5Wd2V2MWNsei9wTTZJZDFEa0phcUYrRlorLzgyVENOZVk4bEZS?=
 =?utf-8?B?dnVjYmEvRXBzRDgrbnhUVjRrVjIzcXFWaDBuNVd2ZkxQTVFnbDBBckFsd1dG?=
 =?utf-8?Q?C9XwHZJnoWbWClUZ8vgLLrMuj?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5e97b362-8e97-40fe-5121-08db8fc67711
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3320.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jul 2023 23:57:56.8819
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: V3amfXWI/JFfHbj/TW5vXM4kPJ+rCS3nAqshUsHRRUQZduU4FoDtg0bcb0ZSH5d6kvifO94QwbemPRQIbdeJ5A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6325
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham 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

> +++ b/arch/x86/kernel/cpu/debugfs.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/debugfs.h>
> +
> +#include <asm/apic.h>
> +#include <asm/processor.h>
> +

I ran checkpatch through the patches and it lists a bunch of errors and
warnings. Most of them are due to issues in the existing code which is
probably not worth fixing.

However, I found a couple of recommendations which might be of interest.

1) Using linux headers instead of the asm ones. Namely,

> Consider using #include <linux/processor.h> instead of <asm/processor.h>
> Consider using #include <linux/topology.h> instead of <asm/topology.h>
> Consider using #include <linux/smp.h> instead of <asm/smp.h>

2) Macros with multiple statements should be enclosed in a do - while loop

From patch 20/38:
> +#define cpuid_subleaf(leaf, subleaf, regs)		\
> +	BUILD_BUG_ON(sizeof(*(regs)) != 16);		\
> +	__cpuid_read(leaf, subleaf, (u32 *)(regs))


I am wondering if either of them matter?


> +static __init int cpu_init_debugfs(void)
> +{
> +	struct dentry *dir, *base = debugfs_create_dir("topo", arch_debugfs_dir);
> +	unsigned long id;
> +	char name [10];
> +

Excess space after name and before the square bracket can be avoided.


Sohil