| Message ID | 1456506526-10803-3-git-send-email-lvivier@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 26, 2016 at 06:08:46PM +0100, Laurent Vivier wrote: > This patch allows to build tests for ppc64 little endian target > (ppc64le) on big and little endian hosts. > > We add a new parameter to configure to select the endianness of the > tests (--endian=little or --endian=big). > > I have built and tested big and little endian tests on a little > endian host, a big endian host, with kvm_hv and kvm_pr, and on > x86_64 with ppc64 as a TCG target. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > v2: replace FIXUP_ENDIAN from linux by a home made version > (B_BE and RETURN_FROM_BE) > > Makefile | 11 ++++++----- > configure | 6 ++++++ > lib/powerpc/asm/ppc_asm.h | 36 ++++++++++++++++++++++++++++++++++++ > lib/ppc64/asm/io.h | 8 ++++++++ > lib/ppc64/asm/ppc_asm.h | 1 + > powerpc/Makefile | 2 +- > powerpc/Makefile.big | 21 +++++++++++++++++++++ > powerpc/Makefile.common | 18 ++++++++++-------- > powerpc/Makefile.little | 21 +++++++++++++++++++++ > powerpc/Makefile.ppc64 | 22 ---------------------- > powerpc/cstart64.S | 14 ++++++++++---- By renaming Makefile.ppc64 to Makefile.{little,big} we lose the 'ppc64' information. Since you've defined the ENDIAN config variable, then how about just doing ifeq ($(ENDIAN),little) CFLAGS = -mlittle-endian LDFLAGS = -EL else CFLAGS = -mbig-endian LDFLAGS = -EB endif Or even just substitute $(ENDIAN) when you want "big"/"little", such as in the CFLAGS. > 11 files changed, 120 insertions(+), 40 deletions(-) > create mode 100644 lib/powerpc/asm/ppc_asm.h > create mode 100644 lib/ppc64/asm/ppc_asm.h > create mode 100644 powerpc/Makefile.big > create mode 100644 powerpc/Makefile.little > delete mode 100644 powerpc/Makefile.ppc64 > > diff --git a/Makefile b/Makefile > index ddba941..8ed244a 100644 > --- a/Makefile > +++ b/Makefile > @@ -40,12 +40,13 @@ include $(TEST_DIR)/Makefile > cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) > > -CFLAGS += -g > -CFLAGS += $(autodepend-flags) -Wall > -CFLAGS += $(call cc-option, -fomit-frame-pointer, "") > -CFLAGS += $(call cc-option, -fno-stack-protector, "") > -CFLAGS += $(call cc-option, -fno-stack-protector-all, "") > +main_CFLAGS = -g > +main_CFLAGS += $(autodepend-flags) -Wall > +main_CFLAGS += $(call cc-option, -fomit-frame-pointer, "") > +main_CFLAGS += $(call cc-option, -fno-stack-protector, "") > +main_CFLAGS += $(call cc-option, -fno-stack-protector-all, "") I guess this is because cc-option doesn't work well with cross-endian compiling? Can we fix the function instead? In any case, do we have drop '-g -Wall and -MMD -MF $(dir $*).$(notdir $*).d' too? > > +CFLAGS += $(main_CFLAGS) > CXXFLAGS += $(CFLAGS) > > autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d > diff --git a/configure b/configure > index a685cca..0043ee9 100755 > --- a/configure > +++ b/configure > @@ -10,6 +10,7 @@ ar=ar > arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'` > host=$arch > cross_prefix= > +endian=big # default for ppc64, the only user If we default to 'little', then we don't have to worry about the current architectures using it. Is big the better default for some reason? Also, as both big and little work on both big and little, then maybe we should require it to be explicitly added (to make sure the user knows which they're using). > > usage() { > cat <<-EOF > @@ -23,6 +24,7 @@ usage() { > --ld=LD ld linker to use ($ld) > --prefix=PREFIX where to install things ($prefix) > --kerneldir=DIR kernel build directory for kvm.h ($kerneldir) > + --endian=ENDIAN endianness to compile for (little or big) > EOF > exit 1 > } > @@ -50,6 +52,9 @@ while [[ "$1" = -* ]]; do > --cross-prefix) > cross_prefix="$arg" > ;; > + --endian) > + endian="$arg" > + ;; Looks like a inconsistent whitespace here (should use tabs). > --cc) > cc="$arg" > ;; > @@ -139,4 +144,5 @@ AR=$cross_prefix$ar > API=$api > TEST_DIR=$testdir > FIRMWARE=$firmware > +ENDIAN=$endian > EOF > diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h > new file mode 100644 > index 0000000..f0ab241 > --- /dev/null > +++ b/lib/powerpc/asm/ppc_asm.h > @@ -0,0 +1,36 @@ > +#ifndef _ASMPOWERPC_PPC_ASM_H > +#define _ASMPOWERPC_PPC_ASM_H Adding this file is a good idea, we should probably move LOAD_REG_IMMEDIATE and LOAD_REG_ADDR here too. > + > +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ > + > +#define B_BE(addr) \ > + mtctr addr; \ > + nop; \ > + bctr > + > +#define RETURN_FROM_BE > + > +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > + > +#define B_BE(addr) \ > + mfmsr r11; \ > + xori r11,r11,1; \ > + mtsrr0 addr; \ > + mtsrr1 r11; \ > + rfid; \ > + b . > + > +#define RETURN_FROM_BE \ > + .long 0x05000048; /* bl . + 4 */ \ > + .long 0xa602487d; /* mflr r10 */ \ > + .long 0x20004a39; /* addi r10,r10,32 */ \ > + .long 0xa600607d; /* mfmsr r11 */ \ > + .long 0x01006b69; /* xori r11,r11,1 */ \ > + .long 0xa6035a7d; /* mtsrr0 r10 */ \ > + .long 0xa6037b7d; /* mtsrr1 r11 */ \ > + .long 0x2400004c; /* rfid */ \ > + .long 0x00000048; /* b . */ \ I think we only need RETURN_FROM_BE. B_BE is currently only used in enter_rtas, where it's fine now, but based on some of David's comments, I think we may eventually want to change even more state before the rtas call, which means the BE version will need the rfid anyway. Thus we can just drop the LE version straight into enter_rtas, replacing the xor with explicitly setting BE mode. > + > +#endif /* __BYTE_ORDER__ */ > + > +#endif /* _ASMPOWERPC_PPC_ASM_H */ > diff --git a/lib/ppc64/asm/io.h b/lib/ppc64/asm/io.h > index c0801d4..4f2c31b 100644 > --- a/lib/ppc64/asm/io.h > +++ b/lib/ppc64/asm/io.h > @@ -1,5 +1,13 @@ > #ifndef _ASMPPC64_IO_H_ > #define _ASMPPC64_IO_H_ > + > +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > +#define __cpu_is_be() (0) > +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ > #define __cpu_is_be() (1) > +#else > +#error Undefined byte order > +#endif > + > #include <asm-generic/io.h> > #endif > diff --git a/lib/ppc64/asm/ppc_asm.h b/lib/ppc64/asm/ppc_asm.h > new file mode 100644 > index 0000000..e3929ee > --- /dev/null > +++ b/lib/ppc64/asm/ppc_asm.h > @@ -0,0 +1 @@ > +#include "../../powerpc/asm/ppc_asm.h" > diff --git a/powerpc/Makefile b/powerpc/Makefile > index 369a38b..cc39a09 100644 > --- a/powerpc/Makefile > +++ b/powerpc/Makefile > @@ -1 +1 @@ > -include $(TEST_DIR)/Makefile.$(ARCH) > +include $(TEST_DIR)/Makefile.$(ENDIAN) > diff --git a/powerpc/Makefile.big b/powerpc/Makefile.big > new file mode 100644 > index 0000000..d81c52d > --- /dev/null > +++ b/powerpc/Makefile.big > @@ -0,0 +1,21 @@ > +# > +# ppc64 big-endian makefile > +# > +# Authors: Andrew Jones <drjones@redhat.com> > +# > +bits = 64 > + > +arch_CFLAGS = -mbig-endian > +arch_LDFLAGS = -EB > + > +cstart.o = $(TEST_DIR)/cstart64.o > +reloc.o = $(TEST_DIR)/reloc64.o > +cflatobjs += lib/ppc64/spinlock.o > + > +# ppc64 specific tests > +tests = > + > +include $(TEST_DIR)/Makefile.common > + > +arch_clean: powerpc_clean > + $(RM) lib/ppc64/.*.d > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common > index cc27ac8..b088af6 100644 > --- a/powerpc/Makefile.common > +++ b/powerpc/Makefile.common > @@ -11,14 +11,15 @@ all: $(TEST_DIR)/boot_rom.bin test_cases > > ################################################################## > > -CFLAGS += $(arch_CFLAGS) > -CFLAGS += -std=gnu99 > -CFLAGS += -ffreestanding > -CFLAGS += -Wextra > -CFLAGS += -O2 > -CFLAGS += -I lib -I lib/libfdt > -CFLAGS += -Wa,-mregnames > -CFLAGS += -fpie > +common_CFLAGS = -std=gnu99 > +common_CFLAGS += -ffreestanding > +common_CFLAGS += -Wextra > +common_CFLAGS += -O2 > +common_CFLAGS += -I lib -I lib/libfdt > +common_CFLAGS += -Wa,-mregnames > +common_CFLAGS += -fpie > + > +CFLAGS += $(arch_CFLAGS) $(common_CFLAGS) I'm not sure what we gain by renaming to common_CFLAGS. Doesn't just %.elf: CFLAGS += $(arch_CFLAGS) work? > > asm-offsets = lib/$(ARCH)/asm-offsets.h > include scripts/asm-offsets.mak > @@ -48,6 +49,7 @@ $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf > dd if=/dev/zero of=$@ bs=256 count=1 > $(OBJCOPY) -O binary $^ >(cat - >>$@) > > +$(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian $(common_CFLAGS) $(main_CFLAGS) And just $(TEST_DIR)/boot_rom.elf: CFLAGS += -mbig-endian > $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o > $(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $< > > diff --git a/powerpc/Makefile.little b/powerpc/Makefile.little > new file mode 100644 > index 0000000..c37d2fc > --- /dev/null > +++ b/powerpc/Makefile.little > @@ -0,0 +1,21 @@ > +# > +# ppc64 little-endian makefile > +# > +# Authors: Andrew Jones <drjones@redhat.com> > +# > +bits = 64 > + > +arch_CFLAGS = -mlittle-endian > +arch_LDFLAGS = -EL > + > +cstart.o = $(TEST_DIR)/cstart64.o > +reloc.o = $(TEST_DIR)/reloc64.o > +cflatobjs += lib/ppc64/spinlock.o > + > +# ppc64 specific tests > +tests = > + > +include $(TEST_DIR)/Makefile.common > + > +arch_clean: powerpc_clean > + $(RM) lib/ppc64/.*.d > diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64 > deleted file mode 100644 > index 1cf277e..0000000 > --- a/powerpc/Makefile.ppc64 > +++ /dev/null > @@ -1,22 +0,0 @@ > -# > -# ppc64 makefile > -# > -# Authors: Andrew Jones <drjones@redhat.com> > -# > -bits = 64 > -ldarch = elf64-powerpc > - > -arch_CFLAGS = -mbig-endian > -arch_LDFLAGS = -EB > - > -cstart.o = $(TEST_DIR)/cstart64.o > -reloc.o = $(TEST_DIR)/reloc64.o > -cflatobjs += lib/ppc64/spinlock.o > - > -# ppc64 specific tests > -tests = > - > -include $(TEST_DIR)/Makefile.common > - > -arch_clean: powerpc_clean > - $(RM) lib/ppc64/.*.d > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > index f5530fb..60557a9 100644 > --- a/powerpc/cstart64.S > +++ b/powerpc/cstart64.S > @@ -7,6 +7,7 @@ > */ > #define __ASSEMBLY__ > #include <asm/hcall.h> > +#include <asm/ppc_asm.h> > > #define LOAD_REG_IMMEDIATE(reg,expr) \ > lis reg,(expr)@highest; \ > @@ -25,6 +26,7 @@ > */ > .globl start > start: > + RETURN_FROM_BE > /* > * We were loaded at QEMU's kernel load address, but we're not > * allowed to link there due to how QEMU deals with linker VMAs, > @@ -93,11 +95,15 @@ halt: > enter_rtas: > mflr r0 > std r0, 16(r1) > + > + LOAD_REG_ADDR(r10,rtas_return_loc) > + mtlr r10 > + > LOAD_REG_ADDR(r10, rtas_blob) > -//FIXME: change this bctrl to an rtas-prep, rfid, rtas-return sequence > - mtctr r10 > - nop > - bctrl > + B_BE(r10) > + > +rtas_return_loc: > + RETURN_FROM_BE > ld r0, 16(r1) > mtlr r0 > blr Besides my earlier comment about always needing rfid to prep for the rtas call (which my FIXUP comment here poorly indicated), then this looks good to me. Although I'm not sure I like the name RETURN_FROM_BE, as sometimes we're returning to LE, and sometimes we're not. In the not case, it's a bit confusing. Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/2016 19:45, Andrew Jones wrote: > On Fri, Feb 26, 2016 at 06:08:46PM +0100, Laurent Vivier wrote: >> This patch allows to build tests for ppc64 little endian target >> (ppc64le) on big and little endian hosts. >> >> We add a new parameter to configure to select the endianness of the >> tests (--endian=little or --endian=big). >> >> I have built and tested big and little endian tests on a little >> endian host, a big endian host, with kvm_hv and kvm_pr, and on >> x86_64 with ppc64 as a TCG target. >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> v2: replace FIXUP_ENDIAN from linux by a home made version >> (B_BE and RETURN_FROM_BE) >> >> Makefile | 11 ++++++----- >> configure | 6 ++++++ >> lib/powerpc/asm/ppc_asm.h | 36 ++++++++++++++++++++++++++++++++++++ >> lib/ppc64/asm/io.h | 8 ++++++++ >> lib/ppc64/asm/ppc_asm.h | 1 + >> powerpc/Makefile | 2 +- >> powerpc/Makefile.big | 21 +++++++++++++++++++++ >> powerpc/Makefile.common | 18 ++++++++++-------- >> powerpc/Makefile.little | 21 +++++++++++++++++++++ >> powerpc/Makefile.ppc64 | 22 ---------------------- >> powerpc/cstart64.S | 14 ++++++++++---- > > By renaming Makefile.ppc64 to Makefile.{little,big} we lose the > 'ppc64' information. Since you've defined the ENDIAN config > variable, then how about just doing > > ifeq ($(ENDIAN),little) > CFLAGS = -mlittle-endian > LDFLAGS = -EL > else > CFLAGS = -mbig-endian > LDFLAGS = -EB > endif > > Or even just substitute $(ENDIAN) when you want "big"/"little", > such as in the CFLAGS. I need the LDFLAGS too, so I will rename the file to Makefile.ppc64 and use the "ifeq". > >> 11 files changed, 120 insertions(+), 40 deletions(-) >> create mode 100644 lib/powerpc/asm/ppc_asm.h >> create mode 100644 lib/ppc64/asm/ppc_asm.h >> create mode 100644 powerpc/Makefile.big >> create mode 100644 powerpc/Makefile.little >> delete mode 100644 powerpc/Makefile.ppc64 >> >> diff --git a/Makefile b/Makefile >> index ddba941..8ed244a 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -40,12 +40,13 @@ include $(TEST_DIR)/Makefile >> cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ >> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) >> >> -CFLAGS += -g >> -CFLAGS += $(autodepend-flags) -Wall >> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "") >> -CFLAGS += $(call cc-option, -fno-stack-protector, "") >> -CFLAGS += $(call cc-option, -fno-stack-protector-all, "") >> +main_CFLAGS = -g >> +main_CFLAGS += $(autodepend-flags) -Wall >> +main_CFLAGS += $(call cc-option, -fomit-frame-pointer, "") >> +main_CFLAGS += $(call cc-option, -fno-stack-protector, "") >> +main_CFLAGS += $(call cc-option, -fno-stack-protector-all, "") > > I guess this is because cc-option doesn't work well with cross-endian > compiling? Can we fix the function instead? In any case, do we have > drop '-g -Wall and -MMD -MF $(dir $*).$(notdir $*).d' too? In fact, my problem here is to always compile the boot_rom in gin endian mode and the other files using the provided endianness. So I have to extract all the common flags and add the good endianness according the file to build. But I agree, I did a minimalist change (renaming): I can try to extract flags needed to build boot_rom to not change the common CFLAGS. >> >> +CFLAGS += $(main_CFLAGS) >> CXXFLAGS += $(CFLAGS) >> >> autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d >> diff --git a/configure b/configure >> index a685cca..0043ee9 100755 >> --- a/configure >> +++ b/configure >> @@ -10,6 +10,7 @@ ar=ar >> arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'` >> host=$arch >> cross_prefix= >> +endian=big # default for ppc64, the only user > > If we default to 'little', then we don't have to worry about the current > architectures using it. Is big the better default for some reason? Also, > as both big and little work on both big and little, then maybe we should > require it to be explicitly added (to make sure the user knows which > they're using). This is not true: on old PowerPC machines, using ppc970 (like my PowerMac G5), the little endian mode doesn't work. It's why I set the default to big endian. But I agree: it's a good idea to have it added explicitly to know what we are using. >> >> usage() { >> cat <<-EOF >> @@ -23,6 +24,7 @@ usage() { >> --ld=LD ld linker to use ($ld) >> --prefix=PREFIX where to install things ($prefix) >> --kerneldir=DIR kernel build directory for kvm.h ($kerneldir) >> + --endian=ENDIAN endianness to compile for (little or big) >> EOF >> exit 1 >> } >> @@ -50,6 +52,9 @@ while [[ "$1" = -* ]]; do >> --cross-prefix) >> cross_prefix="$arg" >> ;; >> + --endian) >> + endian="$arg" >> + ;; > > Looks like a inconsistent whitespace here (should use tabs). > >> --cc) >> cc="$arg" >> ;; >> @@ -139,4 +144,5 @@ AR=$cross_prefix$ar >> API=$api >> TEST_DIR=$testdir >> FIRMWARE=$firmware >> +ENDIAN=$endian >> EOF >> diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h >> new file mode 100644 >> index 0000000..f0ab241 >> --- /dev/null >> +++ b/lib/powerpc/asm/ppc_asm.h >> @@ -0,0 +1,36 @@ >> +#ifndef _ASMPOWERPC_PPC_ASM_H >> +#define _ASMPOWERPC_PPC_ASM_H > > Adding this file is a good idea, we should probably move > LOAD_REG_IMMEDIATE and LOAD_REG_ADDR here too. I can add them in the series. >> + >> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ >> + >> +#define B_BE(addr) \ >> + mtctr addr; \ >> + nop; \ >> + bctr >> + >> +#define RETURN_FROM_BE >> + >> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ >> + >> +#define B_BE(addr) \ >> + mfmsr r11; \ >> + xori r11,r11,1; \ >> + mtsrr0 addr; \ >> + mtsrr1 r11; \ >> + rfid; \ >> + b . >> + >> +#define RETURN_FROM_BE \ >> + .long 0x05000048; /* bl . + 4 */ \ >> + .long 0xa602487d; /* mflr r10 */ \ >> + .long 0x20004a39; /* addi r10,r10,32 */ \ >> + .long 0xa600607d; /* mfmsr r11 */ \ >> + .long 0x01006b69; /* xori r11,r11,1 */ \ >> + .long 0xa6035a7d; /* mtsrr0 r10 */ \ >> + .long 0xa6037b7d; /* mtsrr1 r11 */ \ >> + .long 0x2400004c; /* rfid */ \ >> + .long 0x00000048; /* b . */ \ > > I think we only need RETURN_FROM_BE. B_BE is currently only used > in enter_rtas, where it's fine now, but based on some of David's > comments, I think we may eventually want to change even more > state before the rtas call, which means the BE version will need > the rfid anyway. Thus we can just drop the LE version straight into > enter_rtas, replacing the xor with explicitly setting BE mode. OK >> + >> +#endif /* __BYTE_ORDER__ */ >> + >> +#endif /* _ASMPOWERPC_PPC_ASM_H */ >> diff --git a/lib/ppc64/asm/io.h b/lib/ppc64/asm/io.h >> index c0801d4..4f2c31b 100644 >> --- a/lib/ppc64/asm/io.h >> +++ b/lib/ppc64/asm/io.h >> @@ -1,5 +1,13 @@ >> #ifndef _ASMPPC64_IO_H_ >> #define _ASMPPC64_IO_H_ >> + >> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ >> +#define __cpu_is_be() (0) >> +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ >> #define __cpu_is_be() (1) >> +#else >> +#error Undefined byte order >> +#endif >> + >> #include <asm-generic/io.h> >> #endif >> diff --git a/lib/ppc64/asm/ppc_asm.h b/lib/ppc64/asm/ppc_asm.h >> new file mode 100644 >> index 0000000..e3929ee >> --- /dev/null >> +++ b/lib/ppc64/asm/ppc_asm.h >> @@ -0,0 +1 @@ >> +#include "../../powerpc/asm/ppc_asm.h" >> diff --git a/powerpc/Makefile b/powerpc/Makefile >> index 369a38b..cc39a09 100644 >> --- a/powerpc/Makefile >> +++ b/powerpc/Makefile >> @@ -1 +1 @@ >> -include $(TEST_DIR)/Makefile.$(ARCH) >> +include $(TEST_DIR)/Makefile.$(ENDIAN) >> diff --git a/powerpc/Makefile.big b/powerpc/Makefile.big >> new file mode 100644 >> index 0000000..d81c52d >> --- /dev/null >> +++ b/powerpc/Makefile.big >> @@ -0,0 +1,21 @@ >> +# >> +# ppc64 big-endian makefile >> +# >> +# Authors: Andrew Jones <drjones@redhat.com> >> +# >> +bits = 64 >> + >> +arch_CFLAGS = -mbig-endian >> +arch_LDFLAGS = -EB >> + >> +cstart.o = $(TEST_DIR)/cstart64.o >> +reloc.o = $(TEST_DIR)/reloc64.o >> +cflatobjs += lib/ppc64/spinlock.o >> + >> +# ppc64 specific tests >> +tests = >> + >> +include $(TEST_DIR)/Makefile.common >> + >> +arch_clean: powerpc_clean >> + $(RM) lib/ppc64/.*.d >> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common >> index cc27ac8..b088af6 100644 >> --- a/powerpc/Makefile.common >> +++ b/powerpc/Makefile.common >> @@ -11,14 +11,15 @@ all: $(TEST_DIR)/boot_rom.bin test_cases >> >> ################################################################## >> >> -CFLAGS += $(arch_CFLAGS) >> -CFLAGS += -std=gnu99 >> -CFLAGS += -ffreestanding >> -CFLAGS += -Wextra >> -CFLAGS += -O2 >> -CFLAGS += -I lib -I lib/libfdt >> -CFLAGS += -Wa,-mregnames >> -CFLAGS += -fpie >> +common_CFLAGS = -std=gnu99 >> +common_CFLAGS += -ffreestanding >> +common_CFLAGS += -Wextra >> +common_CFLAGS += -O2 >> +common_CFLAGS += -I lib -I lib/libfdt >> +common_CFLAGS += -Wa,-mregnames >> +common_CFLAGS += -fpie >> + >> +CFLAGS += $(arch_CFLAGS) $(common_CFLAGS) > > I'm not sure what we gain by renaming to common_CFLAGS. Doesn't just > > %.elf: CFLAGS += $(arch_CFLAGS) > > work? > >> >> asm-offsets = lib/$(ARCH)/asm-offsets.h >> include scripts/asm-offsets.mak >> @@ -48,6 +49,7 @@ $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf >> dd if=/dev/zero of=$@ bs=256 count=1 >> $(OBJCOPY) -O binary $^ >(cat - >>$@) >> >> +$(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian $(common_CFLAGS) $(main_CFLAGS) > > And just > $(TEST_DIR)/boot_rom.elf: CFLAGS += -mbig-endian I can try. > >> $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o >> $(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $< >> >> diff --git a/powerpc/Makefile.little b/powerpc/Makefile.little >> new file mode 100644 >> index 0000000..c37d2fc >> --- /dev/null >> +++ b/powerpc/Makefile.little >> @@ -0,0 +1,21 @@ >> +# >> +# ppc64 little-endian makefile >> +# >> +# Authors: Andrew Jones <drjones@redhat.com> >> +# >> +bits = 64 >> + >> +arch_CFLAGS = -mlittle-endian >> +arch_LDFLAGS = -EL >> + >> +cstart.o = $(TEST_DIR)/cstart64.o >> +reloc.o = $(TEST_DIR)/reloc64.o >> +cflatobjs += lib/ppc64/spinlock.o >> + >> +# ppc64 specific tests >> +tests = >> + >> +include $(TEST_DIR)/Makefile.common >> + >> +arch_clean: powerpc_clean >> + $(RM) lib/ppc64/.*.d >> diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64 >> deleted file mode 100644 >> index 1cf277e..0000000 >> --- a/powerpc/Makefile.ppc64 >> +++ /dev/null >> @@ -1,22 +0,0 @@ >> -# >> -# ppc64 makefile >> -# >> -# Authors: Andrew Jones <drjones@redhat.com> >> -# >> -bits = 64 >> -ldarch = elf64-powerpc >> - >> -arch_CFLAGS = -mbig-endian >> -arch_LDFLAGS = -EB >> - >> -cstart.o = $(TEST_DIR)/cstart64.o >> -reloc.o = $(TEST_DIR)/reloc64.o >> -cflatobjs += lib/ppc64/spinlock.o >> - >> -# ppc64 specific tests >> -tests = >> - >> -include $(TEST_DIR)/Makefile.common >> - >> -arch_clean: powerpc_clean >> - $(RM) lib/ppc64/.*.d >> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S >> index f5530fb..60557a9 100644 >> --- a/powerpc/cstart64.S >> +++ b/powerpc/cstart64.S >> @@ -7,6 +7,7 @@ >> */ >> #define __ASSEMBLY__ >> #include <asm/hcall.h> >> +#include <asm/ppc_asm.h> >> >> #define LOAD_REG_IMMEDIATE(reg,expr) \ >> lis reg,(expr)@highest; \ >> @@ -25,6 +26,7 @@ >> */ >> .globl start >> start: >> + RETURN_FROM_BE >> /* >> * We were loaded at QEMU's kernel load address, but we're not >> * allowed to link there due to how QEMU deals with linker VMAs, >> @@ -93,11 +95,15 @@ halt: >> enter_rtas: >> mflr r0 >> std r0, 16(r1) >> + >> + LOAD_REG_ADDR(r10,rtas_return_loc) >> + mtlr r10 >> + >> LOAD_REG_ADDR(r10, rtas_blob) >> -//FIXME: change this bctrl to an rtas-prep, rfid, rtas-return sequence >> - mtctr r10 >> - nop >> - bctrl >> + B_BE(r10) >> + >> +rtas_return_loc: >> + RETURN_FROM_BE >> ld r0, 16(r1) >> mtlr r0 >> blr > > Besides my earlier comment about always needing rfid to prep for the > rtas call (which my FIXUP comment here poorly indicated), then this > looks good to me. Although I'm not sure I like the name RETURN_FROM_BE, > as sometimes we're returning to LE, and sometimes we're not. In the > not case, it's a bit confusing. > > Thanks, > drew > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/2016 19:45, Andrew Jones wrote: >> > + LOAD_REG_ADDR(r10,rtas_return_loc) >> > + mtlr r10 >> > + >> > LOAD_REG_ADDR(r10, rtas_blob) >> > -//FIXME: change this bctrl to an rtas-prep, rfid, rtas-return sequence >> > - mtctr r10 >> > - nop >> > - bctrl >> > + B_BE(r10) Note that this doesn't apply. I've applied patch 1 though. Paolo >> > + >> > +rtas_return_loc: >> > + RETURN_FROM_BE >> > ld r0, 16(r1) >> > mtlr r0 >> > blr > Besides my earlier comment about always needing rfid to prep for the > rtas call (which my FIXUP comment here poorly indicated), then this > looks good to me. Although I'm not sure I like the name RETURN_FROM_BE, > as sometimes we're returning to LE, and sometimes we're not. In the > not case, it's a bit confusing. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29/02/2016 17:06, Paolo Bonzini wrote: > > > On 26/02/2016 19:45, Andrew Jones wrote: >>>> + LOAD_REG_ADDR(r10,rtas_return_loc) >>>> + mtlr r10 >>>> + >>>> LOAD_REG_ADDR(r10, rtas_blob) >>>> -//FIXME: change this bctrl to an rtas-prep, rfid, rtas-return sequence >>>> - mtctr r10 >>>> - nop >>>> - bctrl >>>> + B_BE(r10) > > Note that this doesn't apply. I've applied patch 1 though. Yes, Drew has changed this part, and anyway I have to rework this. Thanks, Laurent > Paolo > >>>> + >>>> +rtas_return_loc: >>>> + RETURN_FROM_BE >>>> ld r0, 16(r1) >>>> mtlr r0 >>>> blr >> Besides my earlier comment about always needing rfid to prep for the >> rtas call (which my FIXUP comment here poorly indicated), then this >> looks good to me. Although I'm not sure I like the name RETURN_FROM_BE, >> as sometimes we're returning to LE, and sometimes we're not. In the >> not case, it's a bit confusing. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/2016 19:45, Andrew Jones wrote: ... >> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common >> > index cc27ac8..b088af6 100644 >> > --- a/powerpc/Makefile.common >> > +++ b/powerpc/Makefile.common >> > @@ -11,14 +11,15 @@ all: $(TEST_DIR)/boot_rom.bin test_cases >> > >> > ################################################################## >> > >> > -CFLAGS += $(arch_CFLAGS) >> > -CFLAGS += -std=gnu99 >> > -CFLAGS += -ffreestanding >> > -CFLAGS += -Wextra >> > -CFLAGS += -O2 >> > -CFLAGS += -I lib -I lib/libfdt >> > -CFLAGS += -Wa,-mregnames >> > -CFLAGS += -fpie >> > +common_CFLAGS = -std=gnu99 >> > +common_CFLAGS += -ffreestanding >> > +common_CFLAGS += -Wextra >> > +common_CFLAGS += -O2 >> > +common_CFLAGS += -I lib -I lib/libfdt >> > +common_CFLAGS += -Wa,-mregnames >> > +common_CFLAGS += -fpie >> > + >> > +CFLAGS += $(arch_CFLAGS) $(common_CFLAGS) > I'm not sure what we gain by renaming to common_CFLAGS. Doesn't just > > %.elf: CFLAGS += $(arch_CFLAGS) > > work? > >> > >> > asm-offsets = lib/$(ARCH)/asm-offsets.h >> > include scripts/asm-offsets.mak >> > @@ -48,6 +49,7 @@ $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf >> > dd if=/dev/zero of=$@ bs=256 count=1 >> > $(OBJCOPY) -O binary $^ >(cat - >>$@) >> > >> > +$(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian $(common_CFLAGS) $(main_CFLAGS) > And just > $(TEST_DIR)/boot_rom.elf: CFLAGS += -mbig-endian > We can't do that because boot_rom.elf is also a .elf, so the both rules apply and we end up with in the case of boot_rom.elf (and --endian=little): CFLAGS += -mlittle-endian -mbig-endian and it doesn't work. We must overwrite CFLAGS in the case of boot_rom.elf. But boot_rom.S is a one line of assembly language, we don't need the other flags, in fact... Laurent -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Makefile b/Makefile index ddba941..8ed244a 100644 --- a/Makefile +++ b/Makefile @@ -40,12 +40,13 @@ include $(TEST_DIR)/Makefile cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) -CFLAGS += -g -CFLAGS += $(autodepend-flags) -Wall -CFLAGS += $(call cc-option, -fomit-frame-pointer, "") -CFLAGS += $(call cc-option, -fno-stack-protector, "") -CFLAGS += $(call cc-option, -fno-stack-protector-all, "") +main_CFLAGS = -g +main_CFLAGS += $(autodepend-flags) -Wall +main_CFLAGS += $(call cc-option, -fomit-frame-pointer, "") +main_CFLAGS += $(call cc-option, -fno-stack-protector, "") +main_CFLAGS += $(call cc-option, -fno-stack-protector-all, "") +CFLAGS += $(main_CFLAGS) CXXFLAGS += $(CFLAGS) autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d diff --git a/configure b/configure index a685cca..0043ee9 100755 --- a/configure +++ b/configure @@ -10,6 +10,7 @@ ar=ar arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'` host=$arch cross_prefix= +endian=big # default for ppc64, the only user usage() { cat <<-EOF @@ -23,6 +24,7 @@ usage() { --ld=LD ld linker to use ($ld) --prefix=PREFIX where to install things ($prefix) --kerneldir=DIR kernel build directory for kvm.h ($kerneldir) + --endian=ENDIAN endianness to compile for (little or big) EOF exit 1 } @@ -50,6 +52,9 @@ while [[ "$1" = -* ]]; do --cross-prefix) cross_prefix="$arg" ;; + --endian) + endian="$arg" + ;; --cc) cc="$arg" ;; @@ -139,4 +144,5 @@ AR=$cross_prefix$ar API=$api TEST_DIR=$testdir FIRMWARE=$firmware +ENDIAN=$endian EOF diff --git a/lib/powerpc/asm/ppc_asm.h b/lib/powerpc/asm/ppc_asm.h new file mode 100644 index 0000000..f0ab241 --- /dev/null +++ b/lib/powerpc/asm/ppc_asm.h @@ -0,0 +1,36 @@ +#ifndef _ASMPOWERPC_PPC_ASM_H +#define _ASMPOWERPC_PPC_ASM_H + +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ + +#define B_BE(addr) \ + mtctr addr; \ + nop; \ + bctr + +#define RETURN_FROM_BE + +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + +#define B_BE(addr) \ + mfmsr r11; \ + xori r11,r11,1; \ + mtsrr0 addr; \ + mtsrr1 r11; \ + rfid; \ + b . + +#define RETURN_FROM_BE \ + .long 0x05000048; /* bl . + 4 */ \ + .long 0xa602487d; /* mflr r10 */ \ + .long 0x20004a39; /* addi r10,r10,32 */ \ + .long 0xa600607d; /* mfmsr r11 */ \ + .long 0x01006b69; /* xori r11,r11,1 */ \ + .long 0xa6035a7d; /* mtsrr0 r10 */ \ + .long 0xa6037b7d; /* mtsrr1 r11 */ \ + .long 0x2400004c; /* rfid */ \ + .long 0x00000048; /* b . */ \ + +#endif /* __BYTE_ORDER__ */ + +#endif /* _ASMPOWERPC_PPC_ASM_H */ diff --git a/lib/ppc64/asm/io.h b/lib/ppc64/asm/io.h index c0801d4..4f2c31b 100644 --- a/lib/ppc64/asm/io.h +++ b/lib/ppc64/asm/io.h @@ -1,5 +1,13 @@ #ifndef _ASMPPC64_IO_H_ #define _ASMPPC64_IO_H_ + +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +#define __cpu_is_be() (0) +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ #define __cpu_is_be() (1) +#else +#error Undefined byte order +#endif + #include <asm-generic/io.h> #endif diff --git a/lib/ppc64/asm/ppc_asm.h b/lib/ppc64/asm/ppc_asm.h new file mode 100644 index 0000000..e3929ee --- /dev/null +++ b/lib/ppc64/asm/ppc_asm.h @@ -0,0 +1 @@ +#include "../../powerpc/asm/ppc_asm.h" diff --git a/powerpc/Makefile b/powerpc/Makefile index 369a38b..cc39a09 100644 --- a/powerpc/Makefile +++ b/powerpc/Makefile @@ -1 +1 @@ -include $(TEST_DIR)/Makefile.$(ARCH) +include $(TEST_DIR)/Makefile.$(ENDIAN) diff --git a/powerpc/Makefile.big b/powerpc/Makefile.big new file mode 100644 index 0000000..d81c52d --- /dev/null +++ b/powerpc/Makefile.big @@ -0,0 +1,21 @@ +# +# ppc64 big-endian makefile +# +# Authors: Andrew Jones <drjones@redhat.com> +# +bits = 64 + +arch_CFLAGS = -mbig-endian +arch_LDFLAGS = -EB + +cstart.o = $(TEST_DIR)/cstart64.o +reloc.o = $(TEST_DIR)/reloc64.o +cflatobjs += lib/ppc64/spinlock.o + +# ppc64 specific tests +tests = + +include $(TEST_DIR)/Makefile.common + +arch_clean: powerpc_clean + $(RM) lib/ppc64/.*.d diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index cc27ac8..b088af6 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -11,14 +11,15 @@ all: $(TEST_DIR)/boot_rom.bin test_cases ################################################################## -CFLAGS += $(arch_CFLAGS) -CFLAGS += -std=gnu99 -CFLAGS += -ffreestanding -CFLAGS += -Wextra -CFLAGS += -O2 -CFLAGS += -I lib -I lib/libfdt -CFLAGS += -Wa,-mregnames -CFLAGS += -fpie +common_CFLAGS = -std=gnu99 +common_CFLAGS += -ffreestanding +common_CFLAGS += -Wextra +common_CFLAGS += -O2 +common_CFLAGS += -I lib -I lib/libfdt +common_CFLAGS += -Wa,-mregnames +common_CFLAGS += -fpie + +CFLAGS += $(arch_CFLAGS) $(common_CFLAGS) asm-offsets = lib/$(ARCH)/asm-offsets.h include scripts/asm-offsets.mak @@ -48,6 +49,7 @@ $(TEST_DIR)/boot_rom.bin: $(TEST_DIR)/boot_rom.elf dd if=/dev/zero of=$@ bs=256 count=1 $(OBJCOPY) -O binary $^ >(cat - >>$@) +$(TEST_DIR)/boot_rom.elf: CFLAGS = -mbig-endian $(common_CFLAGS) $(main_CFLAGS) $(TEST_DIR)/boot_rom.elf: $(TEST_DIR)/boot_rom.o $(LD) -EB -nostdlib -Ttext=0x100 --entry=start --build-id=none -o $@ $< diff --git a/powerpc/Makefile.little b/powerpc/Makefile.little new file mode 100644 index 0000000..c37d2fc --- /dev/null +++ b/powerpc/Makefile.little @@ -0,0 +1,21 @@ +# +# ppc64 little-endian makefile +# +# Authors: Andrew Jones <drjones@redhat.com> +# +bits = 64 + +arch_CFLAGS = -mlittle-endian +arch_LDFLAGS = -EL + +cstart.o = $(TEST_DIR)/cstart64.o +reloc.o = $(TEST_DIR)/reloc64.o +cflatobjs += lib/ppc64/spinlock.o + +# ppc64 specific tests +tests = + +include $(TEST_DIR)/Makefile.common + +arch_clean: powerpc_clean + $(RM) lib/ppc64/.*.d diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64 deleted file mode 100644 index 1cf277e..0000000 --- a/powerpc/Makefile.ppc64 +++ /dev/null @@ -1,22 +0,0 @@ -# -# ppc64 makefile -# -# Authors: Andrew Jones <drjones@redhat.com> -# -bits = 64 -ldarch = elf64-powerpc - -arch_CFLAGS = -mbig-endian -arch_LDFLAGS = -EB - -cstart.o = $(TEST_DIR)/cstart64.o -reloc.o = $(TEST_DIR)/reloc64.o -cflatobjs += lib/ppc64/spinlock.o - -# ppc64 specific tests -tests = - -include $(TEST_DIR)/Makefile.common - -arch_clean: powerpc_clean - $(RM) lib/ppc64/.*.d diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S index f5530fb..60557a9 100644 --- a/powerpc/cstart64.S +++ b/powerpc/cstart64.S @@ -7,6 +7,7 @@ */ #define __ASSEMBLY__ #include <asm/hcall.h> +#include <asm/ppc_asm.h> #define LOAD_REG_IMMEDIATE(reg,expr) \ lis reg,(expr)@highest; \ @@ -25,6 +26,7 @@ */ .globl start start: + RETURN_FROM_BE /* * We were loaded at QEMU's kernel load address, but we're not * allowed to link there due to how QEMU deals with linker VMAs, @@ -93,11 +95,15 @@ halt: enter_rtas: mflr r0 std r0, 16(r1) + + LOAD_REG_ADDR(r10,rtas_return_loc) + mtlr r10 + LOAD_REG_ADDR(r10, rtas_blob) -//FIXME: change this bctrl to an rtas-prep, rfid, rtas-return sequence - mtctr r10 - nop - bctrl + B_BE(r10) + +rtas_return_loc: + RETURN_FROM_BE ld r0, 16(r1) mtlr r0 blr
This patch allows to build tests for ppc64 little endian target (ppc64le) on big and little endian hosts. We add a new parameter to configure to select the endianness of the tests (--endian=little or --endian=big). I have built and tested big and little endian tests on a little endian host, a big endian host, with kvm_hv and kvm_pr, and on x86_64 with ppc64 as a TCG target. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- v2: replace FIXUP_ENDIAN from linux by a home made version (B_BE and RETURN_FROM_BE) Makefile | 11 ++++++----- configure | 6 ++++++ lib/powerpc/asm/ppc_asm.h | 36 ++++++++++++++++++++++++++++++++++++ lib/ppc64/asm/io.h | 8 ++++++++ lib/ppc64/asm/ppc_asm.h | 1 + powerpc/Makefile | 2 +- powerpc/Makefile.big | 21 +++++++++++++++++++++ powerpc/Makefile.common | 18 ++++++++++-------- powerpc/Makefile.little | 21 +++++++++++++++++++++ powerpc/Makefile.ppc64 | 22 ---------------------- powerpc/cstart64.S | 14 ++++++++++---- 11 files changed, 120 insertions(+), 40 deletions(-) create mode 100644 lib/powerpc/asm/ppc_asm.h create mode 100644 lib/ppc64/asm/ppc_asm.h create mode 100644 powerpc/Makefile.big create mode 100644 powerpc/Makefile.little delete mode 100644 powerpc/Makefile.ppc64