Re: [PATCH] KVM: arm64: Drop mte_allowed check during memslot creation

[Marc Zyngier <maz@kernel.org>]

On Mon, 24 Feb 2025 11:05:33 +0000,
Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> On Mon, Feb 24, 2025 at 03:09:38PM +0530, Aneesh Kumar K.V (Arm) wrote:
> > Before commit d89585fbb308 ("KVM: arm64: unify the tests for VMAs in
> > memslots when MTE is enabled"), kvm_arch_prepare_memory_region() only
> > rejected a memory slot if VM_SHARED was set. This commit unified the
> > checking with user_mem_abort(), with slots being rejected if either
> > VM_MTE_ALLOWED is not set or VM_SHARED set. A subsequent commit
> > c911f0d46879 ("KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE
> > enabled") dropped the VM_SHARED check, so we ended up with memory slots
> > being rejected if VM_MTE_ALLOWED is not set. This wasn't the case before
> > the commit d89585fbb308. The rejection of the memory slot with VM_SHARED
> > set was done to avoid a race condition with the test/set of the
> > PG_mte_tagged flag. Before Commit d77e59a8fccd ("arm64: mte: Lock a page
> > for MTE tag initialization") the kernel avoided allowing MTE with shared
> > pages, thereby preventing two tasks sharing a page from setting up the
> > PG_mte_tagged flag racily.
> > 
> > Commit d77e59a8fccd ("arm64: mte: Lock a page for MTE tag
> > initialization") further updated the locking so that the kernel
> > allows VM_SHARED mapping with MTE. With this commit, we can enable
> > memslot creation with VM_SHARED VMA mapping.
> > 
> > This patch results in a minor tweak to the ABI. We now allow creating
> > memslots that don't have the VM_MTE_ALLOWED flag set.
> 
> As I commented here:
> 
> https://lore.kernel.org/r/Z4e04P1bQlFBDHo7@arm.com
> 
> I'm fine with the change, we basically go back to the original ABI prior
> to relaxing this for VM_SHARED.
> 
> > If the guest uses
> > such a memslot with Allocation Tags, the kernel will generate -EFAULT.
> > ie, instead of failing early, we now fail later during KVM_RUN.
> 
> Nit: more like the kernel "will return -EFAULT" to the VMM rather than
> "generate".
> 
> > This change is needed because, without it, users are not able to use MTE
> > with VFIO passthrough (currently the mapping is either Device or
> > NonCacheable for which tag access check is not applied.), as shown
> > below (kvmtool VMM).
> 
> Another nit: "users are not able to user VFIO passthrough when MTE is
> enabled". At a first read, the above sounded to me like one wants to
> enable MTE for VFIO passthrough mappings.

What the commit message doesn't spell out is how MTE and VFIO are
interacting here. I also don't understand the reference to Device or
NC memory here.

Isn't the issue that DMA doesn't check/update tags, and therefore it
makes little sense to prevent non-tagged memory being associated with
a memslot?

My other concern is that this gives pretty poor consistency to the
guest, which cannot know what can be tagged and what cannot, and
breaks a guarantee that the guest should be able to rely on.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.