Re: [PATCH 20/20] KVM: selftests: Add arch hook to force page tables to be naturally aligned

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The physical page allocator uses the ALIGN() macro with nr_pages as the alignment, which miscalculates physical addresses if nr_pages is not a power of two.
--

commit e164bd04c5d3f08c13c51c2171eea3a520c79ec4
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add arch hook to force page tables to be naturally aligned

This commit adds an arch hook to communicate to the core physical page
allocator that page table allocations need to be naturally aligned. The hook
will be used for upcoming PowerPC support, which has MMU tables that are
larger than the guest base page size.

> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
[ ... ]
> @@ -2110,6 +2116,7 @@ gpa_t __vm_phy_pages_alloc(struct kvm_vm *vm, size_t nr_pages,
>  		break;
>  	case MEM_REGION_PT:
>  		min_gpa = KVM_GUEST_PAGE_TABLE_MIN_PADDR;
> +		naturally_aligned = kvm_arch_needs_naturally_aligned_page_tables();
>  		break;
>  	case MEM_REGION_TEST_EXTRA:
>  		min_gpa = region->region.guest_phys_addr;
> @@ -2120,7 +2127,7 @@ gpa_t __vm_phy_pages_alloc(struct kvm_vm *vm, size_t nr_pages,
>  	}
>  
>  	return ____vm_phy_pages_alloc(vm, nr_pages, min_gpa, vm->memslots[type],
> -				      protected, false);
> +				      protected, naturally_aligned);
>  }

[Severity: Medium]
When naturally_aligned is set to true here, is it guaranteed that nr_pages 
will always be a power of two?

Looking at ____vm_phy_pages_alloc(), the nr_pages parameter is used directly
as the alignment value:

____vm_phy_pages_alloc() {
        size_t alignment = naturally_aligned ? nr_pages : 1;
        ...
        base = ALIGN(base, alignment);
        ...
}

The ALIGN(x, a) macro in the kernel is implemented using bitwise operations
((x + (a - 1)) & ~(a - 1)), which inherently require the alignment
parameter to be a strictly power-of-two value. If a test requests a
non-power-of-two number of pages for page table memory, this causes a
bitwise truncation that results in an incorrectly calculated physical
address.

Because there is no TEST_ASSERT enforcing that nr_pages is a power of two
when naturally_aligned is requested, could this lead to silent memory
overlaps and guest data corruption? Should we enforce that nr_pages is a
power of two, or round the alignment up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826230511.972824-1-seanjc@google.com?part=20