Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Fuad Tabba <fuad.tabba@linux.dev>
Cc: Marc Zyngier <maz@kernel.org>, Oliver Upton <oupton@kernel.org>,
	 kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	 Joey Gouly <joey.gouly@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	 Zenghui Yu <yuzenghui@huawei.com>,
	Steffen Eiden <seiden@linux.ibm.com>,
	 Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	Steven Price <steven.price@arm.com>,
	 Yang Shi <yang@os.amperecomputing.com>,
	David Hildenbrand <david@kernel.org>,
	 Fuad Tabba <tabba@google.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] KVM: arm64: Clear the whole hugetlb folio when storing MTE tags
Date: Tue, 20 Oct 2026 12:58:13 +0100	[thread overview]
Message-ID: <atdWIpk9Qh8ngs-Y@gremlin> (raw)
In-Reply-To: <20261019132103.3836646-3-fuad.tabba@linux.dev>

On Mon, Oct 19, 2026 at 02:21:02PM +0100, Fuad Tabba wrote:
> When KVM_ARM_MTE_COPY_TAGS copies tags into a page, it marks the page as
> tagged. For a hugetlb folio the flag covers every page in the folio. If
> the folio wasn't tagged yet, its other pages are marked as tagged with
> their previous owner's tags still in place. The stage-2 fault path then
> maps them into the guest without clearing them. The ioctl's read side
> also copies them out to the VMM.
>
> Clear the whole folio on its first tagging, before copying in this
> page's tags.
>
> Fixes: 25c17c4b55def ("hugetlb: arm64: add mte support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>

Logic LGTM, with some nits below:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  arch/arm64/kvm/guest.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index b01d6622b8720..cda655fbff657 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -1012,6 +1012,7 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
>  		void *maddr;
>  		unsigned long num_tags;
>  		struct folio *folio;
> +		unsigned long i;
>
>  		if (!page) {
>  			ret = -EFAULT;
> @@ -1042,12 +1043,14 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
>  			/*
>  			 * Only locking to serialise with a concurrent
>  			 * __set_ptes() in the VMM but still overriding the
> -			 * tags, hence ignoring the return value.
> +			 * tags. A hugetlb folio is marked tagged as a whole,
> +			 * so its first tagging clears every page.
>  			 */
> -			if (folio_test_hugetlb(folio))
> -				folio_try_hugetlb_mte_tagging(folio);
> -			else
> +			if (!folio_test_hugetlb(folio))
>  				try_page_mte_tagging(page);
> +			else if (folio_try_hugetlb_mte_tagging(folio))

Slightly awkward logic, testing a negative then the negative of the negative
(i.e. else) plus folio_try_hugetlb_mte_tagging(), makes it a little harder to
understand.

With mte_clear_folio_tags() you can make life simpler:

	if (folio_test_hugetlb(folio)) {
		if (folio_try_hugetlb_mte_tagging(folio))
			mte_clear_folio_tags(folio);
	} else {
		try_page_mte_tagging(page);
	}

> +				for (i = 0; i < folio_nr_pages(folio); i++)
> +					mte_clear_page_tags(page_address(folio_page(folio, i)));

As above, similar comment to 1/3 re: mte_clear_folio_tags(). Would be less churn also.

>  			num_tags = mte_copy_tags_from_user(maddr, tags,
>  							MTE_GRANULES_PER_PAGE);
>
> --
> 2.47.3
>
>

--
Cheers, Lorenzo


  reply	other threads:[~2026-10-20 11:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-19 13:21 [PATCH 0/3] KVM: arm64: Fix stale MTE tags reaching a guest Fuad Tabba
2026-10-19 13:21 ` [PATCH 1/3] KVM: arm64: Clear the whole hugetlb folio when sanitising MTE tags Fuad Tabba
2026-10-20 11:52   ` Lorenzo Stoakes (ARM)
2026-10-19 13:21 ` [PATCH 2/3] KVM: arm64: Clear the whole hugetlb folio when storing " Fuad Tabba
2026-10-20 11:58   ` Lorenzo Stoakes (ARM) [this message]
2026-10-19 13:21 ` [PATCH 3/3] KVM: arm64: Sanitise MTE tags when a permission fault installs a mapping Fuad Tabba

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=atdWIpk9Qh8ngs-Y@gremlin \
    --to=ljs@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=david@kernel.org \
    --cc=fuad.tabba@linux.dev \
    --cc=joey.gouly@arm.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=seiden@linux.ibm.com \
    --cc=steven.price@arm.com \
    --cc=suzuki.poulose@arm.com \
    --cc=tabba@google.com \
    --cc=will@kernel.org \
    --cc=yang@os.amperecomputing.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox