[PATCH v1 3/3] kselftest/arm64: Add tests for SVE vector configuration
Mark Brown
broonie at kernel.org
Wed Jul 28 05:59:18 PDT 2021
On Wed, Jul 28, 2021 at 10:41:58AM +0100, Dave Martin wrote:
> On Tue, Jul 27, 2021 at 07:06:49PM +0100, Mark Brown wrote:
> > We provide interfaces for configuring the SVE vector length seen by
> > processes using prctl and also via /proc for configuring the default
> > values. Provide tests that exercise all these interfaces and verify that
> > they take effect as expected, though at present no test fully enumerates
> > all the possible vector lengths.
> Does "at present" mean that this test doesn't do it either?
> (It doesn't seem to try every VL, unless I've missed something? Is this
> planned?)
Nothing currently does it, and nor does this patch. Planned is a strong
term but yes, ideally we should probe all the VLs.
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
>
> Not used? ^
We call exit() which is declared in stdlib.h.
> > +#define MIN_VL 16
> <asm/sigcontext.h> has SVE_MIN_VL. Maybe we can use that everywhere
> these days?
I partly wanted the vector type neutral name, and I'm considering
modifying the sysfs ABI file to define 0 as a valid vector length for
consistency with accepting -1 as the maximum since SME doesn't have any
architected guarantees as to which particular vector lengths are defined.
> > +/* Verify that we can write a minimum value and have it take effect */
> > +void proc_write_min(struct vec_data *data)
> Could be proc_write_check_min() (though the "check" is a bit more
> redundant here; from "write" it's clear that this function actually
> does something nontrivial).
TBH I'm not sure people will be excssively confused by the idea that a
test would validate the values it was trying to read or write were
accurate.
> > +/* Can we read back a VL from prctl? */
>
> It's certainly possible.
The comment is describing what the test is verifying.
> Since this would test different kernel paths from getting the child
> itself to do RVDL / PR_SVE_GET_VL, it would be a different test though.
> I think this diff is still good, but beefing up the ptrace tests to do
> the appropriate checks would be good too (if we don't have that already).
Yes, the ptrace stuff could have a bit more coverage.
> > + proc_write_min,
> > + proc_write_max,
> Can we also check what happens when writing unsupported values here?
We could.
> If this patch is more about establishing the framework, these could be
> TODOs for now.
It definitely feels like something we can do incrementally.
> Can we be a good citizen and restore sve_default_vector_length to its
> original value?
We do that in the tests that fiddle with the default vector length, it
seems useful to keep it at a value different from min and max as much as
possible to increase the chance that we notice a failure to set things.
> Also, we should probably disable the default vector length writing tests
> if not running with EUID==0. Verifying that writing the default vector
kselftest in general isn't going to have a great time run as non-root
but yes, it wouldn't hurt to check.
> length fails when non-root would be worth testing. If running as root,
> a temporary seteuid(nobody_uid) could be used for that.
This feels like another thing that could be done incrementally.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20210728/0e507930/attachment.sig>
More information about the linux-arm-kernel
mailing list