Re: [PATCH 1/3] xen: Add files needed for minimal Power build

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.06.2023 17:06, Shawn Anastasio wrote:
> Add the build system changes required to build for ppc64le (POWER8+).
> Following in the footsteps of the initial riscv port, only building
> the head.o target, which boots to an infinite loop, is supported:
> 
> $ make XEN_TARGET_ARCH=ppc64 -C xen openpower_defconfig
> $ make XEN_TARGET_ARCH=ppc64 SUBSYSTEMS=xen -C xen TARGET=ppc64/head.o
> 
> 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 ppc64/head.o
> 
> Where possible, this patch uses header definitions and support routines
> from the original Xen PowerPC port present in Xen 3.2.3. Though we are
> targeting a much newer ISA than that original port did, some of the
> definitions have remained similar enough for reuse.
> 
> Signed-off-by: Shawn Anastasio <shawnanastasio@xxxxxxxxxxxxxxxxxxxxx>

Just a few small remarks, as following Andrew's comments I expect the
patch will shrink quite a bit:

> --- /dev/null
> +++ b/config/ppc64.mk
> @@ -0,0 +1,5 @@
> +CONFIG_PPC64 := y
> +CONFIG_PPC64_64 := y
> +CONFIG_PPC64_$(XEN_OS) := y

The first of the 64-s here are a little odd; looking at RISC-V's
counterpart, wouldn't this want to be

CONFIG_PPC := y
CONFIG_PPC_64 := y
CONFIG_PPC_$(XEN_OS) := y

> --- /dev/null
> +++ b/xen/arch/ppc/Kconfig
> @@ -0,0 +1,42 @@
> +config PPC
> +     def_bool y

Is this necessary? Iirc PPC is frequently used as a name for 32-bit PPC
(but then also elsewhere as covering both 32- and 64-bit), so I'm not
sure we want this without having a need for it.

> +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+)

What are the + in here meant to indicate? Since this is about a baseline
ISA, I find such a use (presumably standing for "or newer") ambiguous.

Jan