Re: [PATCH v2 1/4] KVM: selftests: fix maxnode arguments in xapic_ipi_test

["Garg, Shivank" <shivankg@amd.com>]

On Tue, 2026-09-01 at 08:04 -0700, Sean Christopherson wrote:
> On Tue, Sep 01, 2026, Shivank Garg wrote:
> > migrate_pages() syscall expect maxnode to be one greater than the
> > number of bits in the nodemask. do_migrations() passes the size of
> > nodemask in bytes to migrate_pages(). This sets the maxnode to 8,
> > so kernel only checks node IDs 0-6 even though the nodemask covers
> > node IDs 0-63.
> > 
> > Pass the nodemask size in bits plus one because get_nodes() in
> > mempolicy does --maxnode.
> 
> Ok, I'm not crazy.  I read all of this multiple times and the manpages seemed
> completely nonsensical.  Looking at QEMU's use of mbind(), it's the kernel that
> sucks:
> 
>     /*
>      * We can have up to MAX_NODES nodes, but we need to pass maxnode+1
>      * as argument to mbind() due to an old Linux bug (feature?) which
>      * cuts off the last specified node. This means backend->host_nodes
>      * must have MAX_NODES+1 bits available.
>      */
>     assert(sizeof(backend->host_nodes) >=
>            BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long));
> 
> 
> And from https://lore.kernel.org/all/63ccc890-fd57-118b-5997-e0259f507d28@suse.cz:
> 
>  : And this is I think the reason why we can't change this now. I assume
>  : numactl allocates 1024 bits (0 to 1023) and passes 1025 to make sure all
>  : 1024 bits are processed. If we change it now, kernel will process 1025
>  : bits (0 to 1024) and overflow the allocated bitmask. If it happens to be
>  : at the border of mmaped vma, it's a segfault...
> 
> This is quite possibly the most confusing syscall interface ever, and IMO the
> manpage is still straight up wrong (well, the kernel is the one that's buggy,
> but the manpage doesn't reflect the kernel's behavior):
> 
>  : The maxnode argument is the maximum node number in the bit mask plus one
> 
> Beacuse it's not the maximum node number plus one, it's the number of nodes in
> the bitask plus one.
> 
> What a confusing mess.

Agreed. The discussion resurfaced here too:
https://lore.kernel.org/linux-mm/20240720173543.897972-1-jglisse@google.com

I'll fix the manpage for this.

> 
> 
> > Fixes: 678e90a349a4 ("KVM: selftests: Test IPI to halted vCPU in xAPIC while backing page moves")
> > Signed-off-by: Shivank Garg <shivankg@amd.com>
> > ---
> >  tools/testing/selftests/kvm/x86/xapic_ipi_test.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> > index 469e3ab16460..0f11f4d7cc3d 100644
> > --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> > +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> > @@ -291,7 +291,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
> >  		 * KVM_CREATE_VCPU ioctl. If that assumption ever changes this
> >  		 * test may break or give a false positive signal.
> >  		 */
> > -		pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]),
> > +		pages_not_moved = migrate_pages(0, sizeof(nodemasks[from]) * 8 + 1,
> 
> BITS_PER_TYPE()
> 
> However, given that it's basically impossible for developers to get this right,
> we should add "#define MAXNODE_FOR_MASK(mask) (BITS_PER_TYPE(mask) + 1)" with a
> big comment explain why the code looks wrong.
> 
> E.g. patch 3 gets it wrong in get_numa_mem_nodes():
> 
>   static inline unsigned long get_numa_mem_nodes(void)
>   {
>        unsigned long nodemask = 0;
> 
>        /* Get set of first 64 numa nodes available */
>        if (get_mempolicy(NULL, &nodemask, BITS_PER_TYPE(nodemask), NULL,
>                          MPOL_F_MEMS_ALLOWED))
>                return 0;
> 
>        return nodemask;
>   }
> 
> because that will only get the mask for bits 62:0.  Even Sashiko got confused in
> patch 4:
> 
>   When maxnode is passed to kvm_get_mempolicy() here, the kernel must write at
>   least 5 bytes to return 33 bits of node status. This rounds up to 8 bytes (two
>   32-bit words).
> 
> since the disaster of a syscall that is get_mempolicy() and friends will only
> provide 32 bits of node status.
> 
> Looking at the rest of the patches in this series, the main goal of fixing the
> extremely-unlikely-to-happen-in-practice bug in patch 4 needs a lot of work.  To
> move along the other cleanups, I'll send the below plus rebased versions of
> patches 1 and 2.
> 
> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c
> index 2233d871a38f..cd5df88bc642 100644
> --- a/tools/testing/selftests/kvm/guest_memfd_test.c
> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c
> @@ -80,7 +80,7 @@ static void test_mbind(int fd, size_t total_size)
>  {
>  	const unsigned long nodemask_0 = 1; /* nid: 0 */
>  	unsigned long nodemask = 0;
> -	unsigned long maxnode = BITS_PER_TYPE(nodemask);
> +	unsigned long maxnode = MAXNODE_FOR_MASK(nodemask);
>  	int policy;
>  	char *mem;
>  	int ret;
> diff --git a/tools/testing/selftests/kvm/include/numaif.h b/tools/testing/selftests/kvm/include/numaif.h
> index 29572a6d789c..84e82857f1c5 100644
> --- a/tools/testing/selftests/kvm/include/numaif.h
> +++ b/tools/testing/selftests/kvm/include/numaif.h
> @@ -30,6 +30,16 @@ KVM_SYSCALL_DEFINE(mbind, 6, void *, addr, unsigned long, size, int, mode,
>  		   const unsigned long *, nodemask, unsigned long, maxnode,
>  		   unsigned int, flags);
>  
> +/*
> + * Caclucate the @maxnode param for the above syscalls given the mask that will
> + * be passed to the kernel, to account for a longstanding off-by-one bug in the
> + * kernel that isn't properly documented in the manpages.  The manpages say
> + * that @maxnode is "the maximum node ID plus one", but the kernel's actual
> + * behavior is "the number of bits in the mask plus one", i.e. "the maximum
> + * node ID plus two".
> + */
> +#define MAXNODE_FOR_MASK(mask) (BITS_PER_TYPE(mask) + 1)
> +
>  static inline int get_max_numa_node(void)
>  {
>  	struct dirent *de;
> diff --git a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> index 469e3ab16460..1ddcf95d7fe4 100644
> --- a/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> +++ b/tools/testing/selftests/kvm/x86/xapic_ipi_test.c
> @@ -248,7 +248,7 @@ void do_migrations(struct test_data_page *data, int run_secs, int delay_usecs,
>  		delay_usecs);
>  
>  	/* Get set of first 64 numa nodes available */
> -	kvm_get_mempolicy(NULL, &nodemask, sizeof(nodemask) * 8,
> +	kvm_get_mempolicy(NULL, &nodemask, MAXNODE_FOR_MASK(nodemask),
>  			  0, MPOL_F_MEMS_ALLOWED);
>  
>  	fprintf(stderr, "Numa nodes found amongst first %lu possible nodes "

Thanks for this patch. I noticed you've already spun out a series with
this, I'll review it shortly.

Best regards,
Shivank