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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.