Re: [patch v3 16/16] btrace, x86: restrict to Atom

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> From: markus.t.metzger@intel.com
> Date: Tue, 14 Aug 2012 14:59:31 +0200
> 
> From: Markus Metzger <markus.t.metzger@intel.com>
> 
> Restrict branch tracing support to Atom processors.

Did you explain anywhere why this is restricted to Atom?

Even with such an explanation, the approach taken here is wrong:

> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 8475d7a..b0f7b43 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -37,6 +37,7 @@
>  #include "symtab.h"
>  #include "arch-utils.h"
>  #include "xml-syscall.h"
> +#include "linux-btrace.h"
>  
>  #include "i387-tdep.h"
>  #include "i386-xstate.h"
> @@ -704,6 +705,39 @@ i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch,
>    return closure;
>  }
>  
> +/* See i386-linux-tdep.h.  */
> +int
> +i386_linux_supports_btrace (void)
> +{
> +  unsigned int cpuid, model, family;
> +
> +  if (!linux_supports_btrace ())
> +    return 0;
> +
> +  __asm__ __volatile__ ("movl   $1, %%eax;"
> +			"cpuid;"
> +			: "=a" (cpuid)
> +			:: "%ebx", "%ecx", "%edx");
> +
> +  family = (cpuid >> 8) & 0xf;
> +  model = (cpuid >> 4) & 0xf;
> +
> +  switch (family)
> +    {
> +    case 6:
> +      model += (cpuid >> 12) & 0xf0;
> +
> +      switch (model)
> +        {
> +	case 28: /* Atom.  */
> +	case 38:
> +	  return 1;
> +	}
> +    }
> +
> +  return 0;
> +}

You can't have this function in a -tdep.c, since this file needs to be
compilable everywhere (not just on i386/amd64).  But even in a -nat.c
file you shouldn't use inline assembly, since it isn't a standardized
C feature.  I can only guess why you want to restrict this feature to
Atom only, but I suspect that the proper solution is to query the
target whether branch tracing is supported.  That means the -nat.c
code should have code to query for support in the Linux kernel.