| Message ID | 20191211170520.7747-13-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdbstub refactor and SVE support (+check-tcg tweaks) | expand |
On 12/11/19 9:05 AM, Alex Bennée wrote: > +static struct TypeSize vec_lanes[] = { const. > + case 51: > + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); You need to use sve_zcr_len_for_el to get the effective vq. Also, I thought vg == 2 * vq. > + case 51: > + { > + uint64_t val = *(uint64_t *) buf; > + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 12/11/19 9:05 AM, Alex Bennée wrote: >> +static struct TypeSize vec_lanes[] = { > > const. > >> + case 51: >> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); > > You need to use sve_zcr_len_for_el to get the effective vq. > Also, I thought vg == 2 * vq. > > + case 51: >> + { >> + uint64_t val = *(uint64_t *) buf; >> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; > > You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq > decreases, you must call aarch64_sve_narrow_vq. You must call > arm_rebuild_hflags. Hmm thinking about it this is overriding the kernels competencies - it should be read only as it is a "virtual" register. Given gdbserver doesn't use the value and will most likely use dynamic XML we could just drop it altogether. > > > r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 12/11/19 9:05 AM, Alex Bennée wrote: >> +static struct TypeSize vec_lanes[] = { > > const. > >> + case 51: >> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); > > You need to use sve_zcr_len_for_el to get the effective vq. > Also, I thought vg == 2 * vq. > > + case 51: >> + { >> + uint64_t val = *(uint64_t *) buf; >> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; > > You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq > decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. I'm just going to drop vg (and therefor the ability to set it) from the regset. It was only meant to be an indicator and gdb doesn't actually look to it to size it's output. The likely dynamic extension will just re-transmit the whole XML when a change occurs.
On 12/19/19 4:15 PM, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 12/11/19 9:05 AM, Alex Bennée wrote: >>> +static struct TypeSize vec_lanes[] = { >> >> const. >> >>> + case 51: >>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >> >> You need to use sve_zcr_len_for_el to get the effective vq. >> Also, I thought vg == 2 * vq. >> > + case 51: >>> + { >>> + uint64_t val = *(uint64_t *) buf; >>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >> >> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. > > I'm just going to drop vg (and therefor the ability to set it) from the > regset. It was only meant to be an indicator and gdb doesn't actually > look to it to size it's output. The likely dynamic extension will just > re-transmit the whole XML when a change occurs. > I'd verify with GDB first if vg isn't actually required. From looking at GDB's code, it does set vg as one of the register names, and this is regardless of any XML input. It does reference VG here and there in the code, even though it may not use it to size its output.
Luis Machado <luis.machado@linaro.org> writes: > On 12/19/19 4:15 PM, Alex Bennée wrote: >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> On 12/11/19 9:05 AM, Alex Bennée wrote: >>>> +static struct TypeSize vec_lanes[] = { >>> >>> const. >>> >>>> + case 51: >>>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >>> >>> You need to use sve_zcr_len_for_el to get the effective vq. >>> Also, I thought vg == 2 * vq. >>> > + case 51: >>>> + { >>>> + uint64_t val = *(uint64_t *) buf; >>>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >>> >>> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >>> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. >> I'm just going to drop vg (and therefor the ability to set it) from >> the >> regset. It was only meant to be an indicator and gdb doesn't actually >> look to it to size it's output. The likely dynamic extension will just >> re-transmit the whole XML when a change occurs. >> > > I'd verify with GDB first if vg isn't actually required. It works with my tests but perhaps we use our own namespaced XML rather than the gdbstub XML. > From looking at GDB's code, it does set vg as one of the register > names, and this is regardless of any XML input. It does reference VG > here and there in the code, even though it may not use it to size its > output. But this is all special casing for feature name="org.gnu.gdb.aarch64.sve" right?
On 12/20/19 10:14 AM, Alex Bennée wrote: > > Luis Machado <luis.machado@linaro.org> writes: > >> On 12/19/19 4:15 PM, Alex Bennée wrote: >>> Richard Henderson <richard.henderson@linaro.org> writes: >>> >>>> On 12/11/19 9:05 AM, Alex Bennée wrote: >>>>> +static struct TypeSize vec_lanes[] = { >>>> >>>> const. >>>> >>>>> + case 51: >>>>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >>>> >>>> You need to use sve_zcr_len_for_el to get the effective vq. >>>> Also, I thought vg == 2 * vq. >>>> > + case 51: >>>>> + { >>>>> + uint64_t val = *(uint64_t *) buf; >>>>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >>>> >>>> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >>>> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. >>> I'm just going to drop vg (and therefor the ability to set it) from >>> the >>> regset. It was only meant to be an indicator and gdb doesn't actually >>> look to it to size it's output. The likely dynamic extension will just >>> re-transmit the whole XML when a change occurs. >>> >> >> I'd verify with GDB first if vg isn't actually required. > > It works with my tests but perhaps we use our own namespaced XML rather > than the gdbstub XML. > >> From looking at GDB's code, it does set vg as one of the register >> names, and this is regardless of any XML input. It does reference VG >> here and there in the code, even though it may not use it to size its >> output. > > But this is all special casing for feature > name="org.gnu.gdb.aarch64.sve" right? > Yes, vg is only available if feature org.gnu.gdb.aarch64.sve is available.
> On 20 Dec 2019, at 13:18, Luis Machado <luis.machado@linaro.org> wrote: > > On 12/20/19 10:14 AM, Alex Bennée wrote: >> Luis Machado <luis.machado@linaro.org> writes: >>> On 12/19/19 4:15 PM, Alex Bennée wrote: >>>> Richard Henderson <richard.henderson@linaro.org> writes: >>>> >>>>> On 12/11/19 9:05 AM, Alex Bennée wrote: >>>>>> +static struct TypeSize vec_lanes[] = { >>>>> >>>>> const. >>>>> >>>>>> + case 51: >>>>>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >>>>> >>>>> You need to use sve_zcr_len_for_el to get the effective vq. >>>>> Also, I thought vg == 2 * vq. >>>>> > + case 51: >>>>>> + { >>>>>> + uint64_t val = *(uint64_t *) buf; >>>>>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >>>>> >>>>> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >>>>> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. >>>> I'm just going to drop vg (and therefor the ability to set it) from >>>> the >>>> regset. It was only meant to be an indicator and gdb doesn't actually >>>> look to it to size it's output. The likely dynamic extension will just >>>> re-transmit the whole XML when a change occurs. >>>> [...rebooting self after festive break] Yes, when using a gdb stub, changes to the VG value should be silently ignored by the stub. In addition, if the vector length on the system does magically change whilst the program is running, then the stub should continue to use the register sizes for the originally transmitted XML, truncating/extending the register values as required. Why? Because once the stub has sent the XML description to GDB on program start, then GDB assumes the XML description will never change. GDB will error if sent packets with different register lengths. >>> >>> I'd verify with GDB first if vg isn't actually required. >> It works with my tests but perhaps we use our own namespaced XML rather >> than the gdbstub XML. Yeah, if your gdb stub is providing an XML description, then essentially it can provide whatever registers it wants to in the XML, and most stuff will just work. When using a gdb stub, nothing (as far as I can recall) in GDB is relying on VG existing or using its value. >>> From looking at GDB's code, it does set vg as one of the register >>> names, and this is regardless of any XML input. It does reference VG >>> here and there in the code, even though it may not use it to size its >>> output. When using GDB a real SVE Linux box without a stub/gdbserver, then: *Before doing anything with the target description (for example after every breakpoint), GDB always asks the target (ie the GDB aarch64 port) for the XML description again. This function re-reads the vector length; if the length has changed then it constructs a new XML description and forces the registers to be re-read. *When writing the registers back, the aarch64 target checks the VG value and if changed writes that first. This can fail (with an error printed to the output). The rest of the registers are then scaled and written according to the new size. There is no need to change the xml at this stage as it’ll be done the next time GDB uses the description. Blindly enabling the above when using a stub results in in GDB *constantly* asking the stub for a new XML description, spamming the pipe, so this needs something more nuanced. I plan on sending Luis my ideas I had for VG changing when using a stub. >> But this is all special casing for feature >> name="org.gnu.gdb.aarch64.sve" right? > > Yes, vg is only available if feature org.gnu.gdb.aarch64.sve is available. Nod. Alan.
Alan Hayward <Alan.Hayward@arm.com> writes: >> On 20 Dec 2019, at 13:18, Luis Machado <luis.machado@linaro.org> wrote: >> >> On 12/20/19 10:14 AM, Alex Bennée wrote: >>> Luis Machado <luis.machado@linaro.org> writes: >>>> On 12/19/19 4:15 PM, Alex Bennée wrote: >>>>> Richard Henderson <richard.henderson@linaro.org> writes: >>>>> >>>>>> On 12/11/19 9:05 AM, Alex Bennée wrote: >>>>>>> +static struct TypeSize vec_lanes[] = { >>>>>> >>>>>> const. >>>>>> >>>>>>> + case 51: >>>>>>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >>>>>> >>>>>> You need to use sve_zcr_len_for_el to get the effective vq. >>>>>> Also, I thought vg == 2 * vq. >>>>>> > + case 51: >>>>>>> + { >>>>>>> + uint64_t val = *(uint64_t *) buf; >>>>>>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >>>>>> >>>>>> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >>>>>> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. >>>>> I'm just going to drop vg (and therefor the ability to set it) from >>>>> the >>>>> regset. It was only meant to be an indicator and gdb doesn't actually >>>>> look to it to size it's output. The likely dynamic extension will just >>>>> re-transmit the whole XML when a change occurs. >>>>> > > [...rebooting self after festive break] > > Yes, when using a gdb stub, changes to the VG value should be silently ignored by the > stub. > In addition, if the vector length on the system does magically change whilst the program > is running, then the stub should continue to use the register sizes for the originally > transmitted XML, truncating/extending the register values as required. > > Why? Because once the stub has sent the XML description to GDB on program start, then GDB > assumes the XML description will never change. GDB will error if sent packets with different > register lengths. Ack. The test "test-sve-ioctl.py" covers this and works as expected. <snip> > > When using GDB a real SVE Linux box without a stub/gdbserver, then: > <snip> > > Blindly enabling the above when using a stub results in in GDB *constantly* asking the > stub for a new XML description, spamming the pipe, so this needs something more nuanced. > > I plan on sending Luis my ideas I had for VG changing when using a > stub. Is this going to be a more general solution because I'm sure there are other cases where the XML description is out of date. A big one is execution modes (thumb/32/64 bit) and I think x86 runs into similar problems with it's various mode changes in early boot-up. For now I'll just let qemu provide it's own xml without vg shenanigans. >>> But this is all special casing for feature >>> name="org.gnu.gdb.aarch64.sve" right? >> >> Yes, vg is only available if feature org.gnu.gdb.aarch64.sve is available. > > Nod. Thanks and hi ;-)
> On 9 Jan 2020, at 12:08, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Alan Hayward <Alan.Hayward@arm.com> writes: > >>> On 20 Dec 2019, at 13:18, Luis Machado <luis.machado@linaro.org> wrote: >>> >>> On 12/20/19 10:14 AM, Alex Bennée wrote: >>>> Luis Machado <luis.machado@linaro.org> writes: >>>>> On 12/19/19 4:15 PM, Alex Bennée wrote: >>>>>> Richard Henderson <richard.henderson@linaro.org> writes: >>>>>> >>>>>>> On 12/11/19 9:05 AM, Alex Bennée wrote: >>>>>>>> +static struct TypeSize vec_lanes[] = { >>>>>>> >>>>>>> const. >>>>>>> >>>>>>>> + case 51: >>>>>>>> + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); >>>>>>> >>>>>>> You need to use sve_zcr_len_for_el to get the effective vq. >>>>>>> Also, I thought vg == 2 * vq. >>>>>>>> + case 51: >>>>>>>> + { >>>>>>>> + uint64_t val = *(uint64_t *) buf; >>>>>>>> + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; >>>>>>> >>>>>>> You cannot hard-code EL1 without ifdef CONFIG_USER_ONLY. If the effective vq >>>>>>> decreases, you must call aarch64_sve_narrow_vq. You must call arm_rebuild_hflags. >>>>>> I'm just going to drop vg (and therefor the ability to set it) from >>>>>> the >>>>>> regset. It was only meant to be an indicator and gdb doesn't actually >>>>>> look to it to size it's output. The likely dynamic extension will just >>>>>> re-transmit the whole XML when a change occurs. >>>>>> >> >> [...rebooting self after festive break] >> >> Yes, when using a gdb stub, changes to the VG value should be silently ignored by the >> stub. >> In addition, if the vector length on the system does magically change whilst the program >> is running, then the stub should continue to use the register sizes for the originally >> transmitted XML, truncating/extending the register values as required. >> >> Why? Because once the stub has sent the XML description to GDB on program start, then GDB >> assumes the XML description will never change. GDB will error if sent packets with different >> register lengths. > > Ack. The test "test-sve-ioctl.py" covers this and works as expected. > > <snip> >> >> When using GDB a real SVE Linux box without a stub/gdbserver, then: >> > <snip> >> >> Blindly enabling the above when using a stub results in in GDB *constantly* asking the >> stub for a new XML description, spamming the pipe, so this needs something more nuanced. >> >> I plan on sending Luis my ideas I had for VG changing when using a >> stub. > > Is this going to be a more general solution because I'm sure there are > other cases where the XML description is out of date. A big one is > execution modes (thumb/32/64 bit) and I think x86 runs into similar > problems with it's various mode changes in early boot-up. > Yes, X86 has the same issue when it goes through 8bit and 16bit modes during boot (I don’t really know any details of what happens there). I guess it all falls under the same issue. My thought for fixing this for SVE, is to add a “bool target_changed" to the stop-request packet. If set to true, then GDB resets state and requests the XML description again. That mechanism should then work for X86 etc. However, SVE is complicated further because the vector length can change per thread. Currently GDB and the stub just assumes the entire process has a single description. Alternatively, you build knowledge of variable register lengths into the xml description. But, that probably causes all sorts of problems, and doesn’t fix the x86 case. > For now I'll just let qemu provide it's own xml without vg shenanigans. > >>>> But this is all special casing for feature >>>> name="org.gnu.gdb.aarch64.sve" right? >>> >>> Yes, vg is only available if feature org.gnu.gdb.aarch64.sve is available. >> >> Nod. > > Thanks and hi ;-) > > -- > Alex Bennée
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index cc7258d5f1d..25d34bc5197 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -755,6 +755,7 @@ struct ARMCPU { int32_t cpreg_vmstate_array_len; DynamicGDBXMLInfo dyn_sysreg_xml; + DynamicGDBXMLInfo dyn_svereg_xml; /* Timers used by the generic (architected) timer */ QEMUTimer *gt_timer[NUM_GTIMERS]; @@ -958,10 +959,12 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr, int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); -/* Dynamically generates for gdb stub an XML description of the sysregs from - * the cp_regs hashtable. Returns the registered sysregs number. +/* + * Helpers to dynamically generates XML descriptions of the sysregs + * and SVE registers. Returns the number of registers in each set. */ int arm_gen_dynamic_sysreg_xml(CPUState *cpu, int base_reg); +int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg); /* Returns the dynamically generated XML for the gdb stub. * Returns a pointer to the XML contents for the specified XML file or NULL diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index 69c35462a63..546906dbcb2 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -171,12 +171,145 @@ int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg) return cpu->dyn_sysreg_xml.num; } +struct TypeSize { + const char *gdb_type; + int size; + const char sz, suffix; +}; + +static struct TypeSize vec_lanes[] = { + /* quads */ + { "uint128", 128, 'q', 'u' }, + { "int128", 128, 'q', 's' }, + /* 64 bit */ + { "uint64", 64, 'd', 'u' }, + { "int64", 64, 'd', 's' }, + { "ieee_double", 64, 'd', 'f' }, + /* 32 bit */ + { "uint32", 32, 's', 'u' }, + { "int32", 32, 's', 's' }, + { "ieee_single", 32, 's', 'f' }, + /* 16 bit */ + { "uint16", 16, 'h', 'u' }, + { "int16", 16, 'h', 's' }, + { "ieee_half", 16, 'h', 'f' }, + /* bytes */ + { "uint8", 8, 'b', 'u' }, + { "int8", 8, 'b', 's' }, +}; + + +int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg) +{ + ARMCPU *cpu = ARM_CPU(cs); + 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); + 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\">"); + + /* 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_append_printf(s, + "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>", + ts->str, vec_lanes[i].gdb_type, count); + } + /* + * Now define a union for each size group containing unsigned and + * 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); + } + } + 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>"); + + /* 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\"/>", + i, reg_width, base_reg++); + info->num++; + } + /* fpscr & status registers */ + g_string_append_printf(s, "<reg name=\"fpsr\" bitsize=\"32\"" + " regnum=\"%d\" group=\"float\"" + " type=\"int\"/>", base_reg++); + g_string_append_printf(s, "<reg name=\"fpcr\" bitsize=\"32\"" + " 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\"/>", + 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\"/>", + cpu->sve_max_vq * 16, base_reg++); + g_string_append_printf(s, + "<reg name=\"vg\" bitsize=\"64\"" + " regnum=\"%d\" group=\"vector\"" + " type=\"int\"/>", base_reg++); + info->num += 2; + g_string_append_printf(s, "</feature>"); + cpu->dyn_svereg_xml.desc = g_string_free(s, false); + + return cpu->dyn_svereg_xml.num; +} + + const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname) { ARMCPU *cpu = ARM_CPU(cs); if (strcmp(xmlname, "system-registers.xml") == 0) { return cpu->dyn_sysreg_xml.desc; + } else if (strcmp(xmlname, "sve-registers.xml") == 0) { + return cpu->dyn_svereg_xml.desc; } return NULL; } diff --git a/target/arm/helper.c b/target/arm/helper.c index d00e4fcca86..b6e1fe51d76 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -201,6 +201,15 @@ static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri, } } +/** + * arm_get/set_gdb_*: get/set a gdb register + * @env: the CPU state + * @buf: a buffer to copy to/from + * @reg: register number (offset from start of group) + * + * We return the number of bytes copied + */ + static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg) { ARMCPU *cpu = env_archcpu(env); @@ -224,6 +233,98 @@ static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg) return 0; } +#ifdef TARGET_AARCH64 +static int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg) +{ + ARMCPU *cpu = env_archcpu(env); + + switch (reg) { + /* The first 32 registers are the zregs */ + case 0 ... 31: + { + int vq, len = 0; + for (vq = 0; vq < cpu->sve_max_vq; vq++) { + len += gdb_get_reg128(buf, + env->vfp.zregs[reg].d[vq * 2 + 1], + env->vfp.zregs[reg].d[vq * 2]); + } + return len; + } + case 32: + return gdb_get_reg32(buf, vfp_get_fpsr(env)); + case 33: + return gdb_get_reg32(buf, vfp_get_fpcr(env)); + /* then 16 predicates and the ffr */ + case 34 ... 50: + { + int preg = reg - 34; + int vq, len = 0; + for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) { + len += gdb_get_reg64(buf, env->vfp.pregs[preg].p[vq / 4]); + } + return len; + } + case 51: + return gdb_get_reg64(buf, (cpu->env.vfp.zcr_el[1] & 0xf) + 1); + default: + /* gdbstub asked for something out our range */ + qemu_log_mask(LOG_UNIMP, "%s: out of range register %d", __func__, reg); + break; + } + + return 0; +} + +static int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg) +{ + ARMCPU *cpu = env_archcpu(env); + + /* The first 32 registers are the zregs */ + switch (reg) { + /* The first 32 registers are the zregs */ + case 0 ... 31: + { + int vq, len = 0; + uint64_t *p = (uint64_t *) buf; + for (vq = 0; vq < cpu->sve_max_vq; vq++) { + env->vfp.zregs[reg].d[vq * 2 + 1] = *p++; + env->vfp.zregs[reg].d[vq * 2] = *p++; + len += 16; + } + return len; + } + case 32: + vfp_set_fpsr(env, *(uint32_t *)buf); + return 4; + case 33: + vfp_set_fpcr(env, *(uint32_t *)buf); + return 4; + case 34 ... 50: + { + int preg = reg - 34; + int vq, len = 0; + uint64_t *p = (uint64_t *) buf; + for (vq = 0; vq < cpu->sve_max_vq; vq = vq + 4) { + env->vfp.pregs[preg].p[vq / 4] = *p++; + len += 8; + } + return len; + } + case 51: + { + uint64_t val = *(uint64_t *) buf; + cpu->env.vfp.zcr_el[1] = (val - 1) & 0xf; + return 8; + } + default: + /* gdbstub asked for something out our range */ + break; + } + + return 0; +} +#endif /* TARGET_AARCH64 */ + static bool raw_accessors_invalid(const ARMCPRegInfo *ri) { /* Return true if the regdef would cause an assertion if you called @@ -6981,9 +7082,22 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) CPUARMState *env = &cpu->env; if (arm_feature(env, ARM_FEATURE_AARCH64)) { - gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg, - aarch64_fpu_gdb_set_reg, - 34, "aarch64-fpu.xml", 0); + /* + * The lower part of each SVE register aliases to the FPU + * registers so we don't need to include both. + */ +#ifdef TARGET_AARCH64 + if (isar_feature_aa64_sve(&cpu->isar)) { + gdb_register_coprocessor(cs, arm_gdb_get_svereg, arm_gdb_set_svereg, + arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs), + "sve-registers.xml", 0); + } else +#endif + { + gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg, + aarch64_fpu_gdb_set_reg, + 34, "aarch64-fpu.xml", 0); + } } else if (arm_feature(env, ARM_FEATURE_NEON)) { gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg, 51, "arm-neon.xml", 0); @@ -6997,6 +7111,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu) gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg, arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs), "system-registers.xml", 0); + } /* Sort alphabetically by type name, except for "any". */
We also expose a the helpers to read/write the the registers. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - instead of zNpM expose zN at sve_max_vq width - wrap union in union q(us), d(usf), s(usf), h(usf), b(us) v3 - add a vg pseudo register for current width - spacing fixes - use switch/case for whole group - drop fpsr_pos marker - remove unused variables --- target/arm/cpu.h | 7 ++- target/arm/gdbstub.c | 133 +++++++++++++++++++++++++++++++++++++++++++ target/arm/helper.c | 121 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 256 insertions(+), 5 deletions(-)