From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from angie.orcam.me.uk (angie.orcam.me.uk [78.133.224.34]) by sourceware.org (Postfix) with ESMTP id 005DD3858D38 for ; Mon, 3 Oct 2022 15:13:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 005DD3858D38 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id DF0F192009C; Mon, 3 Oct 2022 17:13:08 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id D888B92009B; Mon, 3 Oct 2022 16:13:08 +0100 (BST) Date: Mon, 3 Oct 2022 16:13:08 +0100 (BST) From: "Maciej W. Rozycki" To: Andrew Burgess cc: Dragan Mladjenovic , gdb-patches@sourceware.org, Chao-ying Fu Subject: Re: [PATCH] gdb: mips: Add MIPSR6 support In-Reply-To: <87edvpgedx.fsf@redhat.com> Message-ID: References: <20220528150340.4707-1-Dragan.Mladjenovic@syrmia.com> <87edvpgedx.fsf@redhat.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-1162.8 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Mon, 03 Oct 2022 15:13:13 -0000 On Mon, 3 Oct 2022, Andrew Burgess wrote: > > + /* Step through the forbidden slot to avoid repeated exceptions we do > > + not currently have access to the BD bit when hitting a breakpoint > > + and therefore cannot tell if the breakpoint hit on the branch or the > > + forbidden slot. */ > > + /* delay_slot_size = 0; */ > > It is probably just because I don't understand the MIPS architecture > well enough, but I don't see the connection between the text of the > comment, and the commented out source line. > > Assuming the comment is explaining why the source line is commented out, > I'd like, at a minimum, to see the source line included within the same > comment block. > > In general though, I'm not a fan of commented out code. The question I > would ask is, when will it be un-commented? If the answer is never, > then lets just remove the commented code. > > The next question I have is, if we can't include this code, then is > something going to go wrong? What would be the expected consequence of > this thing going wrong? > > My ideal comment would include a description of what might go wrong, why > we can't solve the problem right now, and what we would need to do to > fix it, in this case it seems like 'check the BD bit' would be part of > the solution. This way, when someone hits the problem and tries to > debug it, then they will see the comment and thing, "hey, that's the > exact problem I'm seeing....". FWIW there should be no problem with accessing the BD bit, which is in the CP0 Cause register. It is a privileged register, but it's made available to the debugger at least by Linux, and I believe FreeBSD as well, with the BD bit faithfully reported. It's also directly accessible with bare metal targets, though I can imagine there might be complications depending on what debug firmware does (MDI comes to mind, though I can't remember the details offhand), but addressing that can be probably delegated to the relevant bare metal debug stub. Historically we've been avoiding placing breakpoints (either regular ones or temporary single-stepping ones) in branch delay slots though, so why should we start with the inclusion of MIPSr6 support? I guess we could (and that could help in some debug scenarios), provided such a change has been sufficiently verified, but that ought to be a separate submission. In any case we have code already for delay slot avoidance, so I don't think there's a need for R6 to make any extra arrangements apart from letting that code know what the extra instructions are in R6 that have a branch delay slot (either a regular one or a "forbidden" one). As I say, FWIW. Maciej