include/exec/gdbstub.h | 9 +++++++++ target/arm/gdbstub.c | 3 +-- target/xtensa/gdbstub.c | 6 ++---- 3 files changed, 12 insertions(+), 6 deletions(-)
Introduce gdb_get_zeroes() to fill a GByteArray with zeroes.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/exec/gdbstub.h | 9 +++++++++
target/arm/gdbstub.c | 3 +--
target/xtensa/gdbstub.c | 6 ++----
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 30b909ebd2..b52d9933ee 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -125,6 +125,15 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi,
return 16;
}
+static inline int gdb_get_zeroes(GByteArray *array, size_t len)
+{
+ for (size_t i = 0; i < len; i++) {
+ gdb_get_reg8(array, '\0');
+ }
+
+ return len;
+}
+
/**
* gdb_get_reg_ptr: get pointer to start of last element
* @len: length of element
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 8efc535f2a..063551df23 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -47,8 +47,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
if (gdb_has_xml) {
return 0;
}
- memset(mem_buf, 0, 12);
- return 12;
+ return gdb_get_zeroes(mem_buf, 12);
}
switch (n) {
case 24:
diff --git a/target/xtensa/gdbstub.c b/target/xtensa/gdbstub.c
index 0ee3feabe5..4d43f1340a 100644
--- a/target/xtensa/gdbstub.c
+++ b/target/xtensa/gdbstub.c
@@ -105,8 +105,7 @@ int xtensa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
default:
qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported size %d\n",
__func__, n, reg->size);
- memset(mem_buf, 0, reg->size);
- return reg->size;
+ return gdb_get_zeroes(mem_buf, reg->size);
}
case xtRegisterTypeWindow: /*a*/
@@ -115,8 +114,7 @@ int xtensa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
default:
qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported type %d\n",
__func__, n, reg->type);
- memset(mem_buf, 0, reg->size);
- return reg->size;
+ return gdb_get_zeroes(mem_buf, reg->size);
}
}
--
2.21.1
On Tue, 14 Apr 2020 at 11:24, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Introduce gdb_get_zeroes() to fill a GByteArray with zeroes. > > Fixes: a010bdbe719 ("extend GByteArray to read register helpers") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/exec/gdbstub.h | 9 +++++++++ > target/arm/gdbstub.c | 3 +-- > target/xtensa/gdbstub.c | 6 ++---- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > index 30b909ebd2..b52d9933ee 100644 > --- a/include/exec/gdbstub.h > +++ b/include/exec/gdbstub.h > @@ -125,6 +125,15 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi, > return 16; > } > > +static inline int gdb_get_zeroes(GByteArray *array, size_t len) > +{ > + for (size_t i = 0; i < len; i++) { > + gdb_get_reg8(array, '\0'); > + } > + > + return len; > +} The other implementation option here would be guint oldlen = array->len; g_byte_array_set_size(array, oldlen + len); memset(array->data + oldlen, 0, len); For length values < 16 my guess is the perf difference is not going to be noticeable though. thanks -- PMM
On 4/14/20 12:52 PM, Peter Maydell wrote: > On Tue, 14 Apr 2020 at 11:24, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> >> Introduce gdb_get_zeroes() to fill a GByteArray with zeroes. >> >> Fixes: a010bdbe719 ("extend GByteArray to read register helpers") >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/exec/gdbstub.h | 9 +++++++++ >> target/arm/gdbstub.c | 3 +-- >> target/xtensa/gdbstub.c | 6 ++---- >> 3 files changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h >> index 30b909ebd2..b52d9933ee 100644 >> --- a/include/exec/gdbstub.h >> +++ b/include/exec/gdbstub.h >> @@ -125,6 +125,15 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi, >> return 16; >> } >> >> +static inline int gdb_get_zeroes(GByteArray *array, size_t len) >> +{ >> + for (size_t i = 0; i < len; i++) { >> + gdb_get_reg8(array, '\0'); >> + } >> + >> + return len; >> +} > > The other implementation option here would be > > guint oldlen = array->len; > g_byte_array_set_size(array, oldlen + len); > memset(array->data + oldlen, 0, len); I thought about it but I'd rather not access GByteArray internals. > > For length values < 16 my guess is the perf difference is > not going to be noticeable though. On ARM it is called with size=12: target/arm/gdbstub.c:50: memset(mem_buf, 0, 12); On Xtensa it is only used with unimplemented registers: qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported type %d\n", __func__, n, reg->type); If you prefer I can use a static empty buffer of 16 bytes and copy from it: static inline int gdb_get_zeroes(GByteArray *array, size_t len) { static const uint8_t zeroed_buf[16] = { }; g_byte_array_append(array, zeroed_buf, MIN(len, sizeof(zeroed_buf))); for (size_t i = sizeof(zeroed_buf); i < len; i++) { gdb_get_reg8(array, '\0'); } return len; } > > thanks > -- PMM >
On Tue, 14 Apr 2020 at 12:05, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > The other implementation option here would be > > > > guint oldlen = array->len; > > g_byte_array_set_size(array, oldlen + len); > > memset(array->data + oldlen, 0, len); > > I thought about it but I'd rather not access GByteArray internals. AIUI ->len and ->data are not internals -- they're in the documentation as "public fields" and the code example for GByteArray directly accesses ->data. (Contrast GBytes, where there are no defined public fields and instead there are functions g_bytes_get_data and g_bytes_get_size.) thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 14 Apr 2020 at 12:05, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> > The other implementation option here would be >> > >> > guint oldlen = array->len; >> > g_byte_array_set_size(array, oldlen + len); >> > memset(array->data + oldlen, 0, len); >> >> I thought about it but I'd rather not access GByteArray internals. > > AIUI ->len and ->data are not internals -- they're > in the documentation as "public fields" and the code > example for GByteArray directly accesses ->data. They are - and I do it in a few places in the gdbstub when it's easier than going long form. FWIW I prefer Peter's formulation. > (Contrast GBytes, where there are no defined public > fields and instead there are functions g_bytes_get_data > and g_bytes_get_size.) > > thanks > -- PMM -- Alex Bennée
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Introduce gdb_get_zeroes() to fill a GByteArray with zeroes. > > Fixes: a010bdbe719 ("extend GByteArray to read register helpers") > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Queued to for-5.0/more-random-fixes with Peter's alternative. > --- > include/exec/gdbstub.h | 9 +++++++++ > target/arm/gdbstub.c | 3 +-- > target/xtensa/gdbstub.c | 6 ++---- > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h > index 30b909ebd2..b52d9933ee 100644 > --- a/include/exec/gdbstub.h > +++ b/include/exec/gdbstub.h > @@ -125,6 +125,15 @@ static inline int gdb_get_reg128(GByteArray *buf, uint64_t val_hi, > return 16; > } > > +static inline int gdb_get_zeroes(GByteArray *array, size_t len) > +{ > + for (size_t i = 0; i < len; i++) { > + gdb_get_reg8(array, '\0'); > + } > + > + return len; > +} > + > /** > * gdb_get_reg_ptr: get pointer to start of last element > * @len: length of element > diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c > index 8efc535f2a..063551df23 100644 > --- a/target/arm/gdbstub.c > +++ b/target/arm/gdbstub.c > @@ -47,8 +47,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > if (gdb_has_xml) { > return 0; > } > - memset(mem_buf, 0, 12); > - return 12; > + return gdb_get_zeroes(mem_buf, 12); > } > switch (n) { > case 24: > diff --git a/target/xtensa/gdbstub.c b/target/xtensa/gdbstub.c > index 0ee3feabe5..4d43f1340a 100644 > --- a/target/xtensa/gdbstub.c > +++ b/target/xtensa/gdbstub.c > @@ -105,8 +105,7 @@ int xtensa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > default: > qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported size %d\n", > __func__, n, reg->size); > - memset(mem_buf, 0, reg->size); > - return reg->size; > + return gdb_get_zeroes(mem_buf, reg->size); > } > > case xtRegisterTypeWindow: /*a*/ > @@ -115,8 +114,7 @@ int xtensa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n) > default: > qemu_log_mask(LOG_UNIMP, "%s from reg %d of unsupported type %d\n", > __func__, n, reg->type); > - memset(mem_buf, 0, reg->size); > - return reg->size; > + return gdb_get_zeroes(mem_buf, reg->size); > } > } -- Alex Bennée
© 2016 - 2024 Red Hat, Inc.