From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64234 invoked by alias); 13 Jul 2018 16:24:59 -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 64072 invoked by uid 89); 13 Jul 2018 16:24:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,RCVD_IN_DNSWL_LOW,SPF_PASS,UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=nip X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Jul 2018 16:24:43 +0000 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w6DGNkcb102372 for ; Fri, 13 Jul 2018 12:24:42 -0400 Received: from e06smtp01.uk.ibm.com (e06smtp01.uk.ibm.com [195.75.94.97]) by mx0a-001b2d01.pphosted.com with ESMTP id 2k6vfjrpfu-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 13 Jul 2018 12:24:41 -0400 Received: from localhost by e06smtp01.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 13 Jul 2018 17:24:39 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197) by e06smtp01.uk.ibm.com (192.168.101.131) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Fri, 13 Jul 2018 17:24:35 +0100 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w6DGOY4o32112670 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 13 Jul 2018 16:24:34 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EAD965205A; Fri, 13 Jul 2018 19:24:54 +0100 (BST) Received: from oc3748833570.ibm.com (unknown [9.167.239.119]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id CD7855204E; Fri, 13 Jul 2018 19:24:54 +0100 (BST) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id 9A714D80276; Fri, 13 Jul 2018 18:24:33 +0200 (CEST) Subject: Re: [PATCH 13/17] [PowerPC] Add support for HTM registers To: pedromfc@linux.ibm.com (Pedro Franco de Carvalho) Date: Fri, 13 Jul 2018 16:24:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, edjunior@gmail.com In-Reply-To: <20180713135226.2321-14-pedromfc@linux.ibm.com> from "Pedro Franco de Carvalho" at Jul 13, 2018 10:52:22 AM MIME-Version: 1.0 x-cbid: 18071316-4275-0000-0000-00000298BC57 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18071316-4276-0000-0000-000037A0C58C Message-Id: <20180713162433.9A714D80276@oc3748833570.ibm.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit X-SW-Source: 2018-07/txt/msg00417.txt.bz2 Pedro Franco de Carvalho wrote: > The set of checkpointed general-purpose registers is returned by the > linux kernel in the same format as the regular general-purpose > registers, defined in struct pt_regs. However, the architecture > specifies that only some of the registers present in pt_regs are > checkpointed (GPRs 0-31, CR, XER, LR and CTR). The kernel fills the > slots for other registers with other info (e.g., nip is filled with > the contents of TFHAR). GDB doesn't handle these other registers. This > means that core files generated by GDB will show values of zero for > these registers, while core files generated by the kernel will have > other values. Core files generated by the kernel have a note section > for checkpointed GPRs with the same size for both 32-bit and 64-bit > threads, and the values for the registers of a 32-bit thread are > squeezed in the first half, with no useful data in the second > half. GDB generates a smaller note section for 32-bit threads, but can > read both sizes. Well, this is weird. Either the kernel should be fixed to use the smaller size as well, or else GDB should also generate the large section (and zero out the second half). > Access to the checkpointed CR (condition register) can be > confusing. The architecture only specifies that CR fields 1 to 7 (the > 24 least significant bits) are checkpointed, but the kernel provides > all 8 fields (32 bits). The value of field 0 is not masked by ptrace, > so it will sometimes show the result of some kernel operation, > probably treclaim., which sets this field. Huh. We shold report this to the kernel people. > The checkpointed registers are marked not to be saved and > restored. Inferior function calls during an active transaction don't > work well, and it's unclear what should be done in this case. TEXASR > and TFIAR can be altered asynchronously, during transaction failure > recording, so they are also not saved and restored. For consistency > neither is TFHAR. > > Record and replay also doesn't work well when transactions are > involved. This patch doesn't address this, so the values of the HTM > SPRs will sometimes be innacurate when the record/relay target is > enabled. For instance, executing a "tbegin." alters TFHAR and TEXASR, > but these changes are not currently recorded. > > Because the checkpointed registers are only available when a > transaction is active (or suspended), ptrace can return ENODATA when > gdb tries to read these registers and the inferior is not in a > transactional state. The registers are set to the unavailable state > when this happens. When gbd tries to write to one of these registers, > and it is unavailable, an error is raised. When gdb tries to store to > all registers in one go (when store_registers called with -1), the > state of these registers is checked. If they are all unavailable, no > write is attempted, so that writes to all the other registers are > unaffected. If all are available, the write is attempted. Otherwise an > internal_error is raised. This all makes sense to me. > Like for the previous patches, configure.srv is updated here to avoid > breaking the build at this patch. Same comment as before :-) > + tdep->wordsize == 4? PPC32_LINUX_SIZEOF_CGPRREGSET > + : PPC64_LINUX_SIZEOF_CGPRREGSET, Use parentheses for formatting. > +/* Get the checkpointed GPR regset that matches the target wordsize > + and byteorder of GDBARCH. The collect function of the returned > + regset will clear the buffer passed to it before collecting the > + registers to 0 if CLEAR_ON_COLLECT is true and the collect function > + is called with regno == -1 (collect all regs in the regset). This > + is useful for generating core files without garbage in the register > + slots that are not seen by gdb. When collecting for storing the > + registers in the inferior, these slots should not be cleared. */ I'm not sure I like this approach. The problem is only about generating core files, right? Why don't we instead simply change linux-tdep.c:linux_collect_regset_section_cb from buf = (char *) xmalloc (size); regset->collect_regset (regset, data->regcache, -1, buf, size); to use xcalloc instead? This seems prudent anyway. (The same applies to the similar code in fbsd-tdep.c.) Then we can probably even get rid of ppc_linux_collect_vrregset as well. > return vsx_pseudo_register_read (gdbarch, regcache, reg_nr, buffer); > else if (IS_EFP_PSEUDOREG (tdep, reg_nr)) > return efpr_pseudo_register_read (gdbarch, regcache, reg_nr, buffer); > + else if (IS_CVSX_PSEUDOREG (tdep, reg_nr)) > + return cvsx_pseudo_register_read (gdbarch, regcache, reg_nr, buffer); Hmm. The DFP and EFP pseudo registers are simply GDB views onto the normal FP / VR register set. For completeness, shouldn't we also offer similar views onto the checkpointed register set? > + if (IS_VSX_PSEUDOREG (tdep, reg_nr)) > + { > + reg_index = reg_nr - tdep->ppc_vsr0_regnum; > + vr0 = tdep->ppc_vr0_regnum; > + fp0 = tdep->ppc_fp0_regnum; > + vsr0_upper = tdep->ppc_vsr0_upper_regnum; > + } > + else > + { > + reg_index = reg_nr - tdep->ppc_cvsr0_regnum; > + vr0 = PPC_CVR0_REGNUM; > + fp0 = PPC_CF0_REGNUM; > + vsr0_upper = PPC_CVS0H_REGNUM; > + } It looks a bit odd that we need to use variable numbers for the vr0 / fp0 / vsr0 registers, but can hard-code the numbers for the checkpointed register set. Why is this the case? Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com