diff mbox series

[PULL,21/30] target/arm: use official org.gnu.gdb.aarch64.sve layout for registers

Message ID 20210115130828.23968-22-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series [PULL,01/30] tests/docker: Remove Debian 9 remnant lines | expand

Commit Message

Alex Bennée Jan. 15, 2021, 1:08 p.m. UTC
While GDB can work with any XML description given to it there is
special handling for SVE registers on the GDB side which makes the
users life a little better. The changes aren't that major and all the
registers save the $vg reported the same. All that changes is:

  - report org.gnu.gdb.aarch64.sve
  - use gdb nomenclature for names and types
  - minor re-ordering of the types to match reference
  - re-enable ieee_half (as we know gdb supports it now)
  - $vg is now a 64 bit int
  - check $vN and $zN aliasing in test

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Luis Machado <luis.machado@linaro.org>
Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>

Comments

Claudio Fontana Jan. 19, 2021, 1:38 p.m. UTC | #1
Hi Alex,

after updating to latest master today, I am getting the following error with

make check-tcg

qemu-system-aarch64: -gdb unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
warning: Could not load XML target description; ignoring
qemu-system-aarch64: QEMU: Terminated via GDBstub

Seems to indicate it is "ieee_half" -related?

Thanks,

Claudio

On 1/15/21 2:08 PM, Alex Bennée wrote:
> While GDB can work with any XML description given to it there is
> special handling for SVE registers on the GDB side which makes the
> users life a little better. The changes aren't that major and all the
> registers save the $vg reported the same. All that changes is:
> 
>   - report org.gnu.gdb.aarch64.sve
>   - use gdb nomenclature for names and types
>   - minor re-ordering of the types to match reference
>   - re-enable ieee_half (as we know gdb supports it now)
>   - $vg is now a 64 bit int
>   - check $vN and $zN aliasing in test
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Luis Machado <luis.machado@linaro.org>
> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
> 
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 866595b4f1..a8fff2a3d0 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>      { "uint128", 128, 'q', 'u' },
>      { "int128", 128, 'q', 's' },
>      /* 64 bit */
> +    { "ieee_double", 64, 'd', 'f' },
>      { "uint64", 64, 'd', 'u' },
>      { "int64", 64, 'd', 's' },
> -    { "ieee_double", 64, 'd', 'f' },
>      /* 32 bit */
> +    { "ieee_single", 32, 's', 'f' },
>      { "uint32", 32, 's', 'u' },
>      { "int32", 32, 's', 's' },
> -    { "ieee_single", 32, 's', 'f' },
>      /* 16 bit */
> +    { "ieee_half", 16, 'h', 'f' },
>      { "uint16", 16, 'h', 'u' },
>      { "int16", 16, 'h', 's' },
> -    /*
> -     * TODO: currently there is no reliable way of telling
> -     * if the remote gdb actually understands ieee_half so
> -     * we don't expose it in the target description for now.
> -     * { "ieee_half", 16, 'h', 'f' },
> -     */
>      /* bytes */
>      { "uint8", 8, 'b', 'u' },
>      { "int8", 8, 'b', 's' },
> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>      GString *s = g_string_new(NULL);
>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>      g_autoptr(GString) ts = g_string_new("");
> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>      info->num = 0;
>      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.sve\">");
> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>  
>      /* First define types and totals in a whole VL */
>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>          int count = reg_width / vec_lanes[i].size;
> -        g_string_printf(ts, "vq%d%c%c", count,
> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>          g_string_append_printf(s,
>                                 "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>                                 ts->str, vec_lanes[i].gdb_type, count);
> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>       * signed and potentially float versions of each size from 128 to
>       * 8 bits.
>       */
> -    for (bits = 128; bits >= 8; bits /= 2) {
> -        int count = reg_width / bits;
> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> -            if (vec_lanes[i].size == bits) {
> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
> -                                       vec_lanes[i].suffix,
> -                                       count,
> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
> +            if (vec_lanes[j].size == bits) {
> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
> +                                       vec_lanes[j].suffix,
> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>              }
>          }
>          g_string_append(s, "</union>");
>      }
>      /* And now the final union of unions */
> -    g_string_append(s, "<union id=\"vq\">");
> -    for (bits = 128; bits >= 8; bits /= 2) {
> -        int count = reg_width / bits;
> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> -            if (vec_lanes[i].size == bits) {
> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
> -                                       vec_lanes[i].sz, count);
> -                break;
> -            }
> -        }
> +    g_string_append(s, "<union id=\"svev\">");
> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
> +                               suf[i], suf[i]);
>      }
>      g_string_append(s, "</union>");
>  
> +    /* Finally the sve prefix type */
> +    g_string_append_printf(s,
> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
> +                           reg_width / 8);
> +
>      /* Then define each register in parts for each vq */
>      for (i = 0; i < 32; i++) {
>          g_string_append_printf(s,
>                                 "<reg name=\"z%d\" bitsize=\"%d\""
> -                               " regnum=\"%d\" group=\"vector\""
> -                               " type=\"vq\"/>",
> +                               " regnum=\"%d\" type=\"svev\"/>",
>                                 i, reg_width, base_reg++);
>          info->num++;
>      }
> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>                             " regnum=\"%d\" group=\"float\""
>                             " type=\"int\"/>", base_reg++);
>      info->num += 2;
> -    /*
> -     * Predicate registers aren't so big they are worth splitting up
> -     * but we do need to define a type to hold the array of quad
> -     * references.
> -     */
> -    g_string_append_printf(s,
> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
> -                           cpu->sve_max_vq);
> +
>      for (i = 0; i < 16; i++) {
>          g_string_append_printf(s,
>                                 "<reg name=\"p%d\" bitsize=\"%d\""
> -                               " regnum=\"%d\" group=\"vector\""
> -                               " type=\"vqp\"/>",
> +                               " regnum=\"%d\" type=\"svep\"/>",
>                                 i, cpu->sve_max_vq * 16, base_reg++);
>          info->num++;
>      }
>      g_string_append_printf(s,
>                             "<reg name=\"ffr\" bitsize=\"%d\""
>                             " regnum=\"%d\" group=\"vector\""
> -                           " type=\"vqp\"/>",
> +                           " type=\"svep\"/>",
>                             cpu->sve_max_vq * 16, base_reg++);
>      g_string_append_printf(s,
>                             "<reg name=\"vg\" bitsize=\"64\""
> -                           " regnum=\"%d\" group=\"vector\""
> -                           " type=\"uint32\"/>",
> +                           " regnum=\"%d\" type=\"int\"/>",
>                             base_reg++);
>      info->num += 2;
>      g_string_append_printf(s, "</feature>");
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5ab3f5ace3..8a492465d6 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>           * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>           */
>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
> -        return gdb_get_reg32(buf, vq * 2);
> +        return gdb_get_reg64(buf, vq * 2);
>      }
>      default:
>          /* gdbstub asked for something out our range */
> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> index 972cf73c31..b9ef169c1a 100644
> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>          except gdb.error:
>              report(False, "checking zregs (out of range)")
>  
> +        # Check the aliased V registers are set and GDB has correctly
> +        # created them for us having recognised and handled SVE.
> +        try:
> +            for i in range(0, 16):
> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
> +                report(int(val_z) == int(val_v),
> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
> +        except gdb.error:
> +            report(False, "checking vregs (out of range)")
> +
>  
>  def run_test():
>      "Run through the tests one by one"
>
Claudio Fontana Jan. 19, 2021, 1:49 p.m. UTC | #2
On 1/19/21 2:38 PM, Claudio Fontana wrote:
> Hi Alex,
> 
> after updating to latest master today, I am getting the following error with
> 
> make check-tcg
> 
> qemu-system-aarch64: -gdb unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
> warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
> warning: Could not load XML target description; ignoring
> qemu-system-aarch64: QEMU: Terminated via GDBstub
> 
> Seems to indicate it is "ieee_half" -related?
> 
> Thanks,
> 
> Claudio

also later on I get:

  TEST    basic gdbstub support
warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
warning: Could not load XML target description; ignoring
  TEST    basic gdbstub qXfer:auxv:read support
warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
warning: Could not load XML target description; ignoring
  TEST    basic gdbstub SVE support
warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
warning: Could not load XML target description; ignoring
*** stack smashing detected ***: <unknown> terminated
  TEST    basic gdbstub SVE ZLEN support
warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
warning: Could not load XML target description; ignoring
Python 3.6.10 (default, Jan 16 2020, 09:12:04) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)


