Re: [PATCH v4 2/4] xen: Add files needed for minimal ppc64le build

[Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>]

On 6/20/23 5:15 AM, Jan Beulich wrote:
> On 16.06.2023 19:48, Shawn Anastasio wrote:
>> --- /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
> 
> I think the "if" here is at best confusing. If / when ppc32 support
> is added, a potentially different default here would make necessary
> adjustments, yet imo would not be very likely to introduce this very
> "if".

Fair enough, I'll drop the if.

>> --- /dev/null
>> +++ b/xen/arch/ppc/arch.mk
>> @@ -0,0 +1,12 @@
>> +########################################
>> +# Power-specific definitions
>> +
>> +ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8
>> +ppc-march-$(CONFIG_POWER_ISA_3_00) := power9
>> +
>> +CFLAGS += -m64 -mlittle-endian -mcpu=$(ppc-march-y)
>> +CFLAGS += -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx
> 
> Just for my own education: Besides the expected effect, -mstrict-align
> also appears to imply -mbit-align, which I'm not sure is intended here.
> Could you clarify the intentions for me?

If I'm reading the GCC documentation right, -mbit-align is the default
behavior regardless of -mstrict-align. The intention was simply to avoid
the generation of unaligned memory accesses in the kernel, since they
are only handled in hardware up to a certain width.

> As to -mabi=elfv2, it looks as if this limits us to gcc12 and newer.
> That's fine, but I think it wants pointing out in ./README (which has
> a section for this kind of information).

This is not the case. The ELFv2 ABI was released in 2014 and has been
the de facto standard on ppc64le since its inception. I know that ELFv2
support in GCC dates back to at least version 6 (the earliest I can
remember using), but I believe it goes back even further than that.

>> --- /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
> 
> I think I did ask for the same on RISC-V (yet sadly it's still .align
> there): .align is notoriously ambiguous. May I ask that you use .p2align
> (which I think is what is meant here, else .balign)?

Sure, will do.

> Jan

Thanks,
Shawn