Re: [PATCH v4 1/3] LoongArch: KVM: Add PV TLB flush support via steal-time shared memory

[Tao Cui <cui.tao@linux.dev>]

Hi Bibo,

Thank you for the review.

You're right, and honestly I didn't think the
error path through carefully enough here.

The current handling leaves the preempted byte uncleared and skips the
flush, which is exactly the cascade you described: a vCPU re-entered on
the error path keeps running with the bit set, so later flushes keep
deferring it and its TLB stays stale until some later entry happens to
succeed. A stale TLB is a correctness issue, so this path needs
conservative handling even though the fault itself is rare.

I'll drop the VPID on error so the TLB is flushed conservatively and the
logic self-heals:

    if (err) {
        vcpu->arch.vpid = 0;    /* be conservative: flush anyway */
        goto out;
    }

The byte isn't cleared on the fault, but the TLB is clean this round;
the next successful amand_db.w reads the still-set FLUSH_TLB bit and
flushes again (redundantly but harmlessly), then clears the byte. The
same covers the unsafe_get_user() priming-load failure.

I'll fix this in the next version.

Thanks,
Tao

在 2026/6/16 09:03, Bibo Mao 写道:
>> --- a/arch/loongarch/kvm/vcpu.c
>> +++ b/arch/loongarch/kvm/vcpu.c
>> @@ -173,7 +173,39 @@ static void kvm_update_stolen_time(struct kvm_vcpu *vcpu)
>>       }
>>         st = (struct kvm_steal_time __user *)ghc->hva;
>> -    if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PREEMPT)) {
>> +    if (kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_PV_TLB_FLUSH)) {
>> +        u32 old = 0;
>> +        int err = 0;
>> +
>> +        /*
>> +         * Prime the cache line with a normal load before the coherent
>> +         * atomic below; it was observed (when the line was last dirtied
>> +         * by another core) to be needed for amand_db.w to see a current
>> +         * value.  amand_db.w overwrites `old` with the real pre-AND value,
>> +         * so this load contributes only its cache side-effect.
>> +         */
>> +        unsafe_get_user(old, (u32 __user *)&st->preempted, out);
> If there is a failure, it does nothing and go to out directly, however guest TLB remains stale without flush operation.
>> +
>> +        /* Atomically read and clear the preempted byte via amand_db.w. */
>> +        asm volatile(
>> +        "1: amand_db.w %1, %3, %2    \n"
>> +        "2:                \n"
>> +        _ASM_EXTABLE_UACCESS_ERR_ZERO(1b, 2b, %0, %1)
>> +        : "+r" (err), "+&r" (old),
>> +          "+ZB" (*(u32 *)&st->preempted)
>> +        : "r" ((u32)~0xffu)
>> +        : "memory");
>> +
>> +        if (err)
>> +            goto out;
> Ditto, if there is error, guest TLB is not flushed and vCPU continues to run with preempted bit set. The next TLB flush will be skipped with this vCPU since its preempted bit is not clear.
> 
> Regards
> Bibo Mao
>