| Message ID | f2d30eb7fadcfd89f7ac3784cb0b4a4b47d47db1.1662658653.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,01/19] powerpc/Kconfig: Fix non existing CONFIG_PPC_FSL_BOOKE | expand |
Hi Christophe, Thanks for trying to clean up this tangled mess. Christophe Leroy <christophe.leroy@csgroup.eu> writes: > The only 64-bit Book3E CPUs we support is the e500mc. AFAIK the e500mc is 32-bit? We support e5500 and e6500 which are 64-bit Book3E. They're derivatives of the e500mc AIUI. So CONFIG_PPC_E500MC actually means e500mc *and later derivatives*. You can see that with eg: config SPE_POSSIBLE def_bool y depends on E500 && !PPC_E500MC Because e500mc dropped SPE, and so therefore e5500 and e6500 don't have it either. And eg: #ifdef CONFIG_PPC_E500MC _GLOBAL(__setup_cpu_e6500) mflr r6 > However our Kconfig allows configurating a kernel that has 64-bit > Book3E support, but no e500mc support enabled. Such a kernel > would never boot, it doesn't know about any CPUs. That is true. > To fix this, force e500mc to be selected whenever we are > building a 64-bit Book3E kernel. I think that's a reasonable fix, just it's important to differentiate between CONFIG_PPC_E500MC (the symbol) and e500mc (the CPU). > And add a test to detect futur situations where cpu_specs is empty. future > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/kernel/cputable.c | 2 ++ > arch/powerpc/platforms/Kconfig.cputype | 2 ++ > 2 files changed, 4 insertions(+) > .. > diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype > index 5185d942b455..19fd95a06352 100644 > --- a/arch/powerpc/platforms/Kconfig.cputype > +++ b/arch/powerpc/platforms/Kconfig.cputype > @@ -108,6 +108,8 @@ config PPC_BOOK3S_64 > config PPC_BOOK3E_64 > bool "Embedded processors" > select PPC_FSL_BOOK3E > + select E500 > + select PPC_E500MC > select PPC_FPU # Make it a choice ? > select PPC_SMP_MUXED_IPI > select PPC_DOORBELL I think that makes the select of PPC_E500MC below redundant: config PPC_QEMU_E500 bool "QEMU generic e500 platform" select DEFAULT_UIMAGE select E500 select PPC_E500MC if PPC64 cheers
Le 09/09/2022 à 07:50, Michael Ellerman a écrit : > Hi Christophe, > > Thanks for trying to clean up this tangled mess. > > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> The only 64-bit Book3E CPUs we support is the e500mc. > > AFAIK the e500mc is 32-bit? Yes it seems. > > We support e5500 and e6500 which are 64-bit Book3E. > > They're derivatives of the e500mc AIUI. > > So CONFIG_PPC_E500MC actually means e500mc *and later derivatives*. > > You can see that with eg: > > config SPE_POSSIBLE > def_bool y > depends on E500 && !PPC_E500MC > > Because e500mc dropped SPE, and so therefore e5500 and e6500 don't have > it either. > > And eg: > > #ifdef CONFIG_PPC_E500MC > _GLOBAL(__setup_cpu_e6500) > mflr r6 > > >> However our Kconfig allows configurating a kernel that has 64-bit >> Book3E support, but no e500mc support enabled. Such a kernel >> would never boot, it doesn't know about any CPUs. > > That is true. > >> To fix this, force e500mc to be selected whenever we are >> building a 64-bit Book3E kernel. > > I think that's a reasonable fix, just it's important to differentiate > between CONFIG_PPC_E500MC (the symbol) and e500mc (the CPU). Ok, I'll see how I can make it more explicit. > >> And add a test to detect futur situations where cpu_specs is empty. > future >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> arch/powerpc/kernel/cputable.c | 2 ++ >> arch/powerpc/platforms/Kconfig.cputype | 2 ++ >> 2 files changed, 4 insertions(+) >> > .. >> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype >> index 5185d942b455..19fd95a06352 100644 >> --- a/arch/powerpc/platforms/Kconfig.cputype >> +++ b/arch/powerpc/platforms/Kconfig.cputype >> @@ -108,6 +108,8 @@ config PPC_BOOK3S_64 >> config PPC_BOOK3E_64 >> bool "Embedded processors" >> select PPC_FSL_BOOK3E >> + select E500 >> + select PPC_E500MC >> select PPC_FPU # Make it a choice ? >> select PPC_SMP_MUXED_IPI >> select PPC_DOORBELL > > I think that makes the select of PPC_E500MC below redundant: > > config PPC_QEMU_E500 > bool "QEMU generic e500 platform" > select DEFAULT_UIMAGE > select E500 > select PPC_E500MC if PPC64 That's handled in [v1,10/19] powerpc: Remove redundant selection of E500 and E500MC. Should I reorder patches ? Thanks Christophe
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 09/09/2022 à 07:50, Michael Ellerman a écrit : >> Hi Christophe, >> >> Thanks for trying to clean up this tangled mess. >> >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> The only 64-bit Book3E CPUs we support is the e500mc. >> >> AFAIK the e500mc is 32-bit? > > Yes it seems. > >> >> We support e5500 and e6500 which are 64-bit Book3E. >> >> They're derivatives of the e500mc AIUI. >> >> So CONFIG_PPC_E500MC actually means e500mc *and later derivatives*. >> >> You can see that with eg: >> >> config SPE_POSSIBLE >> def_bool y >> depends on E500 && !PPC_E500MC >> >> Because e500mc dropped SPE, and so therefore e5500 and e6500 don't have >> it either. >> >> And eg: >> >> #ifdef CONFIG_PPC_E500MC >> _GLOBAL(__setup_cpu_e6500) >> mflr r6 >> >> >>> However our Kconfig allows configurating a kernel that has 64-bit >>> Book3E support, but no e500mc support enabled. Such a kernel >>> would never boot, it doesn't know about any CPUs. >> >> That is true. >> >>> To fix this, force e500mc to be selected whenever we are >>> building a 64-bit Book3E kernel. >> >> I think that's a reasonable fix, just it's important to differentiate >> between CONFIG_PPC_E500MC (the symbol) and e500mc (the CPU). > > Ok, I'll see how I can make it more explicit. > >> >>> And add a test to detect futur situations where cpu_specs is empty. >> future >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> arch/powerpc/kernel/cputable.c | 2 ++ >>> arch/powerpc/platforms/Kconfig.cputype | 2 ++ >>> 2 files changed, 4 insertions(+) >>> >> .. >>> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype >>> index 5185d942b455..19fd95a06352 100644 >>> --- a/arch/powerpc/platforms/Kconfig.cputype >>> +++ b/arch/powerpc/platforms/Kconfig.cputype >>> @@ -108,6 +108,8 @@ config PPC_BOOK3S_64 >>> config PPC_BOOK3E_64 >>> bool "Embedded processors" >>> select PPC_FSL_BOOK3E >>> + select E500 >>> + select PPC_E500MC >>> select PPC_FPU # Make it a choice ? >>> select PPC_SMP_MUXED_IPI >>> select PPC_DOORBELL >> >> I think that makes the select of PPC_E500MC below redundant: >> >> config PPC_QEMU_E500 >> bool "QEMU generic e500 platform" >> select DEFAULT_UIMAGE >> select E500 >> select PPC_E500MC if PPC64 > > That's handled in [v1,10/19] powerpc: Remove redundant selection of > E500 and E500MC. Should I reorder patches ? No that's fine the way it is, I hadn't looked at patch 10 when I replied. cheers
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c index d8e42ef750f1..2829ea537277 100644 --- a/arch/powerpc/kernel/cputable.c +++ b/arch/powerpc/kernel/cputable.c @@ -2018,6 +2018,8 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr) struct cpu_spec *s = cpu_specs; int i; + BUILD_BUG_ON(!ARRAY_SIZE(cpu_specs)); + s = PTRRELOC(s); for (i = 0; i < ARRAY_SIZE(cpu_specs); i++,s++) { diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index 5185d942b455..19fd95a06352 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -108,6 +108,8 @@ config PPC_BOOK3S_64 config PPC_BOOK3E_64 bool "Embedded processors" select PPC_FSL_BOOK3E + select E500 + select PPC_E500MC select PPC_FPU # Make it a choice ? select PPC_SMP_MUXED_IPI select PPC_DOORBELL
The only 64-bit Book3E CPUs we support is the e500mc. However our Kconfig allows configurating a kernel that has 64-bit Book3E support, but no e500mc support enabled. Such a kernel would never boot, it doesn't know about any CPUs. To fix this, force e500mc to be selected whenever we are building a 64-bit Book3E kernel. And add a test to detect futur situations where cpu_specs is empty. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- arch/powerpc/kernel/cputable.c | 2 ++ arch/powerpc/platforms/Kconfig.cputype | 2 ++ 2 files changed, 4 insertions(+)