Add the build system changes required to build for ppc64le (POWER8+).
As of now the resulting image simply boots to an infinite loop.
$ make XEN_TARGET_ARCH=ppc64 -C xen openpower_defconfig
$ make XEN_TARGET_ARCH=ppc64 SUBSYSTEMS=xen -C xen build
This port targets POWER8+ CPUs running in Little Endian mode specifically,
and does not boot on older machines. Additionally, this initial skeleton
only implements the PaPR/pseries boot protocol which allows it to be
booted in a standard QEMU virtual machine:
$ qemu-system-ppc64 -M pseries-5.2 -m 256M -kernel xen/xen
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
---
config/ppc64.mk | 5 +
xen/Makefile | 5 +-
xen/arch/ppc/Kconfig | 42 ++++++
xen/arch/ppc/Kconfig.debug | 0
xen/arch/ppc/Makefile | 16 +++
xen/arch/ppc/Rules.mk | 0
xen/arch/ppc/arch.mk | 11 ++
xen/arch/ppc/configs/openpower_defconfig | 13 ++
xen/arch/ppc/include/asm/config.h | 63 +++++++++
xen/arch/ppc/include/asm/page-bits.h | 7 +
xen/arch/ppc/ppc64/Makefile | 1 +
xen/arch/ppc/ppc64/asm-offsets.c | 0
xen/arch/ppc/ppc64/head.S | 27 ++++
xen/arch/ppc/xen.lds.S | 173 +++++++++++++++++++++++
14 files changed, 361 insertions(+), 2 deletions(-)
create mode 100644 config/ppc64.mk
create mode 100644 xen/arch/ppc/Kconfig
create mode 100644 xen/arch/ppc/Kconfig.debug
create mode 100644 xen/arch/ppc/Makefile
create mode 100644 xen/arch/ppc/Rules.mk
create mode 100644 xen/arch/ppc/arch.mk
create mode 100644 xen/arch/ppc/configs/openpower_defconfig
create mode 100644 xen/arch/ppc/include/asm/config.h
create mode 100644 xen/arch/ppc/include/asm/page-bits.h
create mode 100644 xen/arch/ppc/ppc64/Makefile
create mode 100644 xen/arch/ppc/ppc64/asm-offsets.c
create mode 100644 xen/arch/ppc/ppc64/head.S
create mode 100644 xen/arch/ppc/xen.lds.S
diff --git a/config/ppc64.mk b/config/ppc64.mk
new file mode 100644
index 0000000000..597f0668c3
--- /dev/null
+++ b/config/ppc64.mk
@@ -0,0 +1,5 @@
+CONFIG_PPC := y
+CONFIG_PPC64 := y
+CONFIG_PPC_$(XEN_OS) := y
+
+CONFIG_XEN_INSTALL_SUFFIX :=
diff --git a/xen/Makefile b/xen/Makefile
index e89fc461fc..db5454fb58 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -38,7 +38,7 @@ EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
ARCH=$(XEN_TARGET_ARCH)
SRCARCH=$(shell echo $(ARCH) | \
sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
- -e s'/riscv.*/riscv/g')
+ -e s'/riscv.*/riscv/g' -e s'/ppc.*/ppc/g')
export ARCH SRCARCH
# Allow someone to change their config file
@@ -244,7 +244,7 @@ include $(XEN_ROOT)/Config.mk
export TARGET_SUBARCH := $(XEN_TARGET_ARCH)
export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \
sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
- -e s'/riscv.*/riscv/g')
+ -e s'/riscv.*/riscv/g' -e s'/ppc.*/ppc/g')
export CONFIG_SHELL := $(SHELL)
export CC CXX LD NM OBJCOPY OBJDUMP ADDR2LINE
@@ -563,6 +563,7 @@ _clean:
$(Q)$(MAKE) $(clean)=xsm
$(Q)$(MAKE) $(clean)=crypto
$(Q)$(MAKE) $(clean)=arch/arm
+ $(Q)$(MAKE) $(clean)=arch/ppc
$(Q)$(MAKE) $(clean)=arch/riscv
$(Q)$(MAKE) $(clean)=arch/x86
$(Q)$(MAKE) $(clean)=test
diff --git a/xen/arch/ppc/Kconfig b/xen/arch/ppc/Kconfig
new file mode 100644
index 0000000000..a0a70adef4
--- /dev/null
+++ b/xen/arch/ppc/Kconfig
@@ -0,0 +1,42 @@
+config PPC
+ def_bool y
+
+config PPC64
+ def_bool y
+ select 64BIT
+
+config ARCH_DEFCONFIG
+ string
+ default "arch/ppc/configs/openpower_defconfig"
+
+menu "Architecture Features"
+
+source "arch/Kconfig"
+
+endmenu
+
+menu "ISA Selection"
+
+choice
+ prompt "Base ISA"
+ default POWER_ISA_2_07B if PPC64
+ help
+ This selects the base ISA version that Xen will target.
+
+config POWER_ISA_2_07B
+ bool "Power ISA 2.07B"
+ help
+ Target version 2.07B of the Power ISA (POWER8)
+
+config POWER_ISA_3_00
+ bool "Power ISA 3.00"
+ help
+ Target version 3.00 of the Power ISA (POWER9)
+
+endchoice
+
+endmenu
+
+source "common/Kconfig"
+
+source "drivers/Kconfig"
diff --git a/xen/arch/ppc/Kconfig.debug b/xen/arch/ppc/Kconfig.debug
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile
new file mode 100644
index 0000000000..10b101cf9c
--- /dev/null
+++ b/xen/arch/ppc/Makefile
@@ -0,0 +1,16 @@
+obj-$(CONFIG_PPC64) += ppc64/
+
+$(TARGET): $(TARGET)-syms
+ cp -f $< $@
+
+$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
+ $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@
+ $(NM) -pa --format=sysv $(@D)/$(@F) \
+ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
+ >$(@D)/$(@F).map
+
+$(obj)/xen.lds: $(src)/xen.lds.S FORCE
+ $(call if_changed_dep,cpp_lds_S)
+
+.PHONY: include
+include:
diff --git a/xen/arch/ppc/Rules.mk b/xen/arch/ppc/Rules.mk
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/ppc/arch.mk b/xen/arch/ppc/arch.mk
new file mode 100644
index 0000000000..c185a5028a
--- /dev/null
+++ b/xen/arch/ppc/arch.mk
@@ -0,0 +1,11 @@
+########################################
+# Power-specific definitions
+
+ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8
+ppc-march-$(CONFIG_POWER_ISA_3_00) := power9
+
+CFLAGS += -mcpu=$(ppc-march-y) -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx
+
+# TODO: Drop override when more of the build is working
+override ALL_OBJS-y = arch/$(TARGET_ARCH)/built_in.o
+override ALL_LIBS-y =
diff --git a/xen/arch/ppc/configs/openpower_defconfig b/xen/arch/ppc/configs/openpower_defconfig
new file mode 100644
index 0000000000..8783eb3488
--- /dev/null
+++ b/xen/arch/ppc/configs/openpower_defconfig
@@ -0,0 +1,13 @@
+# CONFIG_SCHED_CREDIT is not set
+# CONFIG_SCHED_RTDS is not set
+# CONFIG_SCHED_NULL is not set
+# CONFIG_SCHED_ARINC653 is not set
+# CONFIG_TRACEBUFFER is not set
+# CONFIG_HYPFS is not set
+# CONFIG_GRANT_TABLE is not set
+# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
+
+CONFIG_PPC64=y
+CONFIG_DEBUG=y
+CONFIG_DEBUG_INFO=y
+CONFIG_EXPERT=y
diff --git a/xen/arch/ppc/include/asm/config.h b/xen/arch/ppc/include/asm/config.h
new file mode 100644
index 0000000000..7a2862ef7a
--- /dev/null
+++ b/xen/arch/ppc/include/asm/config.h
@@ -0,0 +1,63 @@
+#ifndef __PPC_CONFIG_H__
+#define __PPC_CONFIG_H__
+
+#include <xen/const.h>
+#include <xen/page-size.h>
+
+#if defined(CONFIG_PPC64)
+#define LONG_BYTEORDER 3
+#define ELFSIZE 64
+#define MAX_VIRT_CPUS 1024u
+#else
+#error "Unsupported PowerPC variant"
+#endif
+
+#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
+#define BITS_PER_LONG (BYTES_PER_LONG << 3)
+#define POINTER_ALIGN BYTES_PER_LONG
+
+#define BITS_PER_LLONG 64
+
+/* xen_ulong_t is always 64 bits */
+#define BITS_PER_XEN_ULONG 64
+
+#define CONFIG_PPC_L1_CACHE_SHIFT 7
+#define CONFIG_PAGEALLOC_MAX_ORDER 18
+#define CONFIG_DOMU_MAX_ORDER 9
+#define CONFIG_HWDOM_MAX_ORDER 10
+
+#define OPT_CONSOLE_STR "dtuart"
+#define INVALID_VCPU_ID MAX_VIRT_CPUS
+
+/* Linkage for PPC */
+#ifdef __ASSEMBLY__
+#define ALIGN .align 2
+
+#define ENTRY(name) \
+ .globl name; \
+ ALIGN; \
+ name:
+#endif
+
+#define XEN_VIRT_START _AT(UL, 0x400000)
+
+#define SMP_CACHE_BYTES (1 << 6)
+
+#define STACK_ORDER 2
+#define STACK_SIZE (PAGE_SIZE << STACK_ORDER)
+
+/* 288 bytes below the stack pointer must be preserved by interrupt handlers */
+#define STACK_VOLATILE_AREA 288
+
+/* size of minimum stack frame; C code can write into the caller's stack */
+#define STACK_FRAME_OVERHEAD 32
+
+#endif /* __PPC_CONFIG_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/ppc/include/asm/page-bits.h b/xen/arch/ppc/include/asm/page-bits.h
new file mode 100644
index 0000000000..4c01bf9716
--- /dev/null
+++ b/xen/arch/ppc/include/asm/page-bits.h
@@ -0,0 +1,7 @@
+#ifndef __PPC_PAGE_BITS_H__
+#define __PPC_PAGE_BITS_H__
+
+#define PAGE_SHIFT 16 /* 64 KiB Pages */
+#define PADDR_BITS 48
+
+#endif /* __PPC_PAGE_BITS_H__ */
diff --git a/xen/arch/ppc/ppc64/Makefile b/xen/arch/ppc/ppc64/Makefile
new file mode 100644
index 0000000000..3340058c08
--- /dev/null
+++ b/xen/arch/ppc/ppc64/Makefile
@@ -0,0 +1 @@
+obj-y += head.o
diff --git a/xen/arch/ppc/ppc64/asm-offsets.c b/xen/arch/ppc/ppc64/asm-offsets.c
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/ppc/ppc64/head.S b/xen/arch/ppc/ppc64/head.S
new file mode 100644
index 0000000000..e24331f95c
--- /dev/null
+++ b/xen/arch/ppc/ppc64/head.S
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+.section .text.header, "ax", %progbits
+
+ENTRY(start)
+ /*
+ * Depending on how we were booted, the CPU could be running in either
+ * Little Endian or Big Endian mode. The following trampoline from Linux
+ * cleverly uses an instruction that encodes to a NOP if the CPU's
+ * endianness matches the assumption of the assembler (LE, in our case)
+ * or a branch to code that performs the endian switch in the other case.
+ */
+ tdi 0, 0, 0x48 /* Reverse endian of b . + 8 */
+ b $ + 44 /* Skip trampoline if endian is good */
+ .long 0xa600607d /* mfmsr r11 */
+ .long 0x01006b69 /* xori r11,r11,1 */
+ .long 0x00004039 /* li r10,0 */
+ .long 0x6401417d /* mtmsrd r10,1 */
+ .long 0x05009f42 /* bcl 20,31,$+4 */
+ .long 0xa602487d /* mflr r10 */
+ .long 0x14004a39 /* addi r10,r10,20 */
+ .long 0xa6035a7d /* mtsrr0 r10 */
+ .long 0xa6037b7d /* mtsrr1 r11 */
+ .long 0x2400004c /* rfid */
+
+ /* Now that the endianness is confirmed, continue */
+1: b 1b
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
new file mode 100644
index 0000000000..89fcdc9c00
--- /dev/null
+++ b/xen/arch/ppc/xen.lds.S
@@ -0,0 +1,173 @@
+#include <xen/xen.lds.h>
+
+#undef ENTRY
+#undef ALIGN
+
+OUTPUT_ARCH(powerpc:common64)
+ENTRY(start)
+
+PHDRS
+{
+ text PT_LOAD ;
+#if defined(BUILD_ID)
+ note PT_NOTE ;
+#endif
+}
+
+/**
+ * OF's base load address is 0x400000 (XEN_VIRT_START).
+ * By defining sections this way, we can keep our virtual address base at 0x400000
+ * while keeping the physical base at 0x0.
+ *
+ * Without this, OF incorrectly loads .text at 0x400000 + 0x400000 = 0x800000.
+ * Taken from x86/xen.lds.S
+ */
+#ifdef CONFIG_LD_IS_GNU
+# define DECL_SECTION(x) x : AT(ADDR(#x) - XEN_VIRT_START)
+#else
+# define DECL_SECTION(x) x : AT(ADDR(x) - XEN_VIRT_START)
+#endif
+
+SECTIONS
+{
+ . = XEN_VIRT_START;
+
+ DECL_SECTION(.text) {
+ _stext = .; /* Text section */
+ *(.text.header)
+
+ *(.text.cold)
+ *(.text.unlikely .text.*_unlikely .text.unlikely.*)
+
+ *(.text)
+#ifdef CONFIG_CC_SPLIT_SECTIONS
+ *(.text.*)
+#endif
+
+ *(.fixup)
+ *(.gnu.warning)
+ . = ALIGN(POINTER_ALIGN);
+ _etext = .; /* End of text section */
+ } :text
+
+ . = ALIGN(PAGE_SIZE);
+ DECL_SECTION(.rodata) {
+ _srodata = .; /* Read-only data */
+ *(.rodata)
+ *(.rodata.*)
+ *(.data.rel.ro)
+ *(.data.rel.ro.*)
+
+ VPCI_ARRAY
+
+ . = ALIGN(POINTER_ALIGN);
+ _erodata = .; /* End of read-only data */
+ } :text
+
+ #if defined(BUILD_ID)
+ . = ALIGN(4);
+ DECL_SECTION(.note.gnu.build-id) {
+ __note_gnu_build_id_start = .;
+ *(.note.gnu.build-id)
+ __note_gnu_build_id_end = .;
+ } :note :text
+ #endif
+ _erodata = .; /* End of read-only data */
+
+ . = ALIGN(PAGE_SIZE);
+ DECL_SECTION(.data.ro_after_init) {
+ __ro_after_init_start = .;
+ *(.data.ro_after_init)
+ . = ALIGN(PAGE_SIZE);
+ __ro_after_init_end = .;
+ } : text
+
+ DECL_SECTION(.data.read_mostly) {
+ *(.data.read_mostly)
+ } :text
+
+ . = ALIGN(PAGE_SIZE);
+ DECL_SECTION(.data) { /* Data */
+ *(.data.page_aligned)
+ . = ALIGN(8);
+ __start_schedulers_array = .;
+ *(.data.schedulers)
+ __end_schedulers_array = .;
+
+ HYPFS_PARAM
+
+ *(.data .data.*)
+ CONSTRUCTORS
+ } :text
+
+ . = ALIGN(PAGE_SIZE); /* Init code and data */
+ __init_begin = .;
+ DECL_SECTION(.init.text) {
+ _sinittext = .;
+ *(.init.text)
+ _einittext = .;
+ . = ALIGN(PAGE_SIZE); /* Avoid mapping alt insns executable */
+ } :text
+
+ . = ALIGN(PAGE_SIZE);
+ DECL_SECTION(.init.data) {
+ *(.init.rodata)
+ *(.init.rodata.*)
+
+ . = ALIGN(POINTER_ALIGN);
+ __setup_start = .;
+ *(.init.setup)
+ __setup_end = .;
+
+ __initcall_start = .;
+ *(.initcallpresmp.init)
+ __presmp_initcall_end = .;
+ *(.initcall1.init)
+ __initcall_end = .;
+
+ LOCK_PROFILE_DATA
+
+ *(.init.data)
+ *(.init.data.rel)
+ *(.init.data.rel.*)
+
+ . = ALIGN(8);
+ __ctors_start = .;
+ *(.ctors)
+ *(.init_array)
+ *(SORT(.init_array.*))
+ __ctors_end = .;
+ } :text
+ . = ALIGN(POINTER_ALIGN);
+ __init_end = .;
+
+ DECL_SECTION(.bss) { /* BSS */
+ __bss_start = .;
+ *(.bss.stack_aligned)
+ . = ALIGN(PAGE_SIZE);
+ *(.bss.page_aligned)
+ . = ALIGN(PAGE_SIZE);
+ __per_cpu_start = .;
+ *(.bss.percpu.page_aligned)
+ *(.bss.percpu)
+ . = ALIGN(SMP_CACHE_BYTES);
+ *(.bss.percpu.read_mostly)
+ . = ALIGN(SMP_CACHE_BYTES);
+ __per_cpu_data_end = .;
+ *(.bss .bss.*)
+ . = ALIGN(POINTER_ALIGN);
+ __bss_end = .;
+ } :text
+ _end = . ;
+
+ /* Section for the device tree blob (if any). */
+ DECL_SECTION(.dtb) { *(.dtb) } :text
+
+ DWARF2_DEBUG_SECTIONS
+
+ DISCARD_SECTIONS
+
+ STABS_DEBUG_SECTIONS
+
+ ELF_DETAILS_SECTIONS
+}
--
2.30.2
On 13.06.2023 16:50, Shawn Anastasio wrote: > --- /dev/null > +++ b/xen/arch/ppc/Makefile > @@ -0,0 +1,16 @@ > +obj-$(CONFIG_PPC64) += ppc64/ > + > +$(TARGET): $(TARGET)-syms > + cp -f $< $@ > + > +$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds > + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@ > + $(NM) -pa --format=sysv $(@D)/$(@F) \ > + | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ > + >$(@D)/$(@F).map Elsewhere we recently switched these uses of $(@D)/$(@F) to just $@. Please can you do so here as well? > --- /dev/null > +++ b/xen/arch/ppc/arch.mk > @@ -0,0 +1,11 @@ > +######################################## > +# Power-specific definitions > + > +ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8 > +ppc-march-$(CONFIG_POWER_ISA_3_00) := power9 > + > +CFLAGS += -mcpu=$(ppc-march-y) -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx Wouldn't it make sense to also pass -mlittle here, such that a tool chain defaulting to big-endian can still be used? > --- /dev/null > +++ b/xen/arch/ppc/ppc64/head.S > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > + > +.section .text.header, "ax", %progbits > + > +ENTRY(start) > + /* > + * Depending on how we were booted, the CPU could be running in either > + * Little Endian or Big Endian mode. The following trampoline from Linux > + * cleverly uses an instruction that encodes to a NOP if the CPU's > + * endianness matches the assumption of the assembler (LE, in our case) > + * or a branch to code that performs the endian switch in the other case. > + */ > + tdi 0, 0, 0x48 /* Reverse endian of b . + 8 */ > + b $ + 44 /* Skip trampoline if endian is good */ If I get this right, $ and . are interchangable on Power? If not, then all is fine and there likely is a reason to use . in the comment but $ in the code. But if so, it would be nice if both could match, and I guess with other architectures in mind . would be preferable. > --- /dev/null > +++ b/xen/arch/ppc/xen.lds.S > @@ -0,0 +1,173 @@ > +#include <xen/xen.lds.h> > + > +#undef ENTRY > +#undef ALIGN > + > +OUTPUT_ARCH(powerpc:common64) > +ENTRY(start) > + > +PHDRS > +{ > + text PT_LOAD ; > +#if defined(BUILD_ID) > + note PT_NOTE ; > +#endif > +} > + > +/** > + * OF's base load address is 0x400000 (XEN_VIRT_START). > + * By defining sections this way, we can keep our virtual address base at 0x400000 > + * while keeping the physical base at 0x0. > + * > + * Without this, OF incorrectly loads .text at 0x400000 + 0x400000 = 0x800000. > + * Taken from x86/xen.lds.S > + */ > +#ifdef CONFIG_LD_IS_GNU > +# define DECL_SECTION(x) x : AT(ADDR(#x) - XEN_VIRT_START) > +#else > +# define DECL_SECTION(x) x : AT(ADDR(x) - XEN_VIRT_START) > +#endif > + > +SECTIONS > +{ > + . = XEN_VIRT_START; > + > + DECL_SECTION(.text) { > + _stext = .; /* Text section */ > + *(.text.header) > + > + *(.text.cold) > + *(.text.unlikely .text.*_unlikely .text.unlikely.*) > + > + *(.text) > +#ifdef CONFIG_CC_SPLIT_SECTIONS > + *(.text.*) > +#endif > + > + *(.fixup) > + *(.gnu.warning) > + . = ALIGN(POINTER_ALIGN); > + _etext = .; /* End of text section */ > + } :text > + > + . = ALIGN(PAGE_SIZE); > + DECL_SECTION(.rodata) { > + _srodata = .; /* Read-only data */ > + *(.rodata) > + *(.rodata.*) > + *(.data.rel.ro) > + *(.data.rel.ro.*) > + > + VPCI_ARRAY > + > + . = ALIGN(POINTER_ALIGN); > + _erodata = .; /* End of read-only data */ > + } :text > + > + #if defined(BUILD_ID) > + . = ALIGN(4); > + DECL_SECTION(.note.gnu.build-id) { > + __note_gnu_build_id_start = .; > + *(.note.gnu.build-id) > + __note_gnu_build_id_end = .; > + } :note :text > + #endif > + _erodata = .; /* End of read-only data */ > + > + . = ALIGN(PAGE_SIZE); > + DECL_SECTION(.data.ro_after_init) { > + __ro_after_init_start = .; > + *(.data.ro_after_init) > + . = ALIGN(PAGE_SIZE); > + __ro_after_init_end = .; > + } : text > + > + DECL_SECTION(.data.read_mostly) { > + *(.data.read_mostly) > + } :text > + > + . = ALIGN(PAGE_SIZE); > + DECL_SECTION(.data) { /* Data */ > + *(.data.page_aligned) > + . = ALIGN(8); > + __start_schedulers_array = .; > + *(.data.schedulers) > + __end_schedulers_array = .; > + > + HYPFS_PARAM > + > + *(.data .data.*) > + CONSTRUCTORS > + } :text > + > + . = ALIGN(PAGE_SIZE); /* Init code and data */ > + __init_begin = .; > + DECL_SECTION(.init.text) { > + _sinittext = .; > + *(.init.text) > + _einittext = .; > + . = ALIGN(PAGE_SIZE); /* Avoid mapping alt insns executable */ > + } :text > + > + . = ALIGN(PAGE_SIZE); > + DECL_SECTION(.init.data) { > + *(.init.rodata) > + *(.init.rodata.*) > + > + . = ALIGN(POINTER_ALIGN); > + __setup_start = .; > + *(.init.setup) > + __setup_end = .; > + > + __initcall_start = .; > + *(.initcallpresmp.init) > + __presmp_initcall_end = .; > + *(.initcall1.init) > + __initcall_end = .; > + > + LOCK_PROFILE_DATA > + > + *(.init.data) > + *(.init.data.rel) > + *(.init.data.rel.*) > + > + . = ALIGN(8); > + __ctors_start = .; > + *(.ctors) > + *(.init_array) > + *(SORT(.init_array.*)) > + __ctors_end = .; > + } :text > + . = ALIGN(POINTER_ALIGN); > + __init_end = .; Up to here I think I agree with all uses of ALIGN(), but ... > + DECL_SECTION(.bss) { /* BSS */ > + __bss_start = .; > + *(.bss.stack_aligned) > + . = ALIGN(PAGE_SIZE); > + *(.bss.page_aligned) ... the one between the two .bss parts looks unmotivated. Within a section definition ALIGN() typically only makes sense when followed by a label definition, like ... > + . = ALIGN(PAGE_SIZE); > + __per_cpu_start = .; ... here. > + *(.bss.percpu.page_aligned) > + *(.bss.percpu) > + . = ALIGN(SMP_CACHE_BYTES); This one is an exception, as it is intended to separate the read-mostly part such that there's no shared cache line. Jan > + *(.bss.percpu.read_mostly) > + . = ALIGN(SMP_CACHE_BYTES); > + __per_cpu_data_end = .; > + *(.bss .bss.*) > + . = ALIGN(POINTER_ALIGN); > + __bss_end = .; > + } :text > + _end = . ; > + > + /* Section for the device tree blob (if any). */ > + DECL_SECTION(.dtb) { *(.dtb) } :text > + > + DWARF2_DEBUG_SECTIONS > + > + DISCARD_SECTIONS > + > + STABS_DEBUG_SECTIONS > + > + ELF_DETAILS_SECTIONS > +}
On Wed Jun 14, 2023 at 10:51 AM CDT, Jan Beulich wrote: > On 13.06.2023 16:50, Shawn Anastasio wrote: > > --- /dev/null > > +++ b/xen/arch/ppc/Makefile > > @@ -0,0 +1,16 @@ > > +obj-$(CONFIG_PPC64) += ppc64/ > > + > > +$(TARGET): $(TARGET)-syms > > + cp -f $< $@ > > + > > +$(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds > > + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@ > > + $(NM) -pa --format=sysv $(@D)/$(@F) \ > > + | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ > > + >$(@D)/$(@F).map > > Elsewhere we recently switched these uses of $(@D)/$(@F) to just $@. > Please can you do so here as well? Sure, will fix in v4. > > --- /dev/null > > +++ b/xen/arch/ppc/arch.mk > > @@ -0,0 +1,11 @@ > > +######################################## > > +# Power-specific definitions > > + > > +ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8 > > +ppc-march-$(CONFIG_POWER_ISA_3_00) := power9 > > + > > +CFLAGS += -mcpu=$(ppc-march-y) -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx > > Wouldn't it make sense to also pass -mlittle here, such that a tool > chain defaulting to big-endian can still be used? Good call. On this topic, I suppose I'll also add -m64 to allow 32-bit toolchains to be used as well. > > --- /dev/null > > +++ b/xen/arch/ppc/ppc64/head.S > > @@ -0,0 +1,27 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +.section .text.header, "ax", %progbits > > + > > +ENTRY(start) > > + /* > > + * Depending on how we were booted, the CPU could be running in either > > + * Little Endian or Big Endian mode. The following trampoline from Linux > > + * cleverly uses an instruction that encodes to a NOP if the CPU's > > + * endianness matches the assumption of the assembler (LE, in our case) > > + * or a branch to code that performs the endian switch in the other case. > > + */ > > + tdi 0, 0, 0x48 /* Reverse endian of b . + 8 */ > > + b $ + 44 /* Skip trampoline if endian is good */ > > If I get this right, $ and . are interchangable on Power? If not, > then all is fine and there likely is a reason to use . in the > comment but $ in the code. But if so, it would be nice if both > could match, and I guess with other architectures in mind . would > be preferable. As hinted by the comment, this code was directly inherited from Linux and I'm not sure why the original author chose '$' instead of '.'. That said, as far as I can tell you are correct about the two being interchangeable, and changing the $ to . results in the exact same machine code. I can go ahead and make the change for consistency in v4. > > + DECL_SECTION(.bss) { /* BSS */ > > + __bss_start = .; > > + *(.bss.stack_aligned) > > + . = ALIGN(PAGE_SIZE); > > + *(.bss.page_aligned) > > ... the one between the two .bss parts looks unmotivated. Within > a section definition ALIGN() typically only makes sense when followed > by a label definition, like ... Correct me if I'm wrong, but wouldn't the ALIGN here serve to ensure that the subsequent '.bss.page_aligned' section has the correct alignment that its name implies? > Jan Thanks, Shawn
On 14.06.2023 18:36, Shawn Anastasio wrote: > On Wed Jun 14, 2023 at 10:51 AM CDT, Jan Beulich wrote: >> On 13.06.2023 16:50, Shawn Anastasio wrote: >>> --- /dev/null >>> +++ b/xen/arch/ppc/arch.mk >>> @@ -0,0 +1,11 @@ >>> +######################################## >>> +# Power-specific definitions >>> + >>> +ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8 >>> +ppc-march-$(CONFIG_POWER_ISA_3_00) := power9 >>> + >>> +CFLAGS += -mcpu=$(ppc-march-y) -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx >> >> Wouldn't it make sense to also pass -mlittle here, such that a tool >> chain defaulting to big-endian can still be used? > > Good call. On this topic, I suppose I'll also add -m64 to allow 32-bit > toolchains to be used as well. Turns out this isn't quite enough. When trying to test my little bit of re-basing of Anthony's series, I ran into ld complaining about little endian input when the target format is big endian (like for the compiler I'm using a default configured, i.e. big-endian, binutils build). Looks like we need to pass "-m elf64lppc" to the linker; I'll see if that helps (and where exactly to put it). Jan
On 14.06.2023 18:36, Shawn Anastasio wrote: > On Wed Jun 14, 2023 at 10:51 AM CDT, Jan Beulich wrote: >> On 13.06.2023 16:50, Shawn Anastasio wrote: >>> + DECL_SECTION(.bss) { /* BSS */ >>> + __bss_start = .; >>> + *(.bss.stack_aligned) >>> + . = ALIGN(PAGE_SIZE); >>> + *(.bss.page_aligned) >> >> ... the one between the two .bss parts looks unmotivated. Within >> a section definition ALIGN() typically only makes sense when followed >> by a label definition, like ... > > Correct me if I'm wrong, but wouldn't the ALIGN here serve to ensure > that the subsequent '.bss.page_aligned' section has the correct alignment > that its name implies? Yes and no. Thing is that every contribution to .bss.page_aligned already needs to specify page alignment itself, or else it may break if any earlier contribution was page-aligned, but not a full page in size (which I think the compiler wouldn't allow to happen, but assembly code can result in such). Note how this very ALIGN() was recently dropped from RISC-V code, and my respective Arm side patch is also about to go in. Jan
On Thu Jun 15, 2023 at 1:47 AM CDT, Jan Beulich wrote: > On 14.06.2023 18:36, Shawn Anastasio wrote: > > On Wed Jun 14, 2023 at 10:51 AM CDT, Jan Beulich wrote: > >> On 13.06.2023 16:50, Shawn Anastasio wrote: > >>> + DECL_SECTION(.bss) { /* BSS */ > >>> + __bss_start = .; > >>> + *(.bss.stack_aligned) > >>> + . = ALIGN(PAGE_SIZE); > >>> + *(.bss.page_aligned) > >> > >> ... the one between the two .bss parts looks unmotivated. Within > >> a section definition ALIGN() typically only makes sense when followed > >> by a label definition, like ... > > > > Correct me if I'm wrong, but wouldn't the ALIGN here serve to ensure > > that the subsequent '.bss.page_aligned' section has the correct alignment > > that its name implies? > > Yes and no. Thing is that every contribution to .bss.page_aligned already > needs to specify page alignment itself, or else it may break if any earlier > contribution was page-aligned, but not a full page in size (which I think > the compiler wouldn't allow to happen, but assembly code can result in > such). Note how this very ALIGN() was recently dropped from RISC-V code, > and my respective Arm side patch is also about to go in. That makes sense, thanks. I'll get rid of the ALIGN here in v4. > Jan Thanks, Shawn
© 2016 - 2024 Red Hat, Inc.