From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x843.google.com (mail-qt1-x843.google.com [IPv6:2607:f8b0:4864:20::843]) by sourceware.org (Postfix) with ESMTPS id 43FA13894C35 for ; Mon, 7 Dec 2020 15:58:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 43FA13894C35 Received: by mail-qt1-x843.google.com with SMTP id z3so9678109qtw.9 for ; Mon, 07 Dec 2020 07:58:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=MOgnLAOTRte1qV8kLQFCsF+fSM8hugA7kyc3siqZeg0=; b=c16YN27pb4aca8Na+0sB9n1OTbAapKumk+if9wUn4CPVIcdI4AFYROqUFpHSJ3Eis6 GBD78mjlrznIXAAWbnVB7Z9f0yiDUo8km48msLxK652JDLrVqRE8f/RGR4ObOavwtyaX /OixBWIzBGo00BBoLujuruKjB9/3fyBnc4+gm8/ByImh5FttykA84gmLKZ8Xu/jtI49A gmFYsHWM79sXxI0oIMijvVWkwCUsSzYPyqn08ww8wWEYJFopRTFz/iTDfWh59PPAehqV x17rkGS+jgz9nNg+LB8GtCenOgg9HcZ/TaGdUd/qk8ySVbugiLDjWJt6k32kYUYG75ru 4aOg== X-Gm-Message-State: AOAM530IkJX3O69Iaq4u2RE5M5tOn8RRLqrkml67a7lqO3vzITD4yCbP brKNyVTcB8Hqqt8E26RrrzsFKg== X-Google-Smtp-Source: ABdhPJwIf10Pr6l6qLVpvLi++O4ZFkcFpzHUoE2sPwFcJqVW8oqSylBrPjo0UbvkPPFxcTC5Z7IpuQ== X-Received: by 2002:ac8:c2:: with SMTP id d2mr24742865qtg.207.1607356703548; Mon, 07 Dec 2020 07:58:23 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:370e:2cb7:754f:737c:109f? ([2804:7f0:8284:370e:2cb7:754f:737c:109f]) by smtp.gmail.com with ESMTPSA id d190sm463644qkf.112.2020.12.07.07.58.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Dec 2020 07:58:23 -0800 (PST) Subject: Re: [PATCH 5/8] gdb/riscv: introduce bare metal core dump support To: Andrew Burgess Cc: binutils@sourceware.org, gdb-patches@sourceware.org, Paul Mathieu , Fredrik Hederstierna References: <5ba628c1042f3000947a1f2a6f9e24cf46573fa3.1606930261.git.andrew.burgess@embecosm.com> <20201207151702.GK2729@embecosm.com> From: Luis Machado Message-ID: <36bed17a-6012-a5d9-a29c-64e9cbbef640@linaro.org> Date: Mon, 7 Dec 2020 12:58:19 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201207151702.GK2729@embecosm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPAM_BODY, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Dec 2020 15:58:26 -0000 On 12/7/20 12:17 PM, Andrew Burgess wrote: > * Luis Machado [2020-12-02 15:12:26 -0300]: > >> CC-ing paulmathieu@google.com and fredrik.hederstierna@verisure.com, who >> were also looking into supporting bare metal ARM core files. >> >> Just a quick note... >> >> Back in October we pondered about this and it looked reasonable, at the >> time, to put some effort into documenting the format (unlike the current >> Linux core file format, which lacks good documentation). >> >> My take on it is that we need to settle on a format that works for multiple >> architectures. > > Hi Luis, > > Thanks for your feedback, I'd love to better understand what you are > proposing here. Sure. What I'm proposing here is the same I proposed in this thread about ARM bare metal core files... https://sourceware.org/pipermail/gdb-patches/2020-October/172617.html ... and also suggested by Simon here: https://sourceware.org/pipermail/gdb-patches/2020-October/172270.html In summary, we should document what a bare metal core file is, what pieces it is expected to contain (registers, target descriptions, hardware information, execution environment etc) and what format it will be in. > > Could you expand a little on why we need a format that supports > multiple architectures (i.e. what benefits will this bring)? And how > this would be different to the approach taken here Having a common format makes it easier to maintain the code and expand the features when needed. This is not different from the Linux core file we have today. The Linux core file is a common format. But unlike Linux core files, which have less than ideal documentation and specifications, we want to take this opportunity to start clean and to document everything required to have a proper bare metal core file. I think this initial definition and documentation will benefit developers moving forward. Does that make it more clear? > > Thanks, > Andrew > >> >> On 12/2/20 2:39 PM, Andrew Burgess wrote: >>> This commit adds the ability for bare metal RISC-V target to generate >>> core files from within GDB. >>> >>> The intended use case is that a user will connect to a remote bare >>> metal target, debug up to some error condition, then generate a core >>> file in the normal way using: >>> >>> (gdb) generate-core-file >>> >>> This core file can then be used to revisit the state of the remote >>> target without having to reconnect to the remote target. >>> >>> The core file creation is all placed into a new file >>> riscv-none-tdep.c, this follows the existing naming pattern as >>> riscv-linux-tdep.c, and riscv-fbsd-tdep.c, where 'none' is the ABI >>> name. >>> >>> Currently only the x-regs and f-regs (if present) are written out, and >>> the formatting follows the Linux layout, rather than inventing yet >>> another layout. Future commits will add support for writing out the >>> CSRs. >>> >>> gdb/ChangeLog: >>> >>> * Makefile.in (ALL_TARGET_OBS): Add riscv-none-tdep.o. >>> (ALLDEPFILES): Add riscv-none-tdep.c. >>> * configure.tgt (riscv*-*-*): Include riscv-none-tdep.c. >>> * riscv-none-tdep.c: New file. >>> --- >>> gdb/ChangeLog | 8 + >>> gdb/Makefile.in | 2 + >>> gdb/configure.tgt | 2 +- >>> gdb/riscv-none-tdep.c | 345 ++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 356 insertions(+), 1 deletion(-) >>> create mode 100644 gdb/riscv-none-tdep.c >>> >>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in >>> index a86e8d648e6..f515670d03d 100644 >>> --- a/gdb/Makefile.in >>> +++ b/gdb/Makefile.in >>> @@ -807,6 +807,7 @@ ALL_TARGET_OBS = \ >>> ravenscar-thread.o \ >>> riscv-fbsd-tdep.o \ >>> riscv-linux-tdep.o \ >>> + riscv-none-tdep.o \ >>> riscv-ravenscar-thread.o \ >>> riscv-tdep.o \ >>> rl78-tdep.o \ >>> @@ -2269,6 +2270,7 @@ ALLDEPFILES = \ >>> riscv-fbsd-tdep.c \ >>> riscv-linux-nat.c \ >>> riscv-linux-tdep.c \ >>> + riscv-none-tdep.c \ >>> riscv-ravenscar-thread.c \ >>> riscv-tdep.c \ >>> rl78-tdep.c \ >>> diff --git a/gdb/configure.tgt b/gdb/configure.tgt >>> index 6e039838748..ad88ddd9302 100644 >>> --- a/gdb/configure.tgt >>> +++ b/gdb/configure.tgt >>> @@ -85,7 +85,7 @@ ia64*-*-*) >>> ;; >>> riscv*-*-*) >>> - cpu_obs="riscv-tdep.o arch/riscv.o \ >>> + cpu_obs="riscv-tdep.o riscv-none-tdep.o arch/riscv.o \ >>> ravenscar-thread.o riscv-ravenscar-thread.o";; >>> x86_64-*-*) >>> diff --git a/gdb/riscv-none-tdep.c b/gdb/riscv-none-tdep.c >>> new file mode 100644 >>> index 00000000000..0a4215a60e9 >>> --- /dev/null >>> +++ b/gdb/riscv-none-tdep.c >>> @@ -0,0 +1,345 @@ >>> +/* Copyright (C) 2020 Free Software Foundation, Inc. >>> + >>> + This file is part of GDB. >>> + >>> + This program is free software; you can redistribute it and/or modify >>> + it under the terms of the GNU General Public License as published by >>> + the Free Software Foundation; either version 3 of the License, or >>> + (at your option) any later version. >>> + >>> + This program is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + GNU General Public License for more details. >>> + >>> + You should have received a copy of the GNU General Public License >>> + along with this program. If not, see . */ >>> + >>> +/* This file contain code that is specific for bare-metal RISC-V targets. */ >>> + >>> +#include "defs.h" >>> +#include "inferior.h" >>> +#include "gdbcore.h" >>> +#include "target.h" >>> +#include "arch-utils.h" >>> +#include "regcache.h" >>> +#include "osabi.h" >>> +#include "riscv-tdep.h" >>> +#include "elf-bfd.h" >>> +#include "regset.h" >>> + >>> +/* Function declarations. */ >>> + >>> +static void riscv_collect_regset_section_cb >>> + (const char *sect_name, int supply_size, int collect_size, >>> + const struct regset *regset, const char *human_name, void *cb_data); >>> + >>> +/* Function Definitions. */ >>> + >>> +/* Called to figure out a target description for the corefile being read. >>> + If we get here then the corefile didn't have a target description >>> + embedded inside it, so we need to figure out a default description >>> + based just on the properties of the corefile itself. */ >>> + >>> +static const struct target_desc * >>> +riscv_core_read_description (struct gdbarch *gdbarch, >>> + struct target_ops *target, >>> + bfd *abfd) >>> +{ >>> + error (_("unable to figure out target description for RISC-V core files")); >>> + return nullptr; >>> +} >>> + >>> +/* Determine which signal stopped execution. */ >>> + >>> +static int >>> +find_signalled_thread (struct thread_info *info, void *data) >>> +{ >>> + if (info->suspend.stop_signal != GDB_SIGNAL_0 >>> + && info->ptid.pid () == inferior_ptid.pid ()) >>> + return 1; >>> + >>> + return 0; >>> +} >>> + >>> +/* Structure for passing information from riscv_corefile_thread via an >>> + iterator to riscv_collect_regset_section_cb. */ >>> + >>> +struct riscv_collect_regset_section_cb_data >>> +{ >>> + riscv_collect_regset_section_cb_data >>> + (struct gdbarch *gdbarch, const struct regcache *regcache, >>> + ptid_t ptid, bfd *obfd, enum gdb_signal stop_signal, >>> + gdb::unique_xmalloc_ptr *note_data, int *note_size) >>> + : gdbarch (gdbarch), regcache (regcache), obfd (obfd), >>> + note_data (note_data), note_size (note_size), >>> + stop_signal (stop_signal) >>> + { >>> + /* The LWP is often not available for bare metal target, in which case >>> + use the tid instead. */ >>> + if (ptid.lwp_p ()) >>> + lwp = ptid.lwp (); >>> + else >>> + lwp = ptid.tid (); >>> + } >>> + >>> + struct gdbarch *gdbarch; >>> + const struct regcache *regcache; >>> + bfd *obfd; >>> + gdb::unique_xmalloc_ptr *note_data; >>> + int *note_size; >>> + long lwp; >>> + enum gdb_signal stop_signal; >>> + bool abort_iteration = false; >>> +}; >>> + >>> +/* Records information about the single thread INFO into *NOTE_DATA, and >>> + updates *NOTE_SIZE. OBFD is the core file being generated. GDBARCH is >>> + the architecture the core file is being created for. */ >>> + >>> +static void >>> +riscv_corefile_thread (struct gdbarch *gdbarch, bfd *obfd, >>> + struct thread_info *info, >>> + gdb::unique_xmalloc_ptr *note_data, >>> + int *note_size) >>> +{ >>> + struct regcache *regcache >>> + = get_thread_arch_regcache (info->inf->process_target (), >>> + info->ptid, gdbarch); >>> + >>> + /* Ideally we should be able to read all of the registers known to this >>> + target. Unfortunately, sometimes targets advertise CSRs that can't be >>> + read. We don't want these registers to prevent a core file being >>> + dumped, so we fetch the registers one by one here, and ignore any >>> + errors. This does mean that the register will show up as zero in the >>> + core dump, which might be confusing, but probably better than being >>> + unable to dump a core file. */ >>> + for (int regnum = 0; regnum < gdbarch_num_regs (gdbarch); ++regnum) >>> + { >>> + try >>> + { >>> + target_fetch_registers (regcache, regnum); >>> + } >>> + catch (const gdb_exception_error &e) >>> + { /* Ignore errors. */ } >>> + } >>> + >>> + /* Call riscv_collect_regset_section_cb for each regset, passing in the >>> + DATA object. Appends the core file notes to *(DATA->NOTE_DATA) to >>> + describe all the registers in this thread. */ >>> + riscv_collect_regset_section_cb_data data (gdbarch, regcache, info->ptid, >>> + obfd, info->suspend.stop_signal, >>> + note_data, note_size); >>> + gdbarch_iterate_over_regset_sections (gdbarch, >>> + riscv_collect_regset_section_cb, >>> + &data, regcache); >>> +} >>> + >>> +/* Build the note section for a corefile, and return it in a malloc >>> + buffer. Currently this just dumps all available registers for each >>> + thread. */ >>> + >>> +static gdb::unique_xmalloc_ptr >>> +riscv_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size) >>> +{ >>> + if (!gdbarch_iterate_over_regset_sections_p (gdbarch)) >>> + return NULL; >>> + >>> + /* Data structures into which we accumulate the core file notes. */ >>> + gdb::unique_xmalloc_ptr note_data; >>> + >>> + /* Add note information about the executable and its arguments. */ >>> + char fname[16] = {'\0'}; >>> + char psargs[80] = {'\0'}; >>> + if (get_exec_file (0)) >>> + { >>> + strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname)); >>> + fname[sizeof (fname) - 1] = 0; >>> + strncpy (psargs, get_exec_file (0), sizeof (psargs)); >>> + psargs[sizeof (psargs) - 1] = 0; >>> + >>> + const char *inf_args = get_inferior_args (); >>> + if (inf_args != nullptr && *inf_args != '\0' >>> + && (strlen (inf_args) >>> + < ((int) sizeof (psargs) - (int) strlen (psargs)))) >>> + { >>> + strncat (psargs, " ", >>> + sizeof (psargs) - strlen (psargs)); >>> + strncat (psargs, inf_args, >>> + sizeof (psargs) - strlen (psargs)); >>> + } >>> + >>> + psargs[sizeof (psargs) - 1] = 0; >>> + } >>> + note_data.reset (elfcore_write_prpsinfo (obfd, note_data.release (), >>> + note_size, fname, >>> + psargs)); >>> + >>> + /* Update our understanding of the available threads. */ >>> + try >>> + { >>> + update_thread_list (); >>> + } >>> + catch (const gdb_exception_error &e) >>> + { >>> + exception_print (gdb_stderr, e); >>> + } >>> + >>> + /* As we do in linux-tdep.c, prefer dumping the signalled thread first. >>> + The "first thread" is what tools use to infer the signalled thread. >>> + In case there's more than one signalled thread, prefer the current >>> + thread, if it is signalled. */ >>> + thread_info *signalled_thr, *curr_thr = inferior_thread (); >>> + if (curr_thr->suspend.stop_signal != GDB_SIGNAL_0) >>> + signalled_thr = curr_thr; >>> + else >>> + { >>> + signalled_thr = iterate_over_threads (find_signalled_thread, nullptr); >>> + if (signalled_thr == nullptr) >>> + signalled_thr = curr_thr; >>> + } >>> + >>> + /* First add information about the signalled thread, then add information >>> + about all the other threads, see above for the reasoning. */ >>> + riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_data, note_size); >>> + for (thread_info *thr : current_inferior ()->non_exited_threads ()) >>> + { >>> + if (thr == signalled_thr) >>> + continue; >>> + riscv_corefile_thread (gdbarch, obfd, signalled_thr, ¬e_data, >>> + note_size); >>> + } >>> + >>> + return note_data; >>> +} >>> + >>> +/* Define the general register mapping. This follows the same format as >>> + the RISC-V linux corefile. The linux kernel puts the PC at offset 0, >>> + gdb puts it at offset 32. Register x0 is always 0 and can be ignored. >>> + Registers x1 to x31 are in the same place. */ >>> + >>> +static const struct regcache_map_entry riscv_gregmap[] = >>> +{ >>> + { 1, RISCV_PC_REGNUM, 0 }, >>> + { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */ >>> + { 0 } >>> +}; >>> + >>> +/* Define the FP register mapping. This follows the same format as the >>> + RISC-V linux corefile. The kernel puts the 32 FP regs first, and then >>> + FCSR. */ >>> + >>> +static const struct regcache_map_entry riscv_fregmap[] = >>> +{ >>> + { 32, RISCV_FIRST_FP_REGNUM, 0 }, >>> + { 1, RISCV_CSR_FCSR_REGNUM, 0 }, >>> + { 0 } >>> +}; >>> + >>> +/* Define the general register regset. */ >>> + >>> +static const struct regset riscv_gregset = >>> +{ >>> + riscv_gregmap, riscv_supply_regset, regcache_collect_regset >>> +}; >>> + >>> +/* Define the FP register regset. */ >>> + >>> +static const struct regset riscv_fregset = >>> +{ >>> + riscv_fregmap, riscv_supply_regset, regcache_collect_regset >>> +}; >>> + >>> +/* Callback for iterate_over_regset_sections that records a single regset >>> + in the corefile note section. */ >>> + >>> +static void >>> +riscv_collect_regset_section_cb (const char *sect_name, int supply_size, >>> + int collect_size, const struct regset *regset, >>> + const char *human_name, void *cb_data) >>> +{ >>> + riscv_collect_regset_section_cb_data *data >>> + = (riscv_collect_regset_section_cb_data *) cb_data; >>> + >>> + /* The only flag is REGSET_VARIABLE_SIZE, and we don't use that. */ >>> + gdb_assert (regset->flags == 0); >>> + gdb_assert (supply_size == collect_size); >>> + >>> + if (data->abort_iteration) >>> + return; >>> + >>> + gdb_assert (regset != nullptr && regset->collect_regset); >>> + >>> + /* This is intentionally zero-initialized by using std::vector, so that >>> + any padding bytes in the core file will show a zero. */ >>> + std::vector buf (collect_size); >>> + >>> + regset->collect_regset (regset, data->regcache, -1, buf.data (), >>> + collect_size); >>> + >>> + /* PRSTATUS still needs to be treated specially. */ >>> + if (strcmp (sect_name, ".reg") == 0) >>> + data->note_data->reset (elfcore_write_prstatus >>> + (data->obfd, data->note_data->release (), >>> + data->note_size, data->lwp, >>> + gdb_signal_to_host (data->stop_signal), >>> + buf.data ())); >>> + else >>> + data->note_data->reset (elfcore_write_register_note >>> + (data->obfd, data->note_data->release (), >>> + data->note_size, >>> + sect_name, buf.data (), collect_size)); >>> + >>> + if (data->note_data->get () == NULL) >>> + data->abort_iteration = true; >>> +} >>> + >>> +/* Implement the "iterate_over_regset_sections" gdbarch method. */ >>> + >>> +static void >>> +riscv_iterate_over_regset_sections (struct gdbarch *gdbarch, >>> + iterate_over_regset_sections_cb *cb, >>> + void *cb_data, >>> + const struct regcache *regcache) >>> +{ >>> + /* Write out the GPRs. */ >>> + int sz = 32 * riscv_isa_xlen (gdbarch); >>> + cb (".reg", sz, sz, &riscv_gregset, NULL, cb_data); >>> + >>> + /* Write out the FPRs, but only if present. */ >>> + if (riscv_isa_flen (gdbarch) > 0) >>> + { >>> + sz = (32 * riscv_isa_flen (gdbarch) >>> + + register_size (gdbarch, RISCV_CSR_FCSR_REGNUM)); >>> + cb (".reg2", sz, sz, &riscv_fregset, NULL, cb_data); >>> + } >>> +} >>> + >>> +/* Initialize RISC-V bare-metal ABI info. */ >>> + >>> +static void >>> +riscv_none_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) >>> +{ >>> + /* Find or create a target description from a core file. */ >>> + set_gdbarch_core_read_description (gdbarch, riscv_core_read_description); >>> + >>> + /* How to create a core file for bare metal RISC-V. */ >>> + set_gdbarch_make_corefile_notes (gdbarch, riscv_make_corefile_notes); >>> + >>> + /* Iterate over registers for reading and writing bare metal RISC-V core >>> + files. */ >>> + set_gdbarch_iterate_over_regset_sections >>> + (gdbarch, riscv_iterate_over_regset_sections); >>> + >>> +} >>> + >>> +/* Initialize RISC-V bare-metal target support. */ >>> + >>> +void _initialize_riscv_none_tdep (); >>> +void >>> +_initialize_riscv_none_tdep () >>> +{ >>> + gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_NONE, >>> + riscv_none_init_abi); >>> +} >>> + >>>