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

Steffen Eiden <seiden@xxxxxxxxxxxxx> · Mon, 21 Feb 2022 12:09:58 +0100

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@xxxxxxxxxxxxx>
---
  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@xxxxxxxxxxxxx>
+ */
+
+#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






