From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55890 invoked by alias); 16 Aug 2018 16:53:05 -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 55880 invoked by uid 89); 16 Aug 2018 16:53:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=1.4 required=5.0 tests=AWL,BAYES_50,SPF_HELO_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=transactions, Hardware, machado, Machado X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 16 Aug 2018 16:53:02 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 630804012992; Thu, 16 Aug 2018 16:53:01 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6BD4B1C736; Thu, 16 Aug 2018 16:53:00 +0000 (UTC) Subject: Re: [PATCH v4 12/12] [PowerPC] Add support for HTM registers To: Pedro Franco de Carvalho , gdb-patches@sourceware.org References: <20180815000608.26840-1-pedromfc@linux.ibm.com> <20180815000608.26840-13-pedromfc@linux.ibm.com> Cc: uweigand@de.ibm.com, edjunior@gmail.com From: Pedro Alves Message-ID: <540dad8e-f9a2-8173-a556-c919fbeeb43f@redhat.com> Date: Thu, 16 Aug 2018 16:53:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180815000608.26840-13-pedromfc@linux.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-08/txt/msg00403.txt.bz2 On 08/15/2018 01:06 AM, Pedro Franco de Carvalho wrote: > From: Edjunior Barbosa Machado > > This patch adds support for Hardware Transactional Memory registers > for the powerpc linux native and core file targets, and for the > pwoerpc linux server stub. > > These registers include both the HTM special-purpose registers (TFHAR, > TEXASR and TFIAR) as well as the set of registers that are > checkpointed (saved) when a transaction is initiated, which the > processor restores in the event of a transaction failure. > > 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. I won't pretend to understand the above fully (not an Power expert), but the question I ended up with was, after all this, will the GDB-generated files end up looking like kernel-generated cores? Or are there plans for that? > > The checkpointed XER is required to be 32-bit in the target > description documentation, even though the more recent ISAs define it > as 64-bit wide, since the high-order 32-bits are reserved, and because > in Linux there is no way to get a 64-bit checkpointed XER for 32-bit > threads. If this changes in the future, the target description > feature requirement can be relaxed to allow for a 64-bit checkpointed > XER. > > 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. > > 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. > > The "fill" functions for checkpointed register sets in the server stub > are not implemented for the same reason as for the EBB register set, > since ptrace can also return ENODATA for checkpointed regsets. > > Just like for the EBB registers, tracepoints will not mark the > checkpointed registers as unavailable if the inferior was not in a > transaction, so their content will also show 0 instead of > when inspecting trace data. > > The new tests record the values of the regular registers before > stepping the inferior through a "tbegin." instruction to start a > transaction, then the checkpointed registers are checked against the > recorded pre-transactional values. New values are written to the > checkpointed registers and recorded, the inferior continues until the > transaction aborts (which is usually immediately when it is resumed), > and the regular registers are checked against the recorded values, > because the abort should have reverted the registers to these values. I'd find it useful to have an intro description like this last paragraph above as a comment in the .exp file directly. Likewise other explanations throughout the series felt like would be useful in the code, but I really haven't double checked to see if they were there already. Speaking of comments, I noticed that the series adds a number of functions and globals with no leading comment. Thanks, Pedro Alves