* [PATCH] powerpc: Inline setup_kup()
@ 2020-12-14 12:30 Michael Ellerman
2020-12-14 13:30 ` Christophe Leroy
0 siblings, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2020-12-14 12:30 UTC (permalink / raw)
To: linuxppc-dev
setup_kup() is used by both 64-bit and 32-bit code. However on 64-bit
it must not be __init, because it's used for CPU hotplug, whereas on
32-bit it should be __init because it calls setup_kuap/kuep() which
are __init.
We worked around that problem in the past by marking it __ref, see
commit 67d53f30e23e ("powerpc/mm: fix section mismatch for
setup_kup()").
Marking it __ref basically just omits it from section mismatch
checking, which can lead to bugs, and in fact it did, see commit
44b4c4450f8d ("powerpc/64s: Mark the kuap/kuep functions non __init")
We can avoid all these problems by just making it static inline.
Because all it does is call other functions, making it inline actually
shrinks the 32-bit vmlinux by ~76 bytes.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/kup.h | 8 ++++++--
arch/powerpc/mm/init-common.c | 6 ------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index 5a9820c54da9..46b12c6dc728 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -49,8 +49,6 @@ extern bool disable_kuap;
#include <linux/pgtable.h>
-void setup_kup(void);
-
#ifdef CONFIG_PPC_KUEP
void setup_kuep(bool disabled);
#else
@@ -85,6 +83,12 @@ static inline void restore_user_access(unsigned long flags) { }
#endif /* CONFIG_PPC_BOOK3S_64 */
#endif /* CONFIG_PPC_KUAP */
+static inline void setup_kup(void)
+{
+ setup_kuep(disable_kuep);
+ setup_kuap(disable_kuap);
+}
+
static inline void allow_read_from_user(const void __user *from, unsigned long size)
{
allow_user_access(NULL, from, size, KUAP_READ);
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index afdebb95bcae..3a82f89827a5 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -47,12 +47,6 @@ static int __init parse_nosmap(char *p)
}
early_param("nosmap", parse_nosmap);
-void __ref setup_kup(void)
-{
- setup_kuep(disable_kuep);
- setup_kuap(disable_kuap);
-}
-
#define CTOR(shift) static void ctor_##shift(void *addr) \
{ \
memset(addr, 0, sizeof(void *) << (shift)); \
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: Inline setup_kup()
2020-12-14 12:30 [PATCH] powerpc: Inline setup_kup() Michael Ellerman
@ 2020-12-14 13:30 ` Christophe Leroy
2020-12-15 1:42 ` Michael Ellerman
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2020-12-14 13:30 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Le 14/12/2020 à 13:30, Michael Ellerman a écrit :
> setup_kup() is used by both 64-bit and 32-bit code. However on 64-bit
> it must not be __init, because it's used for CPU hotplug, whereas on
> 32-bit it should be __init because it calls setup_kuap/kuep() which
> are __init.
>
> We worked around that problem in the past by marking it __ref, see
> commit 67d53f30e23e ("powerpc/mm: fix section mismatch for
> setup_kup()").
>
> Marking it __ref basically just omits it from section mismatch
> checking, which can lead to bugs, and in fact it did, see commit
> 44b4c4450f8d ("powerpc/64s: Mark the kuap/kuep functions non __init")
>
> We can avoid all these problems by just making it static inline.
> Because all it does is call other functions, making it inline actually
> shrinks the 32-bit vmlinux by ~76 bytes.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/include/asm/kup.h | 8 ++++++--
> arch/powerpc/mm/init-common.c | 6 ------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
> index 5a9820c54da9..46b12c6dc728 100644
> --- a/arch/powerpc/include/asm/kup.h
> +++ b/arch/powerpc/include/asm/kup.h
> @@ -49,8 +49,6 @@ extern bool disable_kuap;
>
> #include <linux/pgtable.h>
>
> -void setup_kup(void);
> -
> #ifdef CONFIG_PPC_KUEP
> void setup_kuep(bool disabled);
> #else
> @@ -85,6 +83,12 @@ static inline void restore_user_access(unsigned long flags) { }
> #endif /* CONFIG_PPC_BOOK3S_64 */
> #endif /* CONFIG_PPC_KUAP */
>
> +static inline void setup_kup(void)
Should it be __always_inline ?
Christophe
> +{
> + setup_kuep(disable_kuep);
> + setup_kuap(disable_kuap);
> +}
> +
> static inline void allow_read_from_user(const void __user *from, unsigned long size)
> {
> allow_user_access(NULL, from, size, KUAP_READ);
> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
> index afdebb95bcae..3a82f89827a5 100644
> --- a/arch/powerpc/mm/init-common.c
> +++ b/arch/powerpc/mm/init-common.c
> @@ -47,12 +47,6 @@ static int __init parse_nosmap(char *p)
> }
> early_param("nosmap", parse_nosmap);
>
> -void __ref setup_kup(void)
> -{
> - setup_kuep(disable_kuep);
> - setup_kuap(disable_kuap);
> -}
> -
> #define CTOR(shift) static void ctor_##shift(void *addr) \
> { \
> memset(addr, 0, sizeof(void *) << (shift)); \
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: Inline setup_kup()
2020-12-14 13:30 ` Christophe Leroy
@ 2020-12-15 1:42 ` Michael Ellerman
2020-12-15 5:41 ` Christophe Leroy
0 siblings, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2020-12-15 1:42 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 14/12/2020 à 13:30, Michael Ellerman a écrit :
>> setup_kup() is used by both 64-bit and 32-bit code. However on 64-bit
>> it must not be __init, because it's used for CPU hotplug, whereas on
>> 32-bit it should be __init because it calls setup_kuap/kuep() which
>> are __init.
>>
>> We worked around that problem in the past by marking it __ref, see
>> commit 67d53f30e23e ("powerpc/mm: fix section mismatch for
>> setup_kup()").
>>
>> Marking it __ref basically just omits it from section mismatch
>> checking, which can lead to bugs, and in fact it did, see commit
>> 44b4c4450f8d ("powerpc/64s: Mark the kuap/kuep functions non __init")
>>
>> We can avoid all these problems by just making it static inline.
>> Because all it does is call other functions, making it inline actually
>> shrinks the 32-bit vmlinux by ~76 bytes.
>>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>> arch/powerpc/include/asm/kup.h | 8 ++++++--
>> arch/powerpc/mm/init-common.c | 6 ------
>> 2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
>> index 5a9820c54da9..46b12c6dc728 100644
>> --- a/arch/powerpc/include/asm/kup.h
>> +++ b/arch/powerpc/include/asm/kup.h
>> @@ -49,8 +49,6 @@ extern bool disable_kuap;
>>
>> #include <linux/pgtable.h>
>>
>> -void setup_kup(void);
>> -
>> #ifdef CONFIG_PPC_KUEP
>> void setup_kuep(bool disabled);
>> #else
>> @@ -85,6 +83,12 @@ static inline void restore_user_access(unsigned long flags) { }
>> #endif /* CONFIG_PPC_BOOK3S_64 */
>> #endif /* CONFIG_PPC_KUAP */
>>
>> +static inline void setup_kup(void)
>
> Should it be __always_inline ?
Yes I guess so, will fix. Thanks for reviewing.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: Inline setup_kup()
2020-12-15 1:42 ` Michael Ellerman
@ 2020-12-15 5:41 ` Christophe Leroy
2020-12-15 10:20 ` Michael Ellerman
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2020-12-15 5:41 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Le 15/12/2020 à 02:42, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 14/12/2020 à 13:30, Michael Ellerman a écrit :
>>> setup_kup() is used by both 64-bit and 32-bit code. However on 64-bit
>>> it must not be __init, because it's used for CPU hotplug, whereas on
>>> 32-bit it should be __init because it calls setup_kuap/kuep() which
>>> are __init.
>>>
>>> We worked around that problem in the past by marking it __ref, see
>>> commit 67d53f30e23e ("powerpc/mm: fix section mismatch for
>>> setup_kup()").
>>>
>>> Marking it __ref basically just omits it from section mismatch
>>> checking, which can lead to bugs, and in fact it did, see commit
>>> 44b4c4450f8d ("powerpc/64s: Mark the kuap/kuep functions non __init")
>>>
>>> We can avoid all these problems by just making it static inline.
>>> Because all it does is call other functions, making it inline actually
>>> shrinks the 32-bit vmlinux by ~76 bytes.
>>>
>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>> ---
>>> arch/powerpc/include/asm/kup.h | 8 ++++++--
>>> arch/powerpc/mm/init-common.c | 6 ------
>>> 2 files changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
>>> index 5a9820c54da9..46b12c6dc728 100644
>>> --- a/arch/powerpc/include/asm/kup.h
>>> +++ b/arch/powerpc/include/asm/kup.h
>>> @@ -49,8 +49,6 @@ extern bool disable_kuap;
>>>
>>> #include <linux/pgtable.h>
>>>
>>> -void setup_kup(void);
>>> -
>>> #ifdef CONFIG_PPC_KUEP
>>> void setup_kuep(bool disabled);
>>> #else
>>> @@ -85,6 +83,12 @@ static inline void restore_user_access(unsigned long flags) { }
>>> #endif /* CONFIG_PPC_BOOK3S_64 */
>>> #endif /* CONFIG_PPC_KUAP */
>>>
>>> +static inline void setup_kup(void)
>>
>> Should it be __always_inline ?
>
> Yes I guess so, will fix. Thanks for reviewing.
>
While we are talking about __always_inline, do you plan to take the following patch this cycle ?
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/a1d31f84ddb0926813b17fcd5cc7f3fa7b4deac2.1602759123.git.christophe.leroy@csgroup.eu/
Christophe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] powerpc: Inline setup_kup()
2020-12-15 5:41 ` Christophe Leroy
@ 2020-12-15 10:20 ` Michael Ellerman
0 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-12-15 10:20 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 15/12/2020 à 02:42, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 14/12/2020 à 13:30, Michael Ellerman a écrit :
>>>> setup_kup() is used by both 64-bit and 32-bit code. However on 64-bit
>>>> it must not be __init, because it's used for CPU hotplug, whereas on
>>>> 32-bit it should be __init because it calls setup_kuap/kuep() which
>>>> are __init.
>>>>
>>>> We worked around that problem in the past by marking it __ref, see
>>>> commit 67d53f30e23e ("powerpc/mm: fix section mismatch for
>>>> setup_kup()").
>>>>
>>>> Marking it __ref basically just omits it from section mismatch
>>>> checking, which can lead to bugs, and in fact it did, see commit
>>>> 44b4c4450f8d ("powerpc/64s: Mark the kuap/kuep functions non __init")
>>>>
>>>> We can avoid all these problems by just making it static inline.
>>>> Because all it does is call other functions, making it inline actually
>>>> shrinks the 32-bit vmlinux by ~76 bytes.
>>>>
>>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>>> ---
>>>> arch/powerpc/include/asm/kup.h | 8 ++++++--
>>>> arch/powerpc/mm/init-common.c | 6 ------
>>>> 2 files changed, 6 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
>>>> index 5a9820c54da9..46b12c6dc728 100644
>>>> --- a/arch/powerpc/include/asm/kup.h
>>>> +++ b/arch/powerpc/include/asm/kup.h
>>>> @@ -49,8 +49,6 @@ extern bool disable_kuap;
>>>>
>>>> #include <linux/pgtable.h>
>>>>
>>>> -void setup_kup(void);
>>>> -
>>>> #ifdef CONFIG_PPC_KUEP
>>>> void setup_kuep(bool disabled);
>>>> #else
>>>> @@ -85,6 +83,12 @@ static inline void restore_user_access(unsigned long flags) { }
>>>> #endif /* CONFIG_PPC_BOOK3S_64 */
>>>> #endif /* CONFIG_PPC_KUAP */
>>>>
>>>> +static inline void setup_kup(void)
>>>
>>> Should it be __always_inline ?
>>
>> Yes I guess so, will fix. Thanks for reviewing.
>>
>
> While we are talking about __always_inline, do you plan to take the following patch this cycle ?
>
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/a1d31f84ddb0926813b17fcd5cc7f3fa7b4deac2.1602759123.git.christophe.leroy@csgroup.eu/
I intended to, will grab it now.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-15 10:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-14 12:30 [PATCH] powerpc: Inline setup_kup() Michael Ellerman
2020-12-14 13:30 ` Christophe Leroy
2020-12-15 1:42 ` Michael Ellerman
2020-12-15 5:41 ` Christophe Leroy
2020-12-15 10:20 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox