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

Jan Kratochvil
2012-11-27T13:05:33Z

On Tue, 27 Nov 2012 11:49:11 +0100, markus.t.metzger@intel.com wrote:
> 2012-11-27 Markus Metzger <markus.t.metzger@intel.com>
>
> * amd64-linux-nat.c (amd64_linux_supports_btrace): New.
> (_initialize_amd64_linux_nat): Change supports_btrace method.
> * i386-linux-nat.c (i386_linux_supports_btrace): New.
> (_initialize_i386_linux_nat): Change supports_btrace method.

There is i386-nat.c for the common functions between these two files.

> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -1859,6 +1859,36 @@ x86_get_min_fast_tracepoint_insn_len (void)
> }
> }
>
> +/* See linux-low.h. */
> +static int
> +x86_supports_btrace (void)
> +{
> + unsigned int cpuid, model, family;
> +
> + __asm__ __volatile__ ("movl $1, %%eax;"
> + "cpuid;"
> + : "=a" (cpuid)
> + :: "%ebx", "%ecx", "%edx");

As Mark Kettenis expressed issue with '__asm__ __volatile__' here and I also
have some uncertainty about its compatibility (*), as it is in linux-* file
anyway I wound find more safe and more common style in GDB to just read and
parse "/proc/cpuinfo",

(*) Although probably any Linux compiler needst so support it the GCC
compatible way, incl. llvm and icc.

> +
> + family = (cpuid >> 8) & 0xf;
> + model = (cpuid >> 4) & 0xf;
> +
> + switch (family)
> + {
> + case 6:
> + model += (cpuid >> 12) & 0xf0;
> +
> + switch (model)
> + {

Here should be tab, not 8 spaces.

> + case 28: /* Atom. */
> + case 38:

In some instances of this function you list 39 and in other instances you do
not.

(My i7-2620M model 42 is probably a buggy one although it seems to work.)

Thanks,
Jan