Re: [PATCH v14 04/39] arm64/sme: Provide ABI documentation for SME

[Szabolcs Nagy]

The 04/28/2022 10:19, Catalin Marinas wrote:
> On Tue, Apr 19, 2022 at 12:22:12PM +0100, Mark Brown wrote:
> > +* There are a number of optional SME features, presence of these is 
> > reported
> > +  through AT_HWCAP2 through:
> > +
> > +   HWCAP2_SME_I16I64
> > +   HWCAP2_SME_F64F64
> > +   HWCAP2_SME_I8I32
> > +   HWCAP2_SME_F16F32
> > +   HWCAP2_SME_B16F32
> > +   HWCAP2_SME_F32F32
> > +   HWCAP2_SME_FA64
> 
> Marc pointed out that in combination with FEAT_WFxT, we used all the
> HWCAP2 bits (32). While we are ok for now, we'll soon need to look into
> what to do when the next features turn up. Some options:
> 
> 1. Only provide HWCAP2_SME and let the ID_AA64SMFR0_EL1 features be
>probed via MRS emulation. It doesn't solve the problem but it buys us
>a bit of time.
> 
> 2. Don't bother with any new HWCAPs, just rely on MRS emulation (we have
>HWCAP_CPUID advertising this).
> 
> 3. Start using the upper 32-bit of HWCAP and HWCAP2 (we initially didn't
>go into these as there was a slight chance of merging ILP32). Does
>the libc rely on the upper bits for anything? Or does it just assume
>a 32-bit HWCAPs layout?

top 2 bits of a 64bit AT_HWCAP should be reserved for libc.
(glibc uses them internally)

otherwise glibc can work with 64bit hwcaps.

> 
> 4. Introduce HWCAP3.
> 
> Szabolcs, any thoughts?

i'd go with AT_HWCAP3 and keep using the bottom 32bit for now.
(this requires some new code in glibc, but not excessive)

thanks.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v13 04/39] arm64/sme: Provide ABI documentation for SME

[Szabolcs Nagy]

The 04/08/2022 12:42, Mark Brown wrote:
> Provide ABI documentation for SME similar to that for SVE. Due to the very
> large overlap around streaming SVE mode in both implementation and
> interfaces documentation for streaming mode SVE is added to the SVE
> document rather than the SME one.
> 
> Signed-off-by: Mark Brown 
> Reviewed-by: Catalin Marinas 

looks good from userspace pov.

Reviewed-by: Szabolcs Nagy 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v12 06/40] arm64/sme: Provide ABI documentation for SME

[Szabolcs Nagy]

The 04/06/2022 19:50, Mark Brown wrote:
> On Thu, Mar 31, 2022 at 04:05:38PM +0000, Szabolcs Nagy wrote:
> 
> > i think it's slightly better to treat ZA like TPIDR2,
> > so only clear if CLONE_SETTLS is set.
> 
> > otherwise in principle the child can return to the frame
> > where ZA was used and expect it to work (it's hard to
> > come up with a reason why would some code do that, but
> > this is valid in a single-threaded fork child).
> 
> From an implementation point of view it's a bit clearer if we just
> always preserve PSTATE.ZA and ZA contents on clone() and clear them on
> exec.  Do you see a problem with that?

i think that's ok too.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v12 06/40] arm64/sme: Provide ABI documentation for SME

[Szabolcs Nagy]

The 03/11/2022 18:42, Mark Brown wrote:
> On Fri, Mar 11, 2022 at 05:21:21PM +0000, Szabolcs Nagy wrote:
> > The 02/25/2022 16:58, Mark Brown wrote:
> > > +* On creation fork() or clone() the newly created process will have 
> > > PSTATE.SM
> > > +  and PSTATE.ZA cleared.
> 
> > is there a reason why fork() clears ZA?
> 
> > i think this is a minor issue, but the usual expectation is that
> > on thread creation thread local state is reset in the child, but
> > in a forked child the state is the same as in the parent (where
> > ZA is preserved according to the first rule).
> 
> It was partly consistency with SM and the SVE state (though that is also
> covered by just being in a system call unlike ZA) and partly concerns
> about what happens if the fork() happens in library code which isn't SME
> aware - it would end up carrying around a copy of ZA with associated
> power and performance impacts if it doesn't exec().  Overall it seemed
> like there would to be less potential for unpleasant surprises if we
> consistently discard the data.
> 
> That's not a *super* strongly held opinion though, we could switch to
> preserving whenever we preserve TPIDR2.

