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

[Catalin Marinas <catalin.marinas@arm.com>]

On Mon, Feb 24, 2025 at 03:02:39PM +0000, Marc Zyngier wrote:
> On Mon, 24 Feb 2025 14:39:16 +0000,
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> > This patch still prevents such MMIO+MTE mappings but moves the decision
> > to user_mem_abort() and it's slightly more relaxed - only rejecting it
> > if !VM_MTE_ALLOWED _and_ the Stage 2 is cacheable. The side-effect is
> > that it allows device assignment into the guest since Stage 2 is not
> > Normal Cacheable (at least for now, we have some patches Ankit but they
> > handle the MTE case).
> 
> The other side effect is that it also allows non-tagged cacheable
> memory to be given to the MTE-enabled guest, and the guest has no way
> to distinguish between what is tagged and what's not.

It can distinguish in the same way as the host does it, e.g. based on
the firmware tables it gets - only assume the standard RAM can be
tagged. Any device in its IPA space does not support tagging and, if
mapped incorrectly, can trigger external aborts etc. But it's the
responsibility of the VMM to set up the memory slots correctly. If those
non-MTE slots are only used for device assignment, we'd not get any
-EFAULT returns.

Having the requirement that all address ranges assigned to a guest
support MTE makes MTE and device assignment exclusive. FEAT_MTE_PERM
doesn't help either, it only allows cacheable MMIO into guests safely
(from an SError/random accesses perspective).

Anyway, I think I get what you mean. For those memory slots potentially
mapped as cacheable at Stage 2 (e.g. a file mmap()), you'd rather block
them upfront than confusing a potentially buggy VMM later with the
-EFAULT (or some other error).

Do we have another way to tell in kvm_arch_prepare_memory_region()
whether the memory is going to be mapped as Device/NC at Stage 2 or
whether later we'll have FEAT_MTE_PERM set for it? We don't have any
physical address at this point. However, we have vma->vm_page_prot but I
was a big fan of this. Ankit's series for Cacheable VFIO is relying on
this IIRC.

For VFIO specifically, we could do something like below. It won't
trigger any -EFAULT exists. I think it's safe for VM_PFNMAP as well
since these would not be mapped as Cacheable at Stage 2 AFAICT but we
could add some additional checks.

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 1f55b0c7b11d..c48382567f2f 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2184,7 +2184,12 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 		if (!vma)
 			break;
 
-		if (kvm_has_mte(kvm) && !kvm_vma_mte_allowed(vma)) {
+		/*
+		 * VM_ALLOW_ANY_UNCACHED vmas will be mapped as Normal
+		 * NonCacheable at Stage 2 and safe with MTE in guests.
+		 */
+		if (kvm_has_mte(kvm) && !kvm_vma_mte_allowed(vma) &&
+		    !(vma->vm_flags & VM_ALLOW_ANY_UNCACHED)) {
 			ret = -EINVAL;
 			break;
 		}

-- 
Catalin