Re: [PATCH 3/3] selftests: drivers/s390x: Add uvdevice tests

Hey Shuah, thanks for your review.

On 2/19/22 00:28, Shuah Khan wrote:

On 2/17/22 4:37 AM, Steffen Eiden wrote:

...

Adds some selftests to test ioctl error paths of the uv-uapi.

Please add information on how to run this test and example output.

I will do it in a v2

...

Signed-off-by: Steffen Eiden seiden@linux.ibm.com

MAINTAINERS                                   |   1 +   tools/testing/selftests/Makefile              |   1 +   tools/testing/selftests/drivers/.gitignore    |   1 +   .../selftests/drivers/s390x/uvdevice/Makefile |  22 ++   .../selftests/drivers/s390x/uvdevice/config   |   1 +   .../drivers/s390x/uvdevice/test_uvdevice.c    | 280 ++++++++++++++++++   6 files changed, 306 insertions(+)   create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/Makefile   create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/config   create mode 100644 tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c

diff --git a/MAINTAINERS b/MAINTAINERS index c7d8d0fe48cf..c6a0311c3fa8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10462,6 +10462,7 @@ F:    arch/s390/kernel/uv.c   F:    arch/s390/kvm/   F:    arch/s390/mm/gmap.c   F:    drivers/s390/char/uvdevice.c +F:    tools/testing/selftests/drivers/s390x/uvdevice/   F:    tools/testing/selftests/kvm/*/s390x/   F:    tools/testing/selftests/kvm/s390x/ diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c852eb40c4f7..3b8abaee9271 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -9,6 +9,7 @@ TARGETS += core   TARGETS += cpufreq   TARGETS += cpu-hotplug   TARGETS += drivers/dma-buf +TARGETS += drivers/s390x/uvdevice   TARGETS += efivarfs   TARGETS += exec   TARGETS += filesystems diff --git a/tools/testing/selftests/drivers/.gitignore b/tools/testing/selftests/drivers/.gitignore index ca74f2e1c719..09e23b5afa96 100644 --- a/tools/testing/selftests/drivers/.gitignore +++ b/tools/testing/selftests/drivers/.gitignore @@ -1,2 +1,3 @@   # SPDX-License-Identifier: GPL-2.0-only   /dma-buf/udmabuf +/s390x/uvdevice/test_uvdevice diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile new file mode 100644 index 000000000000..5e701d2708d4 --- /dev/null +++ b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile @@ -0,0 +1,22 @@ +include ../../../../../build/Build.include

+UNAME_M := $(shell uname -m)

+ifneq ($(UNAME_M),s390x) +nothing: +.PHONY: all clean run_tests install +.SILENT: +else

+TEST_GEN_PROGS := test_uvdevice

+top_srcdir ?= ../../../../../.. +KSFT_KHDR_INSTALL := 1 +khdr_dir = $(top_srcdir)/usr/include +LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include

+CFLAGS += -Wall -Werror -static -I$(khdr_dir) -I$(LINUX_TOOL_ARCH_INCLUDE)

+include ../../../lib.mk

+endif diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/config b/tools/testing/selftests/drivers/s390x/uvdevice/config new file mode 100644 index 000000000000..f28a04b99eff --- /dev/null +++ b/tools/testing/selftests/drivers/s390x/uvdevice/config @@ -0,0 +1 @@ +CONFIG_S390_UV_UAPI=y diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c b/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c new file mode 100644 index 000000000000..f23663bcab03 --- /dev/null +++ b/tools/testing/selftests/drivers/s390x/uvdevice/test_uvdevice.c @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: GPL-2.0 +/*

• *  selftest for the Ultravisor UAPI device
• *  Copyright IBM Corp. 2022
• *  Author(s): Steffen Eiden seiden@linux.ibm.com
• */

+#include <stdint.h> +#include <fcntl.h> +#include <errno.h> +#include <sys/ioctl.h> +#include <sys/mman.h>

+#include <asm/uvdevice.h>

+#include "../../../kselftest_harness.h"

+#define BUFFER_SIZE 0x200 +FIXTURE(uvio_fixture) { +    int uv_fd; +    struct uvio_ioctl_cb uvio_ioctl; +    uint8_t buffer[BUFFER_SIZE]; +    __u64 fault_page; +};

+FIXTURE_VARIANT(uvio_fixture) { +    unsigned long ioctl_cmd; +    uint32_t arg_size; +};

