| Message ID | 20210917120855.13858-1-broonie@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | selftests: arm64: vec-syscfg updates | expand |
On Fri, Sep 17, 2021 at 01:08:51PM +0100, Mark Brown wrote: > This series fixes up a few issues introduced into vec-syscfg during > refactoring in the review process, then adds a new test which ensures > that the behaviour when we attempt to set a vector length which is not > supported by the current system matches what is documented in the SVE > ABI documentation. > > v2: > - Fix handling of missing VLs when checking that vector length setting > works as expected. With this series applied, I see a test failing under qemu with: # selftests: arm64: vec-syscfg # TAP version 13 # 1..10 # ok 1 SVE default vector length 64 # ok 2 # SKIP Need to be root to write to /proc # ok 3 # SKIP Need to be root to write to /proc # ok 4 SVE current VL is 64 # ok 5 SVE set VL 64 and have VL 64 # ok 6 # SKIP SVE only one VL supported # ok 7 # SKIP SVE only one VL supported # ok 8 # SKIP SVE only one VL supported # ok 9 # SKIP SVE only one VL supported # # SVE VL 272 returned 256 not maximum 0 # # SVE VL 288 returned 256 not maximum 0 # # SVE VL 304 returned 256 not maximum 0 # # SVE VL 320 returned 256 not maximum 0 # # SVE VL 336 returned 256 not maximum 0 # # SVE VL 352 returned 256 not maximum 0 [repeat similar messages for ages] # SVE VL 8160 returned 256 not maximum 0 # # SVE VL 8176 returned 256 not maximum 0 # # SVE VL 8192 returned 256 not maximum 0 # not ok 10 SVE prctl() set all VLs, 496 errors # # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:6 error:0 Will
On Wed, Sep 29, 2021 at 03:31:14PM +0100, Will Deacon wrote: > With this series applied, I see a test failing under qemu with: > # selftests: arm64: vec-syscfg > # TAP version 13 > # 1..10 > # ok 1 SVE default vector length 64 > # ok 2 # SKIP Need to be root to write to /proc > # ok 3 # SKIP Need to be root to write to /proc AFAICT this is due to running as a non-root user, the testsuite was already having serious issues before then... > # ok 4 SVE current VL is 64 > # ok 5 SVE set VL 64 and have VL 64 > # ok 6 # SKIP SVE only one VL supported > # ok 7 # SKIP SVE only one VL supported > # ok 8 # SKIP SVE only one VL supported > # ok 9 # SKIP SVE only one VL supported > # # SVE VL 272 returned 256 not maximum 0 ...as it's starting off by testing an interface that's only writable by root and then relying on that information, the existing tests were also not working usefully. qemu by default supports way more than one vector length. In any case it's just the test added by the last patch that's causing the output here, the first four patches should be fine and fix issues. I'm not sure it's a particularly good idea to run kselftest as a non-root user TBH, it's going to cause you to skip a lot of tests.
On Wed, Sep 29, 2021 at 03:43:23PM +0100, Mark Brown wrote: > On Wed, Sep 29, 2021 at 03:31:14PM +0100, Will Deacon wrote: > > > With this series applied, I see a test failing under qemu with: > > > # selftests: arm64: vec-syscfg > > # TAP version 13 > > # 1..10 > > # ok 1 SVE default vector length 64 > > # ok 2 # SKIP Need to be root to write to /proc > > # ok 3 # SKIP Need to be root to write to /proc > > AFAICT this is due to running as a non-root user, the testsuite was > already having serious issues before then... > > > # ok 4 SVE current VL is 64 > > # ok 5 SVE set VL 64 and have VL 64 > > # ok 6 # SKIP SVE only one VL supported > > # ok 7 # SKIP SVE only one VL supported > > # ok 8 # SKIP SVE only one VL supported > > # ok 9 # SKIP SVE only one VL supported > > # # SVE VL 272 returned 256 not maximum 0 > > ...as it's starting off by testing an interface that's only writable by > root and then relying on that information, the existing tests were also > not working usefully. qemu by default supports way more than one vector > length. In any case it's just the test added by the last patch that's > causing the output here, the first four patches should be fine and fix > issues. > > I'm not sure it's a particularly good idea to run kselftest as a > non-root user TBH, it's going to cause you to skip a lot of tests. Ah, thanks for pointing that out. It would probably be better to skip the tests rather than fail them if they're not running with sufficient permissions, but I'll go ahead and queue your v3 for now. Will
On Wed, Sep 29, 2021 at 04:35:12PM +0100, Will Deacon wrote: > On Wed, Sep 29, 2021 at 03:43:23PM +0100, Mark Brown wrote: > > I'm not sure it's a particularly good idea to run kselftest as a > > non-root user TBH, it's going to cause you to skip a lot of tests. > Ah, thanks for pointing that out. It would probably be better to skip the > tests rather than fail them if they're not running with sufficient > permissions, but I'll go ahead and queue your v3 for now. Yes, that's what my v3 does - it skips the new test if it failed to enumerate minimum and maximum vector lengths, like the other tests do.
On 9/29/21 9:35 AM, Will Deacon wrote: > On Wed, Sep 29, 2021 at 03:43:23PM +0100, Mark Brown wrote: >> On Wed, Sep 29, 2021 at 03:31:14PM +0100, Will Deacon wrote: >> >>> With this series applied, I see a test failing under qemu with: >> >>> # selftests: arm64: vec-syscfg >>> # TAP version 13 >>> # 1..10 >>> # ok 1 SVE default vector length 64 >>> # ok 2 # SKIP Need to be root to write to /proc >>> # ok 3 # SKIP Need to be root to write to /proc >> >> AFAICT this is due to running as a non-root user, the testsuite was >> already having serious issues before then... >> >>> # ok 4 SVE current VL is 64 >>> # ok 5 SVE set VL 64 and have VL 64 >>> # ok 6 # SKIP SVE only one VL supported >>> # ok 7 # SKIP SVE only one VL supported >>> # ok 8 # SKIP SVE only one VL supported >>> # ok 9 # SKIP SVE only one VL supported >>> # # SVE VL 272 returned 256 not maximum 0 >> >> ...as it's starting off by testing an interface that's only writable by >> root and then relying on that information, the existing tests were also >> not working usefully. qemu by default supports way more than one vector >> length. In any case it's just the test added by the last patch that's >> causing the output here, the first four patches should be fine and fix >> issues. >> >> I'm not sure it's a particularly good idea to run kselftest as a >> non-root user TBH, it's going to cause you to skip a lot of tests. > We don't want Kselftest default run to be as root. Users can choose to run as root which would be an explicit choice so they expect and plan for the impact. Example panic test. > Ah, thanks for pointing that out. It would probably be better to skip the > tests rather than fail them if they're not running with sufficient > permissions, but I'll go ahead and queue your v3 for now. > Correct. I would like to see tests skipped not failed if either config or permissions are lacking to run the tests. thanks, -- Shuah
On Wed, Sep 29, 2021 at 10:26:49AM -0600, Shuah Khan wrote: > On 9/29/21 9:35 AM, Will Deacon wrote: > > On Wed, Sep 29, 2021 at 03:43:23PM +0100, Mark Brown wrote: > > > I'm not sure it's a particularly good idea to run kselftest as a > > > non-root user TBH, it's going to cause you to skip a lot of tests. > We don't want Kselftest default run to be as root. Users can choose to > run as root which would be an explicit choice so they expect and plan > for the impact. Example panic test. OTOH if you're trying to verify that the tests aren't broken it's not that great since it'll mean that you'll not be exercising a bunch of the code. > > Ah, thanks for pointing that out. It would probably be better to skip the > > tests rather than fail them if they're not running with sufficient > > permissions, but I'll go ahead and queue your v3 for now. > Correct. I would like to see tests skipped not failed if either config > or permissions are lacking to run the tests. As I said previously that's what my v3 that Will referenced above does.
On 9/29/21 10:37 AM, Mark Brown wrote: > On Wed, Sep 29, 2021 at 10:26:49AM -0600, Shuah Khan wrote: >> On 9/29/21 9:35 AM, Will Deacon wrote: >>> On Wed, Sep 29, 2021 at 03:43:23PM +0100, Mark Brown wrote: > >>>> I'm not sure it's a particularly good idea to run kselftest as a >>>> non-root user TBH, it's going to cause you to skip a lot of tests. > >> We don't want Kselftest default run to be as root. Users can choose to >> run as root which would be an explicit choice so they expect and plan >> for the impact. Example panic test. > > OTOH if you're trying to verify that the tests aren't broken it's not > that great since it'll mean that you'll not be exercising a bunch of the > code. > Correct. Running Kselftest as root is the best approach for full coverage. thanks, -- Shuah
This series fixes up a few issues introduced into vec-syscfg during refactoring in the review process, then adds a new test which ensures that the behaviour when we attempt to set a vector length which is not supported by the current system matches what is documented in the SVE ABI documentation. v2: - Fix handling of missing VLs when checking that vector length setting works as expected. Mark Brown (4): selftests: arm64: Fix printf() format mismatch in vec-syscfg selftests: arm64: Remove bogus error check on writing to files selftests: arm64: Fix and enable test for setting current VL in vec-syscfg selftests: arm64: Verify that all possible vector lengths are handled tools/testing/selftests/arm64/fp/vec-syscfg.c | 89 ++++++++++++++++--- 1 file changed, 76 insertions(+), 13 deletions(-) base-commit: 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f