On Thu, Jan 02, 2014 at 06:24:10PM +0100, Andrew Jones wrote: > On Sat, Dec 28, 2013 at 10:31:58PM -0800, Christoffer Dall wrote: > > On Fri, Dec 13, 2013 at 11:58:05AM +0100, Andrew Jones wrote: > > > We're going to need PSR bit defines and pt_regs. We'll also need > > > pt_regs offsets in assembly code. This patch adapts the linux > > > kernel's ptrace.h and asm-offsets to this framework. Even though > > > lib/arm/asm-offsets.h is a generated file, we still commit it, > > > as it's unlikely to change. Also adapt cp15.h from the kernel, > > > since we'll need bit defines from there too. > > > > Why commit the autogenerated header? That will just create noise in the > > git log... It seems like it's auto-building it every time anyway, so it > > would only be to avoid that step... > > Hmm, it doesn't for me. For me you need to do 'make asm-offsets' to > regenerate it, which will likely only be necessary when we add more > structs. Possibly we'll be adding structs frequently at first, but > eventually we'll have what we need, and then the churn will be low. > Anyway, whatever, it just didn't seem necessary to me, and also nice > to be able to git-diff the asm-offsets.h file, rather than just the > generator. So, it actually doesn't work for me to do 'make asm-offsets' if I do 'rm lib/arm/asm-offsets.h'. It complains about the file missing from other tools that it tries to build first before producing the header file. If I do a 'touch lib/arm/asm-offsets.h', I get warnings about the missing defines from building cstart.o and also if I do a make clean the 'make asm-offsets' doesn't work for me anymore, because it depends on the iomaps being generated, which doesn't work on my machine, so I manually have to copy the iomaps.gen.c (which I generated by hand for now) first, but still I don't get anywhere. All this to say that with this appraoch, the 'make asm-offsets' relies on a lot of logic to be in place, and it doesn't seem particularly easy to make the compilation portable... > > > > diff --git a/config/config-arm.mak b/config/config-arm.mak > > > index d0814186b279c..173e606fbe64c 100644 > > > --- a/config/config-arm.mak > > > +++ b/config/config-arm.mak > > > @@ -1,3 +1,4 @@ > > > +PROCESSOR = cortex-a15 > > > mach = mach-virt > > > iodevs = pl011 virtio_mmio > > > phys_base = 0x40000000 > > > @@ -30,8 +31,7 @@ $(libcflat) $(libeabi): CFLAGS += -ffreestanding -I lib > > > > > > CFLAGS += -Wextra > > > CFLAGS += -marm > > > -#CFLAGS += -mcpu=$(PROCESSOR) > > > -CFLAGS += -mcpu=cortex-a15 > > > +CFLAGS += -mcpu=$(PROCESSOR) > > > CFLAGS += -O2 > > > > unrelated changes? > > yeah... > > > > > > > > > libgcc := $(shell $(CC) -m$(ARCH) --print-libgcc-file-name) > > > @@ -55,7 +55,7 @@ tests_and_config = $(TEST_DIR)/*.flat $(TEST_DIR)/unittests.cfg > > > > > > test_cases: $(tests-common) $(tests) > > > > > > -$(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib > > > +$(TEST_DIR)/%.o scripts/arm/%.o: CFLAGS += -std=gnu99 -ffreestanding -I lib > > > > > > $(TEST_DIR)/boot.elf: $(cstart.o) $(TEST_DIR)/boot.o > > > > > > @@ -67,7 +67,16 @@ lib/$(TEST_DIR)/mach-virt.dts: > > > $(QEMU_BIN) -kernel /dev/null -M virt -machine dumpdtb=$(dtb) > > > fdtdump $(dtb) > $@ > > > > > > +.PHONY: asm-offsets > > > + > > > +asm-offsets: scripts/arm/asm-offsets.flat > > > + $(QEMU_BIN) -device virtio-testdev -display none -serial stdio \ > > > + -M virt -cpu $(PROCESSOR) \ > > > + -kernel $^ > lib/arm/asm-offsets.h || true > > > > this is a shame, you're depending on a full working setup with a correct > > QEMU to just build this thing. Did you consider replicating the > > kernel's Kbuild approach? > > Actually, I'm not, because we commit the asm-offsets.h file :-) So unless > you're changing it, you don't need the above target. I'm not sure what > part of Kbuild you're referring to. I certainly don't want to over-engineer > the build process of these unit tests though. > I don't think constructing a mechanism (which you can copy from the kernel) that makes it possible to compile this tool without relying on a another set of tools (that needs to be of a quite recent revision) is over-engineering a build-process. If you are going this approach, you really need to check for the available tools as part of configure and give nice user messages saying "you need a version of qemu newer than 1.7 compiled for your host architecture with target-arm support on your build system) or something like that. But again, since we can, I think this tool should really just build without relying on that. > > > > In fact, when I started playing around with this, it was quite the > > hassle, because I run with a cross-compiled QEMU for my ARM devices > > which doesn't compile on my build-machine, so now I need a different > > x86-compiled QEMU with mach-virt support compiled somewhere, and I can > > only build unit-tests when I remember to set the QEMU variable in my > > shell. So if we really stick with this approach, why not make it part > > of the ./configure options? That being said, I really want the build > > here only to depend on having an ARM compiler :(( > > Ahh, maybe we should commit the dts that mach-virt generates too then. > I would advise very strongly against that. We want this tool to be something that is broadly applied and used by KVM developers, and since we really don't want another place where the whole keep-the-kernel-and-device-tree-in-sync problem can be an issue. Ideally QEMU just gives you a DTB and the test tool consumes this. -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