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

["BillXiang" <xiangwencheng@lanxincomputing.com>]

On 10/28/2026 2:14 PM, Anup Patel wrote:
> On Mon, Oct 26, 2026 at 3:49 PM guoyaxing <guoyaxing@bosc.ac.cn> wrote:
>>
>>
>>
>> 在 2026/10/24 23:49, Anup Patel 写道:
>>> On Sat, Oct 10, 2026 at 2:36 PM guoyaxing <guoyaxing@bosc.ac.cn> wrote:
>>>>
>>>>
>>>>
>>>> 在 2026/10/10 15:50, Anup Patel 写道:
>>>>> On Sat, Oct 10, 2026 at 4:12 AM guoyaxing <guoyaxing@bosc.ac.cn> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 在 2026/10/10 02:51, Anup Patel 写道:
>>>>>>> On Mon, Aug 17, 2026 at 12:25 PM Yaxing Guo <guoyaxing@bosc.ac.cn> 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@bosc.ac.cn/
>>>>>>
>>>>>>       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?

Regards,
Bill

>>> +            kvm_vcpu_kick(tvcpu);
>>> +        }
>>> +    }
>>> +
>>> +    /* Clear current host CPU from the tlb_sanitize_mask */
>>> +    cpumask_clear_cpu(vcpu->cpu, &vcpu->arch.tlb_sanitize_mask);
>>>
>>>        /*
>>>         * On RISC-V platforms with hardware VMID support, we share same
>>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>>> index dd786f758eb7..c00dc0b191dc 100644
>>> --- a/arch/riscv/kvm/vcpu.c
>>> +++ b/arch/riscv/kvm/vcpu.c
>>> @@ -107,6 +107,8 @@ static void kvm_riscv_reset_vcpu(struct kvm_vcpu
>>> *vcpu, bool kvm_sbi_reset)
>>>
>>>        vcpu->arch.last_exit_cpu = -1;
>>>
>>> +    cpumask_clear(&vcpu->arch.tlb_sanitize_mask);
>>> +
>>>        kvm_riscv_vcpu_context_reset(vcpu, kvm_sbi_reset);
>>>
>>>        kvm_riscv_vcpu_fp_reset(vcpu);
>>>
>>> Regards,
>>> Anup
>>
>> Hi Anup,
>>
>> Thanks for the explanation and for suggesting the per-vCPU sanitization
>> mask. Building on your proposal, I would like to suggest two changes:
>>
>> 1. Separate the invalidation strategy for vCPU migration and a mask hit.
>>
>>      Keep HFENCE.GVMA for vCPU migration, identified by
>>      last_exit_cpu != vcpu->cpu, when hardware VMIDs are available. Use
>>      HFENCE.VVMA when the current host CPU is marked in tlb_sanitize_mask.
>>      A mask hit caused by another vCPU of the same VM running on this CPU
>>      requires VS-stage sanitization; it does not by itself require G-stage
>>      invalidation.
>>
>>      For example, if vCPU0 and vCPU1 both last ran on CPU0, switching from
>>      vCPU1 back to vCPU0 only needs the mask-triggered VVMA. This avoids
>>      requesting G-stage invalidation when the shared G-stage mappings have
>>      not changed. If both migration and a mask hit occur, both fences run.
> 
> HFENCE.GVMA is sufficient for most implementations so to sanitize all TLB
> entries matching a particular VMID.
> 
> Refer, the following text from RISC-V Privileged specification section "5.3.2.
> Hypervisor Memory-Management Fence Instructions":
> 
>    Conceptually, an implementation might contain two
> address-translation caches: one that
>    maps guest virtual addresses to guest physical addresses, and
> another that maps guest
>    physical addresses to supervisor physical addresses. HFENCE.GVMA
> need not flush the
>    former cache, but it must flush entries from the latter cache that
> match the HFENCE.GVMA’s
>    address and VMID arguments.
> 
>    More commonly, implementations contain address-translation caches
> that map guest virtual
>    addresses directly to supervisor physical addresses, removing a
> level of indirection. For such
>    implementations, any entry whose guest virtual address maps to a
> guest physical address that
>    matches the HFENCE.GVMA’s address and VMID arguments must be
> flushed. Selectively
>    flushing entries in this fashion requires tagging them with the
> guest physical address, which is
>    costly, and so a common technique is to flush all entries that match
> the HFENCE.GVMA’s
>    VMID argument, regardless of the address argument.
> 
> Doing HFENCE.GVMA in certain cases and HFENCE.VVMA other cases is
> maintainence burden without much benefits. Also, VCPU migrating from one
> host CPU to another is an infrequent event so no point over optimizing without
> much benefits.
> 
> In short, NACK to your suggestion #1.
> 
>>
>> 2. Move mask updates to kvm_arch_vcpu_put().
>>
>>      Mark the outgoing host CPU in the masks of the other vCPUs of the
>>      same VM when a vCPU is put. This also covers a switch between vCPUs
>>      on one host CPU without either vCPU migrating. Marking only when
>>      migration is detected in kvm_riscv_local_tlb_sanitize() would miss
>>      that case.
>>
>>      The marking now happens on the outgoing CPU before another vCPU can
>>      enter guest mode there. The migration-time marking and kicks are
>>      therefore removed. The trade-off is an O(nr_vcpus) walk on each
>>      qualifying put().
>>
>>      How about the following change ?
>>
>> diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
>> index 2ac3c0a20fce..ae8a40b3429c 100644
>> --- a/arch/riscv/kvm/tlb.c
>> +++ b/arch/riscv/kvm/tlb.c
>> @@ -160,32 +160,15 @@
>>
>>    void kvm_riscv_local_tlb_sanitize(struct kvm_vcpu *vcpu)
>>    {
>> -       struct kvm_vcpu *tvcpu;
>> -       unsigned long i, vmid;
>> -
>> -       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);
>> -                       kvm_vcpu_kick(tvcpu);
>> -               }
>> -       }
>> -
>> -       /* Clear current host CPU from the tlb_sanitize_mask */
>> -       cpumask_clear_cpu(vcpu->cpu, &vcpu->arch.tlb_sanitize_mask);
>> +       unsigned long vmid;
>> +       bool vcpu_migrated, sanitize_vsstage;
>> +
>> +       vcpu_migrated = vcpu->arch.last_exit_cpu != vcpu->cpu;
>> +       sanitize_vsstage = cpumask_test_and_clear_cpu(vcpu->cpu,
>> +                                               &vcpu->arch.tlb_sanitize_mask);
>> +       if (!sanitize_vsstage &&
>> +           (!kvm_riscv_gstage_vmid_bits() || !vcpu_migrated))
>> +               return;
>>
>>          /*
>>           * On RISC-V platforms with hardware VMID support, we share same
>> @@ -199,13 +182,16 @@
>>           */
>>
>>          vmid = READ_ONCE(vcpu->kvm->arch.vmid.vmid);
>> -       kvm_riscv_local_hfence_gvma_vmid_all(vmid);
>> +       if (vcpu_migrated && kvm_riscv_gstage_vmid_bits())
>> +               kvm_riscv_local_hfence_gvma_vmid_all(vmid);
>>
>>          /*
>> -        * Flush VS-stage TLB entries for implementation where VS-stage
>> -        * TLB does not cahce guest physical address and VMID.
>> +        * Flush VS-stage entries marked when another VCPU left this CPU, or when
>> +        * required by the split-TLB vendor workaround on this VCPU's migration.
>>           */
>> -       if (static_branch_unlikely(&kvm_riscv_vsstage_tlb_no_gpa))
>> +           if (sanitize_vsstage ||
>> +               (vcpu_migrated &&
>> +               static_branch_unlikely(&kvm_riscv_vsstage_tlb_no_gpa)))
>>                      kvm_riscv_local_hfence_vvma_all(vmid);
>>    }
>>
>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>> index f2e52f3b0786..6209c66b24e1 100644
>> --- a/arch/riscv/kvm/vcpu.c
>> +++ b/arch/riscv/kvm/vcpu.c
>> @@ -644,8 +644,18 @@
>>
>>    void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>    {
>> +       struct kvm_vcpu *tvcpu;
>> +       unsigned long i;
>> +       int cpu = vcpu->cpu;
>>          void *nsh;
>>          struct kvm_vcpu_csr *csr = &vcpu->arch.guest_csr;
>> +
>> +       if (cpu >= 0 && vcpu->arch.last_exit_cpu == cpu) {
>> +               kvm_for_each_vcpu(i, tvcpu, vcpu->kvm) {
>> +                       if (tvcpu != vcpu)
>> +                               cpumask_set_cpu(cpu, &tvcpu->arch.tlb_sanitize_mask);
>> +               }
>> +       }
>>
>>          vcpu->cpu = -1;
>>
>>      Would these changes make sense to you, or is there anythingI might
>> be overlooking?
> 
> The kvm_arch_vcpu_put() is called every time a VCPU exits to
> user space for MMIO emulation and handling other exits. Now,
> imagine VCPU0 and VCPU1 running on same host CPU where
> VCPU0 is doing lot of MMIO exits causing VCPU1 to sanitize
> TLB frequently on the same host CPU.
> 
> Based on above, NACK to your suggestion #2
> 
> Regards,
> Anup

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv