Re: [PATCH 3/4] target/arm: Support reading ZA[] from gdbstub

[Peter Maydell]

On Thu, 22 Jun 2023 at 16:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Mirror the existing support for SVE.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


> @@ -247,6 +247,61 @@ int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t 
> *buf, int reg)
>      return 0;
>  }
>
> +static int max_svq(ARMCPU *cpu)
> +{
> +    return 32 - clz32(cpu->sme_vq.map);
> +}
> +
> +int aarch64_gdb_get_za_reg(CPUARMState *env, GByteArray *buf, int reg)
> +{
> +    ARMCPU *cpu = env_archcpu(env);
> +    int max_vq = max_svq(cpu);
> +    int cur_vq = EX_TBFLAG_A64(env->hflags, SVL) + 1;
> +    int i;
> +
> +    if (reg >= max_vq * 16) {
> +        return 0;
> +    }
> +
> +    /* If ZA is unset, or reg out of range, the contents are zero. */
> +    if (FIELD_EX64(env->svcr, SVCR, ZA) && reg < cur_vq * 16) {
> +        for (i = 0; i < cur_vq; i++) {
> +            gdb_get_reg128(buf, env->zarray[reg].d[i * 2 + 1],
> +                           env->zarray[reg].d[i * 2]);
> +        }
> +    } else {
> +        cur_vq = 0;
> +    }
> +
> +    for (i = cur_vq; i < max_vq; i++) {
> +        gdb_get_reg128(buf, 0, 0);
> +    }
> +
> +    return max_vq * 16;
> +}
> +
> +int aarch64_gdb_set_za_reg(CPUARMState *env, uint8_t *buf, int reg)
> +{
> +    ARMCPU *cpu = env_archcpu(env);
> +    uint64_t *p = (uint64_t *) buf;
> +    int max_vq = max_svq(cpu);
> +    int cur_vq = EX_TBFLAG_A64(env->hflags, SVL) + 1;
> +    int i;
> +
> +    if (reg >= max_vq * 16) {
> +        return 0;
> +    }
> +
> +    /* If ZA is unset, or reg out of range, the contents are zero. */
> +    if (FIELD_EX64(env->svcr, SVCR, ZA) && reg < cur_vq * 16) {
> +        for (i = 0; i < cur_vq; i++) {
> +            env->zarray[reg].d[i * 2 + 1] = *p++;
> +            env->zarray[reg].d[i * 2 + 0] = *p++;

This looks like it won't do the right thing on a big-endian
system. (And the existing SVE code also looks wrong.)
The gdb_get_reg*() functions handle endianness conversion
from the gdb data buffer; there are no equivalent gdb_set_reg*()
functions so you have to do the byte-swapping yourself.
(This is pretty bug-prone so maybe we should design a better
API here :-))

Compare aarch64_gdb_get/set_fpu_reg() where a gdb_get_reg128()
is matched with a pair of ldq_le_p() and so on.

> +        }
> +    }
> +    return max_vq * 16;
> +}
> +
>  static void output_vector_union_type(GString *s, int reg_width,
>                                       const char *name)
>  {
> @@ -379,3 +434,36 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int 
> orig_base_reg)
>      info->num = base_reg - orig_base_reg;
>      return info->num;
>  }
> +
> +/*
> + * Generate the xml for SME, with matrix size set to the maximum
> + * for the cpu.  Returns the number of registers generated.
> + */
> +int arm_gen_dynamic_zareg_xml(CPUState *cs, int base_reg)
> +{
> +    ARMCPU *cpu = ARM_CPU(cs);
> +    GString *s = g_string_new(NULL);
> +    int vq = max_svq(cpu);
> +    int row_count = vq * 16;
> +    int row_width = vq * 128;
> +    int i;
> +
> +    g_string_printf(s, "<?xml version=\"1.0\"?>");
> +    g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> +    g_string_append_printf(s, "<feature name=\"org.qemu.gdb.aarch64.za\">");

The patches on the GDB end are still under review, but they
use the feature name org.gnu.gdb.aarch64.sme:

https://inbox.sourceware.org/gdb-patches/20230519102508.14020-18-luis.machado@arm.com/
We should follow that (and only commit our end when the GDB
spec for the XML layout is finalized.

Luis kindly gave me a dump of some example XML to save us
from trying to parse it out of the patch:

  <feature name="org.gnu.gdb.aarch64.sme">
    <flags id="svcr_flags" size="8">
      <field name="SM" start="0" end="0" type="bool"/>
      <field name="ZA" start="1" end="1" type="bool"/>
    </flags>
    <vector id="sme_bv" type="uint8" count="32"/>
    <vector id="sme_bvv" type="sme_bv" count="32"/>
    <reg name="svg" bitsize="64" type="int" regnum="91"/>
    <reg name="svcr" bitsize="64" type="svcr_flags" regnum="92"/>
    <reg name="za" bitsize="8192" type="sme_bvv" regnum="93"/>
  </feature>

> +
> +    output_vector_union_type(s, row_width, "zav");
> +
> +    for (i = 0; i < row_count; i++) {
> +        g_string_append_printf(s,
> +                               "<reg name=\"za%d\" bitsize=\"%d\""
> +                               " regnum=\"%d\" type=\"zav\"/>",
> +                               i, row_width, base_reg + i);
> +    }
> +
> +    g_string_append_printf(s, "</feature>");
> +
> +    cpu->dyn_zareg_xml.num = row_count;
> +    cpu->dyn_zareg_xml.desc = g_string_free(s, false);
> +    return row_count;
> +}

thanks
-- PMM