... and here it buzzes.

Thanks,

Claudio

> 
> On 1/15/21 2:08 PM, Alex Bennée wrote:
>> While GDB can work with any XML description given to it there is
>> special handling for SVE registers on the GDB side which makes the
>> users life a little better. The changes aren't that major and all the
>> registers save the $vg reported the same. All that changes is:
>>
>>   - report org.gnu.gdb.aarch64.sve
>>   - use gdb nomenclature for names and types
>>   - minor re-ordering of the types to match reference
>>   - re-enable ieee_half (as we know gdb supports it now)
>>   - $vg is now a 64 bit int
>>   - check $vN and $zN aliasing in test
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Luis Machado <luis.machado@linaro.org>
>> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
>>
>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>> index 866595b4f1..a8fff2a3d0 100644
>> --- a/target/arm/gdbstub.c
>> +++ b/target/arm/gdbstub.c
>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>      { "uint128", 128, 'q', 'u' },
>>      { "int128", 128, 'q', 's' },
>>      /* 64 bit */
>> +    { "ieee_double", 64, 'd', 'f' },
>>      { "uint64", 64, 'd', 'u' },
>>      { "int64", 64, 'd', 's' },
>> -    { "ieee_double", 64, 'd', 'f' },
>>      /* 32 bit */
>> +    { "ieee_single", 32, 's', 'f' },
>>      { "uint32", 32, 's', 'u' },
>>      { "int32", 32, 's', 's' },
>> -    { "ieee_single", 32, 's', 'f' },
>>      /* 16 bit */
>> +    { "ieee_half", 16, 'h', 'f' },
>>      { "uint16", 16, 'h', 'u' },
>>      { "int16", 16, 'h', 's' },
>> -    /*
>> -     * TODO: currently there is no reliable way of telling
>> -     * if the remote gdb actually understands ieee_half so
>> -     * we don't expose it in the target description for now.
>> -     * { "ieee_half", 16, 'h', 'f' },
>> -     */
>>      /* bytes */
>>      { "uint8", 8, 'b', 'u' },
>>      { "int8", 8, 'b', 's' },
>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>      GString *s = g_string_new(NULL);
>>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>      g_autoptr(GString) ts = g_string_new("");
>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>      info->num = 0;
>>      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.sve\">");
>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>  
>>      /* First define types and totals in a whole VL */
>>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>          int count = reg_width / vec_lanes[i].size;
>> -        g_string_printf(ts, "vq%d%c%c", count,
>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>          g_string_append_printf(s,
>>                                 "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>                                 ts->str, vec_lanes[i].gdb_type, count);
>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>       * signed and potentially float versions of each size from 128 to
>>       * 8 bits.
>>       */
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>> -                                       vec_lanes[i].suffix,
>> -                                       count,
>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>> +            if (vec_lanes[j].size == bits) {
>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>> +                                       vec_lanes[j].suffix,
>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>              }
>>          }
>>          g_string_append(s, "</union>");
>>      }
>>      /* And now the final union of unions */
>> -    g_string_append(s, "<union id=\"vq\">");
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>> -                                       vec_lanes[i].sz, count);
>> -                break;
>> -            }
>> -        }
>> +    g_string_append(s, "<union id=\"svev\">");
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>> +                               suf[i], suf[i]);
>>      }
>>      g_string_append(s, "</union>");
>>  
>> +    /* Finally the sve prefix type */
>> +    g_string_append_printf(s,
>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>> +                           reg_width / 8);
>> +
>>      /* Then define each register in parts for each vq */
>>      for (i = 0; i < 32; i++) {
>>          g_string_append_printf(s,
>>                                 "<reg name=\"z%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vq\"/>",
>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>                                 i, reg_width, base_reg++);
>>          info->num++;
>>      }
>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>                             " regnum=\"%d\" group=\"float\""
>>                             " type=\"int\"/>", base_reg++);
>>      info->num += 2;
>> -    /*
>> -     * Predicate registers aren't so big they are worth splitting up
>> -     * but we do need to define a type to hold the array of quad
>> -     * references.
>> -     */
>> -    g_string_append_printf(s,
>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>> -                           cpu->sve_max_vq);
>> +
>>      for (i = 0; i < 16; i++) {
>>          g_string_append_printf(s,
>>                                 "<reg name=\"p%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vqp\"/>",
>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>                                 i, cpu->sve_max_vq * 16, base_reg++);
>>          info->num++;
>>      }
>>      g_string_append_printf(s,
>>                             "<reg name=\"ffr\" bitsize=\"%d\""
>>                             " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"vqp\"/>",
>> +                           " type=\"svep\"/>",
>>                             cpu->sve_max_vq * 16, base_reg++);
>>      g_string_append_printf(s,
>>                             "<reg name=\"vg\" bitsize=\"64\""
>> -                           " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"uint32\"/>",
>> +                           " regnum=\"%d\" type=\"int\"/>",
>>                             base_reg++);
>>      info->num += 2;
>>      g_string_append_printf(s, "</feature>");
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 5ab3f5ace3..8a492465d6 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>           * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>           */
>>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>> -        return gdb_get_reg32(buf, vq * 2);
>> +        return gdb_get_reg64(buf, vq * 2);
>>      }
>>      default:
>>          /* gdbstub asked for something out our range */
>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> index 972cf73c31..b9ef169c1a 100644
>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>          except gdb.error:
>>              report(False, "checking zregs (out of range)")
>>  
>> +        # Check the aliased V registers are set and GDB has correctly
>> +        # created them for us having recognised and handled SVE.
>> +        try:
>> +            for i in range(0, 16):
>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>> +                report(int(val_z) == int(val_v),
>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>> +        except gdb.error:
>> +            report(False, "checking vregs (out of range)")
>> +
>>  
>>  def run_test():
>>      "Run through the tests one by one"
>>
> 
>
Alex Bennée Jan. 19, 2021, 2:50 p.m. UTC | #3
Claudio Fontana <cfontana@suse.de> writes:

> Hi Alex,
>
> after updating to latest master today, I am getting the following error with
>
> make check-tcg
>
> qemu-system-aarch64: -gdb unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
> warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
> warning: Could not load XML target description; ignoring
> qemu-system-aarch64: QEMU: Terminated via GDBstub
>
> Seems to indicate it is "ieee_half" -related?

*sigh*

yes - it is. I thought this was solved by the GDB version check in
14/30. What does your gdb report?

>
> Thanks,
>
> Claudio
>
> On 1/15/21 2:08 PM, Alex Bennée wrote:
>> While GDB can work with any XML description given to it there is
>> special handling for SVE registers on the GDB side which makes the
>> users life a little better. The changes aren't that major and all the
>> registers save the $vg reported the same. All that changes is:
>> 
>>   - report org.gnu.gdb.aarch64.sve
>>   - use gdb nomenclature for names and types
>>   - minor re-ordering of the types to match reference
>>   - re-enable ieee_half (as we know gdb supports it now)
>>   - $vg is now a 64 bit int
>>   - check $vN and $zN aliasing in test
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Luis Machado <luis.machado@linaro.org>
>> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
>> 
>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>> index 866595b4f1..a8fff2a3d0 100644
>> --- a/target/arm/gdbstub.c
>> +++ b/target/arm/gdbstub.c
>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>      { "uint128", 128, 'q', 'u' },
>>      { "int128", 128, 'q', 's' },
>>      /* 64 bit */
>> +    { "ieee_double", 64, 'd', 'f' },
>>      { "uint64", 64, 'd', 'u' },
>>      { "int64", 64, 'd', 's' },
>> -    { "ieee_double", 64, 'd', 'f' },
>>      /* 32 bit */
>> +    { "ieee_single", 32, 's', 'f' },
>>      { "uint32", 32, 's', 'u' },
>>      { "int32", 32, 's', 's' },
>> -    { "ieee_single", 32, 's', 'f' },
>>      /* 16 bit */
>> +    { "ieee_half", 16, 'h', 'f' },
>>      { "uint16", 16, 'h', 'u' },
>>      { "int16", 16, 'h', 's' },
>> -    /*
>> -     * TODO: currently there is no reliable way of telling
>> -     * if the remote gdb actually understands ieee_half so
>> -     * we don't expose it in the target description for now.
>> -     * { "ieee_half", 16, 'h', 'f' },
>> -     */
>>      /* bytes */
>>      { "uint8", 8, 'b', 'u' },
>>      { "int8", 8, 'b', 's' },
>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>      GString *s = g_string_new(NULL);
>>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>      g_autoptr(GString) ts = g_string_new("");
>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>      info->num = 0;
>>      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.sve\">");
>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>  
>>      /* First define types and totals in a whole VL */
>>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>          int count = reg_width / vec_lanes[i].size;
>> -        g_string_printf(ts, "vq%d%c%c", count,
>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>          g_string_append_printf(s,
>>                                 "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>                                 ts->str, vec_lanes[i].gdb_type, count);
>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>       * signed and potentially float versions of each size from 128 to
>>       * 8 bits.
>>       */
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>> -                                       vec_lanes[i].suffix,
>> -                                       count,
>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>> +            if (vec_lanes[j].size == bits) {
>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>> +                                       vec_lanes[j].suffix,
>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>              }
>>          }
>>          g_string_append(s, "</union>");
>>      }
>>      /* And now the final union of unions */
>> -    g_string_append(s, "<union id=\"vq\">");
>> -    for (bits = 128; bits >= 8; bits /= 2) {
>> -        int count = reg_width / bits;
>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>> -            if (vec_lanes[i].size == bits) {
>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>> -                                       vec_lanes[i].sz, count);
>> -                break;
>> -            }
>> -        }
>> +    g_string_append(s, "<union id=\"svev\">");
>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>> +                               suf[i], suf[i]);
>>      }
>>      g_string_append(s, "</union>");
>>  
>> +    /* Finally the sve prefix type */
>> +    g_string_append_printf(s,
>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>> +                           reg_width / 8);
>> +
>>      /* Then define each register in parts for each vq */
>>      for (i = 0; i < 32; i++) {
>>          g_string_append_printf(s,
>>                                 "<reg name=\"z%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vq\"/>",
>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>                                 i, reg_width, base_reg++);
>>          info->num++;
>>      }
>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>                             " regnum=\"%d\" group=\"float\""
>>                             " type=\"int\"/>", base_reg++);
>>      info->num += 2;
>> -    /*
>> -     * Predicate registers aren't so big they are worth splitting up
>> -     * but we do need to define a type to hold the array of quad
>> -     * references.
>> -     */
>> -    g_string_append_printf(s,
>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>> -                           cpu->sve_max_vq);
>> +
>>      for (i = 0; i < 16; i++) {
>>          g_string_append_printf(s,
>>                                 "<reg name=\"p%d\" bitsize=\"%d\""
>> -                               " regnum=\"%d\" group=\"vector\""
>> -                               " type=\"vqp\"/>",
>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>                                 i, cpu->sve_max_vq * 16, base_reg++);
>>          info->num++;
>>      }
>>      g_string_append_printf(s,
>>                             "<reg name=\"ffr\" bitsize=\"%d\""
>>                             " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"vqp\"/>",
>> +                           " type=\"svep\"/>",
>>                             cpu->sve_max_vq * 16, base_reg++);
>>      g_string_append_printf(s,
>>                             "<reg name=\"vg\" bitsize=\"64\""
>> -                           " regnum=\"%d\" group=\"vector\""
>> -                           " type=\"uint32\"/>",
>> +                           " regnum=\"%d\" type=\"int\"/>",
>>                             base_reg++);
>>      info->num += 2;
>>      g_string_append_printf(s, "</feature>");
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 5ab3f5ace3..8a492465d6 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>           * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>           */
>>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>> -        return gdb_get_reg32(buf, vq * 2);
>> +        return gdb_get_reg64(buf, vq * 2);
>>      }
>>      default:
>>          /* gdbstub asked for something out our range */
>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> index 972cf73c31..b9ef169c1a 100644
>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>          except gdb.error:
>>              report(False, "checking zregs (out of range)")
>>  
>> +        # Check the aliased V registers are set and GDB has correctly
>> +        # created them for us having recognised and handled SVE.
>> +        try:
>> +            for i in range(0, 16):
>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>> +                report(int(val_z) == int(val_v),
>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>> +        except gdb.error:
>> +            report(False, "checking vregs (out of range)")
>> +
>>  
>>  def run_test():
>>      "Run through the tests one by one"
>>
Claudio Fontana Jan. 19, 2021, 3:11 p.m. UTC | #4
On 1/19/21 3:50 PM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> Hi Alex,
>>
>> after updating to latest master today, I am getting the following error with
>>
>> make check-tcg
>>
>> qemu-system-aarch64: -gdb unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
>> warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
>> warning: Could not load XML target description; ignoring
>> qemu-system-aarch64: QEMU: Terminated via GDBstub
>>
>> Seems to indicate it is "ieee_half" -related?
> 
> *sigh*
> 
> yes - it is. I thought this was solved by the GDB version check in
> 14/30. What does your gdb report?


$ gdb --version
GNU gdb (GDB; openSUSE Leap 15.2) 8.3.1
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

gdb --configuration
This GDB was configured as follows:
   configure --host=x86_64-suse-linux --target=x86_64-suse-linux
             --with-auto-load-dir=$debugdir:$datadir/auto-load
             --with-auto-load-safe-path=$debugdir:$datadir/auto-load
             --with-expat
             --with-gdb-datadir=/usr/share/gdb
             --with-jit-reader-dir=/usr/lib64/gdb
             --without-libunwind-ia64
             --with-lzma
             --without-babeltrace
             --with-intel-pt
             --disable-libmcheck
             --with-mpfr
             --with-python=/usr
             --without-guile
             --disable-source-highlight
             --with-separate-debug-dir=/usr/lib/debug
             --with-system-gdbinit=/etc/gdbinit


does this help?

Let me know if more info is needed. Thanks!

Claudio


> 
>>
>> Thanks,
>>
>> Claudio
>>
>> On 1/15/21 2:08 PM, Alex Bennée wrote:
>>> While GDB can work with any XML description given to it there is
>>> special handling for SVE registers on the GDB side which makes the
>>> users life a little better. The changes aren't that major and all the
>>> registers save the $vg reported the same. All that changes is:
>>>
>>>   - report org.gnu.gdb.aarch64.sve
>>>   - use gdb nomenclature for names and types
>>>   - minor re-ordering of the types to match reference
>>>   - re-enable ieee_half (as we know gdb supports it now)
>>>   - $vg is now a 64 bit int
>>>   - check $vN and $zN aliasing in test
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Reviewed-by: Luis Machado <luis.machado@linaro.org>
>>> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
>>>
>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>>> index 866595b4f1..a8fff2a3d0 100644
>>> --- a/target/arm/gdbstub.c
>>> +++ b/target/arm/gdbstub.c
>>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>>      { "uint128", 128, 'q', 'u' },
>>>      { "int128", 128, 'q', 's' },
>>>      /* 64 bit */
>>> +    { "ieee_double", 64, 'd', 'f' },
>>>      { "uint64", 64, 'd', 'u' },
>>>      { "int64", 64, 'd', 's' },
>>> -    { "ieee_double", 64, 'd', 'f' },
>>>      /* 32 bit */
>>> +    { "ieee_single", 32, 's', 'f' },
>>>      { "uint32", 32, 's', 'u' },
>>>      { "int32", 32, 's', 's' },
>>> -    { "ieee_single", 32, 's', 'f' },
>>>      /* 16 bit */
>>> +    { "ieee_half", 16, 'h', 'f' },
>>>      { "uint16", 16, 'h', 'u' },
>>>      { "int16", 16, 'h', 's' },
>>> -    /*
>>> -     * TODO: currently there is no reliable way of telling
>>> -     * if the remote gdb actually understands ieee_half so
>>> -     * we don't expose it in the target description for now.
>>> -     * { "ieee_half", 16, 'h', 'f' },
>>> -     */
>>>      /* bytes */
>>>      { "uint8", 8, 'b', 'u' },
>>>      { "int8", 8, 'b', 's' },
>>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>      GString *s = g_string_new(NULL);
>>>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>>      g_autoptr(GString) ts = g_string_new("");
>>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>>      info->num = 0;
>>>      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.sve\">");
>>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>>  
>>>      /* First define types and totals in a whole VL */
>>>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>>          int count = reg_width / vec_lanes[i].size;
>>> -        g_string_printf(ts, "vq%d%c%c", count,
>>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>>          g_string_append_printf(s,
>>>                                 "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>>                                 ts->str, vec_lanes[i].gdb_type, count);
>>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>       * signed and potentially float versions of each size from 128 to
>>>       * 8 bits.
>>>       */
>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>> -        int count = reg_width / bits;
>>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>> -            if (vec_lanes[i].size == bits) {
>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>>> -                                       vec_lanes[i].suffix,
>>> -                                       count,
>>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>>> +            if (vec_lanes[j].size == bits) {
>>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>>> +                                       vec_lanes[j].suffix,
>>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>>              }
>>>          }
>>>          g_string_append(s, "</union>");
>>>      }
>>>      /* And now the final union of unions */
>>> -    g_string_append(s, "<union id=\"vq\">");
>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>> -        int count = reg_width / bits;
>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>> -            if (vec_lanes[i].size == bits) {
>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>>> -                                       vec_lanes[i].sz, count);
>>> -                break;
>>> -            }
>>> -        }
>>> +    g_string_append(s, "<union id=\"svev\">");
>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>>> +                               suf[i], suf[i]);
>>>      }
>>>      g_string_append(s, "</union>");
>>>  
>>> +    /* Finally the sve prefix type */
>>> +    g_string_append_printf(s,
>>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>>> +                           reg_width / 8);
>>> +
>>>      /* Then define each register in parts for each vq */
>>>      for (i = 0; i < 32; i++) {
>>>          g_string_append_printf(s,
>>>                                 "<reg name=\"z%d\" bitsize=\"%d\""
>>> -                               " regnum=\"%d\" group=\"vector\""
>>> -                               " type=\"vq\"/>",
>>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>>                                 i, reg_width, base_reg++);
>>>          info->num++;
>>>      }
>>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>                             " regnum=\"%d\" group=\"float\""
>>>                             " type=\"int\"/>", base_reg++);
>>>      info->num += 2;
>>> -    /*
>>> -     * Predicate registers aren't so big they are worth splitting up
>>> -     * but we do need to define a type to hold the array of quad
>>> -     * references.
>>> -     */
>>> -    g_string_append_printf(s,
>>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>>> -                           cpu->sve_max_vq);
>>> +
>>>      for (i = 0; i < 16; i++) {
>>>          g_string_append_printf(s,
>>>                                 "<reg name=\"p%d\" bitsize=\"%d\""
>>> -                               " regnum=\"%d\" group=\"vector\""
>>> -                               " type=\"vqp\"/>",
>>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>>                                 i, cpu->sve_max_vq * 16, base_reg++);
>>>          info->num++;
>>>      }
>>>      g_string_append_printf(s,
>>>                             "<reg name=\"ffr\" bitsize=\"%d\""
>>>                             " regnum=\"%d\" group=\"vector\""
>>> -                           " type=\"vqp\"/>",
>>> +                           " type=\"svep\"/>",
>>>                             cpu->sve_max_vq * 16, base_reg++);
>>>      g_string_append_printf(s,
>>>                             "<reg name=\"vg\" bitsize=\"64\""
>>> -                           " regnum=\"%d\" group=\"vector\""
>>> -                           " type=\"uint32\"/>",
>>> +                           " regnum=\"%d\" type=\"int\"/>",
>>>                             base_reg++);
>>>      info->num += 2;
>>>      g_string_append_printf(s, "</feature>");
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index 5ab3f5ace3..8a492465d6 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>>           * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>>           */
>>>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>>> -        return gdb_get_reg32(buf, vq * 2);
>>> +        return gdb_get_reg64(buf, vq * 2);
>>>      }
>>>      default:
>>>          /* gdbstub asked for something out our range */
>>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> index 972cf73c31..b9ef169c1a 100644
>>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>>          except gdb.error:
>>>              report(False, "checking zregs (out of range)")
>>>  
>>> +        # Check the aliased V registers are set and GDB has correctly
>>> +        # created them for us having recognised and handled SVE.
>>> +        try:
>>> +            for i in range(0, 16):
>>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>>> +                report(int(val_z) == int(val_v),
>>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>>> +        except gdb.error:
>>> +            report(False, "checking vregs (out of range)")
>>> +
>>>  
>>>  def run_test():
>>>      "Run through the tests one by one"
>>>
> 
>
Alex Bennée Jan. 19, 2021, 3:54 p.m. UTC | #5
Claudio Fontana <cfontana@suse.de> writes:

> On 1/19/21 3:50 PM, Alex Bennée wrote:
>> 
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>>> Hi Alex,
>>>
>>> after updating to latest master today, I am getting the following error with
>>>
>>> make check-tcg
>>>
>>> qemu-system-aarch64: -gdb unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
>>> warning: while parsing target description (at line 47): Vector "svevhf" references undefined type "ieee_half"
>>> warning: Could not load XML target description; ignoring
>>> qemu-system-aarch64: QEMU: Terminated via GDBstub
>>>
>>> Seems to indicate it is "ieee_half" -related?
>> 
>> *sigh*
>> 
>> yes - it is. I thought this was solved by the GDB version check in
>> 14/30. What does your gdb report?
>
>
> $ gdb --version
> GNU gdb (GDB; openSUSE Leap 15.2) 8.3.1
> Copyright (C) 2019 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
>
> gdb --configuration
> This GDB was configured as follows:
>    configure --host=x86_64-suse-linux --target=x86_64-suse-linux
>              --with-auto-load-dir=$debugdir:$datadir/auto-load
>              --with-auto-load-safe-path=$debugdir:$datadir/auto-load
>              --with-expat
>              --with-gdb-datadir=/usr/share/gdb
>              --with-jit-reader-dir=/usr/lib64/gdb
>              --without-libunwind-ia64
>              --with-lzma
>              --without-babeltrace
>              --with-intel-pt
>              --disable-libmcheck
>              --with-mpfr
>              --with-python=/usr
>              --without-guile
>              --disable-source-highlight
>              --with-separate-debug-dir=/usr/lib/debug
>              --with-system-gdbinit=/etc/gdbinit
>
>
> does this help?

So it looks like TDESC_TYPE_IEEE_HALF was only implemented in GDB 9.1
and there is no probing possible during the gdbstub connection. I guess
I can either go back to stubbing it out (which would break gdb's SVE
understanding) or up our minimum GDB version check for running tests.
That would mean less people test GDB (or at least until the distros
catch up) but considering it was zero people not too long ago maybe
that's acceptable?

>
> Let me know if more info is needed. Thanks!
>
> Claudio
>
>
>> 
>>>
>>> Thanks,
>>>
>>> Claudio
>>>
>>> On 1/15/21 2:08 PM, Alex Bennée wrote:
>>>> While GDB can work with any XML description given to it there is
>>>> special handling for SVE registers on the GDB side which makes the
>>>> users life a little better. The changes aren't that major and all the
>>>> registers save the $vg reported the same. All that changes is:
>>>>
>>>>   - report org.gnu.gdb.aarch64.sve
>>>>   - use gdb nomenclature for names and types
>>>>   - minor re-ordering of the types to match reference
>>>>   - re-enable ieee_half (as we know gdb supports it now)
>>>>   - $vg is now a 64 bit int
>>>>   - check $vN and $zN aliasing in test
>>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Reviewed-by: Luis Machado <luis.machado@linaro.org>
>>>> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
>>>>
>>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>>>> index 866595b4f1..a8fff2a3d0 100644
>>>> --- a/target/arm/gdbstub.c
>>>> +++ b/target/arm/gdbstub.c
>>>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
>>>>      { "uint128", 128, 'q', 'u' },
>>>>      { "int128", 128, 'q', 's' },
>>>>      /* 64 bit */
>>>> +    { "ieee_double", 64, 'd', 'f' },
>>>>      { "uint64", 64, 'd', 'u' },
>>>>      { "int64", 64, 'd', 's' },
>>>> -    { "ieee_double", 64, 'd', 'f' },
>>>>      /* 32 bit */
>>>> +    { "ieee_single", 32, 's', 'f' },
>>>>      { "uint32", 32, 's', 'u' },
>>>>      { "int32", 32, 's', 's' },
>>>> -    { "ieee_single", 32, 's', 'f' },
>>>>      /* 16 bit */
>>>> +    { "ieee_half", 16, 'h', 'f' },
>>>>      { "uint16", 16, 'h', 'u' },
>>>>      { "int16", 16, 'h', 's' },
>>>> -    /*
>>>> -     * TODO: currently there is no reliable way of telling
>>>> -     * if the remote gdb actually understands ieee_half so
>>>> -     * we don't expose it in the target description for now.
>>>> -     * { "ieee_half", 16, 'h', 'f' },
>>>> -     */
>>>>      /* bytes */
>>>>      { "uint8", 8, 'b', 'u' },
>>>>      { "int8", 8, 'b', 's' },
>>>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>>      GString *s = g_string_new(NULL);
>>>>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
>>>>      g_autoptr(GString) ts = g_string_new("");
>>>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
>>>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
>>>>      info->num = 0;
>>>>      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.sve\">");
>>>> +    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
>>>>  
>>>>      /* First define types and totals in a whole VL */
>>>>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>>>          int count = reg_width / vec_lanes[i].size;
>>>> -        g_string_printf(ts, "vq%d%c%c", count,
>>>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
>>>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
>>>>          g_string_append_printf(s,
>>>>                                 "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>>>>                                 ts->str, vec_lanes[i].gdb_type, count);
>>>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>>       * signed and potentially float versions of each size from 128 to
>>>>       * 8 bits.
>>>>       */
>>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>>> -        int count = reg_width / bits;
>>>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
>>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>>> -            if (vec_lanes[i].size == bits) {
>>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
>>>> -                                       vec_lanes[i].suffix,
>>>> -                                       count,
>>>> -                                       vec_lanes[i].sz, vec_lanes[i].suffix);
>>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
>>>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
>>>> +            if (vec_lanes[j].size == bits) {
>>>> +                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
>>>> +                                       vec_lanes[j].suffix,
>>>> +                                       vec_lanes[j].sz, vec_lanes[j].suffix);
>>>>              }
>>>>          }
>>>>          g_string_append(s, "</union>");
>>>>      }
>>>>      /* And now the final union of unions */
>>>> -    g_string_append(s, "<union id=\"vq\">");
>>>> -    for (bits = 128; bits >= 8; bits /= 2) {
>>>> -        int count = reg_width / bits;
>>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
>>>> -            if (vec_lanes[i].size == bits) {
>>>> -                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
>>>> -                                       vec_lanes[i].sz, count);
>>>> -                break;
>>>> -            }
>>>> -        }
>>>> +    g_string_append(s, "<union id=\"svev\">");
>>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
>>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
>>>> +        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
>>>> +                               suf[i], suf[i]);
>>>>      }
>>>>      g_string_append(s, "</union>");
>>>>  
>>>> +    /* Finally the sve prefix type */
>>>> +    g_string_append_printf(s,
>>>> +                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
>>>> +                           reg_width / 8);
>>>> +
>>>>      /* Then define each register in parts for each vq */
>>>>      for (i = 0; i < 32; i++) {
>>>>          g_string_append_printf(s,
>>>>                                 "<reg name=\"z%d\" bitsize=\"%d\""
>>>> -                               " regnum=\"%d\" group=\"vector\""
>>>> -                               " type=\"vq\"/>",
>>>> +                               " regnum=\"%d\" type=\"svev\"/>",
>>>>                                 i, reg_width, base_reg++);
>>>>          info->num++;
>>>>      }
>>>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
>>>>                             " regnum=\"%d\" group=\"float\""
>>>>                             " type=\"int\"/>", base_reg++);
>>>>      info->num += 2;
>>>> -    /*
>>>> -     * Predicate registers aren't so big they are worth splitting up
>>>> -     * but we do need to define a type to hold the array of quad
>>>> -     * references.
>>>> -     */
>>>> -    g_string_append_printf(s,
>>>> -                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
>>>> -                           cpu->sve_max_vq);
>>>> +
>>>>      for (i = 0; i < 16; i++) {
>>>>          g_string_append_printf(s,
>>>>                                 "<reg name=\"p%d\" bitsize=\"%d\""
>>>> -                               " regnum=\"%d\" group=\"vector\""
>>>> -                               " type=\"vqp\"/>",
>>>> +                               " regnum=\"%d\" type=\"svep\"/>",
>>>>                                 i, cpu->sve_max_vq * 16, base_reg++);
>>>>          info->num++;
>>>>      }
>>>>      g_string_append_printf(s,
>>>>                             "<reg name=\"ffr\" bitsize=\"%d\""
>>>>                             " regnum=\"%d\" group=\"vector\""
>>>> -                           " type=\"vqp\"/>",
>>>> +                           " type=\"svep\"/>",
>>>>                             cpu->sve_max_vq * 16, base_reg++);
>>>>      g_string_append_printf(s,
>>>>                             "<reg name=\"vg\" bitsize=\"64\""
>>>> -                           " regnum=\"%d\" group=\"vector\""
>>>> -                           " type=\"uint32\"/>",
>>>> +                           " regnum=\"%d\" type=\"int\"/>",
>>>>                             base_reg++);
>>>>      info->num += 2;
>>>>      g_string_append_printf(s, "</feature>");
>>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>>> index 5ab3f5ace3..8a492465d6 100644
>>>> --- a/target/arm/helper.c
>>>> +++ b/target/arm/helper.c
>>>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
>>>>           * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
>>>>           */
>>>>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
>>>> -        return gdb_get_reg32(buf, vq * 2);
>>>> +        return gdb_get_reg64(buf, vq * 2);
>>>>      }
>>>>      default:
>>>>          /* gdbstub asked for something out our range */
>>>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>>> index 972cf73c31..b9ef169c1a 100644
>>>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
>>>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
>>>>          except gdb.error:
>>>>              report(False, "checking zregs (out of range)")
>>>>  
>>>> +        # Check the aliased V registers are set and GDB has correctly
>>>> +        # created them for us having recognised and handled SVE.
>>>> +        try:
>>>> +            for i in range(0, 16):
>>>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
>>>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
>>>> +                report(int(val_z) == int(val_v),
>>>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
>>>> +        except gdb.error:
>>>> +            report(False, "checking vregs (out of range)")
>>>> +
>>>>  
>>>>  def run_test():
>>>>      "Run through the tests one by one"
>>>>
>> 
>>
Luis Machado Jan. 19, 2021, 4:19 p.m. UTC | #6
This is not ideal. GDB should probably have a way to negotiate available
types.

On Tue, 19 Jan 2021 at 12:57, Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Claudio Fontana <cfontana@suse.de> writes:
>
> > On 1/19/21 3:50 PM, Alex Bennée wrote:
> >>
> >> Claudio Fontana <cfontana@suse.de> writes:
> >>
> >>> Hi Alex,
> >>>
> >>> after updating to latest master today, I am getting the following
> error with
> >>>
> >>> make check-tcg
> >>>
> >>> qemu-system-aarch64: -gdb
> unix:path=/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server: info: QEMU
> waiting for connection on:
> disconnected:unix:/tmp/tmp9ru5tgk8qemu-gdbstub/gdbstub.socket,server
> >>> warning: while parsing target description (at line 47): Vector
> "svevhf" references undefined type "ieee_half"
> >>> warning: Could not load XML target description; ignoring
> >>> qemu-system-aarch64: QEMU: Terminated via GDBstub
> >>>
> >>> Seems to indicate it is "ieee_half" -related?
> >>
> >> *sigh*
> >>
> >> yes - it is. I thought this was solved by the GDB version check in
> >> 14/30. What does your gdb report?
> >
> >
> > $ gdb --version
> > GNU gdb (GDB; openSUSE Leap 15.2) 8.3.1
> > Copyright (C) 2019 Free Software Foundation, Inc.
> > License GPLv3+: GNU GPL version 3 or later <
> http://gnu.org/licenses/gpl.html>
> > This is free software: you are free to change and redistribute it.
> > There is NO WARRANTY, to the extent permitted by law.
> >
> > gdb --configuration
> > This GDB was configured as follows:
> >    configure --host=x86_64-suse-linux --target=x86_64-suse-linux
> >              --with-auto-load-dir=$debugdir:$datadir/auto-load
> >              --with-auto-load-safe-path=$debugdir:$datadir/auto-load
> >              --with-expat
> >              --with-gdb-datadir=/usr/share/gdb
> >              --with-jit-reader-dir=/usr/lib64/gdb
> >              --without-libunwind-ia64
> >              --with-lzma
> >              --without-babeltrace
> >              --with-intel-pt
> >              --disable-libmcheck
> >              --with-mpfr
> >              --with-python=/usr
> >              --without-guile
> >              --disable-source-highlight
> >              --with-separate-debug-dir=/usr/lib/debug
> >              --with-system-gdbinit=/etc/gdbinit
> >
> >
> > does this help?
>
> So it looks like TDESC_TYPE_IEEE_HALF was only implemented in GDB 9.1
> and there is no probing possible during the gdbstub connection. I guess
> I can either go back to stubbing it out (which would break gdb's SVE
> understanding) or up our minimum GDB version check for running tests.
> That would mean less people test GDB (or at least until the distros
> catch up) but considering it was zero people not too long ago maybe
> that's acceptable?
>
> >
> > Let me know if more info is needed. Thanks!
> >
> > Claudio
> >
> >
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Claudio
> >>>
> >>> On 1/15/21 2:08 PM, Alex Bennée wrote:
> >>>> While GDB can work with any XML description given to it there is
> >>>> special handling for SVE registers on the GDB side which makes the
> >>>> users life a little better. The changes aren't that major and all the
> >>>> registers save the $vg reported the same. All that changes is:
> >>>>
> >>>>   - report org.gnu.gdb.aarch64.sve
> >>>>   - use gdb nomenclature for names and types
> >>>>   - minor re-ordering of the types to match reference
> >>>>   - re-enable ieee_half (as we know gdb supports it now)
> >>>>   - $vg is now a 64 bit int
> >>>>   - check $vN and $zN aliasing in test
> >>>>
> >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >>>> Reviewed-by: Luis Machado <luis.machado@linaro.org>
> >>>> Message-Id: <20210108224256.2321-11-alex.bennee@linaro.org>
> >>>>
> >>>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> >>>> index 866595b4f1..a8fff2a3d0 100644
> >>>> --- a/target/arm/gdbstub.c
> >>>> +++ b/target/arm/gdbstub.c
> >>>> @@ -195,22 +195,17 @@ static const struct TypeSize vec_lanes[] = {
> >>>>      { "uint128", 128, 'q', 'u' },
> >>>>      { "int128", 128, 'q', 's' },
> >>>>      /* 64 bit */
> >>>> +    { "ieee_double", 64, 'd', 'f' },
> >>>>      { "uint64", 64, 'd', 'u' },
> >>>>      { "int64", 64, 'd', 's' },
> >>>> -    { "ieee_double", 64, 'd', 'f' },
> >>>>      /* 32 bit */
> >>>> +    { "ieee_single", 32, 's', 'f' },
> >>>>      { "uint32", 32, 's', 'u' },
> >>>>      { "int32", 32, 's', 's' },
> >>>> -    { "ieee_single", 32, 's', 'f' },
> >>>>      /* 16 bit */
> >>>> +    { "ieee_half", 16, 'h', 'f' },
> >>>>      { "uint16", 16, 'h', 'u' },
> >>>>      { "int16", 16, 'h', 's' },
> >>>> -    /*
> >>>> -     * TODO: currently there is no reliable way of telling
> >>>> -     * if the remote gdb actually understands ieee_half so
> >>>> -     * we don't expose it in the target description for now.
> >>>> -     * { "ieee_half", 16, 'h', 'f' },
> >>>> -     */
> >>>>      /* bytes */
> >>>>      { "uint8", 8, 'b', 'u' },
> >>>>      { "int8", 8, 'b', 's' },
> >>>> @@ -223,17 +218,16 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs,
> int base_reg)
> >>>>      GString *s = g_string_new(NULL);
> >>>>      DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
> >>>>      g_autoptr(GString) ts = g_string_new("");
> >>>> -    int i, bits, reg_width = (cpu->sve_max_vq * 128);
> >>>> +    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
> >>>>      info->num = 0;
> >>>>      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.sve\">");
> >>>> +    g_string_append_printf(s, "<feature
> name=\"org.gnu.gdb.aarch64.sve\">");
> >>>>
> >>>>      /* First define types and totals in a whole VL */
> >>>>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> >>>>          int count = reg_width / vec_lanes[i].size;
> >>>> -        g_string_printf(ts, "vq%d%c%c", count,
> >>>> -                        vec_lanes[i].sz, vec_lanes[i].suffix);
> >>>> +        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz,
> vec_lanes[i].suffix);
> >>>>          g_string_append_printf(s,
> >>>>                                 "<vector id=\"%s\" type=\"%s\"
> count=\"%d\"/>",
> >>>>                                 ts->str, vec_lanes[i].gdb_type,
> count);
> >>>> @@ -243,39 +237,37 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs,
> int base_reg)
> >>>>       * signed and potentially float versions of each size from 128 to
> >>>>       * 8 bits.
> >>>>       */
> >>>> -    for (bits = 128; bits >= 8; bits /= 2) {
> >>>> -        int count = reg_width / bits;
> >>>> -        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
> >>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> >>>> -            if (vec_lanes[i].size == bits) {
> >>>> -                g_string_append_printf(s, "<field name=\"%c\"
> type=\"vq%d%c%c\"/>",
> >>>> -                                       vec_lanes[i].suffix,
> >>>> -                                       count,
> >>>> -                                       vec_lanes[i].sz,
> vec_lanes[i].suffix);
> >>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> >>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> >>>> +        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
> >>>> +        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
> >>>> +            if (vec_lanes[j].size == bits) {
> >>>> +                g_string_append_printf(s, "<field name=\"%c\"
> type=\"svev%c%c\"/>",
> >>>> +                                       vec_lanes[j].suffix,
> >>>> +                                       vec_lanes[j].sz,
> vec_lanes[j].suffix);
> >>>>              }
> >>>>          }
> >>>>          g_string_append(s, "</union>");
> >>>>      }
> >>>>      /* And now the final union of unions */
> >>>> -    g_string_append(s, "<union id=\"vq\">");
> >>>> -    for (bits = 128; bits >= 8; bits /= 2) {
> >>>> -        int count = reg_width / bits;
> >>>> -        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> >>>> -            if (vec_lanes[i].size == bits) {
> >>>> -                g_string_append_printf(s, "<field name=\"%c\"
> type=\"vq%dn\"/>",
> >>>> -                                       vec_lanes[i].sz, count);
> >>>> -                break;
> >>>> -            }
> >>>> -        }
> >>>> +    g_string_append(s, "<union id=\"svev\">");
> >>>> +    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
> >>>> +        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
> >>>> +        g_string_append_printf(s, "<field name=\"%c\"
> type=\"svevn%c\"/>",
> >>>> +                               suf[i], suf[i]);
> >>>>      }
> >>>>      g_string_append(s, "</union>");
> >>>>
> >>>> +    /* Finally the sve prefix type */
> >>>> +    g_string_append_printf(s,
> >>>> +                           "<vector id=\"svep\" type=\"uint8\"
> count=\"%d\"/>",
> >>>> +                           reg_width / 8);
> >>>> +
> >>>>      /* Then define each register in parts for each vq */
> >>>>      for (i = 0; i < 32; i++) {
> >>>>          g_string_append_printf(s,
> >>>>                                 "<reg name=\"z%d\" bitsize=\"%d\""
> >>>> -                               " regnum=\"%d\" group=\"vector\""
> >>>> -                               " type=\"vq\"/>",
> >>>> +                               " regnum=\"%d\" type=\"svev\"/>",
> >>>>                                 i, reg_width, base_reg++);
> >>>>          info->num++;
> >>>>      }
> >>>> @@ -287,31 +279,22 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs,
> int base_reg)
> >>>>                             " regnum=\"%d\" group=\"float\""
> >>>>                             " type=\"int\"/>", base_reg++);
> >>>>      info->num += 2;
> >>>> -    /*
> >>>> -     * Predicate registers aren't so big they are worth splitting up
> >>>> -     * but we do need to define a type to hold the array of quad
> >>>> -     * references.
> >>>> -     */
> >>>> -    g_string_append_printf(s,
> >>>> -                           "<vector id=\"vqp\" type=\"uint16\"
> count=\"%d\"/>",
> >>>> -                           cpu->sve_max_vq);
> >>>> +
> >>>>      for (i = 0; i < 16; i++) {
> >>>>          g_string_append_printf(s,
> >>>>                                 "<reg name=\"p%d\" bitsize=\"%d\""
> >>>> -                               " regnum=\"%d\" group=\"vector\""
> >>>> -                               " type=\"vqp\"/>",
> >>>> +                               " regnum=\"%d\" type=\"svep\"/>",
> >>>>                                 i, cpu->sve_max_vq * 16, base_reg++);
> >>>>          info->num++;
> >>>>      }
> >>>>      g_string_append_printf(s,
> >>>>                             "<reg name=\"ffr\" bitsize=\"%d\""
> >>>>                             " regnum=\"%d\" group=\"vector\""
> >>>> -                           " type=\"vqp\"/>",
> >>>> +                           " type=\"svep\"/>",
> >>>>                             cpu->sve_max_vq * 16, base_reg++);
> >>>>      g_string_append_printf(s,
> >>>>                             "<reg name=\"vg\" bitsize=\"64\""
> >>>> -                           " regnum=\"%d\" group=\"vector\""
> >>>> -                           " type=\"uint32\"/>",
> >>>> +                           " regnum=\"%d\" type=\"int\"/>",
> >>>>                             base_reg++);
> >>>>      info->num += 2;
> >>>>      g_string_append_printf(s, "</feature>");
> >>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
> >>>> index 5ab3f5ace3..8a492465d6 100644
> >>>> --- a/target/arm/helper.c
> >>>> +++ b/target/arm/helper.c
> >>>> @@ -276,7 +276,7 @@ static int arm_gdb_get_svereg(CPUARMState *env,
> GByteArray *buf, int reg)
> >>>>           * while the ZCR works in Vector Quads (VQ) which is 128bit
> chunks.
> >>>>           */
> >>>>          int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
> >>>> -        return gdb_get_reg32(buf, vq * 2);
> >>>> +        return gdb_get_reg64(buf, vq * 2);
> >>>>      }
> >>>>      default:
> >>>>          /* gdbstub asked for something out our range */
> >>>> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> >>>> index 972cf73c31..b9ef169c1a 100644
> >>>> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> >>>> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> >>>> @@ -40,6 +40,17 @@ class TestBreakpoint(gdb.Breakpoint):
> >>>>          except gdb.error:
> >>>>              report(False, "checking zregs (out of range)")
> >>>>
> >>>> +        # Check the aliased V registers are set and GDB has correctly
> >>>> +        # created them for us having recognised and handled SVE.
> >>>> +        try:
> >>>> +            for i in range(0, 16):
> >>>> +                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
> >>>> +                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
> >>>> +                report(int(val_z) == int(val_v),
> >>>> +                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
> >>>> +        except gdb.error:
> >>>> +            report(False, "checking vregs (out of range)")
> >>>> +
> >>>>
> >>>>  def run_test():
> >>>>      "Run through the tests one by one"
> >>>>
> >>
> >>
>
>
> --
> Alex Bennée
>
diff mbox series

Patch

diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 866595b4f1..a8fff2a3d0 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -195,22 +195,17 @@  static const struct TypeSize vec_lanes[] = {
     { "uint128", 128, 'q', 'u' },
     { "int128", 128, 'q', 's' },
     /* 64 bit */
+    { "ieee_double", 64, 'd', 'f' },
     { "uint64", 64, 'd', 'u' },
     { "int64", 64, 'd', 's' },
-    { "ieee_double", 64, 'd', 'f' },
     /* 32 bit */
+    { "ieee_single", 32, 's', 'f' },
     { "uint32", 32, 's', 'u' },
     { "int32", 32, 's', 's' },
-    { "ieee_single", 32, 's', 'f' },
     /* 16 bit */
+    { "ieee_half", 16, 'h', 'f' },
     { "uint16", 16, 'h', 'u' },
     { "int16", 16, 'h', 's' },
-    /*
-     * TODO: currently there is no reliable way of telling
-     * if the remote gdb actually understands ieee_half so
-     * we don't expose it in the target description for now.
-     * { "ieee_half", 16, 'h', 'f' },
-     */
     /* bytes */
     { "uint8", 8, 'b', 'u' },
     { "int8", 8, 'b', 's' },
@@ -223,17 +218,16 @@  int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
     GString *s = g_string_new(NULL);
     DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
     g_autoptr(GString) ts = g_string_new("");
-    int i, bits, reg_width = (cpu->sve_max_vq * 128);
+    int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
     info->num = 0;
     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.sve\">");
+    g_string_append_printf(s, "<feature name=\"org.gnu.gdb.aarch64.sve\">");
 
     /* First define types and totals in a whole VL */
     for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
         int count = reg_width / vec_lanes[i].size;
-        g_string_printf(ts, "vq%d%c%c", count,
-                        vec_lanes[i].sz, vec_lanes[i].suffix);
+        g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
         g_string_append_printf(s,
                                "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
                                ts->str, vec_lanes[i].gdb_type, count);
@@ -243,39 +237,37 @@  int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
      * signed and potentially float versions of each size from 128 to
      * 8 bits.
      */
-    for (bits = 128; bits >= 8; bits /= 2) {
-        int count = reg_width / bits;
-        g_string_append_printf(s, "<union id=\"vq%dn\">", count);
-        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-            if (vec_lanes[i].size == bits) {
-                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%d%c%c\"/>",
-                                       vec_lanes[i].suffix,
-                                       count,
-                                       vec_lanes[i].sz, vec_lanes[i].suffix);
+    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+        g_string_append_printf(s, "<union id=\"svevn%c\">", suf[i]);
+        for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
+            if (vec_lanes[j].size == bits) {
+                g_string_append_printf(s, "<field name=\"%c\" type=\"svev%c%c\"/>",
+                                       vec_lanes[j].suffix,
+                                       vec_lanes[j].sz, vec_lanes[j].suffix);
             }
         }
         g_string_append(s, "</union>");
     }
     /* And now the final union of unions */