+FIXTURE_VARIANT_ADD(uvio_fixture, qui) { +    .ioctl_cmd = UVIO_IOCTL_QUI, +    .arg_size = BUFFER_SIZE, +};

+FIXTURE_VARIANT_ADD(uvio_fixture, att) { +    .ioctl_cmd = UVIO_IOCTL_ATT, +    .arg_size = sizeof(struct uvio_attest), +};

+FIXTURE_SETUP(uvio_fixture) +{ +    self->uv_fd = open("/dev/uv", O_RDWR);

+    self->uvio_ioctl.argument_addr = (__u64)self->buffer; +    self->uvio_ioctl.argument_len = variant->arg_size; +    self->fault_page = +        (__u64)mmap(NULL, (size_t)getpagesize(), PROT_NONE, MAP_ANONYMOUS, -1, 0); +}

+FIXTURE_TEARDOWN(uvio_fixture) +{ +    if (self->uv_fd) +        close(self->uv_fd); +    munmap((void *)self->fault_page, (size_t)getpagesize()); +}

+TEST_F(uvio_fixture, fault_ioctl_arg) +{ +    int rc, errno_cache;

+    rc = ioctl(self->uv_fd, variant->ioctl_cmd, NULL); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT);

+    rc = ioctl(self->uv_fd, variant->ioctl_cmd, self->fault_page); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT); +}

+TEST_F(uvio_fixture, fault_uvio_arg) +{ +    int rc, errno_cache;

+    self->uvio_ioctl.argument_addr = 0; +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT);

+    self->uvio_ioctl.argument_addr = self->fault_page; +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT); +}

+/*

• • Test to verify that IOCTLs with invalid values in the

ioctl_control block

• • are rejected.
• */

+TEST_F(uvio_fixture, inval_ioctl_cb) +{ +    int rc, errno_cache;

+    self->uvio_ioctl.argument_len = 0; +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL);

+    self->uvio_ioctl.argument_len = (uint32_t)-1; +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL); +    self->uvio_ioctl.argument_len = variant->arg_size;

+    self->uvio_ioctl.flags = (uint32_t)-1; +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL); +    self->uvio_ioctl.flags = 0;

+    memset(self->uvio_ioctl.reserved14, 0xff, sizeof(self->uvio_ioctl.reserved14)); +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL);

+    memset(&self->uvio_ioctl, 0x11, sizeof(self->uvio_ioctl)); +    rc = ioctl(self->uv_fd, variant->ioctl_cmd, &self->uvio_ioctl); +    ASSERT_EQ(rc, -1); +}

+TEST_F(uvio_fixture, inval_ioctl_cmd) +{ +    int rc, errno_cache; +    uint8_t nr = _IOC_NR(variant->ioctl_cmd); +    unsigned long cmds[] = { +        _IOWR('a', nr, struct uvio_ioctl_cb), +        _IOWR(UVIO_TYPE_UVC, nr, int), +        _IO(UVIO_TYPE_UVC, nr), +        _IOR(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb), +        _IOW(UVIO_TYPE_UVC, nr, struct uvio_ioctl_cb), +    };

+    for (size_t i = 0; i < ARRAY_SIZE(cmds); i++) { +        rc = ioctl(self->uv_fd, cmds[i], &self->uvio_ioctl); +        errno_cache = errno; +        ASSERT_EQ(rc, -1); +        ASSERT_EQ(errno_cache, EINVAL); +    } +}

+struct test_attest_buffer { +    uint8_t arcb[0x180]; +    uint8_t meas[64]; +    uint8_t add[32]; +};

+FIXTURE(attest_fixture) { +    int uv_fd; +    struct uvio_ioctl_cb uvio_ioctl; +    struct uvio_attest uvio_attest; +    struct test_attest_buffer attest_buffer; +    __u64 fault_page; +};

