From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 121622 invoked by alias); 21 Mar 2015 20:18:49 -0000 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 Received: (qmail 121605 invoked by uid 89); 21 Mar 2015 20:18:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: glazunov.sibelius.xs4all.nl Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sat, 21 Mar 2015 20:18:48 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id t2LKIL0q022331; Sat, 21 Mar 2015 21:18:28 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id t2LKI3jm000615; Sat, 21 Mar 2015 21:18:03 +0100 (CET) Date: Sat, 21 Mar 2015 20:18:00 -0000 Message-Id: <201503212018.t2LKI3jm000615@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: jhb@FreeBSD.org CC: palves@redhat.com, gdb-patches@sourceware.org In-reply-to: <550DC328.40800@FreeBSD.org> (message from John Baldwin on Sat, 21 Mar 2015 15:14:48 -0400) Subject: Re: [PATCH] Add support for the x86 XSAVE extended state on FreeBSD/x86. References: <2672674.t3ZJOKnpzU@ralph.baldwin.cx> <5509D93A.5030707@redhat.com> <550DC328.40800@FreeBSD.org> X-SW-Source: 2015-03/txt/msg00693.txt.bz2 > Date: Sat, 21 Mar 2015 15:14:48 -0400 > From: John Baldwin > > >> + if (x86_xsave_len != 0) > >> + { > >> + switch (xcr0 & X86_XSTATE_ALL_MASK) > >> + { > >> + case X86_XSTATE_MPX_AVX512_MASK: > >> + case X86_XSTATE_AVX512_MASK: > >> + if (is64) > >> + return tdesc_amd64_avx512; > >> + else > >> + return tdesc_i386_avx512; > >> + case X86_XSTATE_MPX_MASK: > >> + if (is64) > >> + return tdesc_amd64_mpx; > >> + else > >> + return tdesc_i386_mpx; > >> + case X86_XSTATE_AVX_MASK: > >> + if (is64) > >> + return tdesc_amd64_avx; > >> + else > >> + return tdesc_i386_avx; > >> + default: > >> + if (is64) > >> + return tdesc_amd64; > >> + else > >> + return tdesc_i386; > >> + } > > > > These xcr0 -> tdesc mappings need to appear in multiple places. > > I wonder whether it'd make sense to put them in a single helper > > function (in the fbsd tdep file) that takes "xcr0" and "is64" as > > parameters, and returns the corresponding tdesc. > > There are a couple of options I've thought about for this. One > has been to have a shared to_read_description implementation in > an x86fbsd-nat.c (Linux uses a shared one in x86-linux-nat.c). > However, these case statements are also not really FreeBSD (or BSD) > specific. What if I added functions in amd64-tdep.c and i386-tdep.c > that returned the correct target description for a given xcr0 > value? Something like: > > struct target_desc * > i386_target_description(uint64_t xcr0) > { > > /* i386 switch statement here */ > } > > That could be reused for the core read_description callback as > well as the native ones. This could also be reused by other > systems that grow XSAVE support in the future. Probably a good idea. I'm working on XSAVE support in the OpenBSD kernel, so I'll eventually need this as well. I have no real objection to adding the ptrace-specific bits to the generic BSD native code like your diff is doing. I'll probably try to use the same interface for my OpenBSD implementation. I have one concern about that code though. The _supply_xsave() and _collect_xsave() functions don't accept a length, so they can't do any bounds checking. Therefore, 'xstat_bv' as returned by the kernel must be set correctly (i.e. not have bits sets that imply state beyond x86_save_len is present. Does the FreeBSD kernel guarantee that? Oh, and please rename x86_xsave_len into amd64bsd_xsave_len and i386bsd_xsave_len to keep the "namesapce" clean.