2023-07-25 14:23:09

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 0/5] remaining x86 -Wmissing-prototype warnings

From: Arnd Bergmann <[email protected]>

Most of the patches addressing -Wmissing-prototype warnings were
already merged, including 16 of the 20 x86 patches I sent before,
and all except 10 other patches that I need to resubmit to other
trees.

I checked that these five are all still required and that there
were no remaining comments I needed to address. Four patches
are from v1 of the series, the last one was sent separately
last time, and the contents are unchanged from the original
submission.

These patches have passed a few thousand randconfig builds with
the warning enabled.

Link: https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/lkml/[email protected]/

Arnd Bergmann (5):
[RESEND] x86: apic: hide unused safe_smp_processor_id on UP
[RESEND] x86: avoid unneeded __div64_32 function definition
[RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings
[RESEND] x86: purgatory: include header for warn() declaration
[RESEND] x86: alternative: add __alt_reloc_selftest prototype

arch/x86/boot/compressed/error.c | 2 +-
arch/x86/boot/compressed/error.h | 2 +-
arch/x86/include/asm/div64.h | 2 ++
arch/x86/include/asm/qspinlock_paravirt.h | 2 ++
arch/x86/kernel/alternative.c | 1 +
arch/x86/kernel/apic/ipi.c | 2 ++
arch/x86/kernel/paravirt.c | 2 ++
arch/x86/purgatory/purgatory.c | 1 +
kernel/locking/qspinlock_paravirt.h | 20 ++++++++++----------
9 files changed, 22 insertions(+), 12 deletions(-)

--
2.39.2

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: [email protected]
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]


2023-07-25 14:24:58

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 5/5] [RESEND] x86: alternative: add __alt_reloc_selftest prototype

From: Arnd Bergmann <[email protected]>

The newly introduced selftest function causes a warning when -Wmissing-prototypes
is enabled:

arch/x86/kernel/alternative.c:1461:32: error: no previous prototype for '__alt_reloc_selftest' [-Werror=missing-prototypes]

Since it's only used locally, add the prototype directly in front of it.

Fixes: 270a69c4485d ("x86/alternative: Support relocations in alternatives")
Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/kernel/alternative.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 2dcf3a06af090..934c23f24a3f8 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1531,6 +1531,7 @@ static noinline void __init int3_selftest(void)

static __initdata int __alt_reloc_selftest_addr;

+extern void __init __alt_reloc_selftest(void *arg);
__visible noinline void __init __alt_reloc_selftest(void *arg)
{
WARN_ON(arg != &__alt_reloc_selftest_addr);
--
2.39.2


2023-07-25 14:26:09

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 4/5] [RESEND] x86: purgatory: include header for warn() declaration

From: Arnd Bergmann <[email protected]>

The purgatory code has uses parts of the decompressor and provides
its own warn() function, but has to include the corresponding
header file to avoid a -Wmissing-prototypes warning.

It turns out that this the function prototype actually differs
from the declaration, so change it to get a constant pointer
in the declaration and the other definition as well.

Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/boot/compressed/error.c | 2 +-
arch/x86/boot/compressed/error.h | 2 +-
arch/x86/purgatory/purgatory.c | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/error.c b/arch/x86/boot/compressed/error.c
index 5313c5cb2b802..19a8251de506a 100644
--- a/arch/x86/boot/compressed/error.c
+++ b/arch/x86/boot/compressed/error.c
@@ -7,7 +7,7 @@
#include "misc.h"
#include "error.h"

