Re: [PATCH] KVM: arm64: selftests: Test VNCR translation invalidation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: selftests: inverted page_mask in prepare_memory()
--

commit 0e10db3329f0ad09596d72a6be111f7048817b36
Author: Itaru Kitayama <itaru.kitayama@fujitsu.com>

KVM: arm64: selftests: Test VNCR translation invalidation

This commit adds a standalone arm64 kvm selftest that remaps the backing
page for vncr_el2 across stage-1 page-table updates and tlb invalidations
to verify that redirected register accesses reach the updated backing page.

> diff --git a/tools/testing/selftests/kvm/arm64/nv_vncr_tlbi_test.c b/tools/testing/selftests/kvm/arm64/nv_vncr_tlbi_test.c
> new file mode 100644
> index 0000000000000..93219d036de6d
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/arm64/nv_vncr_tlbi_test.c
[ ... ]
> +/* Add a guest virtual alias for an existing guest physical page. */
> +static u64 map_alias(struct kvm_vm *vm, u64 gpa)
> +{
> +	u64 va = vm_unused_gva_gap(vm, vm->page_size,
> +				  KVM_UTIL_MIN_VADDR);
> +
> +	virt_map(vm, va, gpa, 1);
> +	return va;
> +}
> +
> +static void prepare_memory(struct kvm_vm *vm)
> +{
> +	u64 a = vm_alloc_page(vm);
> +	u64 b = vm_alloc_page(vm);
> +	u64 page_mask = ~((u64)vm->page_size - 1);
> +	u64 stack = vm_alloc_pages(vm, DEFAULT_STACK_PGS);
> +	u64 ptep_gpa, ptep_alias;
> +	u64 *ptep;
> +
> +	cfg.slot_a = (u64 *)(a + VNCR_TPIDR_OFFSET);
> +	cfg.slot_b = (u64 *)(b + VNCR_TPIDR_OFFSET);
> +	cfg.l2_sp = stack + DEFAULT_STACK_PGS * vm->page_size;
> +
> +	/* Initially, the separate VNCR virtual address maps page A. */
> +	cfg.vncr_va = map_alias(vm, addr_gva2gpa(vm, a));
> +
> +	ptep = virt_get_pte_hva(vm, cfg.vncr_va);
> +	cfg.pte_a = *ptep;
> +	cfg.pte_b = *virt_get_pte_hva(vm, b);
> +
> +	TEST_ASSERT(cfg.pte_a != cfg.pte_b,
> +		    "Backing pages must have different descriptors");
> +	TEST_ASSERT((cfg.pte_a & (PTE_VALID | PTE_TYPE_PAGE)) ==
> +		    (PTE_VALID | PTE_TYPE_PAGE),
> +		    "VNCR mapping must use a leaf page descriptor");
> +
> +	/*
> +	 * Map the page containing the descriptor into L1, so L1 can
> +	 * perform its own page-table update and architectural TLBI.
> +	 */
> +	ptep_gpa = addr_hva2gpa(vm, ptep);
> +	ptep_alias = map_alias(vm, ptep_gpa & ~page_mask);
> +	cfg.vncr_ptep = (u64 *)(ptep_alias + (ptep_gpa & page_mask));

[Severity: Medium]
Are page_mask and ~page_mask inverted when mapping ptep_gpa and computing
cfg.vncr_ptep in prepare_memory()?

Since page_mask is initialized to ~((u64)vm->page_size - 1), it holds the
upper page-frame mask while ~page_mask holds the intra-page offset mask.

Passing ptep_gpa & ~page_mask to map_alias() supplies the intra-page byte
offset of ptep_gpa rather than the page-aligned guest physical address,
which trips the alignment assertion in map_alias() -> virt_map() ->
virt_pg_map() -> _virt_pg_map():

tools/testing/selftests/kvm/lib/arm64/processor.c:_virt_pg_map() {
    ...
	TEST_ASSERT((gpa % vm->page_size) == 0,
		    "Physical address not on page boundary,\n"
		    "  gpa: 0x%lx vm->page_size: 0x%x", gpa, vm->page_size);
    ...
}

Similarly, adding (ptep_gpa & page_mask) to ptep_alias adds the page-frame
base address rather than the intra-page offset, leaving cfg.vncr_ptep
pointing to an unmapped guest virtual address when remap_vncr() writes to
*cfg.vncr_ptep.

Should this pass ptep_gpa & page_mask to map_alias() and add ptep_gpa &
~page_mask to ptep_alias?

> +
> +	sync_global_to_guest(vm, cfg);
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-b4-vncr-mapping-test-v1-1-e9ad54d506be@fujitsu.com?part=1