Re: [Qemu-devel] [PATCH v2 07/15] gdbstub: add multiprocess support to Xfer:features:read:

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Luc,

On 01/10/2018 13:56, Luc Michel wrote:
> Change the Xfer:features:read: packet handling to support the
> multiprocess extension. This packet is used to request the XML
> description of the CPU. In multiprocess mode, different descriptions can
> be sent for different processes.
> 
> This function now takes the process to send the description for as a
> parameter, and use a buffer in the process structure to store the
> generated description.
> 
> It takes the first CPU of the process to generate the description.
> 
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> ---
>  gdbstub.c | 44 ++++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 63d7913269..9065e8e140 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -792,55 +792,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
>      }
>  
>      return cpu;
>  }
>  
> -static const char *get_feature_xml(const char *p, const char **newp,
> -                                   CPUClass *cc)
> +static const char *get_feature_xml(const GDBState *s, const char *p,
> +                                   const char **newp, GDBProcess *process)
>  {
>      size_t len;
>      int i;
>      const char *name;
> -    static char target_xml[1024];

As noted in your patch #2, I'd add GDBProcess::target_xml in this same
patch.

With this change:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    CPUState *cpu = get_first_cpu_in_process(s, process);
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>  
>      len = 0;
>      while (p[len] && p[len] != ':')
>          len++;
>      *newp = p + len;
>  
>      name = NULL;
>      if (strncmp(p, "target.xml", len) == 0) {
> +        char *buf = process->target_xml;
> +        const size_t buf_sz = sizeof(process->target_xml);
> +
>          /* Generate the XML description for this CPU.  */
> -        if (!target_xml[0]) {
> +        if (!buf[0]) {
>              GDBRegisterState *r;
> -            CPUState *cpu = first_cpu;
>  
> -            pstrcat(target_xml, sizeof(target_xml),
> +            pstrcat(buf, buf_sz,
>                      "<?xml version=\"1.0\"?>"
>                      "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
>                      "<target>");
>              if (cc->gdb_arch_name) {
>                  gchar *arch = cc->gdb_arch_name(cpu);
> -                pstrcat(target_xml, sizeof(target_xml), "<architecture>");
> -                pstrcat(target_xml, sizeof(target_xml), arch);
> -                pstrcat(target_xml, sizeof(target_xml), "</architecture>");
> +                pstrcat(buf, buf_sz, "<architecture>");
> +                pstrcat(buf, buf_sz, arch);
> +                pstrcat(buf, buf_sz, "</architecture>");
>                  g_free(arch);
>              }
> -            pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> -            pstrcat(target_xml, sizeof(target_xml), cc->gdb_core_xml_file);
> -            pstrcat(target_xml, sizeof(target_xml), "\"/>");
> +            pstrcat(buf, buf_sz, "<xi:include href=\"");
> +            pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
> +            pstrcat(buf, buf_sz, "\"/>");
>              for (r = cpu->gdb_regs; r; r = r->next) {
> -                pstrcat(target_xml, sizeof(target_xml), "<xi:include href=\"");
> -                pstrcat(target_xml, sizeof(target_xml), r->xml);
> -                pstrcat(target_xml, sizeof(target_xml), "\"/>");
> +                pstrcat(buf, buf_sz, "<xi:include href=\"");
> +                pstrcat(buf, buf_sz, r->xml);
> +                pstrcat(buf, buf_sz, "\"/>");
>              }
> -            pstrcat(target_xml, sizeof(target_xml), "</target>");
> +            pstrcat(buf, buf_sz, "</target>");
>          }
> -        return target_xml;
> +        return buf;
>      }
>      if (cc->gdb_get_dynamic_xml) {
> -        CPUState *cpu = first_cpu;
>          char *xmlname = g_strndup(p, len);
>          const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
>  
>          g_free(xmlname);
>          if (xml) {
> @@ -1246,10 +1248,11 @@ out:
>  }
>  
>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>  {
>      CPUState *cpu;
> +    GDBProcess *process;
>      CPUClass *cc;
>      const char *p;
>      uint32_t pid, tid;
>      int ch, reg_size, type, res;
>      uint8_t mem_buf[MAX_PACKET_LENGTH];
> @@ -1620,18 +1623,19 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>          }
>          if (strncmp(p, "Xfer:features:read:", 19) == 0) {
>              const char *xml;
>              target_ulong total_len;
>  
> -            cc = CPU_GET_CLASS(first_cpu);
> +            process = gdb_get_cpu_process(s, s->g_cpu);
> +            cc = CPU_GET_CLASS(s->g_cpu);
>              if (cc->gdb_core_xml_file == NULL) {
>                  goto unknown_command;
>              }
>  
>              gdb_has_xml = true;
>              p += 19;
> -            xml = get_feature_xml(p, &p, cc);
> +            xml = get_feature_xml(s, p, &p, process);
>              if (!xml) {
>                  snprintf(buf, sizeof(buf), "E00");
>                  put_packet(s, buf);
>                  break;
>              }
>