[PATCH v2 1/2] powerpc/ps3: Change updateboltedpp panic to info

[Christophe Leroy]

Le 16/01/2023 à 21:08, Geoff Levand a écrit :
> Hi,
> 
> On 1/15/23 16:06, Michael Ellerman wrote:
>> Geoff Levand <geoff at infradead.org> writes:
>>> On 1/9/23 09:41, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 03/01/2023 à 18:51, Geoff Levand a écrit :
>>>>> Commit fdacae8a84024474afff234bdd1dbe19ad597a10 (powerpc: Activate
>>>>> CONFIG_STRICT_KERNEL_RWX by default) causes ps3_hpte_updateboltedpp()
>>>>> to be called.  Change the panic statment in ps3_hpte_updateboltedpp()
>>>>> to a pr_info statement so that bootup can continue.
>>>>
>>>> But if I understand correctly, it means that CONFIG_STRICT_KERNEL_RWX
>>>> won't work then.
>>>>
>>>> So, shouldn't we keep the panic and forbid CONFIG_STRICT_KERNEL_RWX on PS3 ?
>>>
>>> mmu_hash_ops.updateboltedpp returns void, so I can't return an error code to
>>> indicate the feature is not supported.
>>
>> We could change that in the medium term.
>>
>>> I could do something like this in arch/powerpc/Kconfig:
>>>
>>> -       select ARCH_HAS_STRICT_KERNEL_RWX       if (PPC_BOOK3S || PPC_8xx || 40x) && !HIBERNATION
>>> +       select ARCH_HAS_STRICT_KERNEL_RWX       if (PPC_BOOK3S || PPC_8xx || 40x) && !PPC_PS3 && !HIBERNATION
>>>
>>> But then the ppc64_defconfig would be built without STRICT_KERNEL_RWX.
>>
>> Yeah that would be a pity.
>>
>> We could do the above and disable PS3 in ppc64_defconfig, allowing
>> ppc64_defconfig to still have STRICT_KERNEL_RWX.
> 
> I really want to keep PS3 included in ppc64_defconfig.  Not that I expect
> anyone to boot a ppc64_defconfig kernel on PS3, but that is one of the
> 'standard' configs that is built by some automated builders, and generally by
> anyone doing changes to the powerpc arch, and I want to keep getting those
> build tests for PS3.
> 
>> I assume actual PS3 users would use a ps3_defconfig anyway right?
> 
> Yeah, a derivative of it.  They are most likely are using 'Jailbreak' firmware
> that allows them to run Linux in the gameos partition.
> 
>> Relatedly are there any actual PS3 users left? ;)
> 
> It seems there are more users now than a few years ago.  I think they buy PS5s
> to play the latest games, and use their old console to mess around with Linux.
> I generally get a private inquiry every 3 or 4 weeks.  Usually asking how to
> update their kernel, or how to install a modern distro.
> 
>>> What other 'clean' way is there?
>>
>> If we want to have a multi-platform kernel image that can boot on PS3
>> and other platforms, and have strict kernel RWX, then we need some
>> runtime logic to deal with that.
>>
>> I'd rather not do that though, because it adds complexity to deal with a
>> pretty obscure corner case, and I suspect no one really boots a
>> ppc64_defconfig on actual PS3 hardware these days.
>>
>> So my preference is we disable PS3 in ppc64_defconfig, and make PS3
>> incompatible with STRICT_KERNEL_RWX.
> 
> As mentioned, I'd really like to keep PS3 included in ppc64_defconfig.  My
> original patch that basically just ignores the call to
> mmu_hash_ops.updateboltedpp allows that, and I haven't experienced any problems
> with it yet.

When you say you have not experienced any problems with it, do you mean 
that STRICT_RWX works for you ? When you select CONFIG_DEBUG_RODATA_TEST 
it tells you that it works ? Otherwise it looks incorrect to enable 
something that doesn't work.

> 
> My preference would be to keep PS3 in ppc64_defconfig, and either apply my
> original patch, or I keep that patch in my ps3-linux repo on kernel.org. Then,
> if we end up adding runtime support for RWX I then fixup PS3 to use that.
> 

In that case I see two solutions:
1/ Implement updateboltedpp for PS3.
2/ Implement arch_parse_debug_rodata() to always set rodata_enabled = 
false on PS3, and update free_initmem() to only call mark_initmem_nx() 
when strict_kernel_rwx_enabled() returns true.

Christophe