On Wed, Jul 28, 2021 at 10:41:51AM +0100, Dave Martin wrote:
> This test was originally a diagnostic tool, so the way VLs are probed
> aims to be efficient, rather than being defensive against the kernel
> doing weird stuff.
Yeah, the whole probing thing doesn't really fit into kselftest's idea
of what a test is - I just put this in here since it seemed like a cheap
extra validation that we could add in with little bother rather than
because it's a complete and thorough validation of every possible thing
that could go wrong. I'd be just as happy to not modify this at all but
since it does try the intermediate VLs it didn't seem like a terrible
idea to throw in some validation while we're at it.
> If the kernel returns a vl > than the one we tried to set, we'll end
> up in an infinite loop because of the way the loop deliberately uses the
> kernel's return value to skip unsupported VLs. So, it might be better
> to probe every single architecturally possible VL and sanity check
> everything instead.
Yup, that'd obviously be a complete rewrite though. We'd not only need
to probe every possible vector length, but also validate that any
unsupported vector lengths report the expected vector length instead
when we try them.
> > + if (rdvl_sve() != vl)
> > + ksft_exit_fail_msg("Set VL %d, RDVL %d\n",
> > + vl, rdvl_sve());
> If this fails, the VL vl wasn't "Set" at all. I found this a bit
> confusing from just looking at this hunk.
> Can we write something like "PR_SVE_SET_VL reports VL %d, RDVL %d"?
Sure.
> I'm not sure of the correct option for forcing SVE off against the
> compiler default -- check with the tools folks. If there isn't a
> proper way to do this, it's a toolchain defect so we should flag it up,
> but -mgeneral-regs-only might work for us even though it's a bit of a
> sledgehammer.
-march should do the trick (if it doesn't the base kernel already has
issues). This is a separate issue that affects all the arm64 selftests
I think.
Attachment:
signature.asc
Description: PGP signature
