public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/9] KVM: selftests: Add __packed attribute fallback
       [not found] <20250829142556.72577-1-aqibaf@amazon.com>
@ 2025-08-29 14:25 ` Aqib Faruqui
  2025-08-29 22:46   ` Sean Christopherson
  2025-08-29 14:25 ` [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback Aqib Faruqui
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Aqib Faruqui @ 2025-08-29 14:25 UTC (permalink / raw)
  To: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel
  Cc: nh-open-source, aqibaf

Kernel UAPI headers use __packed but don't provide the definition in
userspace builds.

Add a fallback definition matching the kernel's implementation. This
follows the same pattern used by BPF and SGX selftests.

Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 23a506d7e..7fae7f5e7 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -5,6 +5,10 @@
 #ifndef SELFTEST_KVM_UTIL_H
 #define SELFTEST_KVM_UTIL_H
 
+#ifndef __packed
+#define __packed __attribute__((__packed__))
+#endif
+
 #include "test_util.h"
 
 #include <linux/compiler.h>
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback
       [not found] <20250829142556.72577-1-aqibaf@amazon.com>
  2025-08-29 14:25 ` [PATCH 2/9] KVM: selftests: Add __packed attribute fallback Aqib Faruqui
@ 2025-08-29 14:25 ` Aqib Faruqui
  2025-08-29 22:37   ` Sean Christopherson
  2025-08-29 14:25 ` [PATCH 4/9] selftests: kselftest: Add memfd_create syscall compatibility Aqib Faruqui
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Aqib Faruqui @ 2025-08-29 14:25 UTC (permalink / raw)
  To: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel
  Cc: nh-open-source, aqibaf

The pthread_attr_setaffinity_np function is a GNU extension that may not
be available in non-glibc C libraries. Some KVM selftests use this
function for CPU affinity control.

Add a function declaration and weak stub implementation for non-glibc
builds. This allows tests to build, with the affinity setting being a
no-op and errno set for the caller when the actual function is not available.

Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
---
 tools/testing/selftests/kvm/include/kvm_util.h |  4 ++++
 tools/testing/selftests/kvm/lib/kvm_util.c     | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 7fae7f5e7..8177178b5 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -31,6 +31,10 @@
 #include "kvm_util_types.h"
 #include "sparsebit.h"
 
+#ifndef __GLIBC__
+int pthread_attr_setaffinity_np(pthread_attr_t *attr, size_t cpusetsize, const cpu_set_t *cpuset);
+#endif /* __GLIBC__ */
+
 #define KVM_DEV_PATH "/dev/kvm"
 #define KVM_MAX_VCPUS 512
 
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index c3f5142b0..5ce80303d 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -20,6 +20,17 @@
 
 #define KVM_UTIL_MIN_PFN	2
 
+#ifndef __GLIBC__
+int __attribute__((weak))
+pthread_attr_setaffinity_np(pthread_attr_t *__attr,
+				size_t __cpusetsize,
+				const cpu_set_t *__cpuset)
+{
+	errno = ENOSYS;
+	return -1;
+}
+#endif
+
 uint32_t guest_random_seed;
 struct guest_random_state guest_rng;
 static uint32_t last_guest_seed;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/9] selftests: kselftest: Add memfd_create syscall compatibility
       [not found] <20250829142556.72577-1-aqibaf@amazon.com>
  2025-08-29 14:25 ` [PATCH 2/9] KVM: selftests: Add __packed attribute fallback Aqib Faruqui
  2025-08-29 14:25 ` [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback Aqib Faruqui
@ 2025-08-29 14:25 ` Aqib Faruqui
  2025-08-29 14:25 ` [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86 Aqib Faruqui
  2025-08-29 14:25 ` [PATCH 6/9] KVM: selftests: Add backtrace fallback Aqib Faruqui
  4 siblings, 0 replies; 12+ messages in thread
From: Aqib Faruqui @ 2025-08-29 14:25 UTC (permalink / raw)
  To: Shuah Khan, Paolo Bonzini, linux-kselftest, linux-kernel, kvm
  Cc: nh-open-source, aqibaf

