From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43132 invoked by alias); 10 Nov 2016 12:43:22 -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 43103 invoked by uid 89); 10 Nov 2016 12:43:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=dating, complication, oddities, dealings X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 10 Nov 2016 12:43:09 +0000 Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19]) by Forcepoint Email with ESMTPS id 9A39E50E46D15; Thu, 10 Nov 2016 12:43:02 +0000 (GMT) Received: from [10.20.78.69] (10.20.78.69) by HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.294.0; Thu, 10 Nov 2016 12:43:04 +0000 Date: Thu, 10 Nov 2016 12:43:00 -0000 From: "Maciej W. Rozycki" To: Yao Qi CC: Bhushan Attarde , "gdb-patches@sourceware.org" , Matthew Fortune , James Hogan , Andrew Bennett , Jaydeep Patil Subject: Re: [PATCH 01/24] MIPS: Handle run-time reconfigurable FPR size In-Reply-To: Message-ID: References: <1467038991-6600-1-git-send-email-bhushan.attarde@imgtec.com> User-Agent: Alpine 2.20.17 (DEB 179 2016-10-28) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2016-11/txt/msg00246.txt.bz2 Yao, I'll chime in in Bhushan's stead a bit as I've beaten the oddities of the MIPS FPU to death throughout my dealings with the architecture (Bhushan, you're of course welcome to reply too if you have anything to add). On Tue, 8 Nov 2016, Yao Qi wrote: > > This is implemented by retaining the raw register size for the FPRs at its > > native size (64-bits; this is required for remote packet offsets to work > > out correctly) and then truncating the cooked register size or not as > > required. I have skipped `maintenance print registers' dumps here for > > brevity, the types flip between "double" and "float" as expected. > > > > If I understand you correctly, 64-bits are still transferred in remote protocol > if FPRs are 32-bit. Nope. In the FR=0 mode existing XML-described stubs present a file of 32 32-bit FPRs; this applies to MIPS32 targets only as so far we've had an assumption that MIPS32 <=> FR=0 and MIPS64 <=> FR=1. I think switching the view at this time would be an undesireable complication, as it would be yet another variant we'd have to handle forever along with the new one. FWIW I think the FR=0 mode should have been chosen to present the FPR file as 16 64-bit FPRs instead, with the cooked register view used to access register halves if required, but then I think a further complication would arise with non-XML-described stubs. > > The change also supports both XML and non-XML targets, but currently > > bare-iron targets for which this update has any significant meaning do not > > really support XML. Any XML target is supposed to always provide an FPU > > description that matches the current setting of CP0.Status.FR, the new > > code verifies this is always the case and rejects the description as > > invalid otherwise. > > How do I understand "Any XML target is supposed to always provide > an FPU description that matches the current setting of CP0.Status.FR"? > I don't see how it is done in current GDB/GDBserver. I do hope the last patch which adds `gdbserver' features en masse, which I didn't get to to looking through yet, has it (although I think the individual features should be included along with corresponding base target and native support instead). > "reconfigure FPRs" is a target description change to me, instead of a > regcache change. The regcache change is partly historic, because parts of the series (dating back to 2005) were implemented even before we had XML descriptions, and an FR transition is also supposed to work with non-XML-described MIPS64 bare-metal targets (where the FPR file is presented as 32 64-bit FPRs regardless of the FR setting). I do agree a target description should take precedence where available, and certainly with Linux targets where you cannot get a MIPS64 FR=0 configuration or a non-XML-described MIPS32 FR=1 configuration). What do you propose if we don't have a target description though? We can't handle MIPS32 FR=1, because stubs won't transfer the upper halves of FPRs, but we can handle MIPS64 FR=0, because we can simply ignore the irrelevant upper halves. > There are two different ways to support handling target description > change in general. Other thoughts are welcome too. > > 1) > - Implement target hook to_thread_architecture in ARCH-linux-nat.c, > in which we can use ptrace to read the register A which indicates > the target description is changed or not. In MIPS, register A is > CP0.Status. Create target info and return the right gdbarch. > [In MIPS, we have one gdbarch for 32-bit FPR and one gdbarch for > 64-bit FPR] > - Add register A to expedited registers, which is included in the stop > reply from GDBserver. I think I'd prefer this to be the $fp_mode register if available (as it'll include the FRE setting as well), and the $status register otherwise. Linux kernels which support the FRE setting will always make $fp_mode available, as it'll be a part of the same change (unless someone sneaks a broken patch through behind my back, as I still yet have to see the kernel parts of this framework, let alone review them). > - Add new gdbarch method target_description_changed_p, and its > parameter is a vector of expedited registers got in > remote.c:process_stop_reply. In default, it returns false. In > MIPS, we can tell whether target description is changed by the > value of and current gdbarch. > - In remote.c:process_stop_reply, if > gdbarch_target_description_changed_p returns true, invalidate all > regcaches, request target description from gdbserver again, and get > the updated target description. > > 2) > - Add enum TARGET_WAITKIND_ARCH_CHANGED, > - In ARCH-linux-nat.c, interpret/override linux_nat_wait. If > linux_nat_wait returns, and there is a real event of stop, call > ptrace to get the register A value, if it is different from what we > remember in current gdbarch, mark the event pending and return > TARGET_WAITKIND_ARCH_CHANGED, > - In remote, add a new stop reply, T00arch, for example, > https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html > If remote.c:process_stop_reply sees T00arch, return > TARGET_WAITKIND_ARCH_CHANGED. > - In gdb core, if TARGET_WAITKIND_ARCH_CHANGED is reported by > target_wait, call target_find_description, to update target > description. > - In gdbserver, add target method target_stopped_by_changed_arch, > and use it in remote-utils.c:prepare_resume_reply. Get register A > value by ptrace too, and return true if it is different from the > knowledge in process_info.tdesc. The rest looks like a good plan to me, although I think I like your #1 proposal a bit better. Maciej