Re: [PATCH 3/6] LoongArch: KVM: Add separate vmid feature support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Missing VMID updating logic when `cpu_has_guestid` is true breaks stage-2 MMU isolation.
--

--- Patch [3]: [PATCH 3/6] LoongArch: KVM: Add separate vmid feature support ---
commit a0b8bed9027bd243aca0ea026c90d7c9e7dd4d26
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Add separate vmid feature support

This commit introduces support for a separate VMID feature on LoongArch
by checking the cpu_has_guestid flag. It adds different VMID update logic
and new stage-1 and stage-2 TLB flush functions.

> diff --git a/arch/loongarch/kvm/main.c b/arch/loongarch/kvm/main.c
> index c1e1d8424fa7e..3df46e02594f4 100644
> --- a/arch/loongarch/kvm/main.c
> +++ b/arch/loongarch/kvm/main.c

[ ... ]

> @@ -282,10 +285,15 @@ static void kvm_check_vmid(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long vmid;
>  
> -	vmid = vcpu->arch.vpid & vpid_mask;
> -	if (vcpu->arch.vmid != vmid) {
> -		vcpu->arch.vmid = vmid;
> -		kvm_clear_request(KVM_REQ_TLB_FLUSH_GPA, vcpu);
> +	/* On some machines like 3A5000, vmid needs the same with vpid */
> +	if (!cpu_has_guestid) {
> +		vmid = vcpu->arch.vpid & vpid_mask;
> +		if (vcpu->arch.vmid != vmid) {
> +			vcpu->arch.vmid = vmid;
> +			kvm_clear_request(KVM_REQ_TLB_FLUSH_GPA, vcpu);
> +		}
> +
> +		return;
>  	}
>  }

[Severity: Critical]
Does this bypass VMID assignment when cpu_has_guestid is true?

If cpu_has_guestid is true, the function falls through and returns without
assigning vcpu->arch.vmid. Since vcpu->arch.vmid is zero-initialized, doesn't
it remain 0 (which is reserved for the root/host environment)?

When kvm_switch_to_guest() in arch/loongarch/kvm/switch.S loads
vcpu->arch.vmid and writes it to hardware GTLBC.TGID:

	/* Set VMID for gpa --> hpa mapping */
	ld.d		t1, a2, KVM_ARCH_VMID
	csrrd		t0, LOONGARCH_CSR_GTLBC
	bstrins.w	t0, t1, CSR_GTLBC_TGID_SHIFT_END, CSR_GTLBC_TGID_SHIFT
	csrwr		t0, LOONGARCH_CSR_GTLBC

Will this cause guests to run with VMID 0 and break stage-2 MMU isolation?

> diff --git a/arch/loongarch/kvm/tlb.c b/arch/loongarch/kvm/tlb.c
> index 38daf936021d1..1d95e2208e82e 100644
> --- a/arch/loongarch/kvm/tlb.c
> +++ b/arch/loongarch/kvm/tlb.c

[ ... ]

> +/* Invalidate all stage2 TLB entries including GPA-->HPA  mappings */
> +void kvm_flush_tlb_all_stage2(void)
> +{
> +	lockdep_assert_irqs_disabled();
> +	invtlb_all(INVTLB_ALLGID_GPA_TO_HPA, 0, 0);
> +}

[Severity: Critical]
Is this function reachable?

Because the VMID updating logic appears to be omitted from kvm_check_vmid()
when cpu_has_guestid is true, is this stage-2 TLB flush function ever called
during VMID wraparound?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727072109.2151675-1-maobibo@loongson.cn?part=3