The memfd_create function and related MFD_* flags may not be available
in non-glibc C libraries. Some selftests use memfd_create for
memory backing operations.

Add fallback definitions for MFD_CLOEXEC and MFD_HUGETLB flags, and
provide a memfd_create wrapper.

Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
---
 tools/testing/selftests/kselftest.h        | 19 +++++++++++++++++++
 tools/testing/selftests/kvm/lib/kvm_util.c |  1 +
 2 files changed, 20 insertions(+)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index c3b6d2604..f362c6766 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -57,6 +57,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <sys/utsname.h>
+#include <sys/syscall.h>
 #endif
 
 #ifndef ARRAY_SIZE
@@ -80,6 +81,24 @@
 #endif
 #endif /* end arch */
 
+#ifndef MFD_CLOEXEC
+#define MFD_CLOEXEC 0x0001U
+#endif
+
+#ifndef MFD_HUGETLB
+#define MFD_HUGETLB 0x0004U
+#endif
+
+static inline int memfd_create(const char *name, unsigned int flags)
+{
+#ifdef __NR_memfd_create
+	return syscall(__NR_memfd_create, name, flags);
+#else
+	errno = ENOSYS;
+	return -1;
+#endif
+}
+
 /* define kselftest exit codes */
 #define KSFT_PASS  0
 #define KSFT_FAIL  1
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 5ce80303d..cb5209f6a 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -15,6 +15,7 @@
 #include <sys/resource.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 #include <unistd.h>
 #include <linux/kernel.h>
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86
       [not found] <20250829142556.72577-1-aqibaf@amazon.com>
                   ` (2 preceding siblings ...)
  2025-08-29 14:25 ` [PATCH 4/9] selftests: kselftest: Add memfd_create syscall compatibility Aqib Faruqui
@ 2025-08-29 14:25 ` Aqib Faruqui
  2025-08-29 20:38   ` Sean Christopherson
  2025-08-29 14:25 ` [PATCH 6/9] KVM: selftests: Add backtrace fallback Aqib Faruqui
  4 siblings, 1 reply; 12+ messages in thread
From: Aqib Faruqui @ 2025-08-29 14:25 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini, Shuah Khan, kvm,
	linux-kselftest, linux-kernel
  Cc: nh-open-source, aqibaf

Prevent PAGE_SIZE redefinition warnings that can occur due to namespace
pollution from included headers.

Add an #ifndef directive before defining PAGE_SIZE to avoid redefinition
conflicts.

Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
---
 tools/testing/selftests/kvm/include/x86/processor.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 2efb05c2f..3f93d1b4f 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -368,7 +368,9 @@ static inline unsigned int x86_model(unsigned int eax)
 #define PHYSICAL_PAGE_MASK      GENMASK_ULL(51, 12)
 
 #define PAGE_SHIFT		12
+#ifndef PAGE_SIZE
 #define PAGE_SIZE		(1ULL << PAGE_SHIFT)
+#endif
 #define PAGE_MASK		(~(PAGE_SIZE-1) & PHYSICAL_PAGE_MASK)
 
 #define HUGEPAGE_SHIFT(x)	(PAGE_SHIFT + (((x) - 1) * 9))
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/9] KVM: selftests: Add backtrace fallback
       [not found] <20250829142556.72577-1-aqibaf@amazon.com>
                   ` (3 preceding siblings ...)
  2025-08-29 14:25 ` [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86 Aqib Faruqui
@ 2025-08-29 14:25 ` Aqib Faruqui
  4 siblings, 0 replies; 12+ messages in thread
From: Aqib Faruqui @ 2025-08-29 14:25 UTC (permalink / raw)
  To: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel
  Cc: nh-open-source, aqibaf

The backtrace() function is a GNU extension available in glibc but may
not be present in non-glibc libraries. KVM selftests use backtrace() for
error reporting and debugging.

Add conditional inclusion of execinfo.h only for glibc builds and
provide a weak stub implementation of backtrace() that returns 0 (stack
trace empty) for non-glibc systems.

Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
---
 tools/testing/selftests/kvm/lib/assert.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index b49690658..c9778dc6c 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -6,11 +6,19 @@
  */
 #include "test_util.h"
 
