Re: [PATCH 16/22] KVM: arm64: nv: Expose FEAT_HAFDBS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] KVM will double-fault on the first write to any DBM-enabled page in a nested guest due to stale dirty state evaluation.
--

commit 559268b60f71702530e79a26ecefcf5b5cf06620
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: nv: Expose FEAT_HAFDBS
    
This commit enables FEAT_HAFDBS (Hardware Access Flag and Dirty Bit Management)
for nested virtual machines by advertising DBM support to the guest.

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index 4d128003ae7a5..2408d604a1e18 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1790,7 +1790,7 @@ u64 limit_nv_id_reg(struct kvm *kvm, u32 reg, u64 val)
>  		if (test_bit(KVM_ARM_VCPU_HAS_EL2_E2H0, kvm->arch.vcpu_features))
>  			val &= ~ID_AA64MMFR1_EL1_VH;
>  
> -		val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, AF);
> +		val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64MMFR1_EL1, HAFDBS, DBM);

[Severity: High]
This is a pre-existing issue, but exposing DBM here appears to cause a
performance regression where a nested guest will double-fault on the first
write to any DBM-enabled page.

In walk_nested_s2_pgd(), the code evaluates the stale descriptor before
atomic-swapping the dirty bit into memory:

arch/arm64/kvm/nested.c:walk_nested_s2_pgd() {
    ...
    compute_s2_permissions(vcpu, wi, &ws, out);
    out->level = ws.level;
    out->desc = ws.desc;

    ret = handle_desc_update(vcpu, wi, &ws, out, access);
    if (ret)
        return ret;
    ...
}

If handle_desc_update() successfully updates the descriptor, it returns 0
without updating out->dirty or out->desc:

arch/arm64/kvm/nested.c:handle_desc_update() {
    ...
    ret = __kvm_at_swap_desc(vcpu->kvm, ws->desc_pa, old, new);
    if (!ret || ret == -EAGAIN)
        return ret;

    out->esr = ESR_ELx_FSC_SEA_TTW(ws->level);
    out->desc = ws->desc;
    return 1;
}

Because out->dirty remains false after the successful update, does
adjust_nested_fault_perms() strip the writable permission, forcing a second
VM exit when the guest resumes? 

Should walk_nested_s2_pgd() or handle_desc_update() be modified to re-evaluate
the permissions upon success before enabling DBM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623184201.1518871-1-oupton@kernel.org?part=16