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 2C2443858D3C for ; Fri, 18 Mar 2022 17:27:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2C2443858D3C 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 B22AC77538; Fri, 18 Mar 2022 17:27:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (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 4KKrYZ4Kqjz4l9Z; Fri, 18 Mar 2022 17:27:30 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1647624450; 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=Jx1AO/qQbsPgbeJ1ewhHyhd14oLWp9fHkme7MDY988c=; b=JH/Lr7eHgdEYIkfi9PXtwlGSe+7Q/Uo6Ppul3xbeg6XsBejnooaZKmdwqYfV8jfr7DmTos 2jHkOOELmkT3IChKqUwkrDDrvhxkWmARHh7Ti3qqby9y+SCMSajsGEUN3g+3rkPJsWbVmZ GQ3y8sYeAkGiNFKs3wVQdSvUeCMkWufZ0O+uqGWudNKvqBQzYhZOZ59939887xIyfaCLTM 3KAD1SVdafKBG87wAaM2781TG8Zb6AmP2BwhCRJE9naSDQSQ0Dck64EtU4C76F+PO08+jz U/8p4PQpAqnrA54kKBf8oBS+/1Wnzk4GTfYBQ/q7bkRAiC3A3e5An29nfoq+TQ== Received: from [IPV6:2601:648:8680:ed60:c991:674a:3aaf:8521] (unknown [IPv6:2601:648:8680:ed60:c991:674a:3aaf:8521]) (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 A448C26FAC; Fri, 18 Mar 2022 17:27:29 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <2fbbf0a4-30d4-a41a-0e16-71a737598706@FreeBSD.org> Date: Fri, 18 Mar 2022 10:27:27 -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> <8e08b537-fe97-3c06-58d0-1e41cded2054@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=1647624450; 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=Jx1AO/qQbsPgbeJ1ewhHyhd14oLWp9fHkme7MDY988c=; b=ObNC26jhThSTQXK67yOEtqwWPqap9GhaZCe0iB4wkrC+y0jCO9Sgg/P6NJghWeKSik7qHq JCoCIwBQjlYMcU/o5Cwn2uG7dRJxF+0EnRyN4r9mMbjpT+zFdEuyAbvDUWtfFfvNmH3jmS Q2/JjWo1IP+CwC/KduNFA/IP//uuyNJ3oKYOEPevSaZPLaSzCtEn07FsV7Nw3Qpiw0/tg1 c8As8tSv5gqOWJU/E3ePwdstoBmWOzmL3IP9kV5JmYn/xpJWRXInxm9NbHW6T4Aoa9bGOA N1CMRTy9+FKdqvngmF0IJPP+TDvIZG6XnxiFzOppbvGfNZDOI7s3eT041Ql5BQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1647624450; a=rsa-sha256; cv=none; b=fm6tU0ounNaMeQshNDVNq2yhZi/cXtwIYKTZBBZ3Cg7UIJ8Itc2bKYcgpdkbvJ9jSxSnze zPBjFefwCZEtb+n9bydcfFy5oZzBQ0mtwOwPf+i/j0iVAlHMu0KneSFm5PcW0+Az9sPpQ4 VdncM2zdbr0CDdGedxmKoV0M+kIE/ty2F/uu1sqe/F6idgxoqILKhz85qkKAsFEoC4+iPO kivKGR4nlo0JZOK/s6suAuyOSUoCfvMRXQPPQIB1Z5Aa+aAAxFtLcx3RoWRbNSYAW1JIhx 96D305t3x3Z4xfyfbDmF2xspv/JWEzIU6roHT+Fzgc27N0JbizUPU4wzPKPzQw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-Spam-Status: No, score=-3.8 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: Fri, 18 Mar 2022 17:27:36 -0000 On 3/18/22 6:49 AM, Willgerodt, Felix wrote: > Hi John, > > See comments inline. > >> -----Original Message----- >> From: Gdb-patches > bounces+felix.willgerodt=intel.com@sourceware.org> On Behalf Of John >> Baldwin >> Sent: Donnerstag, 17. März 2022 19:03 >> To: gdb-patches@sourceware.org >> Subject: Re: [RFC PATCH 0/4] Handle variable XSAVE layouts >> >> On 3/17/22 9:20 AM, John Baldwin wrote: >>> On 3/17/22 6:17 AM, Willgerodt, Felix wrote: >>>>> 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. > > Just out of curiosity: If we wouldn't implement i387_set_xsave_layout(), > and read that info from CPUID and the corefile (once that note exists), > would we still need this? Once corefile support exists, this i387_set_xsave_layout() function won't be used. To be clear, in the current patches (which don't yet include all of the Linux-specific changes), CPUID is used for native targets. This function is only used for coredumps, and only in OS-specific gdbarch method to read the XSAVE layout from a core dump. Once OS's add a new core dump note to describe the XSAVE layout, those OS-specific gdbarch methods can read that core dump note instead and will only call i387_set_xsave_layout() for older core dumps without the note. I'm happy to work on the core dump note and I can implement it in FreeBSD easily. I think it would be nice to agree on a common format that is also used in Linux, but I'm not a Linux developer and would need someone else to work on implementing the core dump note in Linux. >>> 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. >> >> I have made these changes and it does appear to work. I do think the >> approach >> of a new TARGET_OBJECT will work (and it will support adding a new >> NT_X86_XSAVE_LAYOUT or the like in the future to better support core >> dumps). >> If you agree with that part of the design (and storing it in the tdep, etc. >> I can try to merge in parts of your patchset (in particular, moving some >> things to gdbsupport and similar gdbserver patches) or I'm happy to let you >> drive. I will send a V2 with the changes to store the layout in the tdep. >> >> -- >> John Baldwin > > > Please feel free to take (and adjust) any code or idea from our patchset > that you like. I just posted it trying to be helpful. > > > I must admit I am not so sure about your approach. Yes it helps > now. But assume in the future there is a 64-byte component Y and a 64-byte > component X. What happens if one CPU drops X and another drops Y? > We could have the same XCR0 and same XSAVE size and no way to > distinguish the layouts. > To me there is no real alternative to getting everything from CPUID. > I personally would at least like to see CPUID implemented for live > processes, and a huge comment that this is a last-resort fallback > for corefiles, that could fail. Until we have the corefile stuff figured out. I have tried to make that apparent in the commit log in the V2 of the series I posted (and have split the commits up a bit further so that it is clear that the intention is to only use the static layouts as a fallback for core dumps, not in native targets). > That said, I am no maintainer and not the right person for deciding. > This is just my point of view. I am not a global maintainer either FWIW. > I looked at the Intel software development manual a bit. There is a > compacted xsave format and a standard format. In GDB we > never check which one is used and assume standard format, afaik. > (I think we have even dropped that information when we are in > I387-tdep.c.) Yes, GDB currently assumes the standard format. FreeBSD doesn't currently make use of the compacted format, and Linux is careful to convert state saved in the compact format to the standard format before exporting the XSAVE state either via ptrace() or via core dump notes. That said, there may come a day when OS's may want to export data in the compacted format. In that case we may want to compute a different set of offsets for each state component. That is the reason I'm inclined to make the core dump note store all of the information for each active leaf (all 4 registers from cpuid) so that they can be used to compute the layout of the compacted format (rather than just storing the ID, size, and standard offset). > Judging by your XCR0 values in your earlier email, you are not in > compacted mode, right? Could you check the CPUID leaves for MPX? > I wonder what they report. The AMD CPU in this case doesn't implement MPX so bits 3 and 4 are always zero in XCR0. Per the SDM for CPUID, the cpuid leaves return a size and offset of 0 for those leaves. This CPU also doesn't support AVX512, so it reports 0's for those three leaves as well. -- John Baldwin