From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.freebsd.org (mx2.freebsd.org [96.47.72.81]) by sourceware.org (Postfix) with ESMTPS id 834573858D28 for ; Tue, 11 Apr 2023 16:02:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 834573858D28 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 4PwrGK2vwdz3gZt; Tue, 11 Apr 2023 16:02:49 +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 4PwrGK0pJ4z4PT0; Tue, 11 Apr 2023 16:02:49 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681228969; 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=nj502KNDRGTfNiNVwZsDqglxdg1bLojgZDS0C4uRN3o=; b=vSIxHnKYVcXlo29vFBnTEYwIaZaMe5Qs9TyvHZ9u8IcngF1mN2C/rvDb0o4Sx+PM7t/pSM /sGsosrGui2J0qu5gmSAxsckoUr4pQEzsr3gqZOp5Cc31gPHC+mIATGxC46v7EzqFsmi+C 3AsLou8DhY+0dammFRdWJ298CdePYu/7DnpUDqK/rjDP84oJPeXjWFpmkc+vOE+zxVQ9kl xEp0RN+FI6V3ssrdEAPvPdcLOeUG74iBTWYFNVEUzNfD0FLhg4IBObQNCIHYjrXr1l1Ekj 6OwNVLQyhmeCg3Wq7ETWwgCQqLIJNiJZWahP+wrq9F49KjEd40t4Enj0E3Aklw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681228969; 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=nj502KNDRGTfNiNVwZsDqglxdg1bLojgZDS0C4uRN3o=; b=IwqcdJ9RP2lkSgJT5rbRU5OBSaAa16NVlmSE4lzp7ExmP4iN1ZL3lAk3amWeEz8b8yZmFq NXn3UL0cp8lie3gA3FBz4HyKezu+3W/c3PvdTD4xUzWQGMAWC6ct+7uBRvQw9gmp/5MRFj ouMlBgD8CvPETGDViwq6ORpJCgTuSQvs0EHUS+5aM0GjPyKy73sFQws9qtPXbtNyh6ZiLN zQTMaJjpP5ToXEF9AoX/jj/d/3mOcmuMym6stfFzVvqQDqXFCp0vpm+hDhq7YK6OG6in07 xgXucDhmxzhdz8j766/yKuOAM0H/N2XiQ/3wYstPY8kx6F+2pwEQchysx+vaAw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1681228969; a=rsa-sha256; cv=none; b=b3AH+QxWRmUoXcoB0sMPJhwdieReJ92Zkqso7fXmCQCfnrQlNRU3h0IZwzyJ+bQa2bQemh +q06tOTstTKu3soriVF3VQJvDJyt3PfCVM7GJtORmvBLDaVB6h2SwEFN3OqYx8jh7NGrSP SKPLKG7642zAxzf5/v7CnDYOPRSNOZXiAZ1jKpkcEIGeeI+bCMuhLZbh2+e/lXFyFQVJua ax83y1RDZ+io6/hMqMpoGRNJR9+XjXLhZOhpxRfpqNmzUGVSennWWq6J9+qzNyMqbog8CW oOtz4GpMj8gQ4Xo7vD+KvIQPqBjrZFkhbJwQNvsaTFmeC0LplmydnAR9RItKOQ== Received: from [IPV6:2601:648:8680:16b0:14bb:55bb:f654:b139] (unknown [IPv6:2601:648:8680:16b0:14bb:55bb:f654:b139]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4PwrGJ3shyz14Bm; Tue, 11 Apr 2023 16:02:48 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <877e12f1-1ad5-cd6a-8b56-22aae3606834@FreeBSD.org> Date: Tue, 11 Apr 2023 09:02:47 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Content-Language: en-US To: Pedro Alves , Simon Marchi , gdb-patches@sourceware.org Cc: Aleksandar Paunovic References: <20230318010905.14294-1-jhb@FreeBSD.org> <20230318010905.14294-2-jhb@FreeBSD.org> <4da73147-3fe8-ecf8-8133-5df102243b74@palves.net> From: John Baldwin Subject: Re: [PATCH v4 01/13] x86: Add an x86_xsave_layout structure to handle variable XSAVE layouts. In-Reply-To: <4da73147-3fe8-ecf8-8133-5df102243b74@palves.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 4/11/23 7:23 AM, Pedro Alves wrote: > On 2023-04-06 8:09 p.m., Simon Marchi via Gdb-patches wrote: > >> >>> +i386_fetch_xsave_layout () >>> +{ >>> + struct x86_xsave_layout layout; >>> + LONGEST len = target_read (current_inferior ()->top_target (), >>> + TARGET_OBJECT_X86_XSAVE_LAYOUT, nullptr, >>> + (gdb_byte *) &layout, 0, sizeof (layout)); >>> + if (len != sizeof (layout)) >>> + return {}; >> >> Do you have an idea of which conditions can make it so `len != sizeof >> (layout)`? If I understand correctly, the contract for >> TARGET_OBJECT_X86_XSAVE_LAYOUT is that the caller passes a pointer to an >> x86_xsave_layout object and the callee fills it. This is all internal >> to GDB. I don't imagine how a target could return something else than >> ;`sizeof (layout)` (indicating success) or TARGET_XFER_E_IO (indicating >> failure), other than being buggy. >> >> All of this to say, should it be an assert? > > How is this meant to work with remote debugging, though? Is > a TARGET_OBJECT_X86_XSAVE_LAYOUT object going to be crossing over the > remote protocol? If so, it's fine if the object is binary, but, we need to consider > the case of the host running GDB not being x86, and not even being little endian. > I.e., we can't just cast byte buffer to x86_xsave_layout and expect that it works. At present this object is not supported across the remote protocol. Instead, both GDB and gdbserver use this internally to extract register values from an XSAVE register set and the individual register values are passed across the remote protocol boundary. That said, in another role I have of maintaining a GDB stub for a hypervisor on FreeBSD, it would be really nice if I could send the XSAVE blob (and some sort of layout information) across the boundary and let GDB deal with pulling out all the sub-registers from the blob instead of doing that work in the GDB stub. I think though if we wanted to ever support that, we would not pass this raw XSAVE_LAYOUT object across the wire, but instead add a new xfer "thing" that matches the new core dump note earlier in this series where you get a set of CPUID leaf values that the layout is then synthesized from. That is, the proposed core dump note would be an array of structures that are something like: struct { uint32_t cpuid_leaf; /* EAX input */ uint32_t cpuid_subleaf; /* ECX input */ uint32_t eax; /* results of CPUID */ uint32_t ebx; uint32_t ecx; uint32_t edx; }; It would be up to the stub to provide enough "meaningful" leafs (e.g. the sub-leaf for each feature active in XCR0 and the 7/0 top-level XSAVE leaf) to construct the XSAVE layout. You could even imagine perhaps a scheme for the remote protocol where you can encode the leaf/subleaf in the annex so that the debugger could request specific leafs as-needed. (In fact, I'd probably prefer that latter method). However, we would only need that if we wanted to support sending the raw XSAVE blob back and forth across the protocol rather than the current approach of just reading/writing individual registers over the protocol. Dealing with registers is probably simpler over-all, just requires a bit of duplicated work in each stub/server to parse the XSAVE layout. >>> +/* Size and offsets of register states in the XSAVE area extended >>> + region. Offsets are set to 0 to indicate the absence of the >>> + associated registers. */ >>> + >>> +struct x86_xsave_layout >>> +{ >>> + int sizeof_xsave = 0; >>> + int avx_offset = 0; >>> + int bndregs_offset = 0; >>> + int bndcfg_offset = 0; >>> + int k_offset = 0; >>> + int zmm_h_offset = 0; >>> + int zmm_offset = 0; >>> + int pkru_offset = 0; >>> + >>> + bool operator!= (const x86_xsave_layout &other) >>> + { >>> + return sizeof_xsave != other.sizeof_xsave >>> + || avx_offset != other.avx_offset >>> + || bndregs_offset != other.bndregs_offset >>> + || bndcfg_offset != other.bndcfg_offset >>> + || k_offset != other.k_offset >>> + || zmm_h_offset != other.zmm_h_offset >>> + || zmm_offset != other.zmm_offset >>> + || pkru_offset != other.pkru_offset; >>> + } >> >> Heh, would be nice to be able to use >> >> https://en.cppreference.com/w/cpp/language/default_comparisons >> >> ! >> > > In this particular case, we could write the op instead as: > > bool operator!= (const x86_xsave_layout &other) > { > /* Just to make sure memcpy isn't comparing padding. */ > static_assert (sizeof (x86_xsave_layout) == 8 * sizeof (int)); > > return memcmp (this, &other, sizeof (x86_xsave_layout)) != 0; > } Hmm, we could (and there are some other cases like the aarch64_features struct similarly used to identify tdesc properties). One risk of the memcmp approach is if there is any compiler-inserted padding then comparing the padding bytes might be UB. Presumably the default comparison in C++20 exists partly so that the compiler can DTRT with well-defined behavior in case there is any padding. -- John Baldwin