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

["Shawn Anastasio" <shawn@xxxxxxxxxx>]

On Fri Jun 9, 2023 at 4:15 AM CDT, Jan Beulich wrote:
> > --- /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>

Good point, this was definitely an oversight on my part. I'll clean this
up in v2.

> > --- /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.

This was mostly modeled after riscv/Kconfig. The idea was that this
option guards PPC support in general (both 32- and 64-bit) and the
subsequent option specifically opts in to 64-bit.

Given that this effort is only targeting 64-bit mode, though, the
argument could definitely be made that having the two options is
redundant, but I believe that is also the case for the risc-v port.

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

Fair point. The + was intended as an "or newer" but I agree that it's
ambiguous (for instance, it might be interpreted as a part of the ISA's
name). I'll remove it in v2, along with a renaming of "POWER ISA" to
"Power ISA" which I originally overlooked.

> Jan

Thanks,
Shawn