linux-kernel - Re: [git-pull -tip] x86: cpu architecture debug code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 10 Mar 2009 10:01:17 +0100
From:	Andreas Herrmann <andreas.herrmann3@....com>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
CC:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...nel.org>,
	x86 maintainers <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [git-pull -tip] x86: cpu architecture debug code

On Mon, Mar 09, 2009 at 12:42:49PM +0530, Jaswinder Singh Rajput wrote:
> On Sun, 2009-03-08 at 12:19 +0530, Jaswinder Singh Rajput wrote:
> > The following changes since commit cb553adb4e83bc5314b5a0c397e546a215b19307:
> >   Ingo Molnar (1):
> >         Merge branch 'tracing/printk'
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tip-cpu.git master
> > 
> > Jaswinder Singh Rajput (1):
> >       x86: cpu architecture debug code
> > 

I still doubt that this is going to be useful (I don't see use cases for it).
Nevertheless I have some comments.

> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index c381330..6427a84 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -9,7 +9,7 @@ endif
>  
>  obj-y			:= intel_cacheinfo.o addon_cpuid_features.o
>  obj-y			+= proc.o capflags.o powerflags.o common.o
> -obj-y			+= vmware.o hypervisor.o
> +obj-y			+= vmware.o hypervisor.o cpu_debug.o
>  
>  obj-$(CONFIG_X86_32)	+= bugs.o cmpxchg.o
>  obj-$(CONFIG_X86_64)	+= bugs_64.o

There is no kernel config option and this code will always be
compiled in. Why?

> diff --git a/arch/x86/kernel/cpu/cpu_debug.c b/arch/x86/kernel/cpu/cpu_debug.c
> new file mode 100755
> index 0000000..e50d73b
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/cpu_debug.c
> @@ -0,0 +1,692 @@

   ...

> +static int cpu_seq_show(struct seq_file *seq, void *v)
> +{
> +	struct cpu_private *priv = seq->private;
> +
> +	if (priv == NULL)
> +		return -EINVAL;
> +
> +	switch (cpu_base[priv->type].flag) {
> +	case CPU_TSS:
> +		smp_call_function_single(priv->cpu, print_stdreg, seq, 1);
> +		break;
> +	case CPU_CR:
> +		smp_call_function_single(priv->cpu, print_cr, seq, 1);
> +		break;
> +	case CPU_DT:
> +		smp_call_function_single(priv->cpu, print_dt, seq, 1);
> +		break;
> +	default:
> +		print_msr(seq, priv->cpu, cpu_base[priv->type].flag);
> +		break;
> +	}
> +	seq_printf(seq, "\n");

(As one example.) Please don't print empty lines.

General thoughts/questions:

Your code is not cpu-hotpluggable. The complete debugfs subtree shows up
for all CPUs - including CPUs that are offline. Was this your intention?

How does this (especially dumping of register state) fit into other
kernel debugging facilities, ie.
 - kgdb
 - sysrq


Regards,
Andreas

-- 
Operating | Advanced Micro Devices GmbH
  System  | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany
 Research | Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
  Center  | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
  (OSRC)  | Registergericht München, HRB Nr. 43632


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/