2023-07-28 13:15:49

by Thomas Gleixner

[permalink] [raw]
Subject: [patch v2 19/38] x86/cpu: Provide debug interface

Provide debug files which dump the topology related information of
cpuinfo_x86. This is useful to validate the upcoming conversion of the
topology evaluation for correctness or bug compatibility.

Signed-off-by: Thomas Gleixner <[email protected]>
---
V2: Don't return ENODEV when offline and make online a field.
---
arch/x86/kernel/cpu/Makefile | 2 +
arch/x86/kernel/cpu/debugfs.c | 58 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)

--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -54,6 +54,8 @@ obj-$(CONFIG_X86_LOCAL_APIC) += perfctr
obj-$(CONFIG_HYPERVISOR_GUEST) += vmware.o hypervisor.o mshyperv.o
obj-$(CONFIG_ACRN_GUEST) += acrn.o

+obj-$(CONFIG_DEBUG_FS) += debugfs.o
+
quiet_cmd_mkcapflags = MKCAP $@
cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $@ $^

--- /dev/null
+++ 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>
+
+static int cpu_debug_show(struct seq_file *m, void *p)
+{
+ unsigned long cpu = (unsigned long)m->private;
+ struct cpuinfo_x86 *c = per_cpu_ptr(&cpu_info, cpu);
+
+ seq_printf(m, "online: %d\n", cpu_online(cpu));
+ if (!c->initialized)
+ return 0;
+
+ seq_printf(m, "initial_apicid: %x\n", c->topo.initial_apicid);
+ seq_printf(m, "apicid: %x\n", c->topo.apicid);
+ seq_printf(m, "pkg_id: %u\n", c->topo.pkg_id);
+ seq_printf(m, "die_id: %u\n", c->topo.die_id);
+ seq_printf(m, "cu_id: %u\n", c->topo.cu_id);
+ seq_printf(m, "core_id: %u\n", c->topo.core_id);
+ seq_printf(m, "logical_pkg_id: %u\n", c->topo.logical_pkg_id);
+ seq_printf(m, "logical_die_id: %u\n", c->topo.logical_die_id);
+ seq_printf(m, "llc_id: %u\n", c->topo.llc_id);
+ seq_printf(m, "l2c_id: %u\n", c->topo.l2c_id);
+ seq_printf(m, "max_cores: %u\n", c->x86_max_cores);
+ seq_printf(m, "max_die_per_pkg: %u\n", __max_die_per_package);
+ seq_printf(m, "smp_num_siblings: %u\n", smp_num_siblings);
+ return 0;
+}
+
+static int cpu_debug_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, cpu_debug_show, inode->i_private);
+}
+
+static const struct file_operations dfs_cpu_ops = {
+ .open = cpu_debug_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static __init int cpu_init_debugfs(void)
+{
+ struct dentry *dir, *base = debugfs_create_dir("topo", arch_debugfs_dir);
+ unsigned long id;
+ char name [10];
+
+ dir = debugfs_create_dir("cpus", base);
+ for_each_possible_cpu(id) {
+ sprintf(name, "%lu", id);
+ debugfs_create_file(name, 0444, dir, (void *)id, &dfs_cpu_ops);
+ }
+ return 0;
+}
+late_initcall(cpu_init_debugfs);



2023-07-29 00:54:19

by Sohil Mehta

[permalink] [raw]
Subject: Re: [patch v2 19/38] x86/cpu: Provide debug interface

> +++ 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