From: Michael Ellerman <mpe@ellerman•id.au>
To: Geoff Levand <geoff@infradead•org>,
Christophe Leroy <christophe.leroy@csgroup•eu>
Cc: "linuxppc-dev@lists•ozlabs.org" <linuxppc-dev@lists•ozlabs.org>
Subject: Re: [PATCH v2 1/2] powerpc/ps3: Change updateboltedpp panic to info
Date: Mon, 16 Jan 2023 11:06:24 +1100 [thread overview]
Message-ID: <87mt6j9wcf.fsf@mpe.ellerman.id.au> (raw)
In-Reply-To: <284aed88-dc26-d74a-cf7a-f5875ba058eb@infradead.org>
Geoff Levand <geoff@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 assume actual PS3 users would use a ps3_defconfig anyway right?
Relatedly are there any actual PS3 users left? ;)
> I could do this in ps3_defconfig:
>
> +# CONFIG_STRICT_KERNEL_RWX is not set
> +# CONFIG_STRICT_MODULE_RWX is not set
>
> But I don't like that way because it seems too easy for users to not add those
> into a custom kernel config, and then they need to figure out what to do after
> their kernel panics on startup.
Yep agreed.
> 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.
cheers
next prev parent reply other threads:[~2023-01-16 0:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-03 17:51 [PATCH v2 0/2] PS3 patches Geoff Levand
2023-01-03 17:51 ` [PATCH v2 1/2] powerpc/ps3: Change updateboltedpp panic to info Geoff Levand
2023-01-09 17:41 ` Christophe Leroy
2023-01-14 21:22 ` Geoff Levand
2023-01-16 0:06 ` Michael Ellerman [this message]
2023-01-16 20:08 ` Geoff Levand
2023-01-17 7:26 ` Christophe Leroy
2023-01-28 22:45 ` Geoff Levand
2023-02-10 0:51 ` Michael Ellerman
2023-01-03 17:51 ` [PATCH v2 2/2] powerpc/ps3: Refresh ps3_defconfig Geoff Levand
2023-02-15 12:41 ` [PATCH v2 0/2] PS3 patches Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mt6j9wcf.fsf@mpe.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=geoff@infradead$(echo .)org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox