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

From: Alan Hayward <Alan.Hayward@arm.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: "damien.hedde@greensocs.com" <damien.hedde@greensocs.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Luis Machado <luis.machado@linaro.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"open list:ARM TCG CPUs" <qemu-arm@nongnu.org>, nd <nd@arm.com>
Subject: Re: [PATCH v3 12/20] target/arm: generate xml description of our SVE registers
Date: Thu, 9 Jan 2020 14:10:44 +0000	[thread overview]
Message-ID: <48A1BD2C-A5E5-49C6-A188-E5D2F73B546C@arm.com> (raw)
In-Reply-To: <87muawj0hz.fsf@linaro.org>

> 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