-#include <execinfo.h>
 #include <sys/syscall.h>
 
+#ifdef __GLIBC__
+#include <execinfo.h> /* backtrace */
+#endif
+
 #include "kselftest.h"
 
+int __attribute__((weak)) backtrace(void **buffer, int size)
+{
+	return 0;
+}
+
 /* Dumps the current stack trace to stderr. */
 static void __attribute__((noinline)) test_dump_stack(void);
 static void test_dump_stack(void)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86
  2025-08-29 14:25 ` [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86 Aqib Faruqui
@ 2025-08-29 20:38   ` Sean Christopherson
       [not found]     ` <33701547-13AA-467D-AC41-A1A05963B1DD@amazon.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2025-08-29 20:38 UTC (permalink / raw)
  To: Aqib Faruqui
  Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel,
	nh-open-source

On Fri, Aug 29, 2025, Aqib Faruqui wrote:
> Prevent PAGE_SIZE redefinition warnings that can occur due to namespace
> pollution from included headers.
> 
> Add an #ifndef directive before defining PAGE_SIZE to avoid redefinition
> conflicts.

Please provide more details on what is causing the conflicts.  Blindly using a
PAGE_SIZE without _knowing_ it's aligned with PAGE_SHIFT and PHYSICAL_PAGE_MASK
is far from ideal.

> Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
> ---
>  tools/testing/selftests/kvm/include/x86/processor.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 2efb05c2f..3f93d1b4f 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -368,7 +368,9 @@ static inline unsigned int x86_model(unsigned int eax)
>  #define PHYSICAL_PAGE_MASK      GENMASK_ULL(51, 12)
>  
>  #define PAGE_SHIFT		12
> +#ifndef PAGE_SIZE
>  #define PAGE_SIZE		(1ULL << PAGE_SHIFT)
> +#endif
>  #define PAGE_MASK		(~(PAGE_SIZE-1) & PHYSICAL_PAGE_MASK)
>  
>  #define HUGEPAGE_SHIFT(x)	(PAGE_SHIFT + (((x) - 1) * 9))
> -- 
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback
  2025-08-29 14:25 ` [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback Aqib Faruqui
@ 2025-08-29 22:37   ` Sean Christopherson
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2025-08-29 22:37 UTC (permalink / raw)
  To: Aqib Faruqui
  Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel,
	nh-open-source

On Fri, Aug 29, 2025, Aqib Faruqui wrote:
> The pthread_attr_setaffinity_np function is a GNU extension that may not
> be available in non-glibc C libraries. Some KVM selftests use this
> function for CPU affinity control.
> 
> Add a function declaration and weak stub implementation for non-glibc
> builds. This allows tests to build, with the affinity setting being a
> no-op and errno set for the caller when the actual function is not available.

Except this isn't a fallback, for all intents and purposes it silently breaks
the test.  A "fallback" is generally something that provides roughly equivalent
functionality.  This particular test will still pass because forced preemption
isn't strictly necessary, but this is still gross.  

Luckily, KVM selftests already provides APIs to pin tasks, just use those and
the problem naturally goes away.

--
From: Sean Christopherson <seanjc@google.com>
Date: Fri, 29 Aug 2025 15:31:44 -0700
Subject: [PATCH] KVM: selftests: Use KVM's task pinning APIs in steal_time
 time

Use pin_self_to_cpu() and pin_task_to_cpu() to pin the vCPU thread and
the stealer thread to pCPU0 in the steal_time.  Eliminating the usage of
pthread_attr_setaffinity_np() in particular allows building the test
again non-glibc C libraries.

Reported-by: Aqib Faruqui <aqibaf@amazon.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/steal_time.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index cce2520af720..663c99c81703 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -341,9 +341,7 @@ int main(int ac, char **av)
 {
 	struct kvm_vcpu *vcpus[NR_VCPUS];
 	struct kvm_vm *vm;
-	pthread_attr_t attr;
 	pthread_t thread;
-	cpu_set_t cpuset;
 	unsigned int gpages;
 	long stolen_time;
 	long run_delay;
@@ -353,11 +351,7 @@ int main(int ac, char **av)
 	verbose = ac > 1 && (!strncmp(av[1], "-v", 3) || !strncmp(av[1], "--verbose", 10));
 
 	/* Set CPU affinity so we can force preemption of the VCPU */
-	CPU_ZERO(&cpuset);
-	CPU_SET(0, &cpuset);
-	pthread_attr_init(&attr);
-	pthread_attr_setaffinity_np(&attr, sizeof(cpu_set_t), &cpuset);
-	pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
+	pin_self_to_cpu(0);
 
 	/* Create a VM and an identity mapped memslot for the steal time structure */
 	vm = vm_create_with_vcpus(NR_VCPUS, guest_code, vcpus);
@@ -389,7 +383,9 @@ int main(int ac, char **av)
 
 		/* Steal time from the VCPU. The steal time thread has the same CPU affinity as the VCPUs. */
 		run_delay = get_run_delay();
-		pthread_create(&thread, &attr, do_steal_time, NULL);
+		pthread_create(&thread, NULL, do_steal_time, NULL);
+		pin_task_to_cpu(thread, 0);
+
 		do
 			sched_yield();
 		while (get_run_delay() - run_delay < MIN_RUN_DELAY_NS);

base-commit: ecbcc2461839e848970468b44db32282e5059925
--

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/9] KVM: selftests: Add __packed attribute fallback
  2025-08-29 14:25 ` [PATCH 2/9] KVM: selftests: Add __packed attribute fallback Aqib Faruqui
@ 2025-08-29 22:46   ` Sean Christopherson
  2025-09-01 15:08     ` Faruqui, Aqib
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2025-08-29 22:46 UTC (permalink / raw)
  To: Aqib Faruqui
  Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel,
	nh-open-source

On Fri, Aug 29, 2025, Aqib Faruqui wrote:
> Kernel UAPI headers use __packed but don't provide the definition in
> userspace builds.
> 
> Add a fallback definition matching the kernel's implementation. This
> follows the same pattern used by BPF and SGX selftests.

Ugh.  No, this needs to be fixed in a central location, not splattered all over
random subsystem selftests.  My first choice would be to copy (and keep synchronize)
all of the include/linux/compiler*.h headers to tools/include/linux/.

If for some reason that's not a viable option, we should yank the __packed and
similar #defines out of tools/include/linux/compiler-gcc.h and place them in
tools/include/linux/compiler.h.  AFAICT, none of them are actually GCC-only.

> Signed-off-by: Aqib Faruqui <aqibaf@amazon.com>
> ---
>  tools/testing/selftests/kvm/include/kvm_util.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 23a506d7e..7fae7f5e7 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -5,6 +5,10 @@
>  #ifndef SELFTEST_KVM_UTIL_H
>  #define SELFTEST_KVM_UTIL_H
>  
> +#ifndef __packed
> +#define __packed __attribute__((__packed__))
> +#endif
> +
>  #include "test_util.h"
>  
>  #include <linux/compiler.h>
> -- 
> 2.47.3
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/9] KVM: selftests: Add __packed attribute fallback
  2025-08-29 22:46   ` Sean Christopherson
@ 2025-09-01 15:08     ` Faruqui, Aqib
  0 siblings, 0 replies; 12+ messages in thread
