From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2610:1c1:1:606c::19:2]) by sourceware.org (Postfix) with ESMTPS id C12693840C2E for ; Thu, 17 Mar 2022 16:20:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C12693840C2E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=FreeBSD.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits)) (Client CN "mx1.freebsd.org", Issuer "R3" (verified OK)) by mx2.freebsd.org (Postfix) with ESMTPS id 7F6A88632B; Thu, 17 Mar 2022 16:20:41 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4KKC6x2xGvz3sxm; Thu, 17 Mar 2022 16:20:41 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1647534041; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i/T3v5AcHJN4DgyGUWBM0FGbPx4CU7nd+Z7Ep/Xu9NM=; b=WRd9xrwNSOvd+PbtZjx31QuKOCkCMojOWDwkAH0vPQvz9pVD0K7oiTkkcF11BH3RJCusOc GbipNM8L8X71QdEcqwMnGHexzJht+crvkiE/Rg/loT5Y7XT1ws4tZrZ9iO4WHDJ9+syRdy ZKl20q2s/9CLq65fNU+3VEpqLHhc8cmKRvkhVCLb9RkZe6WPTp41mc/nlgtZCB86ZOJ2/E 9Cfm8Wci5drYHXxg24zFilyM0q4N4t1zIJo9CO82rGfenyyb8wD6inEQgcH1NXDNiH7VHg Y3W7bmHYR4vZLGW8rvXkkBqJm20f1Phyk4gHDg1+ru4qrBzL3zamUlrQNptCig== Received: from [10.0.1.4] (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id B3A1298D3; Thu, 17 Mar 2022 16:20:40 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <8e08b537-fe97-3c06-58d0-1e41cded2054@FreeBSD.org> Date: Thu, 17 Mar 2022 09:20:39 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 Content-Language: en-US To: "Willgerodt, Felix" , "gdb-patches@sourceware.org" References: <20220316194608.89528-1-jhb@FreeBSD.org> From: John Baldwin Subject: Re: [RFC PATCH 0/4] Handle variable XSAVE layouts In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1647534041; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=i/T3v5AcHJN4DgyGUWBM0FGbPx4CU7nd+Z7Ep/Xu9NM=; b=U33eF/ZN5akOP/WVtLzw1yrcbHDez3L4/2v+w+mL36v6SUJuVs99bmd4u0+0KyiKiDIOdf NBSY58PXIvjLuKCln1FAzS2Ie+FMN+bIe4FKtCfZMW/pthdMGFzZ0pG+CXCRHWLcH5jFrA jTgU9VKN5tODB36mYbfeLffBkkb6638WWZiCbd9qG5FKzsrNSlx36+fRc2CGBsUnR9lwxW BcvSIQkRrcO+haoMR9aHZhtDe3p+zK4EcyrvKav+zsw1t8JcETbSimljQ96l8d9Jmc+g2p 01qdeJAbvk99umhxCy3rjJpN2RD/muD+W7PNykA7yckEUzuFpKRNf24I4qaqLA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1647534041; a=rsa-sha256; cv=none; b=nJbQXJMq77Oecw+6vqWEbLdNGVaz+BYu06dgt05V3sgpdSS6LrJkncGYssjUyzs57BEWvc ox14w0Sw17CXsaEpku2/dQ9SBjmUtW4bgi39Ygu0TAe0JZ4voySlH/4efz4IVjUebIQ4Du MSUuf3BTrSd/4mvjI5AZpwBtQMCSmsPY+7+OoFJDjrEsMM44mXar+kOJpHcpUON4vtFeL3 DOhBkEhEwAb4zieYQ+eUuR5rcFpirFuz+qnXfbhAXKNOKFGWjJ/4R+014wpfAh8KbEeLoA 9VZX0NWUVLO79pQzh7cWmMXlVsrNtZQqg2YfwOctqun0mq9nu48O8PzhoP88sQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Mar 2022 16:20:45 -0000 On 3/17/22 6:17 AM, Willgerodt, Felix wrote: > Hi John, > > We looked at this recently while working on AMX support (I hope I can post > that soon), and decided to leave it hardcoded for now due to the missing > information for corefiles. We weren't aware of the AMD behaviour. > > You can find an old prototype we discussed internally here: > https://github.com/intel/gdb/tree/experimental/xsave_offsets > It is not working for corefiles, which is why we put it on hold for now. > But it might be interesting to see. I just rebased it quickly from an old > state, so there might be problems with the patches. Yes, I like many aspects of this such as moving code to gdbuspport. I do think it is perhaps more future-proof/cleaner to go ahead and assume arbitrary offsets for each region (e.g. don't assume a single relative offset for the 3 AVX-512 regions) which my patch 2 does, but there are certainly many similarities. > I would like to see the offset info in corefiles in the long run, as you > mentioned in your earlier mail. To me your approach of hardcoding > known combinations seems hard to maintain. But obviously making it > a bit better right now, if there are no conflicting combinations. So I view hardcoding combinations as a fallback. I would much prefer adding a new NT_X86_XSAVE_LAYOUT or the like. It could be fetched for live processes using PT_GETREGSET if desired (though using cpuid directly which both of our patch sets do for native is fine). The fallback would exist to handle older core dumps without the new note. For example, if we had the note, then the new function in patch 3 would use that note if it exists and only fall back to calling i387_set_xsave_offsets() when the note isn't present. I'm happy to come up with a scheme for the proposed NT_X86_XSAVE_LAYOUT. A simple layout would be for it to simply contain an array of the x86_extended_feature type from your structures, but I don't know if it wouldn't be better to store "raw" CPUID results in case there are future needs for other bits (like the alignment if we ever wanted to support the compact format in userland for some reason). To that end, perhaps a structure like: struct { uint32_t id; uint32_t size; /* eax */ uint32_t offset; /* ebx */ uint32_t ecx; uint32_t edx; }; Where the note is just an array of those? The total size of the state can be inferred from the size of the NT_X86_XSTATE note. > I responded a bit more below. > >> -----Original Message----- >> From: Gdb-patches > bounces+felix.willgerodt=intel.com@sourceware.org> On Behalf Of John >> Baldwin >> Sent: Mittwoch, 16. März 2022 20:46 >> To: gdb-patches@sourceware.org >> Subject: [RFC PATCH 0/4] Handle variable XSAVE layouts >> >> This is a first attempt at resolving the issue with XSAVE I described >> previously. There are more details in the commit logs, but here I think >> will describe some caveats about the current prototype: >> >> - It is probably terrible performance-wise to be reading the offsets >> from the target every time collect/supply_xsave is called. I'd >> actually much prefer to store these (along with the total XSAVE area >> size) in the tdep. The issue is that you can have gdbarches with the >> same tdesc that use different layouts (e.g. if you open a core dump >> from an Intel CPU on a host with an AMD CPU, the two CPUs could have >> identical XCR0 masks, but the layout in the core dump wouldn't match >> the layout of a live process). Perhaps if I can fetch the offsets >> from the target in i386_gdbarch_init though I can iterate over >> matching arches looking for a match. > > I don't quite understand why storing them in tdep wouldn't work. > We get XCR0 from the coredump, not from the CPU analysing > the coredump. For live targets we would query CPUID on GDB/gdbserver. > I don't see how this would clash in your example, but maybe I missed > something in your patches. The problem is that two tdep's with the same XCR0 value currently have an identical tdesc and thus share the same 'struct gdbarch'. However, an Intel CPU with XCR0 of 0x207 uses a different layout than an AMD CPU with an XCR0 of 0x207. We would thus need separate gdbarches for those. I think though I can make that work if I fetch TARGET_OBJECT_X86_XSAVE_OFFSETS in i386_gdbarch_init() before this loop: /* If there is already a candidate, use it. */ arches = gdbarch_list_lookup_by_info (arches, &info); if (arches != NULL) return arches->gdbarch; And instead only return an existing gdbarch if it has the same XSAVE layout. For example, RISC-V does the following logic to handle differences in gdbarches that aren't fully handled by the tdesc: /* Find a candidate among the list of pre-declared architectures. */ for (arches = gdbarch_list_lookup_by_info (arches, &info); arches != NULL; arches = gdbarch_list_lookup_by_info (arches->next, &info)) { /* Check that the feature set of the ARCHES matches the feature set we are looking for. If it doesn't then we can't reuse this gdbarch. */ riscv_gdbarch_tdep *other_tdep = (riscv_gdbarch_tdep *) gdbarch_tdep (arches->gdbarch); if (other_tdep->isa_features != features || other_tdep->abi_features != abi_features) continue; break; } if (arches != NULL) return arches->gdbarch; I think it would also be handy in this case to extend the xsave_offsets structure to include the total size that can be used in the collect/supply callbacks. >> - The cpuid function I added in patch 3 isn't FreeBSD-specific at all >> (and would work on i386). I could add it to x86-nat.c instead >> easily enough. Even if OS's start providing a new ptrace op >> to fetch this info we probably should ship a cpuid-based variant as >> a fallback? > > We will also need this in gdbserver, so maybe gdbsupport or nat/ is > the better place. I personally don't see a new ptrace op coming, > as ptrace won't help us with corefiles either. But that is just my guess. I would only see the ptrace op being one that comes "for free" via PT_GETREGSET to fetch the new core dump note. However, I'm also happy to just use cpuid always for native targets. >> - The collect size I used in patch 3 for the XSAVE register set >> isn't really correct. Really if I had the "real" XSAVE register >> set size available in the tdep (see point 1) I would not set >> REGSET_VARIABLE_SIZE and instead use tdep->sizeof_xsave for both >> sizes. >> >> - I have no idea how gdbserver is impacted. So far I haven't really >> found similar tables to i387-tdep.c in gdbserver. (It's also harder >> for me to test currently as I haven't yet added FreeBSD support to >> gdbserver). >> > > The gdbserver part is in i387-fp.cc. It is quite similar to i386-tdep.c. > There is a struct i387_xsave, which also assumes a fixed layout. Ok, I had read that, but the structure wasn't obvious to me. >> - I haven't added any support for fetching the offsets on Linux (the >> only other OS that supports XSAVE state currently). I am waiting >> to have the design a bit more finalized before doing that. >> >> John Baldwin (4): >> x86: Add an xsave_offsets structure to handle variable XSAVE layouts. >> core: Support fetching TARGET_OBJECT_X86_XSAVE_OFFSETS from >> architectures. >> Update x86 FreeBSD architectures to support XSAVE offsets. >> Support XSAVE layouts for the current host in the FreeBSD/amd64 >> target. >> >> gdb/amd64-fbsd-nat.c | 73 +++++ >> gdb/amd64-fbsd-tdep.c | 8 +- >> gdb/corelow.c | 22 ++ >> gdb/gdbarch-components.py | 13 + >> gdb/gdbarch-gen.h | 10 + >> gdb/gdbarch.c | 32 +++ >> gdb/i386-fbsd-tdep.c | 33 ++- >> gdb/i386-fbsd-tdep.h | 6 + >> gdb/i387-tdep.c | 592 +++++++++++++++++++++++++------------- >> gdb/i387-tdep.h | 22 ++ >> gdb/target.h | 2 + >> 11 files changed, 607 insertions(+), 206 deletions(-) >> >> -- >> 2.34.1 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 -- John Baldwin