-void warn(char *m)
+void warn(const char *m)
{
error_putstr("\n\n");
error_putstr(m);
diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h
index 86fe33b937154..31f9e080d61a8 100644
--- a/arch/x86/boot/compressed/error.h
+++ b/arch/x86/boot/compressed/error.h
@@ -4,7 +4,7 @@

#include <linux/compiler.h>

-void warn(char *m);
+void warn(const char *m);
void error(char *m) __noreturn;
void panic(const char *fmt, ...) __noreturn __cold;

diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c
index 7558139920f8c..aea47e7939637 100644
--- a/arch/x86/purgatory/purgatory.c
+++ b/arch/x86/purgatory/purgatory.c
@@ -14,6 +14,7 @@
#include <crypto/sha2.h>
#include <asm/purgatory.h>

+#include "../boot/compressed/error.h"
#include "../boot/string.h"

u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(".kexec-purgatory");
--
2.39.2


2023-07-25 14:28:10

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 3/5] [RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings

From: Arnd Bergmann <[email protected]>

__pv_queued_spin_unlock_slowpath is defined in a header file as a global
function, and designed to be called from an inline asm, but there is
no prototype visible in the definition:

kernel/locking/qspinlock_paravirt.h:493:1: error: no previous prototype for '__pv_queued_spin_unlock_slowpath' [-Werror=missing-prototypes]

Add this to the x86 header that contains the inline asm calling it,
and ensure this gets included before the definition, rather than
after it.

The native_pv_lock_init function in turn is only declared in SMP
builds but can be left out in UP to avoid another warning:

arch/x86/kernel/paravirt.c:76:13: error: no previous prototype for 'native_pv_lock_init' [-Werror=missing-prototypes]

Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/include/asm/qspinlock_paravirt.h | 2 ++
arch/x86/kernel/paravirt.c | 2 ++
kernel/locking/qspinlock_paravirt.h | 20 ++++++++++----------
3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/qspinlock_paravirt.h b/arch/x86/include/asm/qspinlock_paravirt.h
index 42b17cf10b10e..85b6e3609cb92 100644
--- a/arch/x86/include/asm/qspinlock_paravirt.h
+++ b/arch/x86/include/asm/qspinlock_paravirt.h
@@ -4,6 +4,8 @@

#include <asm/ibt.h>

+void __lockfunc __pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked);
+
/*
* For x86-64, PV_CALLEE_SAVE_REGS_THUNK() saves and restores 8 64-bit
* registers. For i386, however, only 1 32-bit register needs to be saved
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 89842bb7ec9cc..64a6bba70d183 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -73,11 +73,13 @@ DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);

DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);

+#ifdef CONFIG_SMP
void __init native_pv_lock_init(void)
{
if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
static_branch_disable(&virt_spin_lock_key);
}
+#endif

unsigned int paravirt_patch(u8 type, void *insn_buff, unsigned long addr,
unsigned int len)
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index 6afc249ce697d..6a0184e9c2348 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -485,6 +485,16 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
return (u32)(atomic_read(&lock->val) | _Q_LOCKED_VAL);
}

+/*
+ * Include the architecture specific callee-save thunk of the
+ * __pv_queued_spin_unlock(). This thunk is put together with
+ * __pv_queued_spin_unlock() to make the callee-save thunk and the real unlock
+ * function close to each other sharing consecutive instruction cachelines.
+ * Alternatively, architecture specific version of __pv_queued_spin_unlock()
+ * can be defined.
+ */
+#include <asm/qspinlock_paravirt.h>
+
/*
* PV versions of the unlock fastpath and slowpath functions to be used
* instead of queued_spin_unlock().
@@ -533,16 +543,6 @@ __pv_queued_spin_unlock_slowpath(struct qspinlock *lock, u8 locked)
pv_kick(node->cpu);
}

-/*
- * Include the architecture specific callee-save thunk of the
- * __pv_queued_spin_unlock(). This thunk is put together with
- * __pv_queued_spin_unlock() to make the callee-save thunk and the real unlock
- * function close to each other sharing consecutive instruction cachelines.
- * Alternatively, architecture specific version of __pv_queued_spin_unlock()
- * can be defined.
- */
-#include <asm/qspinlock_paravirt.h>
-
#ifndef __pv_queued_spin_unlock
__visible __lockfunc void __pv_queued_spin_unlock(struct qspinlock *lock)
{
--
2.39.2


2023-07-25 14:30:29

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/5] [RESEND] x86: apic: hide unused safe_smp_processor_id on UP

From: Arnd Bergmann <[email protected]>

When CONFIG_SMP is disabled, the prototype for safe_smp_processor_id()
is hidden, which causes a W=1 warning:

/home/arnd/arm-soc/arch/x86/kernel/apic/ipi.c:316:5: error: no previous prototype for 'safe_smp_processor_id' [-Werror=missing-prototypes]

Since there are no callers in this configuration, just hide the definition
as well.

Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/x86/kernel/apic/ipi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/apic/ipi.c b/arch/x86/kernel/apic/ipi.c
index 2a6509e8c840d..9bfd6e3973845 100644
--- a/arch/x86/kernel/apic/ipi.c
+++ b/arch/x86/kernel/apic/ipi.c
@@ -301,6 +301,7 @@ void default_send_IPI_mask_logical(const struct cpumask *cpumask, int vector)
local_irq_restore(flags);
}

+#ifdef CONFIG_SMP
/* must come after the send_IPI functions above for inlining */
static int convert_apicid_to_cpu(int apic_id)
{
@@ -329,3 +330,4 @@ int safe_smp_processor_id(void)
return cpuid >= 0 ? cpuid : 0;
}
#endif
+#endif
--
2.39.2


2023-08-01 19:44:27

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/5] [RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings

On Tue, Jul 25, 2023 at 03:48:35PM +0200, Arnd Bergmann wrote:
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 89842bb7ec9cc..64a6bba70d183 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -73,11 +73,13 @@ DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);
>
> DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
>
> +#ifdef CONFIG_SMP
> void __init native_pv_lock_init(void)
> {
> if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> static_branch_disable(&virt_spin_lock_key);
> }
> +#endif

Can you add an empty UP stub instead?

We all have a great aversion against ifdeffery...

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-08-01 20:53:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/5] [RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings

On Tue, Aug 1, 2023, at 21:22, Borislav Petkov wrote:
> On Tue, Jul 25, 2023 at 03:48:35PM +0200, Arnd Bergmann wrote:
>> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> index 89842bb7ec9cc..64a6bba70d183 100644
>> --- a/arch/x86/kernel/paravirt.c
>> +++ b/arch/x86/kernel/paravirt.c
>> @@ -73,11 +73,13 @@ DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);
>>
>> DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
>>
>> +#ifdef CONFIG_SMP
>> void __init native_pv_lock_init(void)
>> {
>> if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
>> static_branch_disable(&virt_spin_lock_key);
>> }
>> +#endif
>
> Can you add an empty UP stub instead?
>
> We all have a great aversion against ifdeffery...