i think it's slightly better to treat ZA like TPIDR2,
so only clear if CLONE_SETTLS is set.

otherwise in principle the child can return to the frame
where ZA was used and expect it to work (it's hard to
come up with a reason why would some code do that, but
this is valid in a single-threaded fork child).

sorry for not deciding this earlier.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v12 06/40] arm64/sme: Provide ABI documentation for SME

[Szabolcs Nagy]

The 02/25/2022 16:58, Mark Brown wrote:
> Provide ABI documentation for SME similar to that for SVE. Due to the very
> large overlap around streaming SVE mode in both implementation and
> interfaces documentation for streaming mode SVE is added to the SVE
> document rather than the SME one.
> 
> Signed-off-by: Mark Brown 
> ---
...
> +1.  General
> +---
> +
> +* PSTATE.SM, PSTATE.ZA, the streaming mode vector length, the ZA
> +  register state and TPIDR2_EL0 are tracked per thread.
> +
> +* The presence of SME is reported to userspace via HWCAP2_SME in the aux 
> vector
> +  AT_HWCAP2 entry.  Presence of this flag implies the presence of the SME
> +  instructions and registers, and the Linux-specific system interfaces
> +  described in this document.  SME is reported in /proc/cpuinfo as "sme".
> +
> +* Support for the execution of SME instructions in userspace can also be
> +  detected by reading the CPU ID register ID_AA64PFR1_EL1 using an MRS
> +  instruction, and checking that the value of the SME field is nonzero. [3]
> +
> +  It does not guarantee the presence of the system interfaces described in 
> the
> +  following sections: software that needs to verify that those interfaces are
> +  present must check for HWCAP2_SME instead.
> +
> +* There are a number of optional SME features, presence of these is reported
> +  through AT_HWCAP2 through:
> +
> + HWCAP2_SME_I16I64
> + HWCAP2_SME_F64F64
> + HWCAP2_SME_I8I32
> + HWCAP2_SME_F16F32
> + HWCAP2_SME_B16F32
> + HWCAP2_SME_F32F32
> + HWCAP2_SME_FA64
> +
> +  This list may be extended over time as the SME architecture evolves.
> +
> +  These extensions are also reported via the CPU ID register 
> ID_AA64SMFR0_EL1,
> +  which userspace can read using an MRS instruction.  See elf_hwcaps.txt and
> +  cpu-feature-registers.txt for details.
> +
> +* Debuggers should restrict themselves to interacting with the target via the
> +  NT_ARM_SVE, NT_ARM_SSVE and NT_ARM_ZA regsets.  The recommended way
> +  of detecting support for these regsets is to connect to a target process
> +  first and then attempt a
> +
> + ptrace(PTRACE_GETREGSET, pid, NT_ARM_, &iov).
> +
> +* Whenever ZA register values are exchanged in memory between userspace and
> +  the kernel, the register value is encoded in memory as a series of 
> horizontal
> +  vectors from 0 to VL/8-1 stored in the same endianness invariant format as 
> is
> +  used for SVE vectors.
> +
> +* On thread creation TPIDR2_EL0 is preserved unless CLONE_SETTLS is 
> specified,
> +  in which case it is set to 0.
> +

This looks OK.


> +2.  Vector lengths
> +--
> +
> +SME defines a second vector length similar to the SVE vector length which is
> +controls the size of the streaming mode SVE vectors and the ZA matrix array.
> +The ZA matrix is square with each side having as many bytes as a SVE vector.
> +

i would s/SVE vector/streaming mode SVE vector/
or give SME vector length some other name and use that throughout.

> +
> +3.  Sharing of streaming and non-streaming mode SVE state
> +-
> +
> +It is implementation defined which if any parts of the SVE state are shared
> +between streaming and non-streaming modes.  When switching between modes
> +via software interfaces such as ptrace if no register content is provided as
> +part of switching no state will be assumed to be shared and everything will
> +be zeroed.
> +

