Re: [PATCH] RISC-V: KVM: Flush VS-stage TLB before reusing a host CPU

[Anup Patel]

On Wed, Oct 28, 2026 at 12:46 PM BillXiang
<xiangwencheng@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 10/28/2026 2:14 PM, Anup Patel wrote:
> > On Mon, Oct 26, 2026 at 3:49 PM guoyaxing <guoyaxing@xxxxxxxxxx> wrote:
> >>
> >>
> >>
> >> 在 2026/10/24 23:49, Anup Patel 写道:
> >>> On Sat, Oct 10, 2026 at 2:36 PM guoyaxing <guoyaxing@xxxxxxxxxx> wrote:
> >>>>
> >>>>
> >>>>
> >>>> 在 2026/10/10 15:50, Anup Patel 写道:
> >>>>> On Sat, Oct 10, 2026 at 4:12 AM guoyaxing <guoyaxing@xxxxxxxxxx> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> 在 2026/10/10 02:51, Anup Patel 写道:
> >>>>>>> On Mon, Aug 17, 2026 at 12:25 PM Yaxing Guo <guoyaxing@xxxxxxxxxx> wrote:
> >>>>>>>>
> >>>>>>>> Guest Linux uses mm_cpumask to decide whether an sfence.vma needs to be
> >>>>>>>> local or remote. Under KVM that mask tracks guest CPUs, not the host CPUs
> >>>>>>>> that previously backed a vCPU.
> >>>>>>>>
> >>>>>>>> This can miss stale VS-stage TLB entries when a vCPU migrates across host
> >>>>>>>> CPUs. For example:
> >>>>>>>>
> >>>>>>>>       - vcpu0 runs on host CPU1 and fills a VS-stage TLB entry.
> >>>>>>>>       - vcpu0 migrates to host CPU0.
> >>>>>>>>       - the guest updates the mapping on vcpu0 and issues a local sfence.vma.
> >>>>>>>>       - later, the guest task runs on vcpu1 while vcpu1 is backed by host CPU1.
> >>>>>>>
> >>>>>>> When the vcpu0 is brought back to host CPU1, the kvm_riscv_local_tlb_sanitize()
> >>>>>>> will ensure TLB entries associated with Guest/VM are nuked.
> >>>>>>>
> >>>>>>> By default, kvm_riscv_local_hfence_gvma_vmid_all() will only do HFENCE.GVMA
> >>>>>>> for the associated VMID because most RISC-V H-extension implemenation will
> >>>>>>> tag VS-stage TLB entries with VMID and HFENCE.GVMA must remove both
> >>>>>>> G-stage and VS-stage TLB entries for given VMID.
> >>>>>>>
> >>>>>>> If your HW does not tag VS-stage TLB enteries with VMID then it buggy
> >>>>>>> / inefficient
> >>>>>>> so you have to set kvm_riscv_vsstage_tlb_no_gpa in
> >>>>>>> kvm_riscv_setup_vendor_features()
> >>>>>>> as work-around.
> >>>>>>>
> >>>>>>> NACK to this patch from myside.
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>> Anup
> >>>>>>>
> >>>>>> Hi Anup,
> >>>>>>       Thanks for the review and clarification.
> >>>>>>
> >>>>>>       Regarding “When the vcpu0 is brought back to host CPU1”: the failure
> >>>>>> we hit is not caused by vcpu0 being brought back onto host CPU1. In our
> >>>>>> case, after vcpu0 has already migrated away, neither vCPU migrates
> >>>>>> again. The guest process is later scheduled inside the multi-vCPU guest
> >>>>>> onto vcpu1, which is still running on host CPU1. So the stale TLB is
> >>>>>> re-hit without any further KVM vCPU migration, and
> >>>>>> kvm_riscv_local_tlb_sanitize() is not involved on that path.
> >>>>>>
> >>>>>>       My commit message was a bit too terse on this point. The concrete
> >>>>>> sequence we observed was described earlier in the RFC discussion:
> >>>>>>
> >>>>>>
> >>>>>> https://lore.kernel.org/kvm/0d15b098-b8f7-4080-9f33-e8c49c2331e1@xxxxxxxxxx/
> >>>>>>
> >>>>>>       In short, the issue is: because vcpu0 migrated away, cpu1 still has
> >>>>>> stale TLB entries while cpu0 has the new mapping. After that, neither
> >>>>>> vcpu migrated again, but the process was scheduled from vcpu0 to vcpu1
> >>>>>> (which is on cpu1), causing it to hit the stale TLB. Consider the
> >>>>>> following scenario:
> >>>>>>
> >>>>>>       1. vcpu0 and vcpu1 are both running on cpu1. At this point, vcpu0 is
> >>>>>> on cpu1, and a process on it reads a page, which gets cached in the TLB
> >>>>>> (let's call this the "stale TLB entry").
> >>>>>>
> >>>>>>       2. vcpu0 migrates to cpu0, and then the process (on vcpu0, now on
> >>>>>> cpu0) does a CoW (copy-on-write) and establishes a new mapping.
> >>>>>>
> >>>>>>       3. Neither vcpu0 nor vcpu1 migrates again. At this point, the process
> >>>>>> gets scheduled inside the VM onto vcpu1 (which is on cpu1), reads the
> >>>>>> same page, and hits the stale TLB entry.
> >>>>>
> >>>>> This cannot happen because in step#2 when mapping was updated the
> >>>>> Guest OS will do TLB flush for all its VCPUs which means vcpu1 on cpu0
> >>>>> will also receive TLB flush and the stale TLB will be removed. This means
> >>>>> when process is moved back to vcpu1 on cpu0, HW will fetch updated
> >>>>> PTE and new TLB entry will be populated.
> >>>>>
> >>>>
> >>>> Hi Anup,
> >>>>
> >>>> Thanks for the follow-up. I may be missing something, so I wanted to
> >>>> double-check this against the guest TLB shootdown path.
> >>>>
> >>>> If I follow the Linux guest CoW path correctly, when the mapping is
> >>>> updated in step#2 it goes through:
> >>>>
> >>>>      wp_page_copy()
> >>>>        -> ptep_clear_flush()
> >>>>          -> flush_tlb_page()
> >>>>            -> __flush_tlb_range(mm, mm_cpumask(mm), ...)
> >>>>
> >>>> So the remote SFENCE.VMA appears to be targeted only at CPUs in
> >>>> mm_cpumask(mm), rather than at all VCPUs of the guest. mm_cpumask is
> >>>> updated in switch_mm()/set_mm() for guest CPUs that have actually run
> >>>> that mm.
> >>>>
> >>>> In our case, before the CoW the process had only ever run on vcpu0. >From
> >>>> the guest's point of view it did not migrate: the host migration of
> >>>> vcpu0 from cpu1 to cpu0 is not visible to guest Linux, which only sees
> >>>> vCPUs. So at CoW time mm_cpumask still contains only vcpu0 (or at least
> >>>> does not contain vcpu1), and __flush_tlb_range() would not IPI vcpu1.
> >>>> The local SFENCE on vcpu0 would only cover host cpu0, where vcpu0 is
> >>>> currently running. The stale VS-stage TLB entry left on host cpu1 from
> >>>> when vcpu0 previously ran there would then remain.
> >>>>
> >>>> Later, still without any further vCPU migration, the process is
> >>>> scheduled inside the guest onto vcpu1, which remains on host cpu1, and
> >>>> can hit that stale entry.
> >>>>
> >>>> This also seems more likely when the ASID allocator is enabled, because
> >>>> set_mm_asid() does not always local_flush_tlb_all() on switch_mm(). So
> >>>> even the first time the process lands on vcpu1, a stale ASID-tagged
> >>>> entry on host cpu1 may still be present.
> >>>>
> >>>> In other words, I am not sure this case is covered by a full guest TLB
> >>>> flush to all VCPUs on the mapping update. Guest shootdown looks
> >>>> mm_cpumask-based, and host pCPU migration of a vCPU does not by itself
> >>>> add other vCPUs that never ran the mm into that mask.
> >>>
> >>> This makes sense now considering ASID allocator in Guest Linux is global
> >>> and not local. In any case, KVM RISC-V must not assume that the Guest OS
> >>> has local ASID allocator hence the kvm_riscv_local_tlb_sanitize() must also
> >>> santize TLB for other VCPUs of the same Guest which can potentially run
> >>> on the previous host CPU where current VCPU ran.
> >>>
> >>> Having said the above, there are multiple issues with the current patch:
> >>> 1) The kvm_riscv_local_hfence_gvma_vmid_all() is sufficient for most
> >>> implementations to sanitize both G-stage and VS-stage TLB entries
> >>> because both G-stage and VS-stage TLB entries are generally tagged
> >>> with VMID. The kvm_riscv_vsstage_tlb_no_gpa work-around is for
> >>> implmentations which do not tag VS-stage TLB entries with VMID
> >>> and this patch remove this work-around.
> >>> 2) The other VCPUs on the previous host CPU may be already running
> >>> and may still hit stale TLB entry before kvm_riscv_local_tlb_sanitize()
> >>> gets a chance to sanitize TLB on previous host CPU.
> >>> 3) The per-CPU pointer in struct kvm_arch is racy because we
> >>> may have two VCPUs on same host CPU which are migrated to
> >>> other host CPUs. Instead, it is much simpler to have an atomic
> >>> cpumask in each VCPU where any part of KVM can mark a host
> >>> CPU in the cpumask of the VCPU to force TLB sanitization whenever
> >>> the VCPU runs on that particular host CPU.
> >>>
> >>> How about the following change ?
> >>>
> >>> diff --git a/arch/riscv/include/asm/kvm_host.h
> >>> b/arch/riscv/include/asm/kvm_host.h
> >>> index a30600579231..c975eee7240c 100644
> >>> --- a/arch/riscv/include/asm/kvm_host.h
> >>> +++ b/arch/riscv/include/asm/kvm_host.h
> >>> @@ -9,6 +9,7 @@
> >>>    #ifndef __RISCV_KVM_HOST_H__
> >>>    #define __RISCV_KVM_HOST_H__
> >>>
> >>> +#include <linux/cpumask.h>
> >>>    #include <linux/types.h>
> >>>    #include <linux/kvm.h>
> >>>    #include <linux/kvm_types.h>
> >>> @@ -184,6 +185,9 @@ struct kvm_vcpu_arch {
> >>>        /* ISA feature bits (similar to MISA) */
> >>>        DECLARE_BITMAP(isa, RISCV_ISA_EXT_MAX);
> >>>
> >>> +    /* Sanitize TLB host CPU mask */
> >>> +    cpumask_t tlb_sanitize_mask;
> >>> +
> >>>        /* Vendor, Arch, and Implementation details */
> >>>        unsigned long mvendorid;
> >>>        unsigned long marchid;
> >>> diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
> >>> index 2ae34632cdcb..b1387a00c8b0 100644
> >>> --- a/arch/riscv/kvm/tlb.c
> >>> +++ b/arch/riscv/kvm/tlb.c
> >>> @@ -207,11 +207,31 @@ void kvm_riscv_local_hfence_vvma_all(unsigned long vmid)
> >>>
> >>>    void kvm_riscv_local_tlb_sanitize(struct kvm_vcpu *vcpu)
> >>>    {
> >>> -    unsigned long vmid;
> >>> +    struct kvm_vcpu *tvcpu;
> >>> +    unsigned long i, vmid;
> >>>
> >>> -    if (!kvm_riscv_gstage_vmid_bits() ||
> >>> -        vcpu->arch.last_exit_cpu == vcpu->cpu)
> >>> -        return;
> >>> +    if (!cpumask_test_cpu(vcpu->cpu, &vcpu->arch.tlb_sanitize_mask)) {
> >>> +        if (!kvm_riscv_gstage_vmid_bits() ||
> >>> +            vcpu->arch.last_exit_cpu == vcpu->cpu)
> >>> +            return;
> >>> +    }
> >>> +
> >>> +    /*
> >>> +     * The VCPU might have left stale TLBs on the previous host CPU so
> >>> +     * mark previous host CPU in the tlb_sanitize_mask of other VCPUs.
> >>> +     */
> >>> +    if (vcpu->arch.last_exit_cpu > -1 && vcpu->arch.last_exit_cpu !=
> >>> vcpu->cpu) {
> >>> +        kvm_for_each_vcpu(i, tvcpu, vcpu->kvm) {
> >>> +            if (tvcpu == vcpu)
> >>> +                continue;
> >>> +            cpumask_set_cpu(vcpu->arch.last_exit_cpu,
> >>> +                    &tvcpu->arch.tlb_sanitize_mask);
>
> Hi Anup,
>
> Thanks for the detailed analysis and for proposing the direction. I’d
> like to suggest a small refinement on top of your diff: add
>
> if (READ_ONCE(tvcpu->cpu) == vcpu->arch.last_exit_cpu)
>
> here so that we only kick the vCPUs that are currently running on the
> previous host CPU, while still setting tlb_sanitize_mask for all other
> vCPUs. Would that be OK?

I am okay with this refinement but please add a comment about
this check.

Regards,
Anup