There is already a stub for !CONFIG_PARAVIRT in asm/qspinlock.h,
but the problem is that this header does not get included
anywhere in UP configurations.

The variant below would avoid adding more #ifdefs, by moving
the declaration into asm/paravirt.h to ensure that it's
declared even if there is no caller.

Does this look better to you?

Arnd

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index b49778664d2be..fc3a377bb9b79 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -739,6 +739,7 @@ static __always_inline unsigned long arch_local_irq_save(void)
".popsection")

extern void default_banner(void);
+void native_pv_lock_init(void) __init;

#else /* __ASSEMBLY__ */

@@ -776,8 +777,13 @@ extern void default_banner(void);
#endif /* __ASSEMBLY__ */
#else /* CONFIG_PARAVIRT */
# define default_banner x86_init_noop
+
+static inline void native_pv_lock_init(void)
+{
+}
#endif /* !CONFIG_PARAVIRT */

#ifndef __ASSEMBLY__
diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
index d87451df480bd..cde8357bb226d 100644
--- a/arch/x86/include/asm/qspinlock.h
+++ b/arch/x86/include/asm/qspinlock.h
@@ -74,8 +74,6 @@ static inline bool vcpu_is_preempted(long cpu)
*/
DECLARE_STATIC_KEY_TRUE(virt_spin_lock_key);

-void native_pv_lock_init(void) __init;
-
/*
* Shortcut for the queued_spin_lock_slowpath() function that allows
* virt to hijack it.
@@ -103,10 +101,7 @@ static inline bool virt_spin_lock(struct qspinlock *lock)

return true;
}
-#else
-static inline void native_pv_lock_init(void)
-{
-}
+
#endif /* CONFIG_PARAVIRT */

#include <asm-generic/qspinlock.h>
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 89842bb7ec9cc..066fc19d2568e 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -75,7 +75,8 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);

void __init native_pv_lock_init(void)
{
- if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
+ if (IS_ENABLED(CONFIG_PARAVIRT_SPINLOCKS) &&
+ !boot_cpu_has(X86_FEATURE_HYPERVISOR))
static_branch_disable(&virt_spin_lock_key);
}


2023-08-02 18:53:32

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/5] [RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings

On Wed, Aug 02, 2023 at 07:26:12PM +0200, Borislav Petkov wrote:
> Now lemme look at the rest.

Yap, they look good. I'll queue your next version.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2023-08-02 19:08:21

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 3/5] [RESEND] x86: qspinlock-paravirt: fix mising-prototype warnings

On Tue, Aug 01, 2023 at 10:26:36PM +0200, Arnd Bergmann wrote:
> The variant below would avoid adding more #ifdefs, by moving
> the declaration into asm/paravirt.h to ensure that it's
> declared even if there is no caller.
>
> Does this look better to you?

Yes, thanks. I don't mind empty stubs in the header which the compiler
discards/ignores.

Now lemme look at the rest.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Subject: [tip: x86/cleanups] x86/apic: Hide unused safe_smp_processor_id() on 32-bit UP

The following commit has been merged into the x86/cleanups branch of tip:

Commit-ID: ac1c6283c45a77431af33fbae435f1a29f23a9f4
Gitweb: https://git.kernel.org/tip/ac1c6283c45a77431af33fbae435f1a29f23a9f4
Author: Arnd Bergmann <[email protected]>
AuthorDate: Tue, 25 Jul 2023 15:48:33 +02:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 31 Jul 2023 11:32:25 +02:00

x86/apic: Hide unused safe_smp_processor_id() on 32-bit UP

When CONFIG_SMP is disabled in a 32-bit config, the prototype for
safe_smp_processor_id() is hidden, which causes a W=1 warning:

arch/x86/kernel/apic/ipi.c:316:5: error: no previous prototype for 'safe_smp_processor_id' [-Werror=missing-prototypes]

Since there are no callers in this configuration, just hide the definition
as well.

[ bp: Clarify it is a 32-bit config. ]

Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/kernel/apic/ipi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/apic/ipi.c b/arch/x86/kernel/apic/ipi.c
index 2a6509e..9bfd6e3 100644
--- a/arch/x86/kernel/apic/ipi.c
+++ b/arch/x86/kernel/apic/ipi.c
@@ -301,6 +301,7 @@ void default_send_IPI_mask_logical(const struct cpumask *cpumask, int vector)
local_irq_restore(flags);
}

+#ifdef CONFIG_SMP
/* must come after the send_IPI functions above for inlining */
static int convert_apicid_to_cpu(int apic_id)
{
@@ -329,3 +330,4 @@ int safe_smp_processor_id(void)
return cpuid >= 0 ? cpuid : 0;
}
#endif
+#endif