Re: [PATCH] RISC-V: KVM: Avoid synchronous IPIs on VMID rollover

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Packing generation and VMID into `atomic_long_t` truncates the generation counter on 32-bit platforms, introducing a rapid ABA vulnerability.

Pre-existing issues:
- [Critical] VMID generation counter wraparound to 0 causes new VMs to share reserved hardware VMID 0, breaking cross-VM isolation.
--

commit 41c24296ddc6bb7cc33a83183e982b0c76bf449d
Author: Wenlong Li <wenlongli486@gmail.com>

RISC-V: KVM: Avoid synchronous IPIs on VMID rollover

Make VMID rollover lazy to remove cross-CPU synchronization and avoid
synchronous IPIs. Track active and reserved software VMIDs for each possible
CPU, preserving hardware VMIDs with stale translations, and deferring TLB
flushes until the CPU next activates a VMID.

> diff --git a/arch/riscv/include/asm/kvm_vmid.h b/arch/riscv/include/asm/kvm_vmid.h
> index db61b0525a8d0..78811ab7dba07 100644
> --- a/arch/riscv/include/asm/kvm_vmid.h
> +++ b/arch/riscv/include/asm/kvm_vmid.h
> @@ -6,21 +6,39 @@
>  #ifndef __RISCV_KVM_VMID_H_
>  #define __RISCV_KVM_VMID_H_
>  
> +#include <linux/atomic.h>
>  #include <linux/kvm_types.h>
>  
>  struct kvm_vmid {
>  	/*
> -	 * Writes to vmid_version and vmid happen with vmid_lock held
> -	 * whereas reads happen without any lock held.
> +	 * Software VMID:
> +	 *
> +	 *   [ generation | hardware VMID ]
> +	 *
> +	 * Only the low hardware VMID bits may be written to HGATP
> +	 * or used as a hardware fence VMID.
>  	 */
> -	unsigned long vmid_version;
> -	unsigned long vmid;
> +	atomic_long_t id;
>  };

[Severity: Critical]
Does packing the generation and VMID into a single atomic_long_t truncate
the generation counter on 32-bit platforms, causing a rapid ABA regression?

On RV32 platforms, atomic_long_t is only 32 bits. Reserving the lower bits
for the hardware VMID restricts the available bits for the generation
counter significantly.

Can a dormant VM sleep while an unprivileged guest forces rapid VMID
rollovers, causing the counter to wrap in roughly an hour? If the dormant
VM wakes up with a perfectly matching old generation, it could bypass
allocation and reuse its old hardware VMID that might already belong to
another active VM.

Should this use atomic64_t to prevent this rapid wraparound?

> diff --git a/arch/riscv/kvm/vmid.c b/arch/riscv/kvm/vmid.c
> index c15bdb1dd8bef..05819e4f7cb57 100644
> --- a/arch/riscv/kvm/vmid.c
> +++ b/arch/riscv/kvm/vmid.c
[ ... ]
>  int kvm_riscv_gstage_vmid_init(struct kvm *kvm)
>  {
> -	/* Mark the initial VMID and VMID version invalid */
> -	kvm->arch.vmid.vmid_version = 0;
> -	kvm->arch.vmid.vmid = 0;
> +	atomic_long_set(&kvm->arch.vmid.id, 0);
>  
>  	return 0;
>  }

[Severity: Critical]
This is a pre-existing issue, but does initializing the VM id to 0 in
kvm_riscv_gstage_vmid_init() cause collisions when the global generation
counter eventually wraps around to exactly 0?

When vmid_generation wraps to 0 in new_vmid_locked():

    generation = atomic_long_add_return(VMID_FIRST_VERSION,
                                        &vmid_generation);

The vmid_gen_match() check will evaluate to true for any newly created VM
because its id was initialized to 0:

    static bool vmid_gen_match(unsigned long vmid)
    {
        return !((vmid ^ atomic_long_read(&vmid_generation)) >> vmid_bits);
    }

This causes kvm_riscv_gstage_vmid_update() to skip the slow path allocation:

    if (!vmid_gen_match(vmid)) {
        vmid = new_vmid_locked(kvm_vmid);
        atomic_long_set(&kvm_vmid->id, vmid);
        vmid_changed = true;
    }

Will this result in multiple new VMs sharing the reserved hardware VMID 0
and inadvertently bypassing Stage-2 memory isolation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910133300.956555-1-wenlongli486@gmail.com?part=1