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 :-)
