Re: [PATCH v2 4/5] LoongArch: KVM: Implement vmid updating logic

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing early return in `__kvm_check_vmid` for machines without guest ID support breaks VMID/VPID synchronization.
- [Medium] Data race between non-atomic `cpumask_setall()` and atomic bitwise operations on `tlb_flush_pending`.
- [Low] Dead code: `kvm_arch_flush_remote_tlbs_memslot` is a fictitious arch hook not called by KVM core.
--

--- Patch [4]: [PATCH v2 4/5] LoongArch: KVM: Implement vmid updating logic ---
commit 1626f2256d93ca3c5c607a6a950ca7531c03169a
Author: Bibo Mao <[email protected]>
Date:   Thu Aug 6 02:07:38 2026 +0000

    LoongArch: KVM: Implement vmid updating logic
    
    VMID calculation method is the same with ASID on LoongArch, it is
    percpu vmid calculation method. For every physical CPU, VMID of
    different VM is different, and it is the same for different vCPUs
    of the same VM.

> diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
> index c36e62ab0d917..72c1d23156e50 100644
> --- a/arch/loongarch/kvm/main.c
> +++ b/arch/loongarch/kvm/main.c
[ ... ]
> @@ -291,6 +316,20 @@ static void __kvm_check_vmid(struct kvm_vcpu *vcpu)
>  			kvm_clear_request(KVM_REQ_TLB_FLUSH_GPA, vcpu);
>  		}
>  	}
> +
> +	cpu = smp_processor_id();
> +	if (cpumask_test_and_clear_cpu(cpu, &vcpu->kvm->arch.tlb_flush_pending))
> +		vcpu->kvm->arch.vmid[cpu] = 0;
> +
> +	/*
> +	 * Check if our vmid is of an older version
> +	 */
> +	ver = vcpu->kvm->arch.vmid[cpu] & ~vpid_mask;
> +	old = this_cpu_ptr(vcpu->kvm->arch.vmcs)->vmid_cache & ~vpid_mask;
> +	if (ver != old)
> +		kvm_update_vmid(vcpu, cpu);
> +
> +	vcpu->arch.hw_vmid = vcpu->kvm->arch.vmid[cpu] & vpid_mask;

[Severity: High]
Does this unconditionally overwrite vcpu->arch.hw_vmid for machines without
guest ID support? On machines like 3A5000, hw_vmid is set to match vpid
above, but without an early return or else block, it falls through and gets
overwritten here with a dynamically allocated vmid.

>  }
>  
>  void kvm_check_vpid(struct kvm_vcpu *vcpu)
> diff --git a/arch/loongarch/kvm/mmu.c b/arch/loongarch/kvm/mmu.c
> index 714640a2e6142..80e7a8bdc344a 100644
> --- a/arch/loongarch/kvm/mmu.c
> +++ b/arch/loongarch/kvm/mmu.c
[ ... ]
> @@ -942,6 +943,22 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
>  {
>  }
>  
> +int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
> +{
> +	/*
> +	 * Queue a TLB invalidation for each CPU to perform on next
> +	 * vcpu loading
> +	 */
> +	if (cpu_has_guestid) {
> +		cpumask_setall(&kvm->arch.tlb_flush_pending);

[Severity: Medium]
Could calling cpumask_setall() cause a data race with concurrent atomic
bit operations? 

cpumask_setall() uses a non-atomic memset() internally, while
__kvm_check_vmid() and kvm_handle_mm_fault() use atomic read-modify-write
operations like cpumask_test_and_clear_cpu() and cpumask_set_cpu() on the
same cpumask.

> +		/* Be sure that other CPUS can watch the changes */
> +		smp_wmb();
> +	}
> +
> +	/* Return 1 continue to send ipi to running vCPUs */
> +	return 1;
> +}
> +
> +void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> +					const struct kvm_memory_slot *memslot)
> +{
> +	kvm_flush_remote_tlbs(kvm);
> +}

[Severity: Low]
Is kvm_arch_flush_remote_tlbs_memslot() dead code? There does not appear to be
a KVM core hook by this name, so this architecture-specific function won't be
called by the KVM core during memslot updates.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4