Re: [PATCH v5 4/9] KVM: s390: Fix code typo in gmap_protect_asce_top_level()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Preexisting ABBA deadlock between KVM `mmu_lock` and gmap `children_lock`.
--

commit d8dbe6f4197c6a65f9a60ab673cf05c59e3ecf7b
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix code typo in gmap_protect_asce_top_level()

This commit fixes a typo in gmap_protect_asce_top_level() where asce.dt
was used instead of asce.tl for the length passed to
kvm_s390_get_guest_pages().

> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> index 3192f610f6962..e6e786811db86 100644
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -1262,7 +1262,7 @@ static int gmap_protect_asce_top_level(struct kvm_s390_mmu_cache *mc, struct gma
>  	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
>  	smp_rmb();
>  
> -	rc = kvm_s390_get_guest_pages(sg->kvm, context.f, asce.rsto, asce.dt + 1, false);
> +	rc = kvm_s390_get_guest_pages(sg->kvm, context.f, asce.rsto, asce.tl + 1, false);
>  	if (rc > 0)
>  		rc = -EFAULT;
>  	if (!rc)
>  		rc = _gmap_protect_asce_top_level(mc, sg, &context);

[Severity: High]
This isn't a bug introduced by this patch, but is there a preexisting ABBA
deadlock between parent->children_lock and parent->kvm->mmu_lock here?

In __gmap_protect_asce_top_level() (which handles setting up non-real-space
shadows), the code acquires sg->kvm->mmu_lock first, and then acquires
parent->children_lock:

arch/s390/kvm/gmap.c:__gmap_protect_asce_top_level() {
    guard(write_lock)(&sg->kvm->mmu_lock);
    ...
    scoped_guard(spinlock, &parent->children_lock) {
        ...
}

However, in gmap_create_shadow(), the lock ordering is reversed. It acquires
parent->children_lock first and then acquires parent->kvm->mmu_lock when
creating a real-space shadow:

arch/s390/kvm/gmap.c:gmap_create_shadow() {
    scoped_guard(spinlock, &parent->children_lock) {
        ...
        if (sg->guest_asce.r) {
            scoped_guard(write_lock, &parent->kvm->mmu_lock)
                ...
}

If two vCPUs of the same VM concurrently attempt to create a real-space
nested shadow and a regular nested shadow, could this lead to a soft lockup?

>  	if (rc)
>  		kvm_s390_release_faultin_array(sg->kvm, context.f, true);
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622160739.202044-1-imbrenda@linux.ibm.com?part=4