On Tue, 8 Jul 2025 at 23:14, Vacha Bhavsar
<[email protected]> wrote:
>
> The QEMU GDB stub does not expose the ZA storage SME register to GDB via
> the remote serial protocol, which can be a useful functionality to debug SME
> code. To provide this functionality in Aarch64 target, this patch registers
> the
> SME register set with the GDB stub. To do so, this patch implements the
> aarch64_gdb_get_sme_reg() and aarch64_gdb_set_sme_reg() functions to
> specify how to get and set the SME registers, and the
> arm_gen_dynamic_smereg_feature() function to generate the target
> description in XML format to indicate the target architecture supports SME.
> Finally, this patch includes a dyn_smereg_feature structure to hold this
> GDB XML description of the SME registers for each CPU.
>
> Signed-off-by: Vacha Bhavsar <[email protected]>
> +int aarch64_gdb_set_sme_reg(CPUState *cs, uint8_t *buf, int reg)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> +
> + /* The first 32 registers are the zregs */
> + switch (reg) {
> + /* The first 32 registers are the zregs */
You don't need the same comment twice, and it also doesn't
look like it makes sense here, because the zregs are
SVE regs, not SME regs.
> + case 0:
> + {
> + /* cannot set svg via gdbstub */
> + return 0;
> + }
> + case 1:
> + env->svcr = *buf & 0x03;
This will update env->svcr but won't have the correct effects
on the CPU (e.g. zeroing the ZA storage); I think you need to
call aarch64_set_svcr() here. Also you've declared
SVCR in the XML as a 64-bit register, so you should read it out
of the buffer as a 64-bit value, not short-cut by reading just
one byte.
> + return 8;
> + case 2:
> + int vq, len = 0;
> + int svl = cpu->sve_max_vq * 16;
> + uint64_t *p = (uint64_t *) buf;
I know this is what the existing SVE code does, but does
this really do the right thing on a big-endian host ?
> + for (int i = 0; i < svl; i++) {
> + for (vq = 0; vq < cpu->sve_max_vq; vq++) {
> + env->za_state.za[i].d[vq * 2 + 1] = *p++;
> + env->za_state.za[i].d[vq * 2] = *p++;
> + len += 16;
> + }
> + }
> + return len;
> + default:
> + /* gdbstub asked for something out our range */
"out of"
> + break;
> + }
> +
> + return 0;
> +}
(PS: I would consider this close enough to being a bugfix to be
OK with putting it into 10.1 in the first rc cycle or so even
if it misses softfreeze.)
thanks
-- PMM
