[v2,11/14] target/arm: default SVE length to 64 bytes for linux-user
diff mbox series

Message ID 20191130084602.10818-12-alex.bennee@linaro.org
State New
Headers show
Series
  • gdbstub refactor and SVE support
Related show

Commit Message

Alex Bennée Nov. 30, 2019, 8:45 a.m. UTC
The Linux kernel chooses the default of 64 bytes for SVE registers on
the basis that it is the largest size that won't grow the signal
frame. When debugging larger sizes are also unwieldy in gdb as each
zreg will take over a page of terminal to display.

The user can of course always specify a larger size with the
sve-max-vq property on the command line:

  -cpu max,sve-max-vq=16

This should not make any difference to SVE enabled software as the SVE
is of course vector length agnostic.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/cpu64.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Richard Henderson Dec. 2, 2019, 2:41 a.m. UTC | #1
On 11/30/19 8:45 AM, Alex Bennée wrote:
> The Linux kernel chooses the default of 64 bytes for SVE registers on
> the basis that it is the largest size that won't grow the signal
> frame. When debugging larger sizes are also unwieldy in gdb as each
> zreg will take over a page of terminal to display.
> 
> The user can of course always specify a larger size with the
> sve-max-vq property on the command line:
> 
>   -cpu max,sve-max-vq=16
> 
> This should not make any difference to SVE enabled software as the SVE
> is of course vector length agnostic.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  target/arm/cpu64.c | 3 +++
>  1 file changed, 3 insertions(+)

6 is the largest size that doesn't grow the signal frame.
I imagine 4 was chosen because that's the only real hw atm.

> +        /* Default sve-max-vq to a reasonable numer */
> +        cpu->sve_max_vq = 4;

I also agree that we should match the kernel, but this is not the right way.
Changing max vq is not the same as changing the default vq.

You should change the value of env->vfp.zcr_el[1] in arm_cpu_reset(), and the
user can increase the length with prctl(2) as they would be able to on real
hardware that would have support for longer vector lengths.

Also, I don't think you should mix this up with gdb stuff.


r~
Alex Bennée Dec. 5, 2019, 5:31 p.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> On 11/30/19 8:45 AM, Alex Bennée wrote:
>> The Linux kernel chooses the default of 64 bytes for SVE registers on
>> the basis that it is the largest size that won't grow the signal
>> frame. When debugging larger sizes are also unwieldy in gdb as each
>> zreg will take over a page of terminal to display.
>> 
>> The user can of course always specify a larger size with the
>> sve-max-vq property on the command line:
>> 
>>   -cpu max,sve-max-vq=16
>> 
>> This should not make any difference to SVE enabled software as the SVE
>> is of course vector length agnostic.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  target/arm/cpu64.c | 3 +++
>>  1 file changed, 3 insertions(+)
>
> 6 is the largest size that doesn't grow the signal frame.
> I imagine 4 was chosen because that's the only real hw atm.
>
>> +        /* Default sve-max-vq to a reasonable numer */
>> +        cpu->sve_max_vq = 4;
>
> I also agree that we should match the kernel, but this is not the right way.
> Changing max vq is not the same as changing the default vq.
>
> You should change the value of env->vfp.zcr_el[1] in arm_cpu_reset(), and the
> user can increase the length with prctl(2) as they would be able to on real
> hardware that would have support for longer vector lengths.

No the intention is to default to a lower max VQ because...

> Also, I don't think you should mix this up with gdb stuff.

it is what we use for sizing the registers for the gdbstub. The other
option would be to use the effective zcr_el1 value at the time of the
gdbstub connecting but then things will go horribly wrong if the user
execute a prctl and widens their size.

