From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11856 invoked by alias); 27 Nov 2012 14:29:26 -0000 Received: (qmail 11845 invoked by uid 22791); 27 Nov 2012 14:29:25 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 27 Nov 2012 14:29:15 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qARETDZK014348 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 27 Nov 2012 09:29:13 -0500 Received: from host2.jankratochvil.net (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qARET7gI009000 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 27 Nov 2012 09:29:10 -0500 Date: Tue, 27 Nov 2012 14:29:00 -0000 From: Jan Kratochvil To: "Metzger, Markus T" Cc: "gdb-patches@sourceware.org" , "markus.t.metzger@gmail.com" , "palves@redhat.com" , "tromey@redhat.com" , "kettenis@gnu.org" Subject: Re: [patch v4 13/13] btrace, x86: restrict to Atom Message-ID: <20121127142904.GA30650@host2.jankratochvil.net> References: <1354013351-14791-1-git-send-email-markus.t.metzger@intel.com> <1354013351-14791-14-git-send-email-markus.t.metzger@intel.com> <20121127130500.GA22431@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-11/txt/msg00735.txt.bz2 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