Re: [PATCH 2/3] arm: add useful headers from the linux kernel

[Andrew Jones <drjones@redhat.com>]

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.

> > 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.

> 
> 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.

drew