From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 460B83858438 for ; Tue, 27 Sep 2022 16:41:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 460B83858438 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 711C01063; Tue, 27 Sep 2022 09:41:59 -0700 (PDT) Received: from [10.2.78.76] (unknown [10.2.78.76]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 275B03F792; Tue, 27 Sep 2022 09:41:52 -0700 (PDT) Message-ID: Date: Tue, 27 Sep 2022 17:41:50 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] gdb/arm: Handle lazy FPU register stacking Content-Language: en-GB To: =?UTF-8?Q?Torbj=c3=b6rn_SVENSSON?= , binutils@sourceware.org Cc: tom@tromey.com, brobecker@adacore.com References: <20220927142028.193446-1-torbjorn.svensson@foss.st.com> From: Richard Earnshaw In-Reply-To: <20220927142028.193446-1-torbjorn.svensson@foss.st.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3497.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: I think this would be better sent to gdb-patches@sourceware.com. R. On 27/09/2022 15:20, Torbjörn SVENSSON via Binutils wrote: > Read LSPEN, ASPEN and LSPACT bits from FPCCR and use them for the > first exception frame. All frames after will always have the FPU > registers on the stack, regadless if lazy stacking is active or > inactive. See "Lazy context save of FP state", in B1.5.7, also ARM > AN298, supported by Cortex-M4F architecture for details on lazy FPU > register stacking. The same conditions are valid for other Cortex-M > cores with FPU. > > This patch has been verified on a STM32F4-Discovery board by: > a) writing a non-zero value (lets use 0x1122334455667788 as an > example) to all the D-registers in the main function > b) configured the SysTick to fire > c) in the SysTick_Handler, write some other value (lets use > 0x0022446688aaccee as an example) to one of the D-registers (D0 as > an example) and then do "SVC #0" > d) in the SVC_Handler, write some other value (lets use > 0x0099aabbccddeeff) to one of the D-registers (D0 as an example) > > In GDB, suspend the execution in the SVC_Handler function and compare > the value of the D-registers for the SVC_handler frame and the > SysTick_Handler frame. With the patch, the value of the modified > D-register (D0) should be the new value (0x009..eff) on the > SVC_Handler frame, and the intermediate value (0x002..cee) for the > SysTick_Handler frame. Now compare the D-register value for the > SysTick_Handler frame and the main frame. The main frame should > have the initial value (0x112..788). > > Signed-off-by: Torbjörn SVENSSON > Signed-off-by: Yvan ROUX > --- > gdb/arm-tdep.c | 62 +++++++++++++++++++++++++++++++++++------------- > gdb/frame.c | 2 ++ > gdb/observable.c | 1 + > gdb/observable.h | 3 +++ > 4 files changed, 52 insertions(+), 16 deletions(-) > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index 2810232fcb8..43ce1a45782 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -68,6 +68,7 @@ > #endif > > static bool arm_debug; > +static bool force_fpu_regs_from_stack = false; > > /* Print an "arm" debug statement. */ > > @@ -3337,6 +3338,17 @@ struct frame_unwind arm_stub_unwind = { > arm_stub_unwind_sniffer > }; > > + > +/* The first time an exception frame is seen, the lazy stacking of the FPU > + registers should be considered. Any following exception frames should not > + consider the lazy stacking as the values will be put on the stack before > + branching to the nested exception handler. */ > +static void > +reset_force_fpu_regs_from_stack () > +{ > + force_fpu_regs_from_stack = false; > +} > + > /* Put here the code to store, into CACHE->saved_regs, the addresses > of the saved registers of frame described by THIS_FRAME. CACHE is > returned. */ > @@ -3593,22 +3605,28 @@ arm_m_exception_cache (struct frame_info *this_frame) > gdb_assert (safe_read_memory_unsigned_integer (FPCCR, > ARM_INT_REGISTER_SIZE, > byte_order, &fpccr)); > + bool fpccr_aspen = bit (fpccr, 31); > + bool fpccr_lspen = bit (fpccr, 30); > bool fpccr_ts = bit (fpccr, 26); > + bool fpccr_lspact = bit (fpccr, 0); > > - /* This code does not take into account the lazy stacking, see "Lazy > - context save of FP state", in B1.5.7, also ARM AN298, supported > - by Cortex-M4F architecture. > - To fully handle this the FPCCR register (Floating-point Context > - Control Register) needs to be read out and the bits ASPEN and > - LSPEN could be checked to setup correct lazy stacked FP registers. > - This register is located at address 0xE000EF34. */ > + /* The rule is that the lazy bit is only valid for the first frame, > + all other frames will have the fpu registers stacked. Just > + consider the LSPEN, ASPEN and LSPACT bits for the first frame. > + See "Lazy context save of FP state", in B1.5.7, also ARM AN298, > + supported by Cortex-M4F architecture for details. */ > + bool lazy_fpu_stacking_active = (fpccr_aspen && fpccr_lspen > + && fpccr_lspact); > > /* Extended stack frame type used. */ > - CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20; > - for (int i = 0; i < 8; i++) > + if (force_fpu_regs_from_stack || !lazy_fpu_stacking_active) > { > - cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); > - addr += 8; > + CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20; > + for (int i = 0; i < 8; i++) > + { > + cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); > + addr += 8; > + } > } > cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp > + sp_r0_offset + 0x60); > @@ -3617,11 +3635,14 @@ arm_m_exception_cache (struct frame_info *this_frame) > && fpccr_ts) > { > /* Handle floating-point callee saved registers. */ > - addr = unwound_sp + sp_r0_offset + 0x68; > - for (int i = 8; i < 16; i++) > + if (force_fpu_regs_from_stack || !lazy_fpu_stacking_active) > { > - cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); > - addr += 8; > + CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x68; > + for (int i = 8; i < 16; i++) > + { > + cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr); > + addr += 8; > + } > } > > arm_cache_set_active_sp_value (cache, tdep, > @@ -3633,6 +3654,10 @@ arm_m_exception_cache (struct frame_info *this_frame) > arm_cache_set_active_sp_value (cache, tdep, > unwound_sp + sp_r0_offset + 0x68); > } > + > + /* Lazy stacking handled, all other frames will have the FPU > + registers on the stack. */ > + force_fpu_regs_from_stack = true; > } > else > { > @@ -3746,6 +3771,7 @@ arm_m_exception_prev_register (struct frame_info *this_frame, > return frame_unwind_got_constant (this_frame, ARM_PS_REGNUM, xpsr); > } > > + > return trad_frame_get_prev_register (this_frame, cache->saved_regs, > prev_regnum); > } > @@ -10547,7 +10573,11 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > /* Add some default predicates. */ > if (is_m) > - frame_unwind_append_unwinder (gdbarch, &arm_m_exception_unwind); > + { > + gdb::observers::frame_cache_purged.attach > + (reset_force_fpu_regs_from_stack, "arm_m_force_fpu_regs_from_stack"); > + frame_unwind_append_unwinder (gdbarch, &arm_m_exception_unwind); > + } > frame_unwind_append_unwinder (gdbarch, &arm_stub_unwind); > dwarf2_append_unwinders (gdbarch); > frame_unwind_append_unwinder (gdbarch, &arm_exidx_unwind); > diff --git a/gdb/frame.c b/gdb/frame.c > index fc883b3239e..fdbaa946a34 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -2007,6 +2007,8 @@ reinit_frame_cache (void) > select_frame (NULL); > frame_stash_invalidate (); > > + gdb::observers::frame_cache_purged.notify (); > + > frame_debug_printf ("generation=%d", frame_cache_generation); > } > > diff --git a/gdb/observable.c b/gdb/observable.c > index 0bc8697f137..feee2b3f0a4 100644 > --- a/gdb/observable.c > +++ b/gdb/observable.c > @@ -81,6 +81,7 @@ DEFINE_OBSERVABLE (gdb_exiting); > DEFINE_OBSERVABLE (connection_removed); > DEFINE_OBSERVABLE (target_pre_wait); > DEFINE_OBSERVABLE (target_post_wait); > +DEFINE_OBSERVABLE (frame_cache_purged); > > } /* namespace observers */ > } /* namespace gdb */ > diff --git a/gdb/observable.h b/gdb/observable.h > index 796bf2a43c6..d0c9fc90a66 100644 > --- a/gdb/observable.h > +++ b/gdb/observable.h > @@ -263,6 +263,9 @@ extern observable target_pre_wait; > /* About to leave target_wait (). */ > extern observable target_post_wait; > > +/* Observer called when the frame cache gets purged. */ > +extern observable<> frame_cache_purged; > + > } /* namespace observers */ > > } /* namespace gdb */