Re: [PATCH v6 20/20] elf: Add SVE support for aarch64 rtld-audit

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 21/12/2021 13:45, Szabolcs Nagy wrote:
> The 12/21/2021 15:27, Florian Weimer via Libc-alpha wrote:
>> * Adhemerval Zanella:
>>
>>> To implement lazy binding is enabled when profiling or auditing used,
>>> even when STO_AARCH64_VARIANT_PCS is set.  Also, to not incur in
>>> performance penalties on architecture without SVE, the PLT entrypoint
>>> is set to a newer one, _dl_runtime_profile_sve, which is used iff
>>> 'hwcap' has HWCAP_SVE bit set.
>>>
>>> This should be a fair assumption since SVE has a defined set of
>>> registers for argument passing and return values.  A new ABI with either
>>> different argument passing or different registers would require a
>>> different PLT entry, but I assume this would require another symbol flag
>>> anyway (or at least a different ELF mark to indicate so).
>>
>> Someone familiar with SVE needs to review this.
>>
>> The restriction to the currently-specified vector calling convention
>> seems to be a bit arbitrary to me.  To me, STO_AARCH64_VARIANT_PCS
>> strongly suggests that the toolchain makes every conceivable attempt to
>> support arbitrary calling conventions, but that's probably just me.
> 
> 
> thanks for working on this, but yeah this approach does not work:
> 
> A PLT hook for STO_AARCH64_VARIANT_PCS symbol has to preserve *all*
> registers (except x16,x17,cc), this is the same strict requirement
> as linker inserted veneers follow. (historically the dynamic linker
> lazy binding code made assumptions about the call convention, but if
> there are extern calls with unusual call conventions then this does
> not work and it has to behave like the static linker. this is the
> meaning of variant_pcs, it is not just for sve calls.)
> 
> there is no way to tell which call is an sve call, so i'd suggest
> having 2 profile entries: normal and variant_pcs and the latter is
> used if a module is marked with DT_AARCH64_VARIANT_PCS and it
> preserves all registers.
> 
> when "all registers" increase via arch extensions then the PLT hook
> code has to be updated, but ideally only the variant_pcs hooks
> will require a glibc update to work on a new arch.

I was afraid we would require a change like that and based on 
STO_AARCH64_VARIANT_PCS definition is does seem the only option.

> 
> e.g. the user callback can take
> 
> struct regs {
>   // base arg regs inline;
>   ...
>   void *vpcs_regs;
> };
> > where vpcs_regs!=0 for variant_pcs calls and contains all the regs
> (in a way that can represent various combinations of arch extensions,
> i don't think we have to make it userfriendly to access).

Yeah, that was one idea I had when coding it.  It will make the
_dl_runtime_profile somewhat more complex for SVE, but it the cost
of having a non-specific ABIs for lazy binding.

I think we will need to have this new field on BZ #26643 fix at least,
and chage current code to zero it. It will make it easier to provide
SVE support on 2.36.

> 
> (i think we don't have to get the aarch64 fixes done in 2.35 as this
> is a big design work.)

Agreed, I will drop this patch for v7.