Re: [patch v4 13/13] btrace, x86: restrict to Atom

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Tue, 27 Nov 2012 15:03:48 +0100, Metzger, Markus T wrote:
> > There is i386-nat.c for the common functions between these two files.
> 
> Is it OK put Linux specific code into i386-nat.c?

True it is not so clear, it would be OK as long as the linux_supports_btrace()
call is moved out of it, as otherwise it just checks the CPU hardware feature.

But as you use it also in gdbserver I see now it can be moved to
common/linux-btrace.[ch] with appropriate #ifdef __i386__ and __x86_64__.
common/ currently does not have any per-file arch/target configury like gdb/
and gdbserver/ have, one day it will probably have it but not now.


> I took the __asm__ __volatile__ code from gdb/go32-nat.c. There's similar
> inline assembly code in gdb/gdbserver/linux-tic6x-low.c for checking the cpu
> id on that architecture.

> 
> The go32 code is also checking for features. I'm not sure whether this can
> be done by parsing /proc/cpuinfo.

/proc/cpuinfo does contain this information:
	cpu family  : 6
	model	: 42

I preferred /proc/cpuinfo, for example in virtualization environments with
buggy emulations I find easier to fake /proc/cpuinfo than the cpuid
instruction data.

I find gdb/go32-nat.c and gdb/gdbserver/linux-tic6x-low.c not so relevant key
as it has marginal market share compared to x86*.

But I am OK with the cpuid instruction when it is already in GDB.


> I think Marks point is that he does not want any such check in gdb but
> rather have the kernel handle it. He's right that the kernel should handle
> it. I just think that gdb needs to handle it, as well.

Not speaking for Mark but I also would like such check in Linux kernel
otherwise Linux kernel provides via SYS_perf_event_open invalid data.

With Linux kernel trunk having such fix we can talk how to workaround older
kernels, I agree with you a workaround for older kernels should be there as
you have implemented.  But such workaround could be limited somehow, for
example either checking /proc/version whether we run on a buggy kernel or for
example enabling the feature on any CPU model >= 90 as according to Intel
numbering all those CPUs should have the feature either missing or correct
(but never buggy).

Maintaining the list of CPUs twice in both GDB and Linux kernel does not seem
great to me.


Thanks,
Jan