OK.

> +
> +4.  System call behaviour
> +-
> +
> +* On syscall PSTATE.ZA is preserved, if PSTATE.ZA==1 then the contents of the
> +  ZA matrix are preserved.
> +
> +* On syscall PSTATE.SM will be cleared and the SVE registers will be handled
> +  as normal.
> +
> +* Neither the SVE registers nor ZA are used to pass arguments to or receive
> +  results from any syscall.
> +
> +* On creation fork() or clone() the newly created process will have PSTATE.SM
> +  and PSTATE.ZA cleared.

is there a reason why fork() clears ZA?

i think this is a minor issue, but the usual expectation is that
on thread creation thread local state is reset in the child, but
in a forked child the state is the same as in the parent (where
ZA is preserved according to the first rule).

> +
> +* All other SME state of a thread, including the currently configured vector
> +  length, the state of the PR_SME_VL_INHERIT flag, and the deferred vector
> +  length (if any), is preserved across all syscalls, subject to the specific
> +  exceptions for execve() described in section 6.
> +

OK other than the fork comment.

> +
> +5.  Signal handling
> +---
> +
> +* Signal handlers are invoked with streaming mode and ZA disabled.
> +
> +* A new signal frame record za_context encodes the ZA register contents on
> +  signal delivery. [1]
> +
> +* The signal frame record for ZA always contains basic metadata, in 
> particular
> +  the thread's vector length (in za_context.vl).
> +
> +* The ZA matrix 

Re: [PATCH v11 10/40] arm64/sme: Basic enumeration support

[Szabolcs Nagy]

The 02/21/2022 14:32, Catalin Marinas wrote:
> On Mon, Feb 07, 2022 at 03:20:39PM +, Mark Brown wrote:
> > diff --git a/Documentation/arm64/elf_hwcaps.rst 
> > b/Documentation/arm64/elf_hwcaps.rst
> > index b72ff17d600a..5626cf208000 100644
> > --- a/Documentation/arm64/elf_hwcaps.rst
> > +++ b/Documentation/arm64/elf_hwcaps.rst
> > @@ -259,6 +259,39 @@ HWCAP2_RPRES
> >  
> >  Functionality implied by ID_AA64ISAR2_EL1.RPRES == 0b0001.
> >  
> > +HWCAP2_SME
> > +
> > +Functionality implied by ID_AA64PFR1_EL1.SME == 0b0001, as described
> > +by Documentation/arm64/sme.rst.
> > +
> > +HWCAP2_SME_I16I64
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.I16I64 == 0b.
> > +
> > +HWCAP2_SME_F64F64
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.F64F64 == 0b1.
> > +
> > +HWCAP2_SME_I8I32
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.I8I32 == 0b.
> > +
> > +HWCAP2_SME_F16F32
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.F16F32 == 0b1.
> > +
> > +HWCAP2_SME_B16F32
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.B16F32 == 0b1.
> > +
> > +HWCAP2_SME_F32F32
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.F32F32 == 0b1.
> > +
> > +HWCAP2_SME_FA64
> > +
> > +Functionality implied by ID_AA64SMFR0_EL1.FA64 == 0b1.
> 
> More of a question for the libc people: should we drop the fine-grained
> HWCAP corresponding to the new ID_AA64SMFR0_EL1 register (only keep
> HWCAP2_SME) and get the user space to use the MRS emulation? Would any
> ifunc resolver be affected?
> 

good question.

within glibc HWCAP2_SME is enough (to decide if we need to
deal with additional register state and the lazy ZA save
scheme) but i guess user code that actually uses sme would
need the details (including in ifunc resolvers in principle).

since we have mrs, there is no strict need for hwcaps.
if ifunc resolvers using this info are not widespread then
the mrs emulation overhead is acceptable, but i suspect
hwcaps are nicer to use.

do we have a plan after hwcap2 bits run out? :)
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 26/28] arm64/sve: Add documentation

[Szabolcs Nagy]

On 11/10/17 12:08, Dave Martin wrote:
> How does this look:
> 

