Re: [PATCH 3/8] gdb, gdbserver/aarch64-linux: Allow aarch64_sve_get_vq to return error

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Luis Machado <luis.machado@arm.com> writes:

> On 9/8/22 07:41, Thiago Jung Bauermann via Gdb-patches wrote:
>> If ptrace fails, aarch64_sve_get_vq assumes that SVE isn't supported.
>> Because in a subsequent change this function will need to be called from
>> low_new_thread (which can be called whe the inferior thread isn't stopped),
>> it will need to distinguish between ptrace errors due to SVE not being
>> supported from ptrace errors due to the inferior thread not being stopped.
>
> My understanding is that we will always be able to validate supported
> features when we start/attach to a process. So if, say, we detect SVE
> support through HWCAP's in the beginning of the debugging session,
> that will still hold for any thread that is spawned. What may change
> later on is the VL (still, very unlikely).

I ended up dropping this patch when I implemented your idea of leaving
the process' target description in place and using a thread target
description only for the cases where it is needed (i.e., when the
inferior supports SVE). It simplified the code a lot (thanks!), and it's
not necessary any more to change low_new_thread so this patch became
unnecessary.

As an aside, we don't check for SVE support using HWCAP, just by getting
the NT_ARM_SVE regset via ptrace.

> When a thread is spawned, it will inherit SVE settings from its
> parent. From the Linux Kernel SVE documentation:
>
>   In particular, on return from a fork() or clone(), the parent and new child
>   process or thread share identical SVE configuration, matching that of the
>   parent before the call.
>
> Is that something we can use here? I suppose there may be corner cases
> where the parent spawned a thread and changed its VL size etc.

It is, and for a while I had changes in my local branch which in the
event of a clone or fork event copied the original thread's target
description to the new one.

After a while I noticed that in practice it was redundant, because when
the new thread stops gdbserver calls aarch64_target::arch_update_tdesc
which also sets up the thread's target description so I removed the
changes for the clone and fork events.

> If we can't and a thread is running, we won't be able to tell the VL,
> but we have a state that is meant to indicate "unknown VL", don't we?
> Maybe I'm misremembering, but -1 used to indicate that.

We used -1 in struct gdbarch_info's id field, but it became unused in
commit “Fix thread's gdbarch when SVE vector length changes” and was
removed. We don't have an “unknown VL” state anymore. And with v2 of
these patches, we can always use ptrace to get the VL so it's not
needed.

-- 
Thiago