-    g_string_append(s, "<union id=\"vq\">");
-    for (bits = 128; bits >= 8; bits /= 2) {
-        int count = reg_width / bits;
-        for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-            if (vec_lanes[i].size == bits) {
-                g_string_append_printf(s, "<field name=\"%c\" type=\"vq%dn\"/>",
-                                       vec_lanes[i].sz, count);
-                break;
-            }
-        }
+    g_string_append(s, "<union id=\"svev\">");
+    for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+        const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+        g_string_append_printf(s, "<field name=\"%c\" type=\"svevn%c\"/>",
+                               suf[i], suf[i]);
     }
     g_string_append(s, "</union>");
 
+    /* Finally the sve prefix type */
+    g_string_append_printf(s,
+                           "<vector id=\"svep\" type=\"uint8\" count=\"%d\"/>",
+                           reg_width / 8);
+
     /* Then define each register in parts for each vq */
     for (i = 0; i < 32; i++) {
         g_string_append_printf(s,
                                "<reg name=\"z%d\" bitsize=\"%d\""
-                               " regnum=\"%d\" group=\"vector\""
-                               " type=\"vq\"/>",
+                               " regnum=\"%d\" type=\"svev\"/>",
                                i, reg_width, base_reg++);
         info->num++;
     }
@@ -287,31 +279,22 @@  int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
                            " regnum=\"%d\" group=\"float\""
                            " type=\"int\"/>", base_reg++);
     info->num += 2;