From: Faruqui, Aqib @ 2025-09-01 15:08 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Shuah Khan, kvm@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	nh-open-source@amazon.com

After investigating a little, looks like tools/include/linux/compiler.h already defines __packed correctly. While UAPI headers (e.g. linux/kvm.h) use __packed but don't include it themselves, the include ordering looks fine for this to be handled by the build system.

I'll drop this patch and investigate further why the tools infrastructure isn't working correctly in my setup.

Thanks for the feedback!

-- 
Aqib Faruqui 
Software Dev Intern (Embedded) | EC2 Accelerated Nitro | AWS 
+44 7763104413 




On 29/08/2025, 23:48, "Sean Christopherson" <seanjc@google.com <mailto:seanjc@google.com>> wrote:


CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.






On Fri, Aug 29, 2025, Aqib Faruqui wrote:
> Kernel UAPI headers use __packed but don't provide the definition in
> userspace builds.
>
> Add a fallback definition matching the kernel's implementation. This
> follows the same pattern used by BPF and SGX selftests.


Ugh. No, this needs to be fixed in a central location, not splattered all over
random subsystem selftests. My first choice would be to copy (and keep synchronize)
all of the include/linux/compiler*.h headers to tools/include/linux/.


If for some reason that's not a viable option, we should yank the __packed and
similar #defines out of tools/include/linux/compiler-gcc.h and place them in
tools/include/linux/compiler.h. AFAICT, none of them are actually GCC-only.