>
>
> r~
Richard Henderson Dec. 5, 2019, 7:36 p.m. UTC | #3
On 12/5/19 9:31 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 11/30/19 8:45 AM, Alex Bennée wrote:
>>> The Linux kernel chooses the default of 64 bytes for SVE registers on
>>> the basis that it is the largest size that won't grow the signal
>>> frame. When debugging larger sizes are also unwieldy in gdb as each
>>> zreg will take over a page of terminal to display.
>>>
>>> The user can of course always specify a larger size with the
>>> sve-max-vq property on the command line:
>>>
>>>   -cpu max,sve-max-vq=16
>>>
>>> This should not make any difference to SVE enabled software as the SVE
>>> is of course vector length agnostic.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  target/arm/cpu64.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>
>> 6 is the largest size that doesn't grow the signal frame.
>> I imagine 4 was chosen because that's the only real hw atm.
>>
>>> +        /* Default sve-max-vq to a reasonable numer */
>>> +        cpu->sve_max_vq = 4;
>>
>> I also agree that we should match the kernel, but this is not the right way.
>> Changing max vq is not the same as changing the default vq.
>>
>> You should change the value of env->vfp.zcr_el[1] in arm_cpu_reset(), and the
>> user can increase the length with prctl(2) as they would be able to on real
>> hardware that would have support for longer vector lengths.
> 
> No the intention is to default to a lower max VQ because...
> 
>> Also, I don't think you should mix this up with gdb stuff.
> 
> it is what we use for sizing the registers for the gdbstub. The other
> option would be to use the effective zcr_el1 value at the time of the
> gdbstub connecting but then things will go horribly wrong if the user
> execute a prctl and widens their size.

Why would you care about the size of the registers as passed by default?  You
shouldn't need or want to change that default to make gdbstub work.

The gdbstub should be passing along the vq value (via the "vg" pseudo-register,
iirc), and gdb should be working out what to display based on that.

If that isn't happening, and you are only changing the default so that gdb
quits displaying massive registers when they aren't in use, then you're doing
something wrong with gdb and gdbstub.


r~
Alex Bennée Dec. 6, 2019, 2:52 p.m. UTC | #4
Richard Henderson <richard.henderson@linaro.org> writes:

> On 12/5/19 9:31 AM, Alex Bennée wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> On 11/30/19 8:45 AM, Alex Bennée wrote:
>>>> The Linux kernel chooses the default of 64 bytes for SVE registers on
>>>> the basis that it is the largest size that won't grow the signal
>>>> frame. When debugging larger sizes are also unwieldy in gdb as each
>>>> zreg will take over a page of terminal to display.
>>>>
>>>> The user can of course always specify a larger size with the
>>>> sve-max-vq property on the command line:
>>>>
>>>>   -cpu max,sve-max-vq=16
>>>>
>>>> This should not make any difference to SVE enabled software as the SVE
>>>> is of course vector length agnostic.
>>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> ---
>>>>  target/arm/cpu64.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>
>>> 6 is the largest size that doesn't grow the signal frame.
>>> I imagine 4 was chosen because that's the only real hw atm.
>>>
>>>> +        /* Default sve-max-vq to a reasonable numer */
>>>> +        cpu->sve_max_vq = 4;
>>>
>>> I also agree that we should match the kernel, but this is not the right way.
>>> Changing max vq is not the same as changing the default vq.
>>>
>>> You should change the value of env->vfp.zcr_el[1] in arm_cpu_reset(), and the
>>> user can increase the length with prctl(2) as they would be able to on real
>>> hardware that would have support for longer vector lengths.
>> 
>> No the intention is to default to a lower max VQ because...
>> 
>>> Also, I don't think you should mix this up with gdb stuff.
>> 
>> it is what we use for sizing the registers for the gdbstub. The other
>> option would be to use the effective zcr_el1 value at the time of the
>> gdbstub connecting but then things will go horribly wrong if the user
>> execute a prctl and widens their size.
>
> Why would you care about the size of the registers as passed by default?  You
> shouldn't need or want to change that default to make gdbstub work.
>
> The gdbstub should be passing along the vq value (via the "vg" pseudo-register,
> iirc), and gdb should be working out what to display based on that.
>
> If that isn't happening, and you are only changing the default so that gdb
> quits displaying massive registers when they aren't in use, then you're doing
> something wrong with gdb and gdbstub.

Currently the upstream gdbserver sends the XML based on the VL at start-up. It
doesn't handle changes in the vector size.

Patch
diff mbox series

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index a39d6fcea34..bc5d6c4b974 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -703,6 +703,9 @@  static void aarch64_max_initfn(Object *obj)
          */
         cpu->ctr = 0x80038003; /* 32 byte I and D cacheline size, VIPT icache */
         cpu->dcz_blocksize = 7; /*  512 bytes */
+
+        /* Default sve-max-vq to a reasonable numer */
+        cpu->sve_max_vq = 4;
 #endif
     }