Re: [PATCH 42/45] KVM: x86: selftests: Introduce memory protection attributes test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] KVM: selftests: unused MMIO mapping and non-static arrays in memory_attributes
--

commit 739900eb4be23c429345b95c9dff33aecc77373c
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86: selftests: Introduce memory protection attributes test

This commit introduces a kselftest for KVM memory protection attributes
with architecture-independent test logic and x86-specific helpers for
read, write, and execution access checks.

Link: https://patch.msgid.link/20260918135030.171564-30-pbonzini@redhat.com

> diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
> new file mode 100644
> index 0000000000000..81ec59382f55b
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/memory_attributes.c
> @@ -0,0 +1,344 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024, Amazon.com, Inc. or its affiliates. All Rights Reserved
> + *
> + * Test for KVM_MEMORY_ATTRIBUTES
> + */
> +#include <pthread.h>
> +#include <signal.h>

[ ... ]

> +#define TEST_MEM_SLOT	1
> +#define TEST_MEM_GPA	0x80000000
> +
> +#define MMIO_GPA	0x700000000
> +#define MMIO_GVA	MMIO_GPA
> +
> +enum {
> +	TEST_OP_NOP,
> +	TEST_OP_READ,
> +	TEST_OP_WRITE,
> +	TEST_OP_EXEC,
> +	TEST_OP_EXIT,
> +};
> +
> +const char *test_op_names[] =
> +{
> +	[TEST_OP_READ] = "Read",
> +	[TEST_OP_WRITE] = "Write",
> +	[TEST_OP_EXEC] = "Exec",
> +	[TEST_OP_EXIT] = "Exit",
> +};

[ ... ]

> +const struct memory_access {
> +	const char *name;
> +	uint64_t attrs;
> +	int ops[5];
> +} access_array[] = {

[ ... ]

> +int main(int argc, char *argv[])
> +{
> +	uint32_t guest_page_size = vm_guest_mode_params[VM_MODE_DEFAULT].page_size;
> +	unsigned int ptes_per_page = guest_page_size / 8;
> +	size_t pages = ptes_per_page * 2; /* 2 huge-pages */
> +	size_t size = guest_page_size * pages;
> +	struct kvm_vcpu *vcpu;
> +	gva_t test_mem;
> +	struct kvm_vm *vm;
> +
> +	TEST_REQUIRE(kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) &
> +		     KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
> +
> +	vm = __vm_create_with_one_vcpu(&vcpu, pages, guest_code);
> +
> +	/* add a few extra pages for use by tests */
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> +				    TEST_MEM_GPA, TEST_MEM_SLOT,
> +				    pages + 100, 0);
> +	vm->memslots[MEM_REGION_TEST_DATA] = TEST_MEM_SLOT;
> +
> +	test_mem = vm_alloc_pages(vm, pages);
> +	virt_map(vcpu->vm, MMIO_GVA, MMIO_GPA, 1);

[Severity: Low]
This isn't a bug, but even after the subsequent commit "KVM: x86: selftests:
Introduce memory attributes side-channel tests" in this series, MMIO_GPA and
MMIO_GVA are never accessed anywhere in tools/testing/selftests/kvm/memory_attributes.c
after main() maps them via virt_map(vcpu->vm, MMIO_GVA, MMIO_GPA, 1).

Similarly, signal.h is never used in this file, and the file-local arrays
test_op_names[] and access_array[] are declared without static.

Could the unused MMIO_GPA/MMIO_GVA definitions, virt_map() call, and signal.h
include be dropped, and should test_op_names[] and access_array[] be marked
static?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102174130.8779-1-pbonzini@redhat.com?part=42