Re: [PATCH] powerpc: e500: Fix compilation with gcc e500 compiler

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 08/07/2022 à 22:04, Arnd Bergmann a écrit :
> On Fri, Jul 8, 2022 at 7:12 PM Pali Rohár <pali@kernel.org> wrote:
>>
>> On Monday 04 July 2022 14:07:10 Arnd Bergmann wrote:
>>> Another problem I see is that a kernel that is built for both E500 and E500MC
>>> uses -mcpu=e500mc and may not actually work on the older ones either
>>> (even with your patch).
>>
>> Such configuration is not supported, see arch/powerpc/platforms/Kconfig.cputype:
>>
>> config PPC_E500MC
>>          bool "e500mc Support"
>>          select PPC_FPU
>>          select COMMON_CLK
>>          depends on E500
>>          help
>>            This must be enabled for running on e500mc (and derivatives
>>            such as e5500/e6500), and must be disabled for running on
>>            e500v1 or e500v2.
>>
>> Based on this option you can enable either support for e500v1/e500v2 or
>> for e500mc. But not both.
> 
> This looks like a bad decision in Kconfig though, as there is nothing
> enforcing the rule: If you want support for E500MC, you have to select
> PPC_85xx, which implies E500 and allows selecting any combination
> of E500v1, E500v2 and E500MC based machines, but enabling
> any E500MC based one breaks all the others.
> 
> If this is a hard dependency, I think it should be enforced by making
> E500MC a separate top-level option in the "Processor Type" choice
> statement. However, if they can actually coexist, the help text and
> the Makefile need to be fixed.
> 

In cputable.c you have entries in the cpu_specs table. It show that when 
selecting PPC32 and E500MC, you exclude e500 and e500v2, allthough you 
then fallback on the generic e500 probably by mistake.

Seems to come from commit 3477e71d5319 ("powerpc/booke: Restrict SPE 
exception handlers to e200/e500 cores"), before that we had both e500 
and e500mc in the table at the same time.

But when e500mc was introduced by commit 3dfa8773674e ("powerpc/booke: 
Add support for new e500mc core"), it was already clear that it was not 
compatible with other e500, especially due to the size of the cacheline, 
which is hardcoded at buildtime on PPC32.

The comment in Kconfig.cputype was added my commit 9653018b615b 
("powerpc/e500: add paravirt QEMU platform")

And by the way, today you are able to build a kernel with an empty 
cpu_specs[] table if you select CONFIG_PPC_BOOK3E_64 and select neither 
CONFIG_CORENET_GENERIC nor CONFIG_PPC_QEMU_E500

static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_E500
#ifdef CONFIG_PPC32
#ifndef CONFIG_PPC_E500MC
	{	/* e500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80200000,
		.cpu_name		= "e500",
		.cpu_features		= CPU_FTRS_E500,
	},
	{	/* e500v2 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80210000,
		.cpu_name		= "e500v2",
		.cpu_features		= CPU_FTRS_E500_2,
	},
#else
	{	/* e500mc */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80230000,
		.cpu_name		= "e500mc",
		.cpu_features		= CPU_FTRS_E500MC,
	},
#endif /* CONFIG_PPC_E500MC */
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_PPC_E500MC
	{	/* e5500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80240000,
		.cpu_name		= "e5500",
		.cpu_features		= CPU_FTRS_E5500,
	},
	{	/* e6500 */
		.pvr_mask		= 0xffff0000,
		.pvr_value		= 0x80400000,
		.cpu_name		= "e6500",
		.cpu_features		= CPU_FTRS_E6500,
	},
#endif /* CONFIG_PPC_E500MC */
#ifdef CONFIG_PPC32
	{	/* default match */
		.pvr_mask		= 0x00000000,
		.pvr_value		= 0x00000000,
		.cpu_name		= "(generic E500 PPC)",
		.cpu_features		= CPU_FTRS_E500,
	}
#endif /* CONFIG_PPC32 */