looks reasonable.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC PATCH v3 27/28] arm64: signal: Report signal frame size to userspace via auxv

[Szabolcs Nagy]

On 10/10/17 19:38, Dave Martin wrote:
> Stateful CPU architecture extensions may require the signal frame
> to grow to a size that exceeds the arch's MINSIGSTKSZ #define.
> However, changing this #define is an ABI break.
> 
> To allow userspace the option of determining the signal frame size
> in a more forwards-compatible way, this patch adds a new auxv entry
> tagged with AT_MINSIGSTKSZ, which provides the maximum signal frame
> size that the process can observe during its lifetime.
> 
> If AT_MINSIGSTKSZ is absent from the aux vector, the caller can
> assume that the MINSIGSTKSZ #define is sufficient.  This allows for
> a consistent interface with older kernels that do not provide
> AT_MINSIGSTKSZ.
> 

the posix sigaltstack api shall fail with ENOMEM
if smaller than MINSIGSTKSZ stack size is used.

so it is important to note somewhere if AT_MINSIGSTKSZ
is intended to be always >= MINSIGSTKSZ define (which
is rounded up to 5k) or it may be smaller as it provides
the precise value of the largest signal frame.

(i think it makes sense for it to be a precise value,
but then users should do the >= check before calling
the sigaltstack api, so they should be aware of this
issue)

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 26/28] arm64/sve: Add documentation

[Szabolcs Nagy]

On 10/10/17 19:38, Dave Martin wrote:
> This patch adds basic documentation of the user/kernel interface
> provided by the for SVE.
> 
> Signed-off-by: Dave Martin 
> Cc: Alex Bennée 
> Cc: Mark Rutland 
> Cc: Alan Hayward 
> 
> ---
> 
> Changes since v2
> 
> 
> Changes requested by Alan Hayward:
> 
>  * Added a note that the caller of PTRACE_SETREGSET will need to do a
>GETREGSET if complete certainty about the resulting VL is desired.
> 
> ABI changes:
> 
>  * Documented the changed return value value semantics for PR_SET_SET_VL
>when the PR_SVE_SET_VL_ONEXEC flag is passed.
> ---
...
> +prctl(PR_SVE_SET_VL, unsigned long arg)
> +
> +Sets the vector length of the calling thread and related flags, where
> +arg == vl | flags.
> +
> +vl is the desired vector length, where sve_vl_valid(vl) must be true.
> +
> +flags:
> +
> + PR_SVE_SET_VL_INHERIT
> +
> + Inherit the current vector length across execve().  Otherwise, the
> + vector length is reset to the system default at execve().  (See
> + Section 9.)
> +
> + PR_SVE_SET_VL_ONEXEC
> +
> + Defer the requested vector length change until the next execve().
> + This allows launching of a new program with a different vector
> + length, while avoiding runtime side effects in the caller.
> +
> + This also overrides the effect of PR_SVE_SET_VL_INHERIT for the
> + first execve().
> +
> + Without PR_SVE_SET_VL_ONEXEC, any outstanding deferred vector
> + length change is cancelled.
> +

"next execve" is still ambiguous. (execve has process
global effect so it may plausibly mean next in the
process or next in the calling thread)

"any outstanding deferred vector length change" is
ambiguous. (it may be for all threads in a process or
in the calling thread only)

> +Return value: a nonnegative on success, or a negative value on error:
> + EINVAL: SVE not supported, invalid vector length requested, or
> + invalid flags.
> +
> +On success, the calling thread's vector length is changed to the largest
> +value supported by the system that is less than or equal to vl.
> +If vl == SVE_VL_MAX, the calling thread's vector length is changed to the
> +largest value supported by the system.
> +
> +The returned value describes the resulting configuration, encoded as for
> +PR_SVE_GET_VL.  The vector length reported in this value is the new 
> current
> +vector length for this thread if PR_SVE_SET_VL_ONEXEC was not passed in 
> the
> +input arg; otherwise, the reported vector length is the deferred vector
> +length that will be applied at the next exec.
> +
...
> +9.  System runtime configuration
> +
> +
> +* To mitigate the ABI impact of expansion of the signal frame, a policy
> +  mechanism is provided for administrators, distro maintainers and developers
> +  to set the default vector length for userspace processes:
> +
> +/proc/cpu/sve_default_vector_length
> +

