| Message ID | 20210301183319.12370-1-eafanasova@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,kvm-unit-tests] x86: add ioregionfd fast PIO test | expand |
On Mon, Mar 01, 2021 at 09:33:19PM +0300, Elena Afanasova wrote: > @@ -159,6 +166,8 @@ function run() > print_result "FAIL" $testname "$summary" > fi > > + [ -n "${fifo}" ] && rm -rf $fifo Is there a guarantee that $helper_cmd has terminated? If not then it would be good to store its pid and invoke kill $helper_cmd_pid here (maybe with an error message indicating helper_cmd hung). > diff --git a/x86/ioregionfd-test.c b/x86/ioregionfd-test.c > new file mode 100644 > index 0000000..5ea5e57 > --- /dev/null > +++ b/x86/ioregionfd-test.c > @@ -0,0 +1,84 @@ Please add a comment describing the purpose of this program. > + pollfd.fd = read_fd; > + pollfd.events = POLLIN; > + > + for (;;) { > + ret = poll(&pollfd, 1, -1); > + if (ret < 0) { > + close(read_fd); > + if (write_fd > 0) > + close(write_fd); > + err_exit("poll\n"); > + } Is poll() necessary? I think a blocking read(read_fd) would have the same effect and simplify the code? > diff --git a/x86/ioregionfd_pio.c b/x86/ioregionfd_pio.c > new file mode 100644 > index 0000000..eaf8aad > --- /dev/null > +++ b/x86/ioregionfd_pio.c > @@ -0,0 +1,24 @@ Please add a comment explaining the purpose of this test.
Hi Elena, I think KVM selftests[1] is perhaps a better test framework for this type of test, but I'm not opposed to teaching kvm-unit-tests how to do this too. I have some ideas as to how to do it more generally though. Please see comments below. (If you are interested in trying to do the testing with KVM selftests, and would like some suggestions as to how to get started, then feel free to mail me.) [1] Linux src: tools/testing/selftests/kvm On Mon, Mar 01, 2021 at 09:33:19PM +0300, Elena Afanasova wrote: > Signed-off-by: Elena Afanasova <eafanasova@gmail.com> > --- > scripts/common.bash | 12 +++++-- > scripts/runtime.bash | 9 +++++ > x86/Makefile.common | 5 ++- > x86/Makefile.x86_64 | 2 ++ > x86/ioregionfd-test.c | 84 +++++++++++++++++++++++++++++++++++++++++++ > x86/ioregionfd_pio.c | 24 +++++++++++++ > x86/unittests.cfg | 7 ++++ > 7 files changed, 140 insertions(+), 3 deletions(-) > create mode 100644 x86/ioregionfd-test.c > create mode 100644 x86/ioregionfd_pio.c > > diff --git a/scripts/common.bash b/scripts/common.bash > index 7b983f7..d9f8556 100644 > --- a/scripts/common.bash > +++ b/scripts/common.bash > @@ -14,6 +14,8 @@ function for_each_unittest() > local accel > local timeout > local rematch > + local helper_cmd > + local fifo I'd prefer not to add 'fifo' to the unittests.cfg parameter list, as that's specific to this particular "helper_cmd". Also, while in this case the helper runs along side the test, a generic helper may need to do pre-test and post-test work. So, I think we should follow the migration test model and pass the QEMU command line (minus the new -device pc-testdev part) to the helper program where it then does pre-test stuff (in this case mkfifo), possibly augments the test's QEMU command line (in this case with the -device pc-testdev part), and runs the test (in parallel with ioregionfd-helper in this case), and then does any post-test work (remove the fifos). You can write a Bash script that does most of that and which also launches ioregionfd-helper (notice, I'm suggesting we rename that from iorergionfd-test, as it's not a test, it's a test helper). > > exec {fd}<"$unittests" > > @@ -21,7 +23,7 @@ function for_each_unittest() > if [[ "$line" =~ ^\[(.*)\]$ ]]; then > rematch=${BASH_REMATCH[1]} > if [ -n "${testname}" ]; then > - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" > + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" "$helper_cmd" "$fifo" > fi > testname=$rematch > smp=1 > @@ -32,6 +34,8 @@ function for_each_unittest() > check="" > accel="" > timeout="" > + helper_cmd="" > + fifo="" > elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then > kernel=$TEST_DIR/${BASH_REMATCH[1]} > elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then > @@ -48,10 +52,14 @@ function for_each_unittest() > accel=${BASH_REMATCH[1]} > elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then > timeout=${BASH_REMATCH[1]} > + elif [[ $line =~ ^helper_cmd\ *=\ *(.*)$ ]]; then > + helper_cmd=$TEST_DIR/${BASH_REMATCH[1]} > + elif [[ $line =~ ^fifo\ *=\ *(.*)$ ]]; then > + fifo=${BASH_REMATCH[1]} > fi > done > if [ -n "${testname}" ]; then > - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" > + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" "$helper_cmd" "$fifo" > fi > exec {fd}<&- > } > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > index 132389c..ba94ee5 100644 > --- a/scripts/runtime.bash > +++ b/scripts/runtime.bash > @@ -81,6 +81,8 @@ function run() > local check="${CHECK:-$7}" > local accel="$8" > local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default > + local helper_cmd="${10}" > + local fifo="${11}" > > if [ -z "$testname" ]; then > return > @@ -139,6 +141,11 @@ function run() > echo $cmdline > fi > > + if [ -n "${helper_cmd}" ] && [ -n "${fifo}" ]; then > + mkfifo $fifo > + $helper_cmd $fifo & > + fi > + > # extra_params in the config file may contain backticks that need to be > # expanded, so use eval to start qemu. Use "> >(foo)" instead of a pipe to > # preserve the exit status. > @@ -159,6 +166,8 @@ function run() > print_result "FAIL" $testname "$summary" > fi > > + [ -n "${fifo}" ] && rm -rf $fifo > + > return $ret > } With the suggestion I'm making I think all the changes above in runtime.bash can go away, as we can simply do the following instead diff --git a/x86/run b/x86/run index 8b2425f45640..fdd6121e37ad 100755 --- a/x86/run +++ b/x86/run @@ -39,6 +39,6 @@ fi command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none -serial stdio $pci_testdev" command+=" -machine accel=$ACCEL -kernel" -command="$(timeout_cmd) $command" +command="$(timeout_cmd) $(helper_cmd) $command" run_qemu ${command} "$@" Assuming we have helper_cmd defined in x86/run some how. > > diff --git a/x86/Makefile.common b/x86/Makefile.common > index 55f7f28..a5cd1d2 100644 > --- a/x86/Makefile.common > +++ b/x86/Makefile.common > @@ -82,6 +82,9 @@ $(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o > > $(TEST_DIR)/hyperv_connections.elf: $(TEST_DIR)/hyperv.o > > +$(TEST_DIR)/ioregionfd-test: > + $(CC) -o $@ $(TEST_DIR)/ioregionfd-test.c > + > arch_clean: > - $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \ > + $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf $(TEST_DIR)/ioregionfd-test \ > $(TEST_DIR)/.*.d lib/x86/.*.d \ > diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 > index 8134952..1a7a6b1 100644 > --- a/x86/Makefile.x86_64 > +++ b/x86/Makefile.x86_64 > @@ -24,6 +24,8 @@ tests += $(TEST_DIR)/vmware_backdoors.flat > tests += $(TEST_DIR)/rdpru.flat > tests += $(TEST_DIR)/pks.flat > tests += $(TEST_DIR)/pmu_lbr.flat > +tests += $(TEST_DIR)/ioregionfd_pio.flat > +tests += $(TEST_DIR)/ioregionfd-test Please write as tests += $(TEST_DIR)/ioregionfd_pio.flat $(TEST_DIR)/ioregionfd-test > > ifneq ($(fcf_protection_full),) > tests += $(TEST_DIR)/cet.flat > diff --git a/x86/ioregionfd-test.c b/x86/ioregionfd-test.c > new file mode 100644 > index 0000000..5ea5e57 > --- /dev/null > +++ b/x86/ioregionfd-test.c > @@ -0,0 +1,84 @@ > +#include <linux/ioregion.h> > +#include <string.h> > +#include <poll.h> > +#include <stdarg.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <fcntl.h> > + > +void err_exit(const char *fmt, ...) > +{ > + va_list args; > + > + va_start(args, fmt); > + vfprintf(stderr, fmt, args); > + va_end(args); > + exit(EXIT_FAILURE); > +} > + > +int main(int argc, char *argv[]) > +{ > + struct ioregionfd_cmd cmd; > + struct ioregionfd_resp resp; > + struct pollfd pollfd; > + int read_fd, write_fd = -1; > + unsigned long cdata = 0; > + int ret; > + > + if (argc < 2) > + err_exit("Usage: %s <read_fifo> <write_fifo>\n", argv[0]); > + > + read_fd = open(argv[1], O_RDONLY); > + if (read_fd < 0) > + err_exit("failed to open read fifo %s\n", argv[1]); > + > + if (argc == 3) { > + write_fd = open(argv[2], O_WRONLY); > + if (write_fd < 0) { > + close(read_fd); > + err_exit("failed to open write fifo %s\n", argv[2]); > + } > + } > + > + pollfd.fd = read_fd; > + pollfd.events = POLLIN; > + > + for (;;) { > + ret = poll(&pollfd, 1, -1); > + if (ret < 0) { > + close(read_fd); > + if (write_fd > 0) > + close(write_fd); > + err_exit("poll\n"); > + } > + > + if (pollfd.revents & POLLIN) { > + ret = read(read_fd, &cmd, sizeof(cmd)); > + > + switch (cmd.cmd) { > + case IOREGIONFD_CMD_READ: > + memset(&resp, 0, sizeof(resp)); > + memcpy(&resp.data, &cdata, 1 << cmd.size_exponent); > + ret = write(write_fd, &resp, sizeof(resp)); > + break; > + case IOREGIONFD_CMD_WRITE: > + memcpy(&cdata, &cmd.data, 1 << cmd.size_exponent); > + if (cmd.resp) { > + memset(&resp, 0, sizeof(resp)); > + ret = write(write_fd, &resp, sizeof(resp)); > + } > + break; > + default: > + break; > + } > + } else > + break; > + } > + > + close(read_fd); > + if (write_fd > 0) > + close(write_fd); > + > + return 0; > +} > diff --git a/x86/ioregionfd_pio.c b/x86/ioregionfd_pio.c > new file mode 100644 > index 0000000..eaf8aad > --- /dev/null > +++ b/x86/ioregionfd_pio.c > @@ -0,0 +1,24 @@ > +#include "x86/vm.h" > + > +int main(int ac, char **av) > +{ > + u32 expected = 0x11223344; > + u32 val = 0; > + u32 write_addr = 0x1234; Getting this address from the command line would allow us to only hard code it in one place, unittests.cfg. > + u32 read_addr = 0x1238; > + > + outb(expected, write_addr); > + val = inb(read_addr); > + report(val == (u8)expected, "%x\n", val); > + > + outw(expected, write_addr); > + val = inw(read_addr); > + report(val == (u16)expected, "%x\n", val); > + > + outl(expected, write_addr); > + val = inl(read_addr); > + report(val == expected, "%x\n", val); > + > + return report_summary(); > +} > + > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > index 0698d15..8001808 100644 > --- a/x86/unittests.cfg > +++ b/x86/unittests.cfg > @@ -389,3 +389,10 @@ file = cet.flat > arch = x86_64 > smp = 2 > extra_params = -enable-kvm -m 2048 -cpu host > + > +[ioregion-pio] > +file = ioregionfd_pio.flat > +helper_cmd = ioregionfd-test > +fifo = /tmp/test1 /tmp/test2 > +extra_params = -device pc-testdev,addr=0x1234,size=8,rfifo=/tmp/test2,wfifo=/tmp/test1,pio=true If we wrap the QEMU invocation in the helper_cmd, then we can use mktemp to create our fifo node names. We also can put the address into a variable that we both pass to the QEMU command line augmentation and to the unit test through the unit test's command line. Thanks, drew > +arch = x86_64 > -- > 2.25.1 >
On Fri, 2021-03-05 at 12:04 +0100, Andrew Jones wrote: > Hi Elena, > > I think KVM selftests[1] is perhaps a better test framework for this > type of test, but I'm not opposed to teaching kvm-unit-tests how to > do this too. I have some ideas as to how to do it more generally > though. Please see comments below. > > (If you are interested in trying to do the testing with KVM > selftests, > and would like some suggestions as to how to get started, then feel > free to mail me.) > > [1] Linux src: tools/testing/selftests/kvm > > > On Mon, Mar 01, 2021 at 09:33:19PM +0300, Elena Afanasova wrote: > > Signed-off-by: Elena Afanasova <eafanasova@gmail.com> > > --- > > scripts/common.bash | 12 +++++-- > > scripts/runtime.bash | 9 +++++ > > x86/Makefile.common | 5 ++- > > x86/Makefile.x86_64 | 2 ++ > > x86/ioregionfd-test.c | 84 > > +++++++++++++++++++++++++++++++++++++++++++ > > x86/ioregionfd_pio.c | 24 +++++++++++++ > > x86/unittests.cfg | 7 ++++ > > 7 files changed, 140 insertions(+), 3 deletions(-) > > create mode 100644 x86/ioregionfd-test.c > > create mode 100644 x86/ioregionfd_pio.c > > > > diff --git a/scripts/common.bash b/scripts/common.bash > > index 7b983f7..d9f8556 100644 > > --- a/scripts/common.bash > > +++ b/scripts/common.bash > > @@ -14,6 +14,8 @@ function for_each_unittest() > > local accel > > local timeout > > local rematch > > + local helper_cmd > > + local fifo > > I'd prefer not to add 'fifo' to the unittests.cfg parameter list, as > that's specific to this particular "helper_cmd". Also, while in this > case the helper runs along side the test, a generic helper may need > to do pre-test and post-test work. So, I think we should follow the > migration test model and pass the QEMU command line (minus the new > -device pc-testdev part) to the helper program where it then does > pre-test stuff (in this case mkfifo), possibly augments the test's > QEMU command line (in this case with the -device pc-testdev part), > and runs the test (in parallel with ioregionfd-helper in this case), > and then does any post-test work (remove the fifos). > > You can write a Bash script that does most of that and which also > launches ioregionfd-helper (notice, I'm suggesting we rename that > from iorergionfd-test, as it's not a test, it's a test helper). > > > > > exec {fd}<"$unittests" > > > > @@ -21,7 +23,7 @@ function for_each_unittest() > > if [[ "$line" =~ ^\[(.*)\]$ ]]; then > > rematch=${BASH_REMATCH[1]} > > if [ -n "${testname}" ]; then > > - $(arch_cmd) "$cmd" "$testname" > > "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" > > "$timeout" > > + $(arch_cmd) "$cmd" "$testname" > > "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" > > "$timeout" "$helper_cmd" "$fifo" > > fi > > testname=$rematch > > smp=1 > > @@ -32,6 +34,8 @@ function for_each_unittest() > > check="" > > accel="" > > timeout="" > > + helper_cmd="" > > + fifo="" > > elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then > > kernel=$TEST_DIR/${BASH_REMATCH[1]} > > elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then > > @@ -48,10 +52,14 @@ function for_each_unittest() > > accel=${BASH_REMATCH[1]} > > elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then > > timeout=${BASH_REMATCH[1]} > > + elif [[ $line =~ ^helper_cmd\ *=\ *(.*)$ ]]; then > > + helper_cmd=$TEST_DIR/${BASH_REMATCH[1]} > > + elif [[ $line =~ ^fifo\ *=\ *(.*)$ ]]; then > > + fifo=${BASH_REMATCH[1]} > > fi > > done > > if [ -n "${testname}" ]; then > > - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" > > "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" > > + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" > > "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" > > "$helper_cmd" "$fifo" > > fi > > exec {fd}<&- > > } > > diff --git a/scripts/runtime.bash b/scripts/runtime.bash > > index 132389c..ba94ee5 100644 > > --- a/scripts/runtime.bash > > +++ b/scripts/runtime.bash > > @@ -81,6 +81,8 @@ function run() > > local check="${CHECK:-$7}" > > local accel="$8" > > local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the > > default > > + local helper_cmd="${10}" > > + local fifo="${11}" > > > > if [ -z "$testname" ]; then > > return > > @@ -139,6 +141,11 @@ function run() > > echo $cmdline > > fi > > > > + if [ -n "${helper_cmd}" ] && [ -n "${fifo}" ]; then > > + mkfifo $fifo > > + $helper_cmd $fifo & > > + fi > > + > > # extra_params in the config file may contain backticks that > > need to be > > # expanded, so use eval to start qemu. Use "> >(foo)" instead > > of a pipe to > > # preserve the exit status. > > @@ -159,6 +166,8 @@ function run() > > print_result "FAIL" $testname "$summary" > > fi > > > > + [ -n "${fifo}" ] && rm -rf $fifo > > + > > return $ret > > } > > With the suggestion I'm making I think all the changes above in > runtime.bash > can go away, as we can simply do the following instead > > diff --git a/x86/run b/x86/run > index 8b2425f45640..fdd6121e37ad 100755 > --- a/x86/run > +++ b/x86/run > @@ -39,6 +39,6 @@ fi > > command="${qemu} --no-reboot -nodefaults $pc_testdev -vnc none > -serial stdio $pci_testdev" > command+=" -machine accel=$ACCEL -kernel" > -command="$(timeout_cmd) $command" > +command="$(timeout_cmd) $(helper_cmd) $command" > > run_qemu ${command} "$@" > > > Assuming we have helper_cmd defined in x86/run some how. > > > > > diff --git a/x86/Makefile.common b/x86/Makefile.common > > index 55f7f28..a5cd1d2 100644 > > --- a/x86/Makefile.common > > +++ b/x86/Makefile.common > > @@ -82,6 +82,9 @@ $(TEST_DIR)/hyperv_stimer.elf: > > $(TEST_DIR)/hyperv.o > > > > $(TEST_DIR)/hyperv_connections.elf: $(TEST_DIR)/hyperv.o > > > > +$(TEST_DIR)/ioregionfd-test: > > + $(CC) -o $@ $(TEST_DIR)/ioregionfd-test.c > > + > > arch_clean: > > - $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \ > > + $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf > > $(TEST_DIR)/ioregionfd-test \ > > $(TEST_DIR)/.*.d lib/x86/.*.d \ > > diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 > > index 8134952..1a7a6b1 100644 > > --- a/x86/Makefile.x86_64 > > +++ b/x86/Makefile.x86_64 > > @@ -24,6 +24,8 @@ tests += $(TEST_DIR)/vmware_backdoors.flat > > tests += $(TEST_DIR)/rdpru.flat > > tests += $(TEST_DIR)/pks.flat > > tests += $(TEST_DIR)/pmu_lbr.flat > > +tests += $(TEST_DIR)/ioregionfd_pio.flat > > +tests += $(TEST_DIR)/ioregionfd-test > > Please write as > > tests += $(TEST_DIR)/ioregionfd_pio.flat $(TEST_DIR)/ioregionfd-test > > > > > ifneq ($(fcf_protection_full),) > > tests += $(TEST_DIR)/cet.flat > > diff --git a/x86/ioregionfd-test.c b/x86/ioregionfd-test.c > > new file mode 100644 > > index 0000000..5ea5e57 > > --- /dev/null > > +++ b/x86/ioregionfd-test.c > > @@ -0,0 +1,84 @@ > > +#include <linux/ioregion.h> > > +#include <string.h> > > +#include <poll.h> > > +#include <stdarg.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <unistd.h> > > +#include <fcntl.h> > > + > > +void err_exit(const char *fmt, ...) > > +{ > > + va_list args; > > + > > + va_start(args, fmt); > > + vfprintf(stderr, fmt, args); > > + va_end(args); > > + exit(EXIT_FAILURE); > > +} > > + > > +int main(int argc, char *argv[]) > > +{ > > + struct ioregionfd_cmd cmd; > > + struct ioregionfd_resp resp; > > + struct pollfd pollfd; > > + int read_fd, write_fd = -1; > > + unsigned long cdata = 0; > > + int ret; > > + > > + if (argc < 2) > > + err_exit("Usage: %s <read_fifo> <write_fifo>\n", > > argv[0]); > > + > > + read_fd = open(argv[1], O_RDONLY); > > + if (read_fd < 0) > > + err_exit("failed to open read fifo %s\n", argv[1]); > > + > > + if (argc == 3) { > > + write_fd = open(argv[2], O_WRONLY); > > + if (write_fd < 0) { > > + close(read_fd); > > + err_exit("failed to open write fifo %s\n", > > argv[2]); > > + } > > + } > > + > > + pollfd.fd = read_fd; > > + pollfd.events = POLLIN; > > + > > + for (;;) { > > + ret = poll(&pollfd, 1, -1); > > + if (ret < 0) { > > + close(read_fd); > > + if (write_fd > 0) > > + close(write_fd); > > + err_exit("poll\n"); > > + } > > + > > + if (pollfd.revents & POLLIN) { > > + ret = read(read_fd, &cmd, sizeof(cmd)); > > + > > + switch (cmd.cmd) { > > + case IOREGIONFD_CMD_READ: > > + memset(&resp, 0, sizeof(resp)); > > + memcpy(&resp.data, &cdata, 1 << > > cmd.size_exponent); > > + ret = write(write_fd, &resp, > > sizeof(resp)); > > + break; > > + case IOREGIONFD_CMD_WRITE: > > + memcpy(&cdata, &cmd.data, 1 << > > cmd.size_exponent); > > + if (cmd.resp) { > > + memset(&resp, 0, sizeof(resp)); > > + ret = write(write_fd, &resp, > > sizeof(resp)); > > + } > > + break; > > + default: > > + break; > > + } > > + } else > > + break; > > + } > > + > > + close(read_fd); > > + if (write_fd > 0) > > + close(write_fd); > > + > > + return 0; > > +} > > diff --git a/x86/ioregionfd_pio.c b/x86/ioregionfd_pio.c > > new file mode 100644 > > index 0000000..eaf8aad > > --- /dev/null > > +++ b/x86/ioregionfd_pio.c > > @@ -0,0 +1,24 @@ > > +#include "x86/vm.h" > > + > > +int main(int ac, char **av) > > +{ > > + u32 expected = 0x11223344; > > + u32 val = 0; > > + u32 write_addr = 0x1234; > > Getting this address from the command line would allow us to only > hard code it in one place, unittests.cfg. > > > + u32 read_addr = 0x1238; > > + > > + outb(expected, write_addr); > > + val = inb(read_addr); > > + report(val == (u8)expected, "%x\n", val); > > + > > + outw(expected, write_addr); > > + val = inw(read_addr); > > + report(val == (u16)expected, "%x\n", val); > > + > > + outl(expected, write_addr); > > + val = inl(read_addr); > > + report(val == expected, "%x\n", val); > > + > > + return report_summary(); > > +} > > + > > diff --git a/x86/unittests.cfg b/x86/unittests.cfg > > index 0698d15..8001808 100644 > > --- a/x86/unittests.cfg > > +++ b/x86/unittests.cfg > > @@ -389,3 +389,10 @@ file = cet.flat > > arch = x86_64 > > smp = 2 > > extra_params = -enable-kvm -m 2048 -cpu host > > + > > +[ioregion-pio] > > +file = ioregionfd_pio.flat > > +helper_cmd = ioregionfd-test > > +fifo = /tmp/test1 /tmp/test2 > > +extra_params = -device pc- > > testdev,addr=0x1234,size=8,rfifo=/tmp/test2,wfifo=/tmp/test1,pio=tr > > ue > > If we wrap the QEMU invocation in the helper_cmd, then we can use > mktemp to > create our fifo node names. We also can put the address into a > variable > that we both pass to the QEMU command line augmentation and to the > unit > test through the unit test's command line. > > Thanks, > drew > Hi Andrew, Thank you for the feedback and code review! Will rework. > > +arch = x86_64 > > -- > > 2.25.1 > >
diff --git a/scripts/common.bash b/scripts/common.bash index 7b983f7..d9f8556 100644 --- a/scripts/common.bash +++ b/scripts/common.bash @@ -14,6 +14,8 @@ function for_each_unittest() local accel local timeout local rematch + local helper_cmd + local fifo exec {fd}<"$unittests" @@ -21,7 +23,7 @@ function for_each_unittest() if [[ "$line" =~ ^\[(.*)\]$ ]]; then rematch=${BASH_REMATCH[1]} if [ -n "${testname}" ]; then - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" "$helper_cmd" "$fifo" fi testname=$rematch smp=1 @@ -32,6 +34,8 @@ function for_each_unittest() check="" accel="" timeout="" + helper_cmd="" + fifo="" elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then kernel=$TEST_DIR/${BASH_REMATCH[1]} elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then @@ -48,10 +52,14 @@ function for_each_unittest() accel=${BASH_REMATCH[1]} elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then timeout=${BASH_REMATCH[1]} + elif [[ $line =~ ^helper_cmd\ *=\ *(.*)$ ]]; then + helper_cmd=$TEST_DIR/${BASH_REMATCH[1]} + elif [[ $line =~ ^fifo\ *=\ *(.*)$ ]]; then + fifo=${BASH_REMATCH[1]} fi done if [ -n "${testname}" ]; then - $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" + $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" "$accel" "$timeout" "$helper_cmd" "$fifo" fi exec {fd}<&- } diff --git a/scripts/runtime.bash b/scripts/runtime.bash index 132389c..ba94ee5 100644 --- a/scripts/runtime.bash +++ b/scripts/runtime.bash @@ -81,6 +81,8 @@ function run() local check="${CHECK:-$7}" local accel="$8" local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default + local helper_cmd="${10}" + local fifo="${11}" if [ -z "$testname" ]; then return @@ -139,6 +141,11 @@ function run() echo $cmdline fi + if [ -n "${helper_cmd}" ] && [ -n "${fifo}" ]; then + mkfifo $fifo + $helper_cmd $fifo & + fi + # extra_params in the config file may contain backticks that need to be # expanded, so use eval to start qemu. Use "> >(foo)" instead of a pipe to # preserve the exit status. @@ -159,6 +166,8 @@ function run() print_result "FAIL" $testname "$summary" fi + [ -n "${fifo}" ] && rm -rf $fifo + return $ret } diff --git a/x86/Makefile.common b/x86/Makefile.common index 55f7f28..a5cd1d2 100644 --- a/x86/Makefile.common +++ b/x86/Makefile.common @@ -82,6 +82,9 @@ $(TEST_DIR)/hyperv_stimer.elf: $(TEST_DIR)/hyperv.o $(TEST_DIR)/hyperv_connections.elf: $(TEST_DIR)/hyperv.o +$(TEST_DIR)/ioregionfd-test: + $(CC) -o $@ $(TEST_DIR)/ioregionfd-test.c + arch_clean: - $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \ + $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf $(TEST_DIR)/ioregionfd-test \ $(TEST_DIR)/.*.d lib/x86/.*.d \ diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 index 8134952..1a7a6b1 100644 --- a/x86/Makefile.x86_64 +++ b/x86/Makefile.x86_64 @@ -24,6 +24,8 @@ tests += $(TEST_DIR)/vmware_backdoors.flat tests += $(TEST_DIR)/rdpru.flat tests += $(TEST_DIR)/pks.flat tests += $(TEST_DIR)/pmu_lbr.flat +tests += $(TEST_DIR)/ioregionfd_pio.flat +tests += $(TEST_DIR)/ioregionfd-test ifneq ($(fcf_protection_full),) tests += $(TEST_DIR)/cet.flat diff --git a/x86/ioregionfd-test.c b/x86/ioregionfd-test.c new file mode 100644 index 0000000..5ea5e57 --- /dev/null +++ b/x86/ioregionfd-test.c @@ -0,0 +1,84 @@ +#include <linux/ioregion.h> +#include <string.h> +#include <poll.h> +#include <stdarg.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <fcntl.h> + +void err_exit(const char *fmt, ...) +{ + va_list args; + + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); + exit(EXIT_FAILURE); +} + +int main(int argc, char *argv[]) +{ + struct ioregionfd_cmd cmd; + struct ioregionfd_resp resp; + struct pollfd pollfd; + int read_fd, write_fd = -1; + unsigned long cdata = 0; + int ret; + + if (argc < 2) + err_exit("Usage: %s <read_fifo> <write_fifo>\n", argv[0]); + + read_fd = open(argv[1], O_RDONLY); + if (read_fd < 0) + err_exit("failed to open read fifo %s\n", argv[1]); + + if (argc == 3) { + write_fd = open(argv[2], O_WRONLY); + if (write_fd < 0) { + close(read_fd); + err_exit("failed to open write fifo %s\n", argv[2]); + } + } + + pollfd.fd = read_fd; + pollfd.events = POLLIN; + + for (;;) { + ret = poll(&pollfd, 1, -1); + if (ret < 0) { + close(read_fd); + if (write_fd > 0) + close(write_fd); + err_exit("poll\n"); + } + + if (pollfd.revents & POLLIN) { + ret = read(read_fd, &cmd, sizeof(cmd)); + + switch (cmd.cmd) { + case IOREGIONFD_CMD_READ: + memset(&resp, 0, sizeof(resp)); + memcpy(&resp.data, &cdata, 1 << cmd.size_exponent); + ret = write(write_fd, &resp, sizeof(resp)); + break; + case IOREGIONFD_CMD_WRITE: + memcpy(&cdata, &cmd.data, 1 << cmd.size_exponent); + if (cmd.resp) { + memset(&resp, 0, sizeof(resp)); + ret = write(write_fd, &resp, sizeof(resp)); + } + break; + default: + break; + } + } else + break; + } + + close(read_fd); + if (write_fd > 0) + close(write_fd); + + return 0; +} diff --git a/x86/ioregionfd_pio.c b/x86/ioregionfd_pio.c new file mode 100644 index 0000000..eaf8aad --- /dev/null +++ b/x86/ioregionfd_pio.c @@ -0,0 +1,24 @@ +#include "x86/vm.h" + +int main(int ac, char **av) +{ + u32 expected = 0x11223344; + u32 val = 0; + u32 write_addr = 0x1234; + u32 read_addr = 0x1238; + + outb(expected, write_addr); + val = inb(read_addr); + report(val == (u8)expected, "%x\n", val); + + outw(expected, write_addr); + val = inw(read_addr); + report(val == (u16)expected, "%x\n", val); + + outl(expected, write_addr); + val = inl(read_addr); + report(val == expected, "%x\n", val); + + return report_summary(); +} + diff --git a/x86/unittests.cfg b/x86/unittests.cfg index 0698d15..8001808 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -389,3 +389,10 @@ file = cet.flat arch = x86_64 smp = 2 extra_params = -enable-kvm -m 2048 -cpu host + +[ioregion-pio] +file = ioregionfd_pio.flat +helper_cmd = ioregionfd-test +fifo = /tmp/test1 /tmp/test2 +extra_params = -device pc-testdev,addr=0x1234,size=8,rfifo=/tmp/test2,wfifo=/tmp/test1,pio=true +arch = x86_64
Signed-off-by: Elena Afanasova <eafanasova@gmail.com> --- scripts/common.bash | 12 +++++-- scripts/runtime.bash | 9 +++++ x86/Makefile.common | 5 ++- x86/Makefile.x86_64 | 2 ++ x86/ioregionfd-test.c | 84 +++++++++++++++++++++++++++++++++++++++++++ x86/ioregionfd_pio.c | 24 +++++++++++++ x86/unittests.cfg | 7 ++++ 7 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 x86/ioregionfd-test.c create mode 100644 x86/ioregionfd_pio.c