Re: [PATCH v4 3/3] KVM: selftests: loongarch: Add PV TLB flush performance test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing Makefile integration for the new test file `pv_tlb_flush_test.c`.
- [Medium] Signed integer overflow in thread bounds check leads to stack buffer overflow.
--

commit 56616f9df639ca6715014c26c5864780ced6ff41
Author: Tao Cui <cuitao@kylinos.cn>

KVM: selftests: loongarch: Add PV TLB flush performance test

Add a multi-threaded benchmark to measure PV TLB flush performance
inside LoongArch KVM guests.

> diff --git a/tools/testing/selftests/kvm/loongarch/pv_tlb_flush_test.c b/tools/testing/selftests/kvm/loongarch/pv_tlb_flush_test.c
> new file mode 100644
> index 0000000000000..63efaf9ef1cd0
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/loongarch/pv_tlb_flush_test.c
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * LoongArch PV TLB Flush Performance Test
> + *
> + * Measure the overhead of remote TLB flushes in a KVM guest by spawning
> + * flusher threads that repeatedly mmap/munmap (triggering TLB shootdown
> + * IPIs) alongside idle threads that either sleep or busy-spin.
> + *
> + * With PV TLB flush enabled, IPIs to preempted vCPUs are replaced by
> + * deferred flags in the steal-time shared page, reducing flush latency.
> + *
> + * Usage:
> + *   Compile on LoongArch guest:
> + *     gcc -O2 -static -pthread -o pv_tlb_flush_test pv_tlb_flush_test.c
> + *   Run (inside KVM guest):
> + *     ./pv_tlb_flush_test <flushers> <idle> <iterations> <busy_idle>

[Severity: Medium]
Since this test is provided as a standalone script and not added to the
selftests Makefile (e.g., TEST_PROGS or TEST_GEN_PROGS), will it be
missed during make install and automated test execution?

Also, is it intentional that this test bypasses the standard KVM selftests
framework (kvm_util) which typically provisions VMs and runs guest payloads
programmatically?

[ ... ]

> +int main(int argc, char **argv) {
> +    int nr_flush = 1, nr_idle = 3, i, run;
> +    int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +    if (argc > 1) nr_flush = atoi(argv[1]);
> +    if (argc > 2) nr_idle = atoi(argv[2]);
> +    if (argc > 3) nr_iters = atoi(argv[3]);
> +    if (argc > 4) busy_idle = atoi(argv[4]);
> +
> +    if (nr_flush < 1 || nr_idle < 0 || nr_flush + nr_idle > MAX_THREADS) {

[Severity: Medium]
Could this bounds check be bypassed if a user provides values that sum to a
value exceeding the 32-bit signed integer limit (e.g., nr_flush=1 and
nr_idle=2147483647)? The signed addition would overflow to a negative number.

If bypassed, would the program then loop nr_idle times and write out of
bounds to the stack-allocated arrays like threads and args in the loop below?

[ ... ]

> +        pthread_t threads[MAX_THREADS];
> +        unsigned long results[MAX_THREADS];
> +        int completed[MAX_THREADS];
> +        struct thread_args args[MAX_THREADS];
> +        start_barrier = 0; stop_flag = 0;
> +
> +        for (i = 0; i < nr_idle; i++) {
> +            args[i].cpu = do_pin ? nr_flush + i : -1;
> +            args[i].result = NULL;
> +            args[i].completed = NULL;
> +            if (pthread_create(&threads[i], NULL, idle_thread, &args[i])) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615082154.42144-1-cui.tao@linux.dev?part=3