still wrong.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 26/28] arm64/sve: Add documentation

[Szabolcs Nagy]

On 31/08/17 18:00, Dave Martin wrote:
> +9.  System runtime configuration
> +
> +
> +* To mitigate the ABI impact of expansion of the signal frame, a policy
> +  mechanism is provided for administrators, distro maintainers and developers
> +  to set the default vector length for userspace processes:
> +
> +/proc/cpu/sve_default_vector_length


elsewhere in the patch series i see

/proc/sys/abi/sve_default_vector_length

is this supposed to be the same?

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v2 26/28] arm64/sve: Add documentation

[Szabolcs Nagy]

On 31/08/17 18:00, Dave Martin wrote:
> +prctl(PR_SVE_SET_VL, unsigned long arg)
> +
> +Sets the vector length of the calling thread and related flags, where
> +arg == vl | flags.
> +
> +vl is the desired vector length, where sve_vl_valid(vl) must be true.
> +
> +flags:
> +
> + PR_SVE_SET_VL_INHERIT
> +
> + Inherit the current vector length across execve().  Otherwise, the
> + vector length is reset to the system default at execve().  (See
> + Section 9.)
> +
> + PR_SVE_SET_VL_ONEXEC
> +
> + Defer the requested vector length change until the next execve().
> + This allows launching of a new program with a different vector
> + length, while avoiding runtime side effects in the caller.
> +
> + This also overrides the effect of PR_SVE_SET_VL_INHERIT for the
> + first execve().
> +
> + Without PR_SVE_SET_VL_ONEXEC, any outstanding deferred vector
> + length change is cancelled.
> +

based on later text it seems this works if exeve is
called in the same thread as prctl was called in.

this is a bit weird from user-space pov so it may
make sense to state it here explicitly.

> +Return value: a nonnegative on success, or a negative value on error:
> + EINVAL: SVE not supported, invalid vector length requested, or
> + invalid flags.
> +
> +On success, the calling thread's vector length is changed to the largest
> +value supported by the system that is less than or equal to vl.
> +If vl == SVE_VL_MAX, the calling thread's vector length is changed to the
> +largest value supported by the system.
> +
> +The returned value describes the resulting configuration, encoded as for
> +PR_SVE_GET_VL.
> +
> +Changing the vector length causes all of P0..P15, FFR and all bits of
> +Z0..V31 except for Z0 bits [127:0] .. Z31 bits [127:0] to become
> +unspecified.  Calling PR_SVE_SET_VL with vl equal to the thread's current
> +vector length does not constitute a change to the vector length for this
> +purpose.

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 11/12] avoid using predefined PAGE_SIZE

[Szabolcs Nagy]

On 17/07/15 17:02, Andre Przywara wrote:
> The musl-libc comes with a predefined PAGE_SIZE macro, which may be
> wrong on systems which support multiple smallest page sizes.
> Make sure we use our own (runtime evaluated) definition of PAGE_SIZE.
>

musl only provides PAGE_SIZE on archs where it is
constant, if not, that's a musl bug and should be
fixed there (this is a posix requirement).

the ifdef does not help around this: musl sysconf
will return the same value as you get from the
header (it would be a conformance bug otherwise).

use

#include 
#ifndef PAGE_SIZE
#define PAGE_SIZE sysconf(_SC_PAGE_SIZE)
#endif

> Signed-off-by: Andre Przywara 
> ---
>  include/kvm/kvm.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h
> index 9818046..8b12cd6 100644
> --- a/include/kvm/kvm.h
> +++ b/include/kvm/kvm.h
> @@ -19,6 +19,10 @@
>  #define HOME_DIR getenv("HOME")
>  #define KVM_BINARY_NAME  "lkvm"
>
> +/* Let's determine the actual page size at runtime. */
> +#ifdef PAGE_SIZE
> +#undef PAGE_SIZE
> +#endif
>  #define PAGE_SIZE (sysconf(_SC_PAGE_SIZE))
>
>  #define DEFINE_KVM_EXT(ext)  \
>


-- IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered 
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm