From: Quentin Perret <qperret@google.com>
To: Vincent Donnefort <vdonnefort@google.com>
Cc: maz@kernel.org, oliver.upton@linux.dev, joey.gouly@arm.com,
suzuki.poulose@arm.com, yuzenghui@huawei.com,
catalin.marinas@arm.com, will@kernel.org,
linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
kernel-team@android.com
Subject: Re: [PATCH] KVM: arm64: pkvm: Rollback refcount on hyp share/unshare error
Date: Mon, 30 Mar 2026 09:41:01 +0000 [thread overview]
Message-ID: <acpCq1qZCMFqocs3@google.com> (raw)
In-Reply-To: <20260324172757.2147153-1-vdonnefort@google.com>
Hey Vincent,
On Tuesday 24 Mar 2026 at 17:27:57 (+0000), Vincent Donnefort wrote:
> If one of the HVC __pkvm_host_share_hyp or __pkvm_host_unshare_hyp fails,
> rollback the refcount to ensure the hyp_shared_pfns tracking reflects
> the actual sharing status.
If any of these hypercalls fail I think we're still in trouble as
kvm_{un}share_hyp() work on multi-page ranges and we could leak pages in
a borked state if we fail halfway through. And failing any of these
hypercalls is also sign of a bigger problem somewhere else so I wasn't
too worried.
But if we're going to fix this properly, I'd suggest also improving the
error handling in kvm_share_hyp(). 'Fixing' kvm_unshare_hyp() is a bit
harder because we must tell the caller to leak the data structure that
was shared I presume, so maybe we just keep the WARN and cross our
fingers :)
Cheers,
Quentin
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 17d64a1e11e5..0fb41d2c8b44 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -493,11 +493,17 @@ static int share_pfn_hyp(u64 pfn)
> goto unlock;
> }
>
> + ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn);
> + if (ret) {
> + kfree(this);
> + goto unlock;
> + }
> +
> this->pfn = pfn;
> this->count = 1;
> rb_link_node(&this->node, parent, node);
> rb_insert_color(&this->node, &hyp_shared_pfns);
> - ret = kvm_call_hyp_nvhe(__pkvm_host_share_hyp, pfn);
> +
> unlock:
> mutex_unlock(&hyp_shared_pfns_lock);
>
> @@ -521,9 +527,15 @@ static int unshare_pfn_hyp(u64 pfn)
> if (this->count)
> goto unlock;
>
> + ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn);
> + if (ret) {
> + this->count++;
> + goto unlock;
> + }
> +
> rb_erase(&this->node, &hyp_shared_pfns);
> kfree(this);
> - ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_hyp, pfn);
> +
> unlock:
> mutex_unlock(&hyp_shared_pfns_lock);
>
>
> base-commit: c369299895a591d96745d6492d4888259b004a9e
> --
> 2.53.0.1018.g2bb0e51243-goog
>
next prev parent reply other threads:[~2026-03-30 9:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-24 17:27 [PATCH] KVM: arm64: pkvm: Rollback refcount on hyp share/unshare error Vincent Donnefort
2026-03-30 9:41 ` Quentin Perret [this message]
2026-04-01 3:01 ` Vincent Donnefort
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acpCq1qZCMFqocs3@google.com \
--to=qperret@google.com \
--cc=catalin.marinas@arm.com \
--cc=joey.gouly@arm.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=suzuki.poulose@arm.com \
--cc=vdonnefort@google.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.