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.2On 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 - 2026 Red Hat, Inc.