On Wed, Dec 04, 2013 at 05:42:57PM +0100, Andrew Jones wrote: > This is the initial arm test framework and a first simple test that > checks some bootinfo. kvm isn't needed to run this test. This patch > also adds a common build environment variable, $QEMU_BIN, which > allows makefiles to call on qemu when needed. > > Try it out with > yum install gcc-arm-linux-gnu dtc > export QEMU=[qemu with mach-virt and virtio-testdev] > ./configure --cross-prefix=arm-linux-gnu- --arch=arm > make > ./run_tests.sh You refer to QEMU_BIN but your example uses QEMU= ? It's getting to be a pretty heavy chunk this patch, it would have been slightly easier to review if it was broken up into multiple patches, but ok. > > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx> > > --- > v2: > - add eabi utility functions needed for some toolchains, this > allows us to drop the divmod hacks that were in v1 > - switch to kernel coding style > - some refactoring of setup code for heap init > - some refactoring of the simple bootinfo test for clarity > and reuse opportunity > - update base addr for the new mach-virt version > --- > arm/boot.c | 17 +++++++++++ > arm/cstart.S | 38 +++++++++++++++++++++++ > arm/flat.lds | 18 +++++++++++ > arm/run | 19 ++++++++++++ > arm/unittests.cfg | 11 +++++++ > config/config-arm.mak | 73 +++++++++++++++++++++++++++++++++++++++++++ > configure | 10 ++++-- > lib/arm/eabi_compat.c | 20 ++++++++++++ > lib/arm/io.c | 31 +++++++++++++++++++ > lib/arm/io.h | 24 +++++++++++++++ > lib/arm/setup.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++ > lib/arm/sysinfo.h | 19 ++++++++++++ > lib/libcflat.h | 2 ++ > lib/libio.h | 4 +++ > lib/test_util.c | 34 +++++++++++++++++++++ > lib/test_util.h | 13 ++++++++ > 16 files changed, 416 insertions(+), 2 deletions(-) > create mode 100644 arm/boot.c > create mode 100644 arm/cstart.S > create mode 100644 arm/flat.lds > create mode 100755 arm/run > create mode 100644 arm/unittests.cfg > create mode 100644 config/config-arm.mak > create mode 100644 lib/arm/eabi_compat.c > create mode 100644 lib/arm/io.c > create mode 100644 lib/arm/io.h > create mode 100644 lib/arm/setup.c > create mode 100644 lib/arm/sysinfo.h > create mode 100644 lib/test_util.c > create mode 100644 lib/test_util.h > > diff --git a/arm/boot.c b/arm/boot.c > new file mode 100644 > index 0000000000000..dc42dfc232366 > --- /dev/null > +++ b/arm/boot.c > @@ -0,0 +1,17 @@ > +#include "libcflat.h" > +#include "test_util.h" > +#include "arm/sysinfo.h" > + > +int main(int argc, char **argv) > +{ > + int ret = FAIL; > + > + if (argc >= 1) { > + --argc; > + if (!strcmp(argv[0], "mem") && enough_args(argc, 1)) { > + if (check_u32(mem32.size/1024/1024, 10, argv[1])) > + ret = PASS; > + } > + } > + return ret; > +} consider renaming this file to boottest.c to avoid the confusion that this file is needed to boot anything that runs tests... > diff --git a/arm/cstart.S b/arm/cstart.S > new file mode 100644 > index 0000000000000..05d4bb5becaa0 > --- /dev/null > +++ b/arm/cstart.S > @@ -0,0 +1,38 @@ > + > +#define CR_B (1 << 7) /* Big endian */ > + > +.arm > + > +.section .init > + > +.globl start > +start: > + /* bootloader params are in r0-r2 */ which bootlaoder params are they? What boot protocol is used, what are you expecting to be in the various registers? I assume this tool expects r0-r2 to follow the definitions in Documentation/arm/Booting in the kernel? > + ldr sp, =stacktop > + > + mrc p15, 0, r8, c1, c0, 0 @r8 = sctrl that's sctlr, not sctrl. > + ands r3, r8, #CR_B @set BE, if necessary > + ldrne r3, =cpu_is_be > + movne r4, #1 This is deprecated for ARMv7 according to the ARM ARM. What is the intention here? Does qemu support running this test tool with the system configured for big-endian? If so, I think this is a build option for this binary or you need to come up with some other architecture-compliant method of detecting the endian-state. > + strne r4, [r3] > + bl setup @complete setup > + > + /* start the test */ > + ldr r0, =__argc > + ldr r0, [r0] > + ldr r1, =__argv > + bl main > + bl exit > + b halt > + > +.text > + > +.globl halt > +halt: > +1: wfi > + b 1b > + > +.data > + > +.globl cpu_is_be > +cpu_is_be: .word 0 > diff --git a/arm/flat.lds b/arm/flat.lds > new file mode 100644 > index 0000000000000..3e5d72e24989b > --- /dev/null > +++ b/arm/flat.lds > @@ -0,0 +1,18 @@ > + > +SECTIONS > +{ > + .text : { *(.init) *(.text) *(.text.*) } > + . = ALIGN(4K); > + .data : { *(.data) } > + . = ALIGN(16); > + .rodata : { *(.rodata) } > + . = ALIGN(16); > + .bss : { *(.bss) } > + . = ALIGN(4K); > + edata = .; > + . += 8K; > + . = ALIGN(4K); > + stacktop = .; > +} > + > +ENTRY(start) > diff --git a/arm/run b/arm/run > new file mode 100755 > index 0000000000000..64446e8907564 > --- /dev/null > +++ b/arm/run > @@ -0,0 +1,19 @@ > +#!/bin/bash > + > +qemu="${QEMU:-qemu-system-arm}" > +testdev='virtio-testdev' > + > +if ! $qemu -device '?' 2>&1 | grep $testdev > /dev/null; then > + echo \"$qemu\" has no support for the virtio test device. Exiting. > + exit 2 > +fi > + > +command="$qemu -device $testdev -display none -serial stdio " > +command+="-M virt -cpu cortex-a15 " > +#command+="-enable-kvm " > +command+="-kernel" > +echo $command "$@" > +$command "$@" > +ret=$? > +echo Return value from qemu: $ret > +exit $ret > diff --git a/arm/unittests.cfg b/arm/unittests.cfg > new file mode 100644 > index 0000000000000..c328657b7944a > --- /dev/null > +++ b/arm/unittests.cfg > @@ -0,0 +1,11 @@ > +# Define your new unittest following the convention: > +# [unittest_name] > +# file = foo.flat # Name of the flat file to be used > +# smp = 2 # Number of processors the VM will use during this test > +# extra_params = -append <params...> # Additional parameters used used to QEMU I presume? > +# arch = arm/arm64 # Only if the test case works only on one of them Do we have any verified support for arm64 yet? > +# groups = group1 group2 # Used to identify test cases with run_tests -g ... > + > +[boot_info] > +file = boot.flat > +extra_params = -m 256 -append 'mem 256' > diff --git a/config/config-arm.mak b/config/config-arm.mak > new file mode 100644 > index 0000000000000..d0814186b279c > --- /dev/null > +++ b/config/config-arm.mak > @@ -0,0 +1,73 @@ > +mach = mach-virt > +iodevs = pl011 virtio_mmio > +phys_base = 0x40000000 > + > +cstart.o = $(TEST_DIR)/cstart.o > +bits = 32 > +ldarch = elf32-littlearm > +kernel_offset = 0x10000 > +CFLAGS += -D__arm__ > + > +all: test_cases > + > +cflatobjs += \ > + lib/$(TEST_DIR)/iomaps.gen.o \ > + lib/heap.o \ > + lib/iomaps.o \ > + lib/libio.o \ > + lib/virtio.o \ > + lib/virtio-testdev.o \ > + lib/test_util.o \ > + lib/arm/io.o \ > + lib/arm/setup.o > + > +libeabi := lib/arm/libeabi.a > +eabiobjs += \ > + lib/arm/eabi_compat.o > + > +$(libcflat) $(libeabi): LDFLAGS += -nostdlib > +$(libcflat) $(libeabi): CFLAGS += -ffreestanding -I lib > + > +CFLAGS += -Wextra > +CFLAGS += -marm > +#CFLAGS += -mcpu=$(PROCESSOR) This looks weird, should it not be $(PROCESSOR) and default to cortex-a15 if it's not set? (also note that you can now use -cpu host with mach-virt which may make your life easier). > +CFLAGS += -mcpu=cortex-a15 > +CFLAGS += -O2 Why do we choose this particular optimzation-level in the arch-specific config? > + > +libgcc := $(shell $(CC) -m$(ARCH) --print-libgcc-file-name) > +start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) ))) > + > +FLATLIBS = $(libcflat) $(libgcc) $(libeabi) > +%.elf: %.o $(FLATLIBS) arm/flat.lds > + $(CC) $(CFLAGS) -nostdlib -o $@ \ > + -Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \ > + $(filter %.o, $^) $(FLATLIBS) I have no idea what the above rules are doing :-( > + > +$(libeabi): $(eabiobjs) > + $(AR) rcs $@ $^ > + > +%.flat: %.elf > + $(OBJCOPY) -O binary $^ $@ > + > +tests-common = $(TEST_DIR)/boot.flat > + > +tests_and_config = $(TEST_DIR)/*.flat $(TEST_DIR)/unittests.cfg > + > +test_cases: $(tests-common) $(tests) what's the distinction between tests-common and tests? arm/arm64 vs. one or the other, or? > + > +$(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib > + > +$(TEST_DIR)/boot.elf: $(cstart.o) $(TEST_DIR)/boot.o > + > +lib/$(TEST_DIR)/iomaps.gen.c: lib/$(TEST_DIR)/$(mach).dts > + scripts/gen-devtree-iomaps.pl $^ $(iodevs) > $@ > + > +lib/$(TEST_DIR)/mach-virt.dts: dtb = $(subst .dts,.dtb,$@) > +lib/$(TEST_DIR)/mach-virt.dts: > + $(QEMU_BIN) -kernel /dev/null -M virt -machine dumpdtb=$(dtb) > + fdtdump $(dtb) > $@ > + > +arch_clean: > + $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \ > + $(libeabi) $(eabiobjs) $(TEST_DIR)/.*.d lib/arm/.*.d \ > + lib/$(TEST_DIR)/iomaps.gen.c lib/$(TEST_DIR)/mach-virt.* > diff --git a/configure b/configure > index 6cfc64943f6e6..296c70182ea1d 100755 > --- a/configure > +++ b/configure > @@ -6,8 +6,7 @@ cc=gcc > ld=ld > objcopy=objcopy > ar=ar > -arch=`uname -m | sed -e s/i.86/i386/` > -processor="$arch" > +arch=`uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/'` > cross_prefix= > > usage() { > @@ -17,6 +16,7 @@ usage() { > Options include: > --test-dir=DIR the main directory for tests ($arch) > --arch=ARCH architecture to compile for ($arch) > + --processor=PROCESSOR processor to compile for ($arch) > --cross-prefix=PREFIX cross compiler prefix > --cc=CC c compiler to use ($cc) > --ld=LD ld linker to use ($ld) > @@ -66,6 +66,9 @@ while [[ "$1" = -* ]]; do > ;; > esac > done > +[ -z "$processor" ] && processor="$arch" > +qemu="${QEMU:-qemu-system-$arch}" > + > if [ -z "$testdir" -a \( "$arch" = "i386" -o "$arch" = "x86_64" \) ]; then > testdir=x86 > elif [ -z "$testdir" ]; then > @@ -80,6 +83,7 @@ if [ -f $testdir/run ]; then > fi > > # check for dependent 32 bit libraries > +if [ "$arch" = "i386" -o "$arch" = "x86_64" ]; then > cat << EOF > lib_test.c > #include <stdc++.h> > #include <boost_thread-mt.h> > @@ -94,6 +98,7 @@ if [ $exit -eq 0 ]; then > api=true > fi > rm -f lib_test.c > +fi > > cat <<EOF > config.mak > PREFIX=$prefix > @@ -106,4 +111,5 @@ OBJCOPY=$cross_prefix$objcopy > AR=$cross_prefix$ar > API=$api > TEST_DIR=$testdir > +QEMU_BIN=$qemu > EOF > diff --git a/lib/arm/eabi_compat.c b/lib/arm/eabi_compat.c > new file mode 100644 > index 0000000000000..76e04f5543ee1 > --- /dev/null > +++ b/lib/arm/eabi_compat.c > @@ -0,0 +1,20 @@ > +/* > + * Adapted from u-boot's arch/arm/lib/eabi_compat.c > + */ > +#include "libcflat.h" > + > +int raise(int signum __unused) > +{ > + printf("Divide by zero!\n"); > + exit(ERANGE); > + return 0; > +} > + > +/* Dummy functions to avoid linker complaints */ > +void __aeabi_unwind_cpp_pr0(void) > +{ > +} > + > +void __aeabi_unwind_cpp_pr1(void) > +{ > +} > diff --git a/lib/arm/io.c b/lib/arm/io.c > new file mode 100644 > index 0000000000000..32c896c29450a > --- /dev/null > +++ b/lib/arm/io.c > @@ -0,0 +1,31 @@ > +#include "libcflat.h" > +#include "libio.h" > +#include "iomaps.h" > +#include "virtio-testdev.h" > + > +static volatile u8 *uart0_base; > + > +void puts(const char *s) > +{ > + while (*s) > + *uart0_base = *s++; > +} > + > +void exit(int code) > +{ > + virtio_testdev_exit(code); > + halt(code); > +} > + > +void io_init_early(void) > +{ > + const struct iomap *m = iomaps_find_compatible("arm,pl011"); > + if (!m) > + halt(ENXIO); > + uart0_base = (u8 *)compat_ptr(m->addrs[0]); > +} is io_init_early going to do something else later on or is it just early_console_init? If the latter, then name the function as such. > + > +void io_init(void) > +{ > + virtio_testdev_init(); > +} > diff --git a/lib/arm/io.h b/lib/arm/io.h > new file mode 100644 > index 0000000000000..f058f7e54d4a7 > --- /dev/null > +++ b/lib/arm/io.h > @@ -0,0 +1,24 @@ > +#ifndef _ARM_IO_H_ > +#define _ARM_IO_H_ > + > +#define cpu_is_be cpu_is_be huh? > +extern bool cpu_is_be; > + > +#define __bswap16 bswap16 > +static inline u16 bswap16(u16 val) > +{ > + u16 ret; > + asm volatile("rev16 %0, %1" : "=r" (ret) : "r" (val)); > + return ret; > +} > + > +#define __bswap32 bswap32 > +static inline u32 bswap32(u32 val) > +{ > + u32 ret; > + asm volatile("rev %0, %1" : "=r" (ret) : "r" (val)); > + return ret; > +} > + > +#include "libio.h" > +#endif > diff --git a/lib/arm/setup.c b/lib/arm/setup.c > new file mode 100644 > index 0000000000000..32fa84bd0bb5b > --- /dev/null > +++ b/lib/arm/setup.c > @@ -0,0 +1,85 @@ > +#include "libcflat.h" > +#include "libio.h" > +#include "heap.h" > +#include "arm/sysinfo.h" > + > +#define FDT_SIG 0xd00dfeed > + > +#define KERNEL_OFFSET 0x00010000 > +#define ATAG_OFFSET 0x00000100 > + > +#define ATAG_CORE 0x54410001 > +#define ATAG_MEM 0x54410002 > +#define ATAG_CMDLINE 0x54410009 > + > +extern void start(void); > +extern unsigned long stacktop; > +extern char *__args; > + > +extern void io_init_early(void); > +extern void io_init(void); > +extern void __setup_args(void); > + > +u32 mach_type_id; > +struct tag_core core; > +struct tag_mem32 mem32; > + > +static void read_atags(u32 id, u32 *info) > +{ > + u32 *p = info; > + > + if (!p) { > + printf("Can't find bootinfo. mach-type = %x\n", id); > + exit(ENOEXEC); > + } > + > + /* > + * p[0] count of words for the tag > + * p[1] tag id > + * p[2..] tag data > + */ > + for (; p[0] != 0; p += p[0]) braces, please. > + switch (p[1]) { > + case ATAG_CORE: > + core.flags = p[2]; > + core.pagesize = p[3]; > + core.rootdev = p[4]; > + break; > + case ATAG_MEM: > + mem32.size = p[2]; > + mem32.start = p[3]; > + break; > + case ATAG_CMDLINE: > + __args = (char *)&p[2]; > + break; > + } > +} > + > +static void read_bootinfo(u32 id, u32 *info) > +{ > + u32 *atags = NULL; > + > + mach_type_id = id; > + > + if (info[0] == be32_to_cpu(FDT_SIG)) { > + /* > + * fdt reading is not [yet?] implemented. So calculate > + * the ATAGS addr to read that instead. > + */ > + atags = (u32 *)(start - KERNEL_OFFSET + ATAG_OFFSET); are the atags always supposed to be loaded even for device-tree booting? I could not find this in any ARM booting documents, and QEMU does seem to do this, but only for auto-generated device trees, which could just be a bug in do_cpu_reset? > + } else if (info[1] == ATAG_CORE) > + atags = info; > + > + read_atags(id, atags); > +} > + > +void setup(u32 arg __unused, u32 id, u32 *info) > +{ > + io_init_early(); > + read_bootinfo(id, info); > + __setup_args(); > + heap_init(&stacktop, > + mem32.size - (ptr_to_compat(&stacktop) - mem32.start), > + core.pagesize); > + io_init(); > +} > diff --git a/lib/arm/sysinfo.h b/lib/arm/sysinfo.h > new file mode 100644 > index 0000000000000..f3b076e1a34c4 > --- /dev/null > +++ b/lib/arm/sysinfo.h > @@ -0,0 +1,19 @@ > +#ifndef _ARM_SYSINFO_H_ > +#define _ARM_SYSINFO_H_ > +#include "libcflat.h" > + > +struct tag_core { > + u32 flags; /* bit 0 = read-only */ > + u32 pagesize; > + u32 rootdev; > +}; > + > +struct tag_mem32 { > + u32 size; > + u32 start; /* physical start address */ > +}; > + > +extern u32 mach_type_id; > +extern struct tag_core core; > +extern struct tag_mem32 mem32; > +#endif > diff --git a/lib/libcflat.h b/lib/libcflat.h > index 197b703e53b46..8c6cf1f0735ba 100644 > --- a/lib/libcflat.h > +++ b/lib/libcflat.h > @@ -45,6 +45,7 @@ extern char *strcat(char *dest, const char *src); > extern int strcmp(const char *a, const char *b); > > extern int printf(const char *fmt, ...); > +extern int snprintf(char *buf, int size, const char *fmt, ...); > extern int vsnprintf(char *buf, int size, const char *fmt, va_list va); > > extern void puts(const char *s); > @@ -60,6 +61,7 @@ extern long atol(const char *ptr); > const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > (type *)( (char *)__mptr - offsetof(type,member) );}) > > +#define __unused __attribute__((__unused__)) > #define NULL ((void *)0UL) > #include "errno.h" > #endif > diff --git a/lib/libio.h b/lib/libio.h > index 210b8d78af43f..5ac1056e7002f 100644 > --- a/lib/libio.h > +++ b/lib/libio.h > @@ -6,6 +6,10 @@ > */ > #include "libcflat.h" > > +#ifdef __arm__ > +#include "arm/io.h" > +#endif > + > typedef u32 compat_ptr_t; > > static inline void *compat_ptr(compat_ptr_t ptr) > diff --git a/lib/test_util.c b/lib/test_util.c > new file mode 100644 > index 0000000000000..3de1f74f83455 > --- /dev/null > +++ b/lib/test_util.c > @@ -0,0 +1,34 @@ > +#include "libcflat.h" > +#include "test_util.h" > + > +bool enough_args(int nargs, int needed) > +{ > + if (nargs >= needed) > + return true; > + > + fail("Not enough arguments.\n"); > + return false; > +} > + > +/* > + * Typically one would compare val == strtoul(expected, endp, base), > + * but we don't have, nor at this point really need, strtoul, so we > + * convert val to a string instead. base can only be 10 or 16. > + */ > +bool check_u32(u32 val, int base, char *expected) > +{ > + char *fmt = base == 10 ? "%d" : "%x"; > + char val_str[16]; > + > + snprintf(val_str, 16, fmt, val); > + > + if (base == 16) > + while (*expected == '0' || *expected == 'x') > + ++expected; > + > + if (strcmp(val_str, expected) == 0) > + return true; > + > + fail("expected %s, but have %s\n", expected, val_str); > + return false; > +} > diff --git a/lib/test_util.h b/lib/test_util.h > new file mode 100644 > index 0000000000000..0e3e6c4a80d51 > --- /dev/null > +++ b/lib/test_util.h > @@ -0,0 +1,13 @@ > +#ifndef _TEST_UTIL_H_ > +#define _TEST_UTIL_H_ > +#include "libcflat.h" > + > +#define PASS 0 > +#define FAIL 1 > + > +#define pass(fmt...) printf("PASS: " fmt) > +#define fail(fmt...) printf("FAIL: " fmt) > + > +bool enough_args(int nargs, int needed); > +bool check_u32(u32 val, int base, char *expected); > +#endif > -- > 1.8.1.4 > I was expecting to see a __raw_... IO accessor definitions for ARM here, specifically so we avoid the register-writeback versions that are not supported on ARM. See arch/arm/include/asm/io.h in the kernel. -- Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html