> Signed-off-by: Aqib Faruqui <aqibaf@amazon.com <mailto:aqibaf@amazon.com>>
> ---
> tools/testing/selftests/kvm/include/kvm_util.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 23a506d7e..7fae7f5e7 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -5,6 +5,10 @@
> #ifndef SELFTEST_KVM_UTIL_H
> #define SELFTEST_KVM_UTIL_H
>
> +#ifndef __packed
> +#define __packed __attribute__((__packed__))
> +#endif
> +
> #include "test_util.h"
>
> #include <linux/compiler.h>
> --
> 2.47.3
>




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86
       [not found]     ` <33701547-13AA-467D-AC41-A1A05963B1DD@amazon.com>
@ 2025-09-05  8:39       ` Sean Christopherson
  2025-09-05 13:59         ` Aqib Faruqui
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2025-09-05  8:39 UTC (permalink / raw)
  To: Faruqui, Aqib; +Cc: kvm, linux-kselftest, linux-kernel

+lists

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

On Tue, Sep 02, 2025, Faruqui, Aqib wrote:
> I suppose a fix for blindly using PAGE_SIZE in subsequent macros:
> 
> #ifdef PAGE_SIZE
> #undef PAGE_SIZE
> #endif
> #define PAGE_SIZE		(1ULL << PAGE_SHIFT)
> 
> Is no better and is instead blindly suppressing the compiler's redefinition warning. 
> 
> I'm having trouble finding what causes the conflict, any advice here?

Maybe try a newer compiler?  E.g. gcc-14.2 will spit out the exact location of the
previous definition.

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)
      |         ^~~~~~~~~

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86
  2025-09-05  8:39       ` Sean Christopherson
@ 2025-09-05 13:59         ` Aqib Faruqui
  2025-09-08 18:22           ` Sean Christopherson
  0 siblings, 1 reply; 12+ messages in thread
From: Aqib Faruqui @ 2025-09-05 13:59 UTC (permalink / raw)
  To: seanjc; +Cc: aqibaf, kvm, linux-kernel, linux-kselftest

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

> 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

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?

> 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! Hopefully this one works as it should.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86
  2025-09-05 13:59         ` Aqib Faruqui
@ 2025-09-08 18:22           ` Sean Christopherson
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2025-09-08 18:22 UTC (permalink / raw)
  To: Aqib Faruqui; +Cc: aqibaf, kvm, linux-kernel, linux-kselftest

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-08 18:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250829142556.72577-1-aqibaf@amazon.com>
2025-08-29 14:25 ` [PATCH 2/9] KVM: selftests: Add __packed attribute fallback Aqib Faruqui
2025-08-29 22:46   ` Sean Christopherson
2025-09-01 15:08     ` Faruqui, Aqib
2025-08-29 14:25 ` [PATCH 3/9] KVM: selftests: Add pthread_attr_setaffinity_np fallback Aqib Faruqui
2025-08-29 22:37   ` Sean Christopherson
2025-08-29 14:25 ` [PATCH 4/9] selftests: kselftest: Add memfd_create syscall compatibility Aqib Faruqui
2025-08-29 14:25 ` [PATCH 5/9] KVM: selftests: Prevent PAGE_SIZE redefinition on x86 Aqib Faruqui
2025-08-29 20:38   ` Sean Christopherson
     [not found]     ` <33701547-13AA-467D-AC41-A1A05963B1DD@amazon.com>
2025-09-05  8:39       ` Sean Christopherson
2025-09-05 13:59         ` Aqib Faruqui
2025-09-08 18:22           ` Sean Christopherson
2025-08-29 14:25 ` [PATCH 6/9] KVM: selftests: Add backtrace fallback Aqib Faruqui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox