| 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 |
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" >
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" >> > >
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" >>
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" >>> > >
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" >>>> >> >>
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 >
On Tue, 19 Jan 2021 at 15: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: > >>> 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? > 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? I just ran into this trying to connect qemu-aarch64 to the Ubuntu gdb-multiarch. I don't care about SVE at all in this case, but the 'max' CPU includes SVE by default, so we report it to gdb even if the guest program being run doesn't use SVE at all. This effectively means that usecases that used to work no longer do :-( Luis: do we really have to report to gdb all the possible data types that might be in SVE vector registers? Won't gdb autogenerate pseudoregisters the way it does with Neon d0..d31 ? thanks -- PMM
Hi, On 9/21/21 10:55 AM, Peter Maydell wrote: > On Tue, 19 Jan 2021 at 15: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: >>>>> 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? > >> 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? > > I just ran into this trying to connect qemu-aarch64 to the > Ubuntu gdb-multiarch. I don't care about SVE at all in this > case, but the 'max' CPU includes SVE by default, so we report > it to gdb even if the guest program being run doesn't use SVE at all. > This effectively means that usecases that used to work no longer do :-( > > Luis: do we really have to report to gdb all the possible > data types that might be in SVE vector registers? Won't > gdb autogenerate pseudoregisters the way it does with > Neon d0..d31 ? > > thanks > -- PMM > I'll check what can be done here.
On 10/4/21 3:44 PM, Luis Machado wrote: > Hi, > > On 9/21/21 10:55 AM, Peter Maydell wrote: >> On Tue, 19 Jan 2021 at 15: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: >>>>>> 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? >> >>> 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? >> >> I just ran into this trying to connect qemu-aarch64 to the >> Ubuntu gdb-multiarch. I don't care about SVE at all in this >> case, but the 'max' CPU includes SVE by default, so we report >> it to gdb even if the guest program being run doesn't use SVE at all. >> This effectively means that usecases that used to work no longer do :-( >> >> Luis: do we really have to report to gdb all the possible >> data types that might be in SVE vector registers? Won't >> gdb autogenerate pseudoregisters the way it does with >> Neon d0..d31 ? >> >> thanks >> -- PMM >> > > I'll check what can be done here. Apologies. I got sidetracked with a few other things. I'm getting back to this one. I'm guessing the less painful solution would be for QEMU to report a more compact XML description for SVE, supported by the oldest GDB we would like to validate things on, and leave it to GDB (newer releases) to add whatever pseudo-registers it deems appropriate.
On 11/4/21 6:03 PM, Luis Machado wrote: > On 10/4/21 3:44 PM, Luis Machado wrote: >> Hi, >> >> On 9/21/21 10:55 AM, Peter Maydell wrote: >>> On Tue, 19 Jan 2021 at 15: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: >>>>>>> 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? >>> >>>> 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? >>> >>> I just ran into this trying to connect qemu-aarch64 to the >>> Ubuntu gdb-multiarch. I don't care about SVE at all in this >>> case, but the 'max' CPU includes SVE by default, so we report >>> it to gdb even if the guest program being run doesn't use SVE at all. >>> This effectively means that usecases that used to work no longer do :-( >>> >>> Luis: do we really have to report to gdb all the possible >>> data types that might be in SVE vector registers? Won't >>> gdb autogenerate pseudoregisters the way it does with >>> Neon d0..d31 ? >>> >>> thanks >>> -- PMM >>> >> >> I'll check what can be done here. > > Apologies. I got sidetracked with a few other things. I'm getting back > to this one. > > I'm guessing the less painful solution would be for QEMU to report a > more compact XML description for SVE, supported by the oldest GDB we > would like to validate things on, and leave it to GDB (newer releases) > to add whatever pseudo-registers it deems appropriate. Is it only the ieee_half type that is causing issues here? Or are there other types?
Luis Machado <luis.machado@linaro.org> writes: > On 11/4/21 6:03 PM, Luis Machado wrote: >> On 10/4/21 3:44 PM, Luis Machado wrote: >>> Hi, >>> >>> On 9/21/21 10:55 AM, Peter Maydell wrote: >>>> On Tue, 19 Jan 2021 at 15: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: >>>>>>>> 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? >>>> >>>>> 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? >>>> >>>> I just ran into this trying to connect qemu-aarch64 to the >>>> Ubuntu gdb-multiarch. I don't care about SVE at all in this >>>> case, but the 'max' CPU includes SVE by default, so we report >>>> it to gdb even if the guest program being run doesn't use SVE at all. >>>> This effectively means that usecases that used to work no longer do :-( >>>> >>>> Luis: do we really have to report to gdb all the possible >>>> data types that might be in SVE vector registers? Won't >>>> gdb autogenerate pseudoregisters the way it does with >>>> Neon d0..d31 ? >>>> >>>> thanks >>>> -- PMM >>>> >>> >>> I'll check what can be done here. >> Apologies. I got sidetracked with a few other things. I'm getting >> back to this one. >> I'm guessing the less painful solution would be for QEMU to report a >> more compact XML description for SVE, supported by the oldest GDB we >> would like to validate things on, and leave it to GDB (newer >> releases) to add whatever pseudo-registers it deems appropriate. > > Is it only the ieee_half type that is causing issues here? Or are > there other types? That was the only one I noticed - which makes sense as it's a fairly new type.
On 11/5/21 1:15 PM, Alex Bennée wrote: > > Luis Machado <luis.machado@linaro.org> writes: > >> On 11/4/21 6:03 PM, Luis Machado wrote: >>> On 10/4/21 3:44 PM, Luis Machado wrote: >>>> Hi, >>>> >>>> On 9/21/21 10:55 AM, Peter Maydell wrote: >>>>> On Tue, 19 Jan 2021 at 15: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: >>>>>>>>> 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? >>>>> >>>>>> 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? >>>>> >>>>> I just ran into this trying to connect qemu-aarch64 to the >>>>> Ubuntu gdb-multiarch. I don't care about SVE at all in this >>>>> case, but the 'max' CPU includes SVE by default, so we report >>>>> it to gdb even if the guest program being run doesn't use SVE at all. >>>>> This effectively means that usecases that used to work no longer do :-( >>>>> >>>>> Luis: do we really have to report to gdb all the possible >>>>> data types that might be in SVE vector registers? Won't >>>>> gdb autogenerate pseudoregisters the way it does with >>>>> Neon d0..d31 ? >>>>> >>>>> thanks >>>>> -- PMM >>>>> >>>> >>>> I'll check what can be done here. >>> Apologies. I got sidetracked with a few other things. I'm getting >>> back to this one. >>> I'm guessing the less painful solution would be for QEMU to report a >>> more compact XML description for SVE, supported by the oldest GDB we >>> would like to validate things on, and leave it to GDB (newer >>> releases) to add whatever pseudo-registers it deems appropriate. >> >> Is it only the ieee_half type that is causing issues here? Or are >> there other types? > > That was the only one I noticed - which makes sense as it's a fairly new > type. > > Ok. I'm considering dialing down the number of subtypes for SVE Z registers, which would greatly simplify the generation of the XML for SVE features. But I still need to do some more investigation.
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"