On Fri, Sep 05, 2025, Aqib Faruqui wrote:

Thanks for the suggestion. I don't see your previous redefinition of PAGE_SIZE upstream, just 3 lines above the warning redefinition:

Oh, I manually added a conflicting definition to demonstrate the warning gcc will provide.

...

In file included from include/x86/svm_util.h:13, from include/x86/sev.h:15, from lib/x86/sev.c:5: include/x86/processor.h:373:9: error: "PAGE_SIZE" redefined [-Werror] 373 | #define PAGE_SIZE (1ULL << PAGE_SHIFT) | ^~~~~~~~~ include/x86/processor.h:370:9: note: this is the location of the previous definition 370 | #define PAGE_SIZE BIT(12) | ^~~~~~~~~

But I investigated further and found that both glibc and musl define PAGE_SIZE in sys/user.h:

glibc (sys/user.h): #define PAGE_SHIFT 12 #define PAGE_SIZE (1UL << PAGE_SHIFT)

musl (sys/user.h): #define PAGE_SIZE 4096

But that still doesn't fully explain how the previous definition comes into play. E.g. if I modify x86/processor.h to explicitly #include <sys/user.h>, then I see redefinition warnings:

In file included from x86/tsc_scaling_sync.c:8: include/x86/processor.h:373:9: warning: "PAGE_SIZE" redefined 373 | #define PAGE_SIZE (1ULL << PAGE_SHIFT) | ^~~~~~~~~ In file included from include/x86/processor.h:13: /usr/include/x86_64-linux-gnu/sys/user.h:173:9: note: this is the location of the previous definition 173 | #define PAGE_SIZE (1UL << PAGE_SHIFT) | ^~~~~~~~~

KVM selftests (include/x86/processor.h): #define PAGE_SHIFT 12 #define PAGE_SIZE (1ULL << PAGE_SHIFT)

This creates redefinition warnings with both C libraries on my system. I've already sent a v2 patch series with the PAGE_SIZE patch dropped but I'm not sure what the next course of action would be for this?

I don't see a clean way to resolve this other than to eliminate the #include of whatever is leading to musl defining PAGE_SIZE. I don't want to #undef and re-define PAGE_SIZE because then different compilation units (or even code chunks) could see different definitions. And as below, relying on libc for the #define isn't viable. So I think before we change any code, we need to first figure out exactly how musl's conflicting definition comes into existence, and then go from there.

Ideally, we would skip selftests' definition and assert that the existing definition is compatible, but that won't work because musl's definition isn't actually compatible with KVM selftests' definition, and more importantly isn't compatible with glibc's definition. KVM selftests and glibc both effectively #define PAGE_SIZE to be a u64 (KVM selftests only support 64-bit builds, so glibc's 1UL is an unsigned 64-bit value). But musl's definition is a signed int. I.e. we can't solve the 64-bit unsigned vs. 32-bit signed by relying on libc.

64-bit unsigned vs. 32-bit signed is unlikely to cause problems in practice, and in fact there are no problems in the current code base. But I don't love creating a potential pitfall for musl, especially since it's quite obviously not a common libc for KVM selftests, i.e. could lead to very latent bugs.

E.g. building with this

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index 488d516c4f6f..1b5e92debd20 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -368,7 +368,11 @@ static inline unsigned int x86_model(unsigned int eax) #define PHYSICAL_PAGE_MASK GENMASK_ULL(51, 12)

#define PAGE_SHIFT 12 +#define PAGE_SIZE 4096 +#ifndef PAGE_SIZE #define PAGE_SIZE (1ULL << PAGE_SHIFT) +#endif +kvm_static_assert(PAGE_SIZE == (1ULL << PAGE_SHIFT)); #define PAGE_MASK (~(PAGE_SIZE-1) & PHYSICAL_PAGE_MASK)

#define HUGEPAGE_SHIFT(x) (PAGE_SHIFT + (((x) - 1) * 9)) diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c index ce3ac0fd6dfb..822119e8853a 100644 --- a/tools/testing/selftests/kvm/set_memory_region_test.c +++ b/tools/testing/selftests/kvm/set_memory_region_test.c @@ -608,9 +608,12 @@ static void test_mmio_during_vectoring(void) int main(int argc, char *argv[]) { #ifdef __x86_64__ + u64 total_size = PAGE_SIZE * 1048576; int i, loops; int j, disable_slot_zap_quirk = 0;

+ printf("Total size = %lx\n", total_size); + if (kvm_check_cap(KVM_CAP_DISABLE_QUIRKS2) & KVM_X86_QUIRK_SLOT_ZAP_ALL) disable_slot_zap_quirk = 1; /*

Generates a warning:

set_memory_region_test.c: In function ‘main’: set_memory_region_test.c:611:36: warning: integer overflow in expression of type ‘int’ results in ‘0’ [-Woverflow] 611 | u64 total_size = PAGE_SIZE * 1048576; |

And yields:

$ ./set_memory_region_test Total size = 0

...

Please keep discussions on-list unless there's something that can't/shouldn't be posted publicly, e.g. for confidentiality or security reasons.

Apologies, doing this for the first time!

No worries, we've all been there :-)