Re: [RFC PATCH v2 25/25] DO NOT MERGE: Add nested_tlb_force_flush

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] State Desynchronization (ASID tracking) / Isolation Breach
- [High] Weakened Flush Semantics / Stale MMU Roots
- [Medium] Guideline Violation / Mutable State
--

commit 3fa5ceb50c23aa3458e6cdaf0dc109608a8b6192
Author: Yosry Ahmed <yosry@kernel.org>

DO NOT MERGE: Add nested_tlb_force_flush

Purely for testing only, add a knob that brings back unconditional force
flushes (and resync) on nested transitions.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -32,6 +32,9 @@
>  #include "hyperv.h"
>  #include "pmu.h"
>  
> +static bool nested_tlb_force_flush;
> +module_param(nested_tlb_force_flush, bool, 0644);

[Severity: Medium]
Does this module parameter need to be mutable at runtime (0644)?

KVM guidelines suggest new flags should default to immutable (0444) because
toggling them creates unexpected state-machine transitions that can be
difficult to handle safely.

> [ ... ]
> @@ -691,6 +694,12 @@ static void nested_svm_entry_tlb_flush(struct kvm_vcpu *vcpu)
>  	/* Handle pending Hyper-V TLB flush requests */
>  	kvm_hv_nested_transtion_tlb_flush(vcpu, npt_enabled);
>  
> +	if (nested_tlb_force_flush) {
> +		kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
> +		kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +		return;
> +	}

[Severity: Critical]
If this parameter is toggled off at runtime, can it leave
svm->nested.last_asid out of sync?

Because the early return skips updating last_asid, if the feature is later
disabled, KVM might see the stale last_asid matching a newly assigned L1
ASID:

nested_svm_entry_tlb_flush() {
    ...
    if (svm->nested.ctl.asid != svm->nested.last_asid) {
        svm->nested.last_asid = svm->nested.ctl.asid;
        new_asid = true;
    }
    ...
}

Would this incorrectly evaluate new_asid to false and skip the required
flush, allowing the L2 guest to run with mappings from a different context?

[Severity: High]
Does returning early here bypass the full guest flush guarantees?

When replacing KVM_REQ_TLB_FLUSH_GUEST with KVM_REQ_MMU_SYNC and
KVM_REQ_TLB_FLUSH_CURRENT, it appears to skip synchronizing the previous
mmu roots (via kvm_mmu_sync_prev_roots) and guest-specific flush logic.

If L1 explicitly requests a full TLB flush, could a guest switching CR3
to a cached previous root end up using stale shadow page tables?

>  	if (svm->nested.ctl.asid != svm->nested.last_asid) {
>  		svm->nested.last_asid = svm->nested.ctl.asid;
>  		new_asid = true;
> [ ... ]
> @@ -726,6 +735,12 @@ static void nested_svm_exit_tlb_flush(struct kvm_vcpu *vcpu)
>  
>  	kvm_hv_nested_transtion_tlb_flush(vcpu, npt_enabled);
>  
> +	if (nested_tlb_force_flush) {
> +		kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
> +		kvm_make_request(KVM_REQ_TLB_FLUSH_CURRENT, vcpu);
> +		return;
> +	}

[Severity: High]
Could this early return on VM-Exit similarly replace an explicit
TLB_CONTROL_FLUSH_ALL_ASID request with a weaker current-only flush?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616004155.1435766-1-yosry@kernel.org?part=25