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

[Dave Martin]

On Mon, Oct 09, 2017 at 03:07:23PM +0100, Alex Bennée wrote:
> 
> Dave Martin  writes:
> 
> > On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote:
> >>
> >> Dave Martin  writes:
> >>
> >> > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote:
> >> >> 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?
> >> >
> >> > Good spot, thanks!
> >> >
> >> > /proc/cpu/ was the old location: they should both say /proc/abi/.
> >> > I'll fix it.
> >>
> >> Isn't /sys (or rather sysfs) the preferred location for modern control
> >> knobs that mirror the kernels object model or is SVE a special case for
> >> extending /proc?
> >
> > I couldn't figure out which kernel object this maps to.  There's no
> > device, no driver.  This isn't even per-cpu.
> 
> Hmm I can see:
> 
>   /sys/devices/system/cpu
> 
> On both my x86 and arm64 systems - but I guess this is more ABIish than
> CPU feature related.
> 
> > sysctl is already used for similar knobs to this one, so I followed that
> > precedent -- though if someone argues strongly enough it could be
> > changed.
> >
> > Are there already examples of arch controls like this in sysfs?  I
> > wasn't aware of any, but I didn't look all that hard...
> 
> Given the paucity of the /proc/sys/abi on both systems I guess this sort
> of knob is rare enough that people haven't expressed a strong preference
> for sysfs here. I have no objection to staying with /proc/sys/abi/.

That was my thinking: sysctls tend to control the kernel, especially
process behaviour, whereas /sys/ controls devices and subsystems.
That's not a concrete rule though and not written down, and doubtless a
major new set of sysctls would be shot down regardless of what they do.

Part of the problem with /proc is that people historically put things in
there that have random ad-hoc behaviour and semantics.  The sysctl
framework at least imposes some sanity here.

There is also some support for specialising sysctls to user namespaces,
which makes some sense in that /proc/sys/abi/* should probably be per-
container -- though whether it's ever considered important enough to
actually be implemented is another question.  I certainly don't attempt
to do it today.

I don't know how sysfs interacts with namespaces, but probably it can.

I guess I'll wait for someone to object loudly...

Cheers
---Dave
___
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

[Alex Bennée]

Dave Martin  writes:

> On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote:
>>
>> Dave Martin  writes:
>>
>> > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote:
>> >> 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?
>> >
>> > Good spot, thanks!
>> >
>> > /proc/cpu/ was the old location: they should both say /proc/abi/.
>> > I'll fix it.
>>
>> Isn't /sys (or rather sysfs) the preferred location for modern control
>> knobs that mirror the kernels object model or is SVE a special case for
>> extending /proc?
>
> I couldn't figure out which kernel object this maps to.  There's no
> device, no driver.  This isn't even per-cpu.

Hmm I can see:

  /sys/devices/system/cpu

On both my x86 and arm64 systems - but I guess this is more ABIish than
CPU feature related.

> sysctl is already used for similar knobs to this one, so I followed that
> precedent -- though if someone argues strongly enough it could be
> changed.
>
> Are there already examples of arch controls like this in sysfs?  I
> wasn't aware of any, but I didn't look all that hard...

Given the paucity of the /proc/sys/abi on both systems I guess this sort
of knob is rare enough that people haven't expressed a strong preference
for sysfs here. I have no objection to staying with /proc/sys/abi/.

--
Alex Bennée
___
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

[Dave Martin]

On Mon, Oct 09, 2017 at 10:34:25AM +0100, Alex Bennée wrote:
> 
> Dave Martin  writes:
> 
> > On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote:
> >> 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?
> >
> > Good spot, thanks!
> >
> > /proc/cpu/ was the old location: they should both say /proc/abi/.
> > I'll fix it.
> 
> Isn't /sys (or rather sysfs) the preferred location for modern control
> knobs that mirror the kernels object model or is SVE a special case for
> extending /proc?

I couldn't figure out which kernel object this maps to.  There's no
device, no driver.  This isn't even per-cpu.

sysctl is already used for similar knobs to this one, so I followed that
precedent -- though if someone argues strongly enough it could be
changed.

Are there already examples of arch controls like this in sysfs?  I
wasn't aware of any, but I didn't look all that hard...

Cheers
---Dave
___
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

[Alex Bennée]

Dave Martin  writes:

> On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote:
>> 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?
>
> Good spot, thanks!
>
> /proc/cpu/ was the old location: they should both say /proc/abi/.
> I'll fix it.

Isn't /sys (or rather sysfs) the preferred location for modern control
knobs that mirror the kernels object model or is SVE a special case for
extending /proc?


>
> ---Dave


--
Alex Bennée
___
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

[Dave Martin]

On Fri, Oct 06, 2017 at 04:43:43PM +0100, Szabolcs Nagy wrote:
> 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?

Good spot, thanks!

/proc/cpu/ was the old location: they should both say /proc/abi/.
I'll fix it.

---Dave
___
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

[Dave Martin]

On Thu, Oct 05, 2017 at 05:39:24PM +0100, Szabolcs Nagy wrote:
> On 31/08/17 18:00, Dave Martin wrote:

[...]

> > +   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.

True.  Looking at the prctl(2) man page it looks like other per-thread
properties are inherited across execve() in a similar way, but it's at
least worth a mention.  PR_SET_SECCOMP seems to work like this, for
example.

So, the intention is that you do a prctl(...ONEXEC) in the run up to
execve(), rather than doing it at other random times.  The primary
reason for ONEXEC is to avoid the side-effects of actually changing
the VL.


Looking at this though...
I wonder whether PR_SVE_SET_VL(... PR_SVE_SET_VL_ONEXEC) should return
the VL set for exec, rather than the current VL (which is unchanged by
definition in this case, thus uninteresting).

This would allow the ONEXEC flag to be used to probe for available VLs
without the other side-effects of changing VL, something like:

int old = prctl(PR_SVE_GET_VL);
int ret;

ret = prctl(PR_SVE_SET_VL, 144 | PR_SVE_SET_VL_ONEXEC);
if (ret == -1) {
perror("PR_SVE_SET_VL");
goto error;
}

if ((ret & PR_SVE_VL_LEN_MASK) == 144)
have_vl_144 = true;

if (prctl(PR_SVE_SET_VL, old | PR_SVE_SET_VL_ONEXEC) == -1) {
perror("PR_SVE_SET_VL");
goto error;
}


This does _not_ do the expected thing right now, since there's no
direct way to retrieve thread.sve_vl_onexec directly from the kernel
(and it didn't really seem justified to add one).

Thoughts?

Cheers
---Dave
___
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