+FIXTURE_SETUP(attest_fixture) +{ +    self->uv_fd = open("/dev/uv", O_RDWR);

So each test opne and closes the devuce. Can this file stay open for the duraction of the test run and close it in main() after test exits?

IIRC 'test_harness_run' will never return and instead exit the program. Therefore, I cannot do anything after that function including closing files if I want a clean cleanup of my test.

...

+    self->uvio_ioctl.argument_addr = (__u64)&self->uvio_attest; +    self->uvio_ioctl.argument_len = sizeof(self->uvio_attest);

+    self->uvio_attest.arcb_addr = (__u64)&self->attest_buffer.arcb; +    self->uvio_attest.arcb_len = sizeof(self->attest_buffer.arcb);

+    self->uvio_attest.meas_addr = (__u64)&self->attest_buffer.meas; +    self->uvio_attest.meas_len = sizeof(self->attest_buffer.meas);

+    self->uvio_attest.add_data_addr = (__u64)&self->attest_buffer.add; +    self->uvio_attest.add_data_len = sizeof(self->attest_buffer.add); +    self->fault_page = +        (__u64)mmap(NULL, (size_t)getpagesize(), PROT_NONE, MAP_ANONYMOUS, -1, 0); +}

+FIXTURE_TEARDOWN(attest_fixture) +{ +    if (self->uv_fd) +        close(self->uv_fd); +    munmap((void *)self->fault_page, (size_t)getpagesize()); +}

+static void att_inval_sizes_test(uint32_t *size, uint32_t max_size, bool test_zero, +                 struct __test_metadata *_metadata, +                 FIXTURE_DATA(attest_fixture) *self) +{ +    int rc, errno_cache; +    uint32_t tmp = *size;

+    if (test_zero) { +        *size = 0; +        rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +        errno_cache = errno; +        ASSERT_EQ(rc, -1); +        ASSERT_EQ(errno_cache, EINVAL); +    } +    *size = max_size + 1; +    rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL); +    *size = tmp; +}

+/*

• • Test to verify that attestation IOCTLs with invalid values in the

UVIO

• • attestation control block are rejected.
• */

+TEST_F(attest_fixture, att_inval_request) +{ +    int rc, errno_cache;

+    att_inval_sizes_test(&self->uvio_attest.add_data_len, UVIO_ATT_ADDITIONAL_MAX_LEN, +                 false, _metadata, self); +    att_inval_sizes_test(&self->uvio_attest.meas_len, UVIO_ATT_MEASUREMENT_MAX_LEN, +                 true, _metadata, self); +    att_inval_sizes_test(&self->uvio_attest.arcb_len, UVIO_ATT_ARCB_MAX_LEN, +                 true, _metadata, self);

+    self->uvio_attest.reserved136 = (uint16_t)-1; +    rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EINVAL);

+    memset(&self->uvio_attest, 0x11, sizeof(self->uvio_attest)); +    rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +    ASSERT_EQ(rc, -1); +}

+static void att_inval_addr_test(__u64 *addr, struct __test_metadata *_metadata, +                FIXTURE_DATA(attest_fixture) *self) +{ +    int rc, errno_cache; +    __u64 tmp = *addr;

+    *addr = 0; +    rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT); +    *addr = self->fault_page; +    rc = ioctl(self->uv_fd, UVIO_IOCTL_ATT, &self->uvio_ioctl); +    errno_cache = errno; +    ASSERT_EQ(rc, -1); +    ASSERT_EQ(errno_cache, EFAULT); +    *addr = tmp; +}

+TEST_F(attest_fixture, att_inval_addr) +{ +    att_inval_addr_test(&self->uvio_attest.arcb_addr, _metadata, self); +    att_inval_addr_test(&self->uvio_attest.add_data_addr, _metadata, self); +    att_inval_addr_test(&self->uvio_attest.meas_addr, _metadata, self); +}

+static void __attribute__((constructor)) __constructor_order_last(void) +{ +    if (!__constructor_order) +        __constructor_order = _CONSTRUCTOR_ORDER_BACKWARD; +}

+int main(int argc, char **argv) +{ +    int fd = open("/dev/uv", O_RDWR);

+    if (fd < 0) { +        ksft_exit_skip("No uv-device.\n");

Please add information on which confg options need to be enabled to run this test - CONFIG_S390_UV_UAPI?

OK.

Also add a check and skip when run by non-root if this test requires root privileges.

The test just needs RDWR access to /dev/uv.

...

+        ksft_exit(KSFT_SKIP);

You don't need this - ksft_exit_skip() calls exit(KSFT_SKIP)

thanks, I missed that.

...

+    } +    close(fd);

Closing the file before test inocation?

See comment about fixture setup/cleanup. If you prefer, I can skip the close and use fd in the tests instead of self->uv_fd. However, I am unabale to do an explicit close to /dev/uv after running the tests.

...

+    return test_harness_run(argc, argv); +}

thanks, -- Shuah

Steffen