From: "Alex Bennée" <alex.bennee@linaro.org>
To: Nicolas Eder <nicolas.eder@lauterbach.com>
Cc: qemu-devel@nongnu.org,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
Christian.Boenig@lauterbach.com
Subject: Re: [PATCH v2 25/29] Doxygen documentation added
Date: Fri, 13 Oct 2023 17:34:56 +0100 [thread overview]
Message-ID: <878r86mqc1.fsf@linaro.org> (raw)
In-Reply-To: <20231006090610.26171-26-nicolas.eder@lauterbach.com>
Nicolas Eder <nicolas.eder@lauterbach.com> writes:
> From: neder <nicolas.eder@lauterbach.com>
>
> ---
> include/exec/mcdstub.h | 7 -
> include/mcdstub/syscalls.h | 2 +-
> mcdstub/mcd_shared_defines.h | 1 +
> mcdstub/mcdstub.c | 61 ++--
> mcdstub/mcdstub.h | 579 ++++++++++++++++++++++++++++++++++-
> mcdstub/mcdstub_common.h | 18 ++
> target/arm/mcdstub.c | 6 -
> target/arm/mcdstub.h | 105 +++++++
> 8 files changed, 709 insertions(+), 70 deletions(-)
>
> diff --git a/include/exec/mcdstub.h b/include/exec/mcdstub.h
> index 84f7c811cb..9b7c31a951 100644
> --- a/include/exec/mcdstub.h
> +++ b/include/exec/mcdstub.h
> @@ -3,13 +3,6 @@
>
> #define DEFAULT_MCDSTUB_PORT "1235"
>
> -/* breakpoint defines */
> -#define MCD_BREAKPOINT_SW 0
> -#define MCD_BREAKPOINT_HW 1
> -#define MCD_WATCHPOINT_WRITE 2
> -#define MCD_WATCHPOINT_READ 3
> -#define MCD_WATCHPOINT_ACCESS 4
> -
> /**
> * mcd_tcp_server_start: start the tcp server to connect via mcd
> * @device: connection spec for mcd
> diff --git a/include/mcdstub/syscalls.h b/include/mcdstub/syscalls.h
> index 5547d6d29e..52a26be3a9 100644
> --- a/include/mcdstub/syscalls.h
> +++ b/include/mcdstub/syscalls.h
> @@ -3,4 +3,4 @@
>
> typedef void (*gdb_syscall_complete_cb)(CPUState *cpu, uint64_t ret, int err);
>
> -#endif /* _SYSCALLS_H_ */
> +#endif /* _MCD_SYSCALLS_H_ */
> diff --git a/mcdstub/mcd_shared_defines.h b/mcdstub/mcd_shared_defines.h
> index 5fbd732252..110d36d975 100644
> --- a/mcdstub/mcd_shared_defines.h
> +++ b/mcdstub/mcd_shared_defines.h
> @@ -38,6 +38,7 @@
> /* tcp query arguments */
> #define QUERY_FIRST "f"
> #define QUERY_CONSEQUTIVE "c"
> +#define QUERY_END_INDEX "!"
>
> #define QUERY_ARG_SYSTEM "system"
> #define QUERY_ARG_CORES "cores"
> diff --git a/mcdstub/mcdstub.c b/mcdstub/mcdstub.c
> index 7613ed2c4a..a468a7d7b8 100644
> --- a/mcdstub/mcdstub.c
> +++ b/mcdstub/mcdstub.c
> @@ -218,12 +218,6 @@ int find_cpu_clusters(Object *child, void *opaque)
> s->processes = g_renew(MCDProcess, s->processes, ++s->process_num);
>
> process = &s->processes[s->process_num - 1];
> -
> - /*
> - * GDB process IDs -1 and 0 are reserved. To avoid subtle errors at
> - * runtime, we enforce here that the machine does not use a cluster ID
> - * that would lead to PID 0.
> - */
> assert(cluster->cluster_id != UINT32_MAX);
> process->pid = cluster->cluster_id + 1;
> process->attached = false;
> @@ -925,7 +919,7 @@ void mcd_vm_state_change(void *opaque, bool running, RunState state)
>
> int mcd_put_packet(const char *buf)
> {
> - return mcd_put_packet_binary(buf, strlen(buf), false);
> + return mcd_put_packet_binary(buf, strlen(buf));
> }
>
> void mcd_put_strbuf(void)
> @@ -933,7 +927,7 @@ void mcd_put_strbuf(void)
> mcd_put_packet(mcdserver_state.str_buf->str);
> }
>
> -int mcd_put_packet_binary(const char *buf, int len, bool dump)
> +int mcd_put_packet_binary(const char *buf, int len)
> {
> for (;;) {
> g_byte_array_set_size(mcdserver_state.last_packet, 0);
> @@ -999,12 +993,12 @@ MCDProcess *mcd_get_process(uint32_t pid)
> return NULL;
> }
>
> -CPUState *mcd_get_cpu(uint32_t i_cpu_index)
> +CPUState *mcd_get_cpu(uint32_t cpu_index)
> {
> CPUState *cpu = first_cpu;
>
> while (cpu) {
> - if (cpu->cpu_index == i_cpu_index) {
> + if (cpu->cpu_index == cpu_index) {
> return cpu;
> }
> cpu = mcd_next_attached_cpu(cpu);
> @@ -1344,15 +1338,13 @@ void handle_open_core(GArray *params, void *user_ctx)
>
> void handle_query_reset_f(GArray *params, void *user_ctx)
> {
> - /* TODO: vull reset over the qemu monitor */
> -
> /* 1. check length */
> int nb_resets = mcdserver_state.resets->len;
> if (nb_resets == 1) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> - g_string_printf(mcdserver_state.str_buf, "1!");
> + g_string_printf(mcdserver_state.str_buf, "1%s", QUERY_END_INDEX);
> }
> /* 2. send data */
> mcd_reset_st reset = g_array_index(mcdserver_state.resets, mcd_reset_st, 0);
> @@ -1370,7 +1362,7 @@ void handle_query_reset_c(GArray *params, void *user_ctx)
> int nb_groups = mcdserver_state.resets->len;
> if (query_index + 1 == nb_groups) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> g_string_printf(mcdserver_state.str_buf, "%u!", query_index + 1);
> }
> @@ -1487,9 +1479,9 @@ void handle_query_mem_spaces_f(GArray *params, void *user_ctx)
> int nb_groups = memspaces->len;
> if (nb_groups == 1) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> - g_string_printf(mcdserver_state.str_buf, "1!");
> + g_string_printf(mcdserver_state.str_buf, "1%s", QUERY_END_INDEX);
> }
>
> /* 3. send data */
> @@ -1522,7 +1514,7 @@ void handle_query_mem_spaces_c(GArray *params, void *user_ctx)
> int nb_groups = memspaces->len;
> if (query_index + 1 == nb_groups) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> g_string_printf(mcdserver_state.str_buf, "%u!", query_index + 1);
> }
> @@ -1555,9 +1547,9 @@ void handle_query_reg_groups_f(GArray *params, void *user_ctx)
> int nb_groups = reggroups->len;
> if (nb_groups == 1) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> - g_string_printf(mcdserver_state.str_buf, "1!");
> + g_string_printf(mcdserver_state.str_buf, "1%s", QUERY_END_INDEX);
> }
> /* 3. send data */
> mcd_reg_group_st group = g_array_index(reggroups, mcd_reg_group_st, 0);
> @@ -1580,7 +1572,7 @@ void handle_query_reg_groups_c(GArray *params, void *user_ctx)
> int nb_groups = reggroups->len;
> if (query_index + 1 == nb_groups) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> g_string_printf(mcdserver_state.str_buf, "%u!", query_index + 1);
> }
> @@ -1604,9 +1596,9 @@ void handle_query_regs_f(GArray *params, void *user_ctx)
> int nb_regs = registers->len;
> if (nb_regs == 1) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> - g_string_printf(mcdserver_state.str_buf, "1!");
> + g_string_printf(mcdserver_state.str_buf, "1%s", QUERY_END_INDEX);
> }
> /* 3. send data */
> mcd_reg_st my_register = g_array_index(registers, mcd_reg_st, 0);
> @@ -1637,7 +1629,7 @@ void handle_query_regs_c(GArray *params, void *user_ctx)
> int nb_regs = registers->len;
> if (query_index + 1 == nb_regs) {
> /* indicates this is the last packet */
> - g_string_printf(mcdserver_state.str_buf, "0!");
> + g_string_printf(mcdserver_state.str_buf, "0%s", QUERY_END_INDEX);
> } else {
> g_string_printf(mcdserver_state.str_buf, "%u!", query_index + 1);
> }
> @@ -1672,19 +1664,8 @@ void handle_query_state(GArray *params, void *user_ctx)
> * get state info
> */
> mcd_cpu_state_st state_info = mcdserver_state.cpu_state;
> - mcd_core_event_et event = MCD_CORE_EVENT_NONE;
> - if (state_info.memory_changed) {
> - event = event | MCD_CORE_EVENT_MEMORY_CHANGE;
> - state_info.memory_changed = false;
> - }
> - if (state_info.registers_changed) {
> - event = event | MCD_CORE_EVENT_REGISTER_CHANGE;
> - state_info.registers_changed = false;
> - }
> - if (state_info.target_was_stopped) {
> - event = event | MCD_CORE_EVENT_STOPPED;
> - state_info.target_was_stopped = false;
> - }
> + /* TODO: add event information */
> + uint32_t event = 0;
> /* send data */
> g_string_printf(mcdserver_state.str_buf,
> "%s=%s.%s=%u.%s=%u.%s=%u.%s=%lu.%s=%s.%s=%s.",
> @@ -1863,7 +1844,7 @@ void handle_write_memory(GArray *params, void *user_ctx)
> mcd_put_packet(TCP_EXECUTION_ERROR);
> return;
> }
> - /* read memory */
> + /* write memory */
> mcd_hextomem(mcdserver_state.mem_buf, mcdserver_state.str_buf->str, len);
> if (mcd_write_memory(cpu, mem_address,
> mcdserver_state.mem_buf->data, len) != 0) {
> @@ -1879,7 +1860,7 @@ int mcd_breakpoint_insert(CPUState *cpu, int type, vaddr addr)
> int bp_type = 0;
> CPUClass *cc = CPU_GET_CLASS(cpu);
> if (cc->gdb_stop_before_watchpoint) {
> - //bp_type |= BP_STOP_BEFORE_ACCESS;
> + /* bp_type |= BP_STOP_BEFORE_ACCESS; */
> }
> int return_value = 0;
> switch (type) {
> @@ -1909,7 +1890,7 @@ int mcd_breakpoint_remove(CPUState *cpu, int type, vaddr addr)
> int bp_type = 0;
> CPUClass *cc = CPU_GET_CLASS(cpu);
> if (cc->gdb_stop_before_watchpoint) {
> - //bp_type |= BP_STOP_BEFORE_ACCESS;
> + /* bp_type |= BP_STOP_BEFORE_ACCESS; */
> }
> int return_value = 0;
> switch (type) {
> diff --git a/mcdstub/mcdstub.h b/mcdstub/mcdstub.h
> index d3f15da180..5412b59423 100644
> --- a/mcdstub/mcdstub.h
> +++ b/mcdstub/mcdstub.h
> @@ -12,20 +12,6 @@
> #define MCD_TRIG_OPT_DATA_IS_CONDITION 0x00000008
> #define MCD_TRIG_ACTION_DBG_DEBUG 0x00000001
>
> -typedef uint32_t mcd_core_event_et;
> -/* TODO: replace mcd defines with custom layer */
> -enum {
> - MCD_CORE_EVENT_NONE = 0x00000000,
> - MCD_CORE_EVENT_MEMORY_CHANGE = 0x00000001,
> - MCD_CORE_EVENT_REGISTER_CHANGE = 0x00000002,
> - MCD_CORE_EVENT_TRACE_CHANGE = 0x00000004,
> - MCD_CORE_EVENT_TRIGGER_CHANGE = 0x00000008,
> - MCD_CORE_EVENT_STOPPED = 0x00000010,
> - MCD_CORE_EVENT_CHL_PENDING = 0x00000020,
> - MCD_CORE_EVENT_CUSTOM_LO = 0x00010000,
> - MCD_CORE_EVENT_CUSTOM_HI = 0x40000000,
> -};
> -
None of these changes have to do with adding documentation.
> /* schema defines */
> #define ARG_SCHEMA_QRYHANDLE 'q'
> #define ARG_SCHEMA_STRING 's'
> @@ -187,88 +173,649 @@ static inline int tohex(int v)
> void mcd_sigterm_handler(int signal);
> #endif
>
> +/**
> + * \defgroup mcdstub Main mcdstub functions
> + * All architecture independent mcdstub functions.
> + */
> +
> +/**
> + * \addtogroup mcdstub
> + * @{
> + */
> +
> +/**
> + * \brief Initializes the mcdserver_state struct.
> + *
> + * This function allocates memory for the mcdserver_state struct and sets
> + * all of its members to their inital values. This includes setting the
> + * cpu_state to halted and initializing the query functions with \ref
> + * init_query_cmds_table.
> + */
We already have a documentation standard for functions and it is kdoc:
https://www.kernel.org/doc/html/v5.0/doc-guide/kernel-doc.html
which is integrated into our documentation system. Please use that.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
next prev parent reply other threads:[~2023-10-13 16:37 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-06 9:05 [PATCH v2 00/29] first version of mcdstub Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 01/29] mcdstub initial commit, mcdstub file structure added Nicolas Eder
2023-10-13 15:51 ` Alex Bennée
2023-10-06 9:05 ` [PATCH v2 02/29] TCP chardev added, handshake with TRACE32 working Nicolas Eder
2023-10-13 16:15 ` Alex Bennée
2023-10-06 9:05 ` [PATCH v2 03/29] TCP packet handling added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 04/29] queries for resets and triggers added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 05/29] queries for memory spaces and register groups added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 06/29] query for registers added Nicolas Eder
2023-10-13 16:38 ` Alex Bennée
2023-10-06 9:05 ` [PATCH v2 07/29] query data preparation improved Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 08/29] shared header file added, used for TCP packet data Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 09/29] memory and register query data now stored per core Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 10/29] handler for resets added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 11/29] query for the VM state added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 12/29] handler for reading registers added Nicolas Eder
2023-10-13 16:40 ` Alex Bennée
2023-10-06 9:05 ` [PATCH v2 13/29] handler for reading memory added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 14/29] handler for single step added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 15/29] adapting to the qemu coding style Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 16/29] deleting the mcdd startup option Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 17/29] handler for breakpoints and watchpoints added Nicolas Eder
2023-10-06 9:05 ` [PATCH v2 18/29] making step and go handlers core-specific Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 19/29] adding trigger ID handling for TRACE32 Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 20/29] cp register read/write added Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 21/29] switching between secure and non-secure memory added Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 22/29] transitioning to unsinged integers in TCP packets and removing MCD-API-specific terms Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 23/29] moved all ARM code to the ARM mcdstub and added now commom header file Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 24/29] step and go handlers now propperly perform global operations Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 25/29] Doxygen documentation added Nicolas Eder
2023-10-13 16:34 ` Alex Bennée [this message]
2023-10-06 9:06 ` [PATCH v2 26/29] moved all mcd related header files into include/mcdstub Nicolas Eder
2023-10-13 16:45 ` Alex Bennée
2023-10-06 9:06 ` [PATCH v2 27/29] MCD stub entry added to maintainers file Nicolas Eder
2023-10-13 16:46 ` Alex Bennée
2023-10-06 9:06 ` [PATCH v2 28/29] added description to out-commented gdb function Nicolas Eder
2023-10-06 9:06 ` [PATCH v2 29/29] introducing the DebugClass. It is used to abstract the gdb/mcd set_stop_cpu function Nicolas Eder
2023-10-06 9:50 ` [PATCH v2 00/29] first version of mcdstub Philippe Mathieu-Daudé
2023-10-13 16:47 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878r86mqc1.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=Christian.Boenig@lauterbach.com \
--cc=nicolas.eder@lauterbach.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).