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 968D83858D1E; Fri, 1 Apr 2022 23:30:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 968D83858D1E 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 [IPv6:2610:1c1:1:606c::19:1]) (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 23BD79FAF4; Fri, 1 Apr 2022 23:30:26 +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 4KVbxs1rz6z4qyT; Fri, 1 Apr 2022 23:30:25 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648855825; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+aLCq9o3/sDMRJOFqF20UlpMpdYBCiyL43mL9jxY4kU=; b=bG3SL+0ae2LcrY4pLNNdo4PTzAQoh5WKAl9f+f1StBtsYWoWHJvudgFUUBsJ9AiqaDichU DKUMzIM2wqPjaoHhKLy+0T/2elYqTQs6bZf6rA0+/zw6zQB+O106BDbyAsFJFGo2RMQx2I BFStnrB4VdznivvN87gnzB38KTXYhPOxvStkofwiv1+9yDMMn2fHN7b47dtWWi/RHCR5Hb fxtW23T0zoLfSSxwm1rFd1tqizpXPYP4vEiJ/wTGfp0/3lsR3MjwpJAfd3V/TirzZ9XIxo AM4ppM29gZClo4kosx9XUcRwRYyeHlcMo5GeCklljV0NGEfv0kS5e7PjtAc55w== 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 48B0457E9; Fri, 1 Apr 2022 23:30:24 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Fri, 1 Apr 2022 16:30:23 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US To: Luis Machado , binutils@sourceware.org, gdb-patches@sourceware.org References: <20220323210048.25525-1-jhb@FreeBSD.org> <20220323210048.25525-9-jhb@FreeBSD.org> From: John Baldwin Subject: Re: [PATCH 08/12] Add an aarch64-tls feature which includes the tpidr register. In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1648855825; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+aLCq9o3/sDMRJOFqF20UlpMpdYBCiyL43mL9jxY4kU=; b=g05DDRTq8PqAd5CQQv0AUSZtYZ3PCq1cnxfMpnjeXuKB/O1hV47nT+H8o4OvrqZN2Sj4zX KfslcCTJRPaBXIJyA3+Tf01lh7V/YnmShiDXfHdDnqR3bQVOFB+9QwlyvVrslD3l4uqYYX WqSw1Y4WMhXWVTU8AEQKTg2Z6kK8pRgK+WrAqT5Q558GQ/5JQ33zo/9gyfqNL/48BmmJd3 UCUs3/82E3vsTZmMY1ScSl49ynsvte+ssjGWVFJAHiWx0EToq3zm+yVLVgt72xPRNLBAqh SKAhbwrh8gWqgJ605UUrravm2vQHiqraargBjDikGhVJiLgGjQnSQsH9zAB30Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1648855825; a=rsa-sha256; cv=none; b=RW1yHpw3aUeVpXvRgn+yttV2gBKpTk+mU7uQTojH/xSo/Hv9UhlnDR72OxvxTYTqgtLyoW fa0e0fdw+gQJmbQtrft2HY5zFtGWmNmDOCwk7W4i7sh2G9rvH5J+f+6+G0WgFt/l7u31dm /RUJHnYxQM2pCFD9Y98L5Cp7dtyDRmutMHwQnoGYX82Laym93Ffiq75uvVzVCoCdmNpkSH yxaVLuVe7u4AzIoEi9z/lOsbcC9BopQTrelfdzYfXfq+dmvavqHA/QeInEjvuEEdnX+udj GZJq3qQiVV201JA7L2aQD6XDgCXN760Eu/jdcQdf24EiN1LsmHe8q5Wgt9Arxw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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, 01 Apr 2022 23:30:28 -0000 On 3/28/22 3:16 AM, Luis Machado wrote: >> - return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p); >> + return aarch64_read_description (aarch64_sve_get_vq (tid), pauth_p, mte_p, >> + false); > > This is getting a bit ugly. We should pass a single struct that contains > all available features eventually. But it should be OK right now. Mmm, a struct would be nice, yes. Maybe I will do that as a followup change. >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >> index b714f6194b6..38c5c9e0e00 100644 >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -3555,6 +3565,21 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) >> num_regs += i; >> } >> >> + /* Add the TLS register. */ >> + if (feature_tls != nullptr) >> + { >> + first_tls_regnum = num_regs; >> + >> + /* Validate the descriptor provides the mandatory MTE registers and >> + allocate their numbers. */ > > It should say TLS instead of MTE. And also adjust it to mention it is a > single register (at least for now?). Oops, did miss a MTE rename. Note that the MTE register set also contains a single register but still uses the plural (but I will fix this). If you think it's clearer I could remove the aarch64_tls_register_names and just call tdesc_numbered_register() once without the loop with the specific register name. >> + for (i = 0; i < ARRAY_SIZE (aarch64_tls_register_names); i++) >> + valid_p &= tdesc_numbered_register (feature_tls, tdesc_data.get (), >> + first_tls_regnum + i, >> + aarch64_tls_register_names[i]); >> + >> + num_regs += i; >> + } >> + >> if (!valid_p) >> return nullptr; > > We should probably error/warn loudly about the fact this feature lacks a > mandatory register. It requires the single "tpidr" register I think? >> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h >> index e416e346e9a..79821666ce6 100644 >> --- a/gdb/arch/aarch64.h >> +++ b/gdb/arch/aarch64.h >> @@ -29,6 +29,7 @@ struct aarch64_features >> bool sve = false; >> bool pauth = false; >> bool mte = false; >> + bool tls = false; >> }; > > The tls field doesn't seem to be set/used anywhere. I think it should be > set when we find the tls feature, and used during gdbserver linux/bsd > target description selection. Mm, yes. Arguably this structure should be the thing that replaces the parameters passed to aarch64_read_description(), though I think it would need the vq value and not just a bool for sve to serve that purpose. > Also, the patch adjusts the gdb/aarch64-linux-nat.c file, but doesn't > touch similar code in gdbserver's gdbserver/linux-aarch64-low.cc. We > also support NT_ARM_TLS there, but we don't export this particular > register through a feature. So for this particular patch, I'm just trying to add the arch feature itself. The later patches in the series (some of which don't seem to have made it to the mailing list actually, so I will resend), make use of it in aarch64-fbsd-tdep.c and aarch64-fbsd-nat.c. The changes to aarch64-linux-nat.c in this patch is just to support the new argument to aarch64_read_description(). > I think it makes sense to do so, though I understand that is outside the > boundary of bsd work. Let me know if you'd like me to enable that and > test before the patch goes in. I can probably take a stab at this based on the FreeBSD patches in the series (they should be fairly similar, at least for the tdep.c file). >> >> /* Create the aarch64 target description. A non zero VQ value indicates both >> @@ -36,10 +37,12 @@ struct aarch64_features >> an SVE Z register. HAS_PAUTH_P indicates the presence of the PAUTH >> feature. >> >> - MTE_P indicates the presence of the Memory Tagging Extension feature. */ >> + MTE_P indicates the presence of the Memory Tagging Extension feature. >> + >> + TLS_P indicates the presence of the Thread Local Storage feature. */ >> >> target_desc *aarch64_create_target_description (uint64_t vq, bool has_pauth_p, >> - bool mte_p); >> + bool mte_p, bool tls_p); >> >> /* Register numbers of various important registers. >> Note that on SVE, the Z registers reuse the V register numbers and the V >> @@ -84,6 +87,8 @@ enum aarch64_regnum >> #define AARCH64_PAUTH_CMASK_REGNUM(pauth_reg_base) (pauth_reg_base + 1) >> #define AARCH64_PAUTH_REGS_SIZE (16) >> >> +#define AARCH64_TPIDR_REGNUM(tls_reg_base) (tls_reg_base) >> + > > The purpose of similar macros is to index a register given a base > number. Given the TLS set is just a single register, I don't think this > macro should exist. We should use the value recorded in the tdep struct > instead. Ok, I was just following what had been done for MTE. I'm fine with just making it a single value. Alternatively, it might be nice to allocate a fixed value for the register in enum aarch64_regnum. I only picked a variable base due to copying what was done for the PAUTH registers. If adding a new AARCH64_TPIDR_REGNUM after AARCH64_SVE_VG_REGNUM is ok, it would be simpler (e.g. I could use a static regcache_map instead of having to construct one dynamically). > Also, I couldn't find any uses of AARCH64_TPIDR_REGNUM. Maybe I missed a > patch? It is used in one of the FreeBSD patches that didn't make it to the list (but was included in the cover letter). -- John Baldwin