Re: [PATCH v3 12/20] target/arm: generate xml description of our SVE registers

From: Luis Machado <luis.machado@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: damien.hedde@greensocs.com,
	Peter Maydell <peter.maydell@linaro.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org,
	"open list:ARM TCG CPUs" <qemu-arm@nongnu.org>,
	alan.hayward@arm.com
Subject: Re: [PATCH v3 12/20] target/arm: generate xml description of our SVE registers
Date: Fri, 20 Dec 2019 10:18:54 -0300	[thread overview]
Message-ID: <81e43d43-29f8-ec56-5270-bb90dd1bc077@linaro.org> (raw)
In-Reply-To: <87eewzdtqg.fsf@linaro.org>

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.