-    /*
-     * Predicate registers aren't so big they are worth splitting up
-     * but we do need to define a type to hold the array of quad
-     * references.
-     */
-    g_string_append_printf(s,
-                           "<vector id=\"vqp\" type=\"uint16\" count=\"%d\"/>",
-                           cpu->sve_max_vq);
+
     for (i = 0; i < 16; i++) {
         g_string_append_printf(s,
                                "<reg name=\"p%d\" bitsize=\"%d\""
-                               " regnum=\"%d\" group=\"vector\""
-                               " type=\"vqp\"/>",
+                               " regnum=\"%d\" type=\"svep\"/>",
                                i, cpu->sve_max_vq * 16, base_reg++);
         info->num++;
     }
     g_string_append_printf(s,
                            "<reg name=\"ffr\" bitsize=\"%d\""
                            " regnum=\"%d\" group=\"vector\""
-                           " type=\"vqp\"/>",
+                           " type=\"svep\"/>",
                            cpu->sve_max_vq * 16, base_reg++);
     g_string_append_printf(s,
                            "<reg name=\"vg\" bitsize=\"64\""
-                           " regnum=\"%d\" group=\"vector\""
-                           " type=\"uint32\"/>",
+                           " regnum=\"%d\" type=\"int\"/>",
                            base_reg++);
     info->num += 2;
     g_string_append_printf(s, "</feature>");
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5ab3f5ace3..8a492465d6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -276,7 +276,7 @@  static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
          * while the ZCR works in Vector Quads (VQ) which is 128bit chunks.
          */
         int vq = sve_zcr_len_for_el(env, arm_current_el(env)) + 1;
-        return gdb_get_reg32(buf, vq * 2);
+        return gdb_get_reg64(buf, vq * 2);
     }
     default:
         /* gdbstub asked for something out our range */
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index 972cf73c31..b9ef169c1a 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -40,6 +40,17 @@  class TestBreakpoint(gdb.Breakpoint):
         except gdb.error:
             report(False, "checking zregs (out of range)")
 
+        # Check the aliased V registers are set and GDB has correctly
+        # created them for us having recognised and handled SVE.
+        try:
+            for i in range(0, 16):
+                val_z = gdb.parse_and_eval("$z0.b.u[%d]" % i)
+                val_v = gdb.parse_and_eval("$v0.b.u[%d]" % i)
+                report(int(val_z) == int(val_v),
+                       "v0.b.u[%d] == z0.b.u[%d]" % (i, i))
+        except gdb.error:
+            report(False, "checking vregs (out of range)")
+
 
 def run_test():
     "Run through the tests one by one"