Add support for NoTagAccess memory attribute
Subsystems (4)
v2 cover Add support for NoTagAccess memory attribute
A VMM allows assigning different types of memory regions to the guest and not all memory regions support storing allocation tags. Currently, the kernel doesn't allow enabling the MTE feature in the guest if any of the assigned memory regions don't allow MTE. This prevents the usage of MTE in the guest even though the guest will never use these memory regions as allocation-tagged memory.
One example of such a configuration is a VFIO passthrough-enabled guest. Enabling MTE with such config results in failure as shown below(kvmtool VMM)
[ 617.921030] vfio-pci 0000:01:00.0: resetting [ 618.024719] vfio-pci 0000:01:00.0: reset done Error: 0000:01:00.0: failed to register region with KVM Warning: [0abc:aced] Error activating emulation for BAR 0 Error: 0000:01:00.0: failed to configure regions Warning: Failed init: vfio__init
Fatal: Initialisation failed
This patch series provides a way to enable MTE in such configs. Even though NoTagAccess can only be used with cacheable mapping, having the ability to map both MTE capable and non-capable VMAs allow us to support VFIO with the MTE capability enabled.
Also, there is a possibility of cachable device memory. That memory, if exposed to guest as WB and the guest enables MTE, may trigger SErrors (Arm ARM mention this as "[RBCFRK] The result of an access to an Allocation Tag where Allocation Tag storage is not provided is IMPLEMENTATION DEFINED"). With FEAT_MTE_PERM KVM could trap and do necessary corrective action.
Another use case is virtiofs dax support, which can use a page cache region as a virtio-shm region. We can use MTE_PERM to enable MTE in this config.
In summary, different types of memory, whether WB MMIO or RAM presented as virtio-(pmem, shm etc.) backed by files in the VMM, the VM should be aware it is not standard RAM and should not attempt to enable MTE on it. If it does (either by mistake or malice), FEAT_MTE_PERM will trap and cause a VM exit with KVM_VM_EXIT_MEMORY_FAULT as the reason.
A VM exit with exit reason KVM_VM_EXIT_MEMORY_FAULT, gives sufficient flexibility for any future fault-handling schemes we come up with (One possible use case is to allocate additional allocation tag space for schemes that want to use this for supporting smaller allocation tag pool and retry the guest instruction again). For the current implementation where we expect the guest to be terminated, this can also be achieved from within the hypervisor.
Implementation notes:
For non-MTE-allowed memory, the hypervisor will install stage2 translation with NoTagAccess memory attributes. Guest access of allocation tags with these memory regions will result in a VM Exit. One detail to note here is that NoTagAccess memory attribute can only be applied to Normal cacheable memory ie, using the attribute value of MemAttr[3:0] = 0b0100 implies Normal, NoTagAccess, write-back cacheable memory region. No other memory attribute value will trap the allocation tag access.
Migration notes:
The feature is only exposed to an EL2 guest only if it is capable of nested virtualization. Otherwise, a read of ID_AA64PFR2_EL1_FPMR will not expose MTE_PERM feature. We also want to make sure that an EL2 guest using this feature as part of its stage2 translation can only migrate to a target that supports the feature in the hardware. This is achieved by using KVM_CAP_ARM_MTE_PERM.
Nested virtualization notes:
This being a stage2 translation attribute, it is exposed to EL2 guest only if it is capable of a VirtualEL2 state. When an EL1 guest is started with MTE_PERM capability enabled, the EL2 hypervisor will look at the EL1 stage2 tables to determine whether a NoTagAccess attribute needs to be inserted in the shadow stage2 table at EL2. (Limitation, upstream nested virt support disables MTE in EL1 guest and in a similar fashion, we don't allow MTE_PERM with EL1 guest for now. This also mean we can drop the patch "KVM: arm64: MTE: Nested guest support" because the feature is only used by EL2 guest for now.)
Changes from v1:
* Add KVM_CAP_ARM_MTE_PERM to handle migration.
* Add handling of NoTagAccess with Nested guest stage2.
* Add changes to split some of the kvm_pgtable_prot bits.
Aneesh Kumar K.V (Arm) (7):
arm64: Update the values to binary from hex
KVM: arm64: MTE: Update code comments
arm64: cpufeature: add Allocation Tag Access Permission (MTE_PERM)
feature
KVM: arm64: MTE: Add KVM_CAP_ARM_MTE_PERM
KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
KVM: arm64: MTE: Nested guest support
KVM: arm64: Split some of the kvm_pgtable_prot bits into separate
defines
Documentation/virt/kvm/api.rst | 17 ++++++++++ arch/arm64/include/asm/cpufeature.h | 5 +++ arch/arm64/include/asm/kvm_emulate.h | 5 +++ arch/arm64/include/asm/kvm_host.h | 7 +++++ arch/arm64/include/asm/kvm_nested.h | 10 ++++++ arch/arm64/include/asm/kvm_pgtable.h | 9 ++++-- arch/arm64/include/asm/memory.h | 14 +++++---- arch/arm64/kernel/cpufeature.c | 9 ++++++ arch/arm64/kvm/arm.c | 11 +++++++ arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +- arch/arm64/kvm/hyp/pgtable.c | 43 +++++++++++++++---------- arch/arm64/kvm/mmu.c | 45 ++++++++++++++++++++------- arch/arm64/kvm/nested.c | 28 +++++++++++++++++ arch/arm64/kvm/sys_regs.c | 15 ++++++--- arch/arm64/tools/cpucaps | 1 + include/linux/kvm_host.h | 10 ++++++ include/uapi/linux/kvm.h | 2 ++ 17 files changed, 191 insertions(+), 42 deletions(-)
base-commit: 56e6a3499e14716b9a28a307bb6d18c10e95301e
Signature · 1 line
2.43.0
PATCH v2 1/7 arm64: Update the values to binary from hex
This matches the ARM ARM representation. No functional change in this patch.
--- arch/arm64/include/asm/memory.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
arch/arm64/include/asm/memory.h +6 -6
@@ -178,17 +178,17 @@/* * Memory types for Stage-2 translation */ -#define MT_S2_NORMAL 0xf -#define MT_S2_NORMAL_NC 0x5 -#define MT_S2_DEVICE_nGnRE 0x1 +#define MT_S2_NORMAL 0b1111 +#define MT_S2_NORMAL_NC 0b0101 +#define MT_S2_DEVICE_nGnRE 0b0001/* * Memory types for Stage-2 translation when ID_AA64MMFR2_EL1.FWB is 0001 * Stage-2 enforces Normal-WB and Device-nGnRE */ -#define MT_S2_FWB_NORMAL 6 -#define MT_S2_FWB_NORMAL_NC 5 -#define MT_S2_FWB_DEVICE_nGnRE 1 +#define MT_S2_FWB_NORMAL 0b0110 +#define MT_S2_FWB_NORMAL_NC 0b0101 +#define MT_S2_FWB_DEVICE_nGnRE 0b0001#ifdef CONFIG_ARM64_4K_PAGES #define IOREMAP_MAX_ORDER (PUD_SHIFT)
Signature · 1 line
2.43.0
Re: [PATCH v2 1/7] arm64: Update the values to binary from hex
Quoted 4 lines · Aneesh Kumar K.V (Arm)
This matches the ARM ARM representation. No functional change in this patch.
Signed-off-by Aneesh Kumar K.V (Arm) <[email protected]>
Fine by me if it's easier to compare with the Arm ARM.
PATCH v2 2/7 KVM: arm64: MTE: Update code comments
In reply to Aneesh Kumar K.V (Arm)commit d77e59a8fccd ("arm64: mte: Lock a page for MTE tag initialisation") updated the locking such the kernel now allows VM_SHARED mapping with MTE. Update the code comment to reflect this.
--- arch/arm64/kvm/mmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
arch/arm64/kvm/mmu.c +5 -5
@@ -1391,11 +1391,11 @@ static int get_vma_page_shift(struct vm_area_struct *vma, unsigned long hva)* able to see the page's tags and therefore they must be initialised first. If * PG_mte_tagged is set, tags have already been initialised. * -* The race in the test/set of the PG_mte_tagged flag is handled by: -* - preventing VM_SHARED mappings in a memslot with MTE preventing two VMs -* racing to santise the same page -* - mmap_lock protects between a VM faulting a page in and the VMM performing -* an mprotect() to add VM_MTE +* The race in the test/set of the PG_mte_tagged flag is handled by using +* PG_mte_lock and PG_mte_tagged together. if PG_mte_lock is found unset, we can +* go ahead and clear the page tags. if PG_mte_lock is found set, then the page +* tags are already cleared or there is a parallel tag clearing is going on. We +* wait for the parallel tag clear to finish by waiting on PG_mte_tagged bit. */ static void sanitise_mte_tags (struct kvm *kvm,kvm_pfn_t pfn,unsigned long size)
Signature · 1 line
2.43.0
Catalin Marinas
maintainerRe: [PATCH v2 2/7] KVM: arm64: MTE: Update code comments
Quoted 16 lines · Aneesh Kumar K.V (Arm)
commit
d77e59a8fccd("arm64: mte: Lock a page for MTE tag initialisation") updated the locking such the kernel now allowsVM_SHAREDmapping with MTE. Update the code comment to reflect this.Signed-off-by Aneesh Kumar K.V (Arm) <[email protected]>--- arch/arm64/kvm/mmu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)arch/arm64/kvm/mmu.c +0
-0@@ -1391,11 +1391,11 @@ static int get_vma_page_shift(struct vm_area_struct *vma, unsigned long hva)* able to see the page's tags and therefore they must be initialised first. If * PG_mte_tagged is set, tags have already been initialised.
arch/arm64/kvm/mmu.c static int get_vma_page_shift(struct vm_area_struct *vma, unsigned long hva) · line 1397
Quoted 10 lines
arch/arm64/kvm/mmu.c +4
-5* -* The race in the test/set of the PG_mte_tagged flag is handled by: -* - preventing VM_SHARED mappings in a memslot with MTE preventing two VMs -* racing to santise the same page -* - mmap_lock protects between a VM faulting a page in and the VMM performing -* an mprotect() to add VM_MTE +* The race in the test/set of the PG_mte_tagged flag is handled by using +* PG_mte_lock and PG_mte_tagged together. if PG_mte_lock is found unset, we can +* go ahead and clear the page tags. if PG_mte_lock is found set, then the page +* tags are already cleared or there is a parallel tag clearing is going on. We
^^^^^^^^ remove this (or the other 'is')
arch/arm64/kvm/mmu.c static int get_vma_page_shift(struct vm_area_struct *vma, unsigned long hva) · line 1399
Quoted 2 lines
arch/arm64/kvm/mmu.c +1
-0+* wait for the parallel tag clear to finish by waiting on PG_mte_tagged bit. */
I don't think we need to describe the behaviour of set_page_mte_tagged() and try_page_mte_tagging() in here. How the locking works for tagged pages was hidden in those functions with their own documentation. I would just remove this whole paragraph here, just leave the first one stating that the tags must be initialised if not already done so.
Signature · 1 line
Catalin
PATCH v2 3/7 arm64: cpufeature: add Allocation Tag Access Permission (MTE_PERM) feature
In reply to Aneesh Kumar K.V (Arm)This indicates if the system supports MTE_PERM. This will be used by KVM for stage 2 mapping. This is a CPUCAP_SYSTEM feature because if we enable the feature all cpus must have it.
--- arch/arm64/include/asm/cpufeature.h | 5 +++++ arch/arm64/include/asm/memory.h | 2 ++ arch/arm64/kernel/cpufeature.c | 9 +++++++++ arch/arm64/tools/cpucaps | 1 + 4 files changed, 17 insertions(+)
arch/arm64/include/asm/cpufeature.h +5 -0
@@ -813,6 +813,11 @@ static inline bool system_supports_mte(void)return alternative_has_cap_unlikely (ARM64_MTE ); } +static inline bool system_supports_notagaccess (void ) +{ +return alternative_has_cap_unlikely (ARM64_MTE_PERM ); +} +static inline bool system_has_prio_mask_debugging (void ) { return IS_ENABLED(CONFIG_ARM64_DEBUG_PRIORITY_MASKING) &&
arch/arm64/include/asm/memory.h +2 -0
@@ -179,6 +179,7 @@* Memory types for Stage-2 translation */ #define MT_S2_NORMAL 0b1111 +#define MT_S2_NORMAL_NOTAGACCESS 0b0100#define MT_S2_NORMAL_NC 0b0101#define MT_S2_DEVICE_nGnRE 0b0001
@@ -187,6 +188,7 @@* Stage-2 enforces Normal-WB and Device-nGnRE */ #define MT_S2_FWB_NORMAL 0b0110 +#define MT_S2_FWB_NORMAL_NOTAGACCESS 0b1110#define MT_S2_FWB_NORMAL_NC 0b0101#define MT_S2_FWB_DEVICE_nGnRE 0b0001
arch/arm64/kernel/cpufeature.c +9 -0
@@ -309,6 +309,7 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {static const struct arm64_ftr_bits ftr_id_aa64pfr2[] = {ARM64_FTR_BITS (FTR_VISIBLE ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64PFR2_EL1_FPMR_SHIFT ,4 ,0 ), +ARM64_FTR_BITS (FTR_HIDDEN ,FTR_NONSTRICT ,FTR_LOWER_SAFE ,ID_AA64PFR2_EL1_MTEPERM_SHIFT ,4 ,0 ),ARM64_FTR_END , };
@@ -2818,6 +2819,14 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .matches = has_cpuid_feature, ARM64_CPUID_FIELDS(ID_AA64PFR1_EL1, MTE, MTE3) }, + { + .desc = "MTE Allocation Tag Access Permission", + .capability = ARM64_MTE_PERM, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_cpuid_feature, + ARM64_CPUID_FIELDS(ID_AA64PFR2_EL1, MTEPERM, IMP) + }, + #endif /* CONFIG_ARM64_MTE */ { .desc = "RCpc load-acquire (LDAPR)",
arch/arm64/tools/cpucaps +1 -0
@@ -66,6 +66,7 @@ MPAM MPAM_HCR MTE MTE_ASYMM +MTE_PERM SME SME_FA64 SME2
Signature · 1 line
2.43.0
Re: [PATCH v2 3/7] arm64: cpufeature: add Allocation Tag Access Permission (MTE_PERM) feature
Quoted 7 lines · Aneesh Kumar K.V (Arm)
This indicates if the system supports
MTE_PERM. This will be used by KVM for stage 2 mapping. This is aCPUCAP_SYSTEMfeature because if we enable the feature all cpus must have it.Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Signed-off-by Aneesh Kumar K.V (Arm) <[email protected]>
PATCH v2 4/7 KVM: arm64: MTE: Add KVM_CAP_ARM_MTE_PERM
In reply to Aneesh Kumar K.V (Arm)This will be used by VMM to enable the usage of NoTagAccess memory attribute while mapping pages not supporting allocating tags to guest IPA.
--- Documentation/virt/kvm/api.rst | 14 ++++++++++++++ arch/arm64/include/asm/kvm_host.h | 7 +++++++ arch/arm64/kvm/arm.c | 11 +++++++++++ include/uapi/linux/kvm.h | 1 + 4 files changed, 33 insertions(+)
Documentation/virt/kvm/api.rst +14 -0
@@ -9017,6 +9017,20 @@ Do not use KVM_X86_SW_PROTECTED_VM for "real" VMs, and especially not in production. The behavior and effective ABI for software-protected VMs is unstable. +8.42 KVM_CAP_ARM_MTE_PERM +------------------------ + +:Capability: KVM_CAP_ARM_MTE_PERM +:Architectures: arm64 +:Type: vm + +This capability, if KVM_CHECK_EXTENSION indicates that it is available, means +that the kernel has support for mapping memory regions not supporting +allocations tags into a guest which enables KVM_CAP_ARM_MTE capability. + +In order to use this, it has to be activated by setting this capability via +KVM_ENABLE_CAP ioctl on the VM fd. + 9. Known KVM API problems =========================
arch/arm64/include/asm/kvm_host.h +7 -0
@@ -331,6 +331,9 @@ struct kvm_arch {#define KVM_ARCH_FLAG_ID_REGS_INITIALIZED 7/* Fine-Grained UNDEF initialised */ #define KVM_ARCH_FLAG_FGU_INITIALIZED 8 +/* Memory Tagging Extension NoTagAccess check enabled for the guest */ +#define KVM_ARCH_FLAG_MTE_PERM_ENABLED 9 +unsigned long flags;/* VM-wide vCPU feature set */
@@ -1417,6 +1420,10 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu);#define kvm_vm_has_ran_once (kvm) \ (test_bit(KVM_ARCH_FLAG_HAS_RAN_ONCE, &(kvm)->arch.flags)) +#define kvm_has_mte_perm (kvm) \ + (system_supports_notagaccess() && \ + test_bit(KVM_ARCH_FLAG_MTE_PERM_ENABLED, &(kvm)->arch.flags)) +static inline bool __vcpu_has_feature (const struct kvm_arch *ka,int feature) {return test_bit (feature, ka->vcpu_features);
arch/arm64/kvm/arm.c +11 -0
@@ -150,6 +150,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, }mutex_unlock (&kvm->slots_lock);break ; +case KVM_CAP_ARM_MTE_PERM : +mutex_lock (&kvm->lock); +if (system_supports_notagaccess () && !kvm->created_vcpus) { + r =0 ; +set_bit (KVM_ARCH_FLAG_MTE_PERM_ENABLED , &kvm->arch.flags); + } +mutex_unlock (&kvm->lock); +break ;default :break ; }
@@ -418,6 +426,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES : r =BIT (0 );break ; +case KVM_CAP_ARM_MTE_PERM : + r =system_supports_notagaccess (); +break ;default : r =0 ; }
include/uapi/linux/kvm.h +1 -0
@@ -933,6 +933,7 @@ struct kvm_enable_cap {#define KVM_CAP_PRE_FAULT_MEMORY 236#define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237#define KVM_CAP_X86_GUEST_MODE 238 +#define KVM_CAP_ARM_MTE_PERM 239struct kvm_irq_routing_irqchip {__u32 irqchip;
Signature · 1 line
2.43.0
PATCH v2 5/7 KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
In reply to Aneesh Kumar K.V (Arm)Currently, the kernel won't start a guest if the MTE feature is enabled and the guest RAM is backed by memory which doesn't support access tags. Update this such that the kernel uses the NoTagAccess memory attribute while mapping pages from VMAs for which MTE is not allowed. The fault from accessing the access tags with such pages is forwarded to VMM so that VMM can decide to kill the guest or take any corrective actions
--- Documentation/virt/kvm/api.rst | 3 +++ arch/arm64/include/asm/kvm_emulate.h | 5 +++++ arch/arm64/include/asm/kvm_pgtable.h | 1 + arch/arm64/kvm/hyp/pgtable.c | 16 +++++++++++++--- arch/arm64/kvm/mmu.c | 17 ++++++++++++++--- include/linux/kvm_host.h | 10 ++++++++++ include/uapi/linux/kvm.h | 1 + 7 files changed, 47 insertions(+), 6 deletions(-)
Documentation/virt/kvm/api.rst +3 -0
@@ -7115,6 +7115,9 @@ describes properties of the faulting access that are likely pertinent: - KVM_MEMORY_EXIT_FLAG_PRIVATE - When set, indicates the memory fault occurred on a private memory access. When clear, indicates the fault occurred on a shared access. + - KVM_MEMORY_EXIT_FLAG_NOTAGACCESS - When set, indicates the memory fault + occurred due to allocation tag access on a memory region that doesn't support + allocation tags. Note! KVM_EXIT_MEMORY_FAULT is unique among all KVM exit reasons in that it accompanies a return code of '-1', not '0'! errno will always be set to EFAULT
arch/arm64/include/asm/kvm_emulate.h +5 -0
@@ -378,6 +378,11 @@ static inline bool kvm_vcpu_trap_is_exec_fault(const struct kvm_vcpu *vcpu)return kvm_vcpu_trap_is_iabt (vcpu) && !kvm_vcpu_abt_iss1tw (vcpu); } +static inline bool kvm_vcpu_trap_is_tagaccess (const struct kvm_vcpu *vcpu) +{ +return !!(ESR_ELx_ISS2 (kvm_vcpu_get_esr (vcpu)) & ESR_ELx_TagAccess); +} + static __always_inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu) {return kvm_vcpu_get_esr (vcpu) & ESR_ELx_FSC;
arch/arm64/include/asm/kvm_pgtable.h +1 -0
@@ -252,6 +252,7 @@ enum kvm_pgtable_prot {KVM_PGTABLE_PROT_DEVICE =BIT (3 ),KVM_PGTABLE_PROT_NORMAL_NC =BIT (4 ), +KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS =BIT (5 ),KVM_PGTABLE_PROT_SW0 =BIT (55 ), KVM_PGTABLE_PROT_SW1 = BIT(56),
arch/arm64/kvm/hyp/pgtable.c +13 -3
@@ -677,9 +677,11 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot p {kvm_pte_t attr;u32 sh =KVM_PTE_LEAF_ATTR_LO_S2_SH_IS ; +unsigned long prot_mask =KVM_PGTABLE_PROT_DEVICE | +KVM_PGTABLE_PROT_NORMAL_NC | +KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS ; -switch (prot & (KVM_PGTABLE_PROT_DEVICE | -KVM_PGTABLE_PROT_NORMAL_NC )) { +switch (prot & prot_mask) {case KVM_PGTABLE_PROT_DEVICE |KVM_PGTABLE_PROT_NORMAL_NC :return -EINVAL ;case KVM_PGTABLE_PROT_DEVICE :
@@ -692,6 +694,12 @@ static int stage2_set_prot_attr(struct kvm_pgtable *pgt, enum kvm_pgtable_prot preturn -EINVAL ; attr =KVM_S2_MEMATTR (pgt,NORMAL_NC );break ; +case KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS : +if (system_supports_notagaccess ()) + attr =KVM_S2_MEMATTR (pgt,NORMAL_NOTAGACCESS ); +else +return -EINVAL ; +break ;default : attr =KVM_S2_MEMATTR (pgt,NORMAL ); }
@@ -872,7 +880,9 @@ static void stage2_unmap_put_pte(const struct kvm_pgtable_visit_ctx *ctx,static bool stage2_pte_cacheable (struct kvm_pgtable *pgt,kvm_pte_t pte) {u64 memattr = pte &KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR ; -return kvm_pte_valid (pte) && memattr ==KVM_S2_MEMATTR (pgt,NORMAL ); +return kvm_pte_valid (pte) && + ((memattr ==KVM_S2_MEMATTR (pgt,NORMAL )) || + (memattr ==KVM_S2_MEMATTR (pgt,NORMAL_NOTAGACCESS ))); }static bool stage2_pte_executable (kvm_pte_t pte)
arch/arm64/kvm/mmu.c +14 -3
@@ -1660,9 +1660,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,if (!fault_is_perm && !device &&kvm_has_mte (kvm)) {/* Check the VMM hasn't introduced a new disallowed VMA */ -if (mte_allowed) { +if (mte_allowed)sanitise_mte_tags (kvm, pfn, vma_pagesize); - }else { +else if (kvm_has_mte_perm (kvm)) + prot |=KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS ; +else { ret = -EFAULT ; goto out_unlock; }
@@ -1840,6 +1842,14 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu) gfn = ipa >>PAGE_SHIFT ; memslot =gfn_to_memslot (vcpu->kvm, gfn); + +if (kvm_vcpu_trap_is_tagaccess (vcpu)) { +/* exit to host and handle the error */ +kvm_prepare_notagaccess_exit (vcpu, gfn <<PAGE_SHIFT ,PAGE_SIZE ); + ret =0 ; + goto out; + } + hva =gfn_to_hva_memslot_prot (memslot, gfn, &writable); write_fault =kvm_is_write_fault (vcpu);if (kvm_is_error_hva (hva) || (write_fault && !writable)) {
@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }
include/linux/kvm_host.h +10 -0
@@ -2471,6 +2471,16 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu, vcpu->run->memory_fault.flags |=KVM_MEMORY_EXIT_FLAG_PRIVATE ; } +static inline void kvm_prepare_notagaccess_exit (struct kvm_vcpu *vcpu, +gpa_t gpa,gpa_t size) +{ + vcpu->run->exit_reason =KVM_EXIT_MEMORY_FAULT ; + vcpu->run->memory_fault.flags =KVM_MEMORY_EXIT_FLAG_NOTAGACCESS ; + vcpu->run->memory_fault.gpa = gpa; + vcpu->run->memory_fault.size = size; +} + +#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES static inline unsigned long kvm_get_memory_attributes (struct kvm *kvm,gfn_t gfn) {
include/uapi/linux/kvm.h +1 -0
@@ -442,6 +442,7 @@ struct kvm_run {/* KVM_EXIT_MEMORY_FAULT */ struct {#define KVM_MEMORY_EXIT_FLAG_PRIVATE (1ULL << 3) +#define KVM_MEMORY_EXIT_FLAG_NOTAGACCESS (1ULL << 4)__u64 flags;__u64 gpa;__u64 size;
Signature · 1 line
2.43.0
Catalin Marinas
maintainerRe: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 8 lines · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled and the guest RAM is backed by memory which doesn't support access tags. Update this such that the kernel uses the NoTagAccess memory attribute while mapping pages from VMAs for which MTE is not allowed. The fault from accessing the access tags with such pages is forwarded to VMM so that VMM can decide to kill the guest or take any corrective actions
Signed-off-by Aneesh Kumar K.V (Arm) <[email protected]>
Mostly nitpicks below (apart from the slot registration). The decision on whether that's the best approach lies with Oliver/Marc.
arch/arm64/include/asm/kvm_emulate.h static inline bool kvm_vcpu_trap_is_exec_fault(const struct kvm_vcpu *vcpu) · line 384
Quoted 12 lines
arch/arm64/include/asm/kvm_emulate.h +4
-0@@ -378,6 +378,11 @@ static inline bool kvm_vcpu_trap_is_exec_fault(const struct kvm_vcpu *vcpu)return kvm_vcpu_trap_is_iabt (vcpu) && !kvm_vcpu_abt_iss1tw (vcpu); } +static inline bool kvm_vcpu_trap_is_tagaccess (const struct kvm_vcpu *vcpu) +{ +return !!(ESR_ELx_ISS2 (kvm_vcpu_get_esr (vcpu)) & ESR_ELx_TagAccess); +}
The function type is already bool, no need for the "!!".
Quoted 8 lines
arch/arm64/kvm/mmu.c +0
-0@@ -1660,9 +1660,11 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,if (!fault_is_perm && !device &&kvm_has_mte (kvm)) {/* Check the VMM hasn't introduced a new disallowed VMA */
arch/arm64/kvm/mmu.c static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, · line 1670
Quoted 10 lines
arch/arm64/kvm/mmu.c +4
-2-if (mte_allowed) { +if (mte_allowed)sanitise_mte_tags (kvm, pfn, vma_pagesize); - }else { +else if (kvm_has_mte_perm (kvm)) + prot |=KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS ; +else { ret = -EFAULT ; goto out_unlock; }
Don't remove the brackets at the end of "if (mte_allowed) {" etc. The coding style does require them if at least one of the branches is multi-line (the last "else").
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
arch/arm64/kvm/mmu.c +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }
I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for VM_PFNMAP).
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file mmap() passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this for VM_PFNMAP mappings further down in this function (and move the VM_PFNMAP check above; see Suzuki's internal patch, unless he posted it publicly already).
Signature · 1 line
Catalin
Re: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
arch/arm64/kvm/mmu.c +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).
Quoted 7 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case). With MTE_PERM, we can essentially skip the kvm_vma_mte_allowed(vma) check because we handle all types in the fault handler.
-aneesh
Catalin Marinas
maintainerRe: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 27 lines · Aneesh Kumar K.V
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
arch/arm64/kvm/mmu.c +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).Quoted 6 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).
Quoted 2 lines
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case).
Ah, I forgot about this use case. So with virtiofs DAX, does a host page cache page (host VMM mmap()) get mapped directly into the guest as a separate memory slot? In this case, the host vma would not have VM_MTE_ALLOWED set.
Quoted 3 lines
With
MTE_PERM, we can essentially skip thekvm_vma_mte_allowed(vma)check because we handle all types in the fault handler.
This was pretty much the early behaviour when we added KVM support for MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed VM_SHARED because of some non-trivial race. Commit d89585fbb308 ("KVM:
arm64: unify the tests for VMAs in memslots when MTE is enabled") changed this behaviour and the VM_MTE_ALLOWED check happens upfront. A subsequent commit removed the VM_SHARED check.
It's a minor ABI change but I'm trying to figure out why we needed this upfront check rather than simply dropping the VM_SHARED check. Adding Peter in case he remembers. I can't see any race if we simply skipped this check altogether, irrespective of FEAT_MTE_PERM.
Signature · 1 line
Catalin
Re: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 48 lines · Catalin Marinas
Quoted 27 lines · Aneesh Kumar K.V
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
arch/arm64/kvm/mmu.c +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).Quoted 6 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).Quoted 2 lines
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case).
Ah, I forgot about this use case. So with virtiofs DAX, does a host page cache page (host VMM
mmap()) get mapped directly into the guest as a separate memory slot? In this case, the host vma would not haveVM_MTE_ALLOWEDset.Quoted 3 lines
With
MTE_PERM, we can essentially skip thekvm_vma_mte_allowed(vma)check because we handle all types in the fault handler.This was pretty much the early behaviour when we added KVM support for MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed
VM_SHAREDbecause of some non-trivial race. Commitd89585fbb308("KVM:
arm64: unify the tests for VMAs in memslots when MTE is enabled") changed this behaviour and theVM_MTE_ALLOWEDcheck happens upfront. A subsequent commit removed theVM_SHAREDcheck.
Quoted 4 lines
It's a minor ABI change but I'm trying to figure out why we needed this upfront check rather than simply dropping the
VM_SHAREDcheck. Adding Peter in case he remembers. I can't see any race if we simply skipped this check altogether, irrespective ofFEAT_MTE_PERM.
I don't see a problem with removing the upfront check. The reason I kept the check was IIRC just that there was already a check there and its logic needed to be adjusted for my VM_SHARED changes.
Peter
Suzuki K Poulose
reviewerRe: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Hi,
Quoted 24 lines · Peter Collingbourne
Quoted 22 lines · Catalin Marinas
Quoted 20 lines · Aneesh Kumar K.V
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
Review int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
(quoted diff) +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, if (!vma) break; - if (kvm_has_mte(kvm) && !kvm_vma_mte_allowed(vma)) { + if (kvm_has_mte(kvm) && + !kvm_has_mte_perm(kvm) && !kvm_vma_mte_allowed(vma)) { ret = -EINVAL; break; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).
For the record, here is the patch Catalin was referring to.
--->8---
kvm: arm64: Allow device mappings with MTE
When a VM is configured to use MTE, we prevent mapping a "Device" to the VM.
e.g: with kvmtool (with additional debugging to dump error code and addresses)
$ lkvm run ... --vfio 0000:01:00.0 ...
Info: Using IOMMU type 3 for VFIO container
Error: 0000:01:00.0: failed to register region with KVM
50020000-50022000: -22
Warning: [0abc:aced] Error activating emulation for BAR 0
Error: 0000:01:00.0: failed to configure regions
Warning: Failed init: vfio__init
Only check for the MTE allowed for a VMA if it is not backed by "device"
--- arch/arm64/kvm/mmu.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
arch/arm64/kvm/mmu.c +3 -5
@@ -2295,17 +2295,15 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { - ret = -EINVAL ; -break ; - } -if (vma->vm_flags &VM_PFNMAP ) {/* IO region dirty page logging not allowed */ if (new->flags &KVM_MEM_LOG_DIRTY_PAGES ) { ret = -EINVAL ;break ; } + }else if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { + ret = -EINVAL ; +break ; } hva =min (reg_end, vma->vm_end); }while (hva < reg_end);
Signature · 4 lines
2.34.1 Suzuki
Quoted 30 lines
Quoted 20 lines
Quoted 3 lines
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case).
Ah, I forgot about this use case. So with virtiofs DAX, does a host page cache page (host VMM
mmap()) get mapped directly into the guest as a separate memory slot? In this case, the host vma would not haveVM_MTE_ALLOWEDset.Quoted 3 lines
With
MTE_PERM, we can essentially skip thekvm_vma_mte_allowed(vma)check because we handle all types in the fault handler.This was pretty much the early behaviour when we added KVM support for MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed
VM_SHAREDbecause of some non-trivial race. Commitd89585fbb308("KVM:
arm64: unify the tests for VMAs in memslots when MTE is enabled") changed this behaviour and theVM_MTE_ALLOWEDcheck happens upfront. A subsequent commit removed theVM_SHAREDcheck.Quoted 4 lines
It's a minor ABI change but I'm trying to figure out why we needed this upfront check rather than simply dropping the
VM_SHAREDcheck. Adding Peter in case he remembers. I can't see any race if we simply skipped this check altogether, irrespective ofFEAT_MTE_PERM.I don't see a problem with removing the upfront check. The reason I kept the check was IIRC just that there was already a check there and its logic needed to be adjusted for my
VM_SHAREDchanges.Peter
Catalin Marinas
maintainerRe: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
In reply to Peter CollingbourneQuoted 49 lines · Peter Collingbourne
Quoted 47 lines · Catalin Marinas
Quoted 27 lines · Aneesh Kumar K.V
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
arch/arm64/kvm/mmu.c +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { +if (kvm_has_mte (kvm) && + !kvm_has_mte_perm (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).Quoted 6 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).Quoted 2 lines
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case).
Ah, I forgot about this use case. So with virtiofs DAX, does a host page cache page (host VMM
mmap()) get mapped directly into the guest as a separate memory slot? In this case, the host vma would not haveVM_MTE_ALLOWEDset.Quoted 3 lines
With
MTE_PERM, we can essentially skip thekvm_vma_mte_allowed(vma)check because we handle all types in the fault handler.This was pretty much the early behaviour when we added KVM support for MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed
VM_SHAREDbecause of some non-trivial race. Commitd89585fbb308("KVM:
arm64: unify the tests for VMAs in memslots when MTE is enabled") changed this behaviour and theVM_MTE_ALLOWEDcheck happens upfront. A subsequent commit removed theVM_SHAREDcheck.
Quoted 8 lines
Quoted 4 lines
It's a minor ABI change but I'm trying to figure out why we needed this upfront check rather than simply dropping the
VM_SHAREDcheck. Adding Peter in case he remembers. I can't see any race if we simply skipped this check altogether, irrespective ofFEAT_MTE_PERM.I don't see a problem with removing the upfront check. The reason I kept the check was IIRC just that there was already a check there and its logic needed to be adjusted for my
VM_SHAREDchanges.
Prior to commit d89585fbb308, 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 (!VM_MTE_ALLOWED || VM_SHARED). A subsequent commit dropped the VM_SHARED check, so we ended up with memory slots being rejected only if !VM_MTE_ALLOWED (of course, if kvm_has_mte()). This wasn't the case before the VM_SHARED relaxation.
So if you don't remember any strong reason for this change, I think we should go back to the original behaviour of deferring the VM_MTE_ALLOWED check to user_mem_abort() (and still permitting VM_SHARED).
Signature · 1 line
Catalin
Re: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 67 lines · Catalin Marinas
Quoted 49 lines · Peter Collingbourne
Quoted 47 lines · Catalin Marinas
Quoted 27 lines · Aneesh Kumar K.V
Quoted 2 lines · Catalin Marinas
Quoted 1 line · Aneesh Kumar K.V (Arm)
Currently, the kernel won't start a guest if the MTE feature is enabled
...
Quoted 14 lines
Review int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2169
Quoted 10 lines
(quoted diff) +2
-1@@ -2152,7 +2162,8 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm, if (!vma) break; - if (kvm_has_mte(kvm) && !kvm_vma_mte_allowed(vma)) { + if (kvm_has_mte(kvm) && + !kvm_has_mte_perm(kvm) && !kvm_vma_mte_allowed(vma)) { ret = -EINVAL; break; }I don't think we should change this, or at least not how it's done above (Suzuki raised a related issue internally relaxing this for
VM_PFNMAP).Quoted 6 lines
For standard memory slots, we want to reject them upfront rather than deferring to the fault handler. An example here is file
mmap()passed as standard RAM to the VM. It's an unnecessary change in behaviour IMHO. I'd only relax this forVM_PFNMAPmappings further down in this function (and move theVM_PFNMAPcheck above; see Suzuki's internal patch, unless he posted it publicly already).Quoted 2 lines
But we want to handle memslots backed by pagecache pages for virtio-shm here (virtiofs dax use case).
Ah, I forgot about this use case. So with virtiofs DAX, does a host page cache page (host VMM
mmap()) get mapped directly into the guest as a separate memory slot? In this case, the host vma would not haveVM_MTE_ALLOWEDset.Quoted 3 lines
With
MTE_PERM, we can essentially skip thekvm_vma_mte_allowed(vma)check because we handle all types in the fault handler.This was pretty much the early behaviour when we added KVM support for MTE, allow !VM_MTE_ALLOWED and trap them later. However, we disallowed
VM_SHAREDbecause of some non-trivial race. Commitd89585fbb308("KVM:
arm64: unify the tests for VMAs in memslots when MTE is enabled") changed this behaviour and theVM_MTE_ALLOWEDcheck happens upfront. A subsequent commit removed theVM_SHAREDcheck.Quoted 8 lines
Quoted 4 lines
It's a minor ABI change but I'm trying to figure out why we needed this upfront check rather than simply dropping the
VM_SHAREDcheck. Adding Peter in case he remembers. I can't see any race if we simply skipped this check altogether, irrespective ofFEAT_MTE_PERM.I don't see a problem with removing the upfront check. The reason I kept the check was IIRC just that there was already a check there and its logic needed to be adjusted for my
VM_SHAREDchanges.Prior to commit
d89585fbb308,kvm_arch_prepare_memory_region()only rejected a memory slot ifVM_SHAREDwas set. This commit unified the checking withuser_mem_abort(), with slots being rejected if (!VM_MTE_ALLOWED ||VM_SHARED). A subsequent commit dropped theVM_SHAREDcheck, so we ended up with memory slots being rejected only if !VM_MTE_ALLOWED (of course, ifkvm_has_mte()). This wasn't the case before theVM_SHAREDrelaxation.
Quoted 4 lines
So if you don't remember any strong reason for this change, I think we should go back to the original behaviour of deferring the
VM_MTE_ALLOWEDcheck touser_mem_abort()(and still permittingVM_SHARED).
Something as below?
From 466237a6f0a165152c157ab4a73f34c400cffe34 Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V (Arm)" <[email protected]>
Date: Tue, 28 Jan 2025 14:21:52 +0530
Subject: [PATCH] KVM: arm64: Drop mte_allowed check during memslot creation
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. 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.
This change is needed because, without it, users are not able to use MTE with VFIO passthrough, as shown below (kvmtool VMM).
[ 617.921030] vfio-pci 0000:01:00.0: resetting [ 618.024719] vfio-pci 0000:01:00.0: reset done Error: 0000:01:00.0: failed to register region with KVM Warning: [0abc:aced] Error activating emulation for BAR 0 Error: 0000:01:00.0: failed to configure regions Warning: Failed init: vfio__init
Fatal: Initialisation failed
--- arch/arm64/kvm/mmu.c | 5 ----- 1 file changed, 5 deletions(-)
arch/arm64/kvm/mmu.c +0 -5
@@ -2146,11 +2146,6 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { - ret = -EINVAL ; -break ; - } -if (vma->vm_flags &VM_PFNMAP ) {/* IO region dirty page logging not allowed */ if (new->flags &KVM_MEM_LOG_DIRTY_PAGES ) {
Signature · 1 line
2.43.0
Catalin Marinas
maintainerRe: [PATCH v2 5/7] KVM: arm64: MTE: Use stage-2 NoTagAccess memory attribute if supported
Quoted 32 lines · Aneesh Kumar K.V
Quoted 3 lines · Catalin Marinas
So if you don't remember any strong reason for this change, I think we should go back to the original behaviour of deferring the
VM_MTE_ALLOWEDcheck touser_mem_abort()(and still permittingVM_SHARED).Something as below?
From
466237a6f0a165152c157ab4a73f34c400cffe34Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V (Arm)" <[email protected]>
Date: Tue, 28 Jan 2025 14:21:52 +0530
Subject: [PATCH] KVM: arm64: Dropmte_allowedcheck during memslot creationBefore 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 ifVM_SHAREDwas set. This commit unified the checking withuser_mem_abort(), with slots being rejected if eitherVM_MTE_ALLOWEDis not set orVM_SHAREDset. A subsequent commitc911f0d46879("KVM: arm64: permit allVM_MTE_ALLOWEDmappings with MTE enabled") dropped theVM_SHAREDcheck, so we ended up with memory slots being rejected ifVM_MTE_ALLOWEDis not set. This wasn't the case before the commitd89585fbb308. The rejection of the memory slot withVM_SHAREDset was done to avoid a race condition with the test/set of thePG_mte_taggedflag. Before Commitd77e59a8fccd("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 thePG_mte_taggedflag racily.Commit
d77e59a8fccd("arm64: mte: Lock a page for MTE tag initialization") further updated the locking so that the kernel allowsVM_SHAREDmapping with MTE. With this commit, we can enable memslot creation withVM_SHAREDVMA mapping.
Quoted 7 lines
This patch results in a minor tweak to the ABI. We now allow creating memslots that don't have the
VM_MTE_ALLOWEDflag set. If the guest uses such a memslot with Allocation Tags, the kernel will generate-EFAULT. ie, instead of failing early, we now fail later duringKVM_RUN.This change is needed because, without it, users are not able to use MTE with VFIO passthrough, as shown below (kvmtool VMM).
You might want to clearly state it that the VFIO passthrough is Device or NonCacheable (for the time being, parallel series from Ankit on allowing cacheable).
Quoted 19 lines
[ 617.921030] vfio-pci 0000:01:00.0: resetting [ 618.024719] vfio-pci 0000:01:00.0: reset done Error: 0000:01:00.0: failed to register region with KVM Warning: [0abc:aced] Error activating emulation for BAR 0 Error: 0000:01:00.0: failed to configure regions Warning: Failed init: vfio__initFatal: Initialisation failed
Signed-off-by Aneesh Kumar K.V (Arm) <[email protected]>--- arch/arm64/kvm/mmu.c | 5 ----- 1 file changed, 5 deletions(-)arch/arm64/kvm/mmu.c +0
-0
arch/arm64/kvm/mmu.c int kvm_arch_prepare_memory_region(struct kvm *kvm, · line 2152
Quoted 7 lines
arch/arm64/kvm/mmu.c +0
-4@@ -2146,11 +2146,6 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { - ret = -EINVAL ; -break ; - }
Works for me. It's up to the KVM maintainers whether they are ok with this ABI change.
Signature · 1 line
Catalin
PATCH v2 6/7 KVM: arm64: MTE: Nested guest support
In reply to Aneesh Kumar K.V (Arm)Currently MTE feature is not enabled for a EL1 guest and similarly we disable MTE_PERM. But we do add code in this patch to allow using KVM_CAP_ARM_MTE_PERM with an EL1 guest. This will allow the usage of MTE in nested guest even if some of the memory backing nested guest RAM is not MTE capable (ex: page cache pages).
--- arch/arm64/include/asm/kvm_nested.h | 10 ++++++++++ arch/arm64/kvm/mmu.c | 10 ++++++++++ arch/arm64/kvm/nested.c | 28 ++++++++++++++++++++++++++++ arch/arm64/kvm/sys_regs.c | 15 +++++++++++---- 4 files changed, 59 insertions(+), 4 deletions(-)
arch/arm64/include/asm/kvm_nested.h +10 -0
@@ -86,6 +86,8 @@ struct kvm_s2_trans {bool writable;bool readable;int level; +int s2_fwb; +int mem_attr;u32 esr;u64 desc; };
@@ -120,10 +122,18 @@ static inline bool kvm_s2_trans_executable(struct kvm_s2_trans *trans)return !(trans->desc &BIT (54 )); } +static inline bool kvm_s2_trans_tagaccess (struct kvm_s2_trans *trans) +{ +if (trans->s2_fwb) +return (trans->mem_attr &MT_S2_FWB_NORMAL_NOTAGACCESS ) !=MT_S2_FWB_NORMAL_NOTAGACCESS ; +return (trans->mem_attr &MT_S2_NORMAL_NOTAGACCESS ) !=MT_S2_NORMAL_NOTAGACCESS ; +} +extern int kvm_walk_nested_s2 (struct kvm_vcpu *vcpu,phys_addr_t gipa,struct kvm_s2_trans *result);extern int kvm_s2_handle_perm_fault (struct kvm_vcpu *vcpu,struct kvm_s2_trans *trans); +int kvm_s2_handle_notagaccess_fault (struct kvm_vcpu *vcpu,struct kvm_s2_trans *trans);extern int kvm_inject_s2_fault (struct kvm_vcpu *vcpu,u64 esr_el2);extern void kvm_nested_s2_wp (struct kvm *kvm);extern void kvm_nested_s2_unmap (struct kvm *kvm,bool may_block);
arch/arm64/kvm/mmu.c +10 -0
@@ -1640,6 +1640,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, goto out_unlock; } +if (nested && !kvm_s2_trans_tagaccess (nested)) + mte_allowed = false; +/* * If we are not forced to use page mapping, check if we are * backed by a THP and thus use block mapping if possible.
@@ -1836,6 +1839,13 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu) goto out_unlock; } + ret =kvm_s2_handle_notagaccess_fault (vcpu, &nested_trans); +if (ret) { + esr =kvm_s2_trans_esr (&nested_trans); +kvm_inject_s2_fault (vcpu, esr); + goto out_unlock; + } + ipa =kvm_s2_trans_output (&nested_trans); nested = &nested_trans; }
arch/arm64/kvm/nested.c +28 -0
@@ -290,6 +290,7 @@ static int walk_nested_s2_pgd(phys_addr_t ipa, out->writable = desc & (0b10 <<6 ); out->level = level; out->desc = desc; + out->mem_attr = desc &KVM_PTE_LEAF_ATTR_LO_S2_MEMATTR ;return 0 ; }
@@ -340,6 +341,7 @@ int kvm_walk_nested_s2(struct kvm_vcpu *vcpu, phys_addr_t gipa, wi.be =vcpu_read_sys_reg (vcpu,SCTLR_EL2 ) & SCTLR_ELx_EE; + result->s2_fwb = !!(*vcpu_hcr (vcpu) &HCR_FWB ); ret =walk_nested_s2_pgd (gipa, &wi, result);if (ret) result->esr |= (kvm_vcpu_get_esr (vcpu) & ~ESR_ELx_FSC);
@@ -733,6 +735,27 @@ int kvm_s2_handle_perm_fault(struct kvm_vcpu *vcpu, struct kvm_s2_trans *trans)return forward_fault; } +int kvm_s2_handle_notagaccess_fault (struct kvm_vcpu *vcpu,struct kvm_s2_trans *trans) +{ +bool forward_fault = false; + + trans->esr =0 ; + +if (!kvm_vcpu_trap_is_tagaccess (vcpu)) +return 0 ; + +if (!kvm_s2_trans_tagaccess (trans)) + forward_fault = true; +else + forward_fault = false; + +/* forward it as a permission fault with tag access set in ISS2 */ +if (forward_fault) + trans->esr =esr_s2_fault (vcpu, trans->level, ESR_ELx_FSC_PERM); + +return forward_fault; +} +int kvm_inject_s2_fault (struct kvm_vcpu *vcpu,u64 esr_el2) {vcpu_write_sys_reg (vcpu, vcpu->arch.fault.far_el2,FAR_EL2 );
@@ -844,6 +867,11 @@ static void limit_nv_id_regs(struct kvm *kvm) NV_FTR(PFR1, CSV2_frac));kvm_set_vm_id_reg (kvm,SYS_ID_AA64PFR1_EL1 , val); +/* For now no MTE_PERM support because MTE is disabled above */ + val =kvm_read_vm_id_reg (kvm,SYS_ID_AA64PFR2_EL1 ); + val &= ~NV_FTR (PFR2 ,MTEPERM ); +kvm_set_vm_id_reg (kvm,SYS_ID_AA64PFR2_EL1 , val); +/* Hide ECV, ExS, Secure Memory */ val =kvm_read_vm_id_reg (kvm,SYS_ID_AA64MMFR0_EL1 ); val &= ~(NV_FTR(MMFR0, ECV) |
arch/arm64/kvm/sys_regs.c +11 -4
@@ -1557,7 +1557,7 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu, const struct sys_reg_desc *r) {u32 id =reg_to_encoding (r); -u64 val; +u64 val, mask;if (sysreg_visible_as_raz (vcpu, r))return 0 ;
@@ -1587,8 +1587,14 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu, val &= ~ARM64_FEATURE_MASK (ID_AA64PFR1_EL1_MPAM_frac);break ;case SYS_ID_AA64PFR2_EL1 : -/* We only expose FPMR */ - val &=ID_AA64PFR2_EL1_FPMR ; + mask =ID_AA64PFR2_EL1_FPMR ; +/* +* Since this is a stage-2 specific feature, only pass +* if vcpu can run in vEL2 +*/ +if (vcpu_has_nv (vcpu)) + mask |=ID_AA64PFR2_EL1_MTEPERM ; + val &= mask;break ;case SYS_ID_AA64ISAR1_EL1 :if (!vcpu_has_ptrauth (vcpu))
@@ -2566,7 +2572,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_AA64PFR1_EL1_MPAM_frac | ID_AA64PFR1_EL1_RAS_frac | ID_AA64PFR1_EL1_MTE)), -ID_WRITABLE (ID_AA64PFR2_EL1 ,ID_AA64PFR2_EL1_FPMR ), +ID_WRITABLE (ID_AA64PFR2_EL1 ,ID_AA64PFR2_EL1_FPMR | +ID_AA64PFR2_EL1_MTEPERM ),ID_UNALLOCATED (4 ,3 ),ID_WRITABLE (ID_AA64ZFR0_EL1 , ~ID_AA64ZFR0_EL1_RES0 ), ID_HIDDEN(ID_AA64SMFR0_EL1),
Signature · 1 line
2.43.0
PATCH v2 7/7 KVM: arm64: Split some of the kvm_pgtable_prot bits into separate defines
In reply to Aneesh Kumar K.V (Arm)Some of the kvm_pgtable_prot values are mutually exclusive, like KVM_PGTABLE_PROT_NORMAL_NC and KVM_PGTABLE_PROT_DEVICE. This patch splits the Normal memory non-cacheable and NoTagAccess attributes into separate #defines. With this change, the kvm_pgtable_prot bits only indicate whether it is a device or normal memory mapping.
There are no functional changes in this patch.
--- arch/arm64/include/asm/kvm_pgtable.h | 10 +++--- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +- arch/arm64/kvm/hyp/pgtable.c | 47 +++++++++++++-------------- arch/arm64/kvm/mmu.c | 10 +++--- 4 files changed, 36 insertions(+), 33 deletions(-)
arch/arm64/include/asm/kvm_pgtable.h +6 -4
@@ -239,7 +239,6 @@ enum kvm_pgtable_stage2_flags {* @KVM_PGTABLE_PROT_W: Write permission. * @KVM_PGTABLE_PROT_R: Read permission. * @KVM_PGTABLE_PROT_DEVICE: Device attributes. -* @KVM_PGTABLE_PROT_NORMAL_NC: Normal noncacheable attributes. * @KVM_PGTABLE_PROT_SW0: Software bit 0. * @KVM_PGTABLE_PROT_SW1: Software bit 1. * @KVM_PGTABLE_PROT_SW2: Software bit 2.
@@ -251,8 +250,6 @@ enum kvm_pgtable_prot {KVM_PGTABLE_PROT_R =BIT (2 ),KVM_PGTABLE_PROT_DEVICE =BIT (3 ), -KVM_PGTABLE_PROT_NORMAL_NC =BIT (4 ), -KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS =BIT (5 ),KVM_PGTABLE_PROT_SW0 =BIT (55 ), KVM_PGTABLE_PROT_SW1 = BIT(56),
@@ -263,6 +260,11 @@ enum kvm_pgtable_prot {#define KVM_PGTABLE_PROT_RW (KVM_PGTABLE_PROT_R | KVM_PGTABLE_PROT_W)#define KVM_PGTABLE_PROT_RWX (KVM_PGTABLE_PROT_RW | KVM_PGTABLE_PROT_X) +/* different memory attribute requested */ +#define KVM_PGTABLE_ATTR_NORMAL_NC 0x1 +#define KVM_PGTABLE_ATTR_NORMAL_NOTAGACCESS 0x2 + +#define PKVM_HOST_MEM_PROT KVM_PGTABLE_PROT_RWX#define PKVM_HOST_MMIO_PROT KVM_PGTABLE_PROT_RW
@@ -606,7 +608,7 @@ kvm_pte_t *kvm_pgtable_stage2_create_unlinked(struct kvm_pgtable *pgt,* Return: 0 on success, negative error code on failure. */ int kvm_pgtable_stage2_map (struct kvm_pgtable *pgt,u64 addr,u64 size, -u64 phys,enum kvm_pgtable_prot prot, +u64 phys,enum kvm_pgtable_prot prot,int mem_attr,void *mc,enum kvm_pgtable_walk_flags flags);/**
arch/arm64/kvm/hyp/nvhe/mem_protect.c +1 -1
@@ -411,7 +411,7 @@ static inline int __host_stage2_idmap(u64 start, u64 end, enum kvm_pgtable_prot prot) {return kvm_pgtable_stage2_map (&host_mmu.pgt, start, end - start, start, - prot, &host_s2_pool,0 ); + prot,0 , &host_s2_pool,0 ); }/*
arch/arm64/kvm/hyp/pgtable.c +23 -24
79 lines not on this page · raw message
arch/arm64/kvm/mmu.c +6 -4
@@ -1130,8 +1130,8 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,break ;write_lock (&kvm->mmu_lock); - ret =kvm_pgtable_stage2_map (pgt, addr,PAGE_SIZE , pa, prot, - &cache,0 ); + ret =kvm_pgtable_stage2_map (pgt, addr,PAGE_SIZE , + pa, prot,0 , &cache,0 );write_unlock (&kvm->mmu_lock);if (ret)break ;
@@ -1452,6 +1452,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,enum kvm_pgtable_prot prot =KVM_PGTABLE_PROT_R ;struct kvm_pgtable *pgt;struct page *page; +int normal_memattr =0 ;if (fault_is_perm) fault_granule =kvm_vcpu_trap_get_perm_fault_granule (vcpu);
@@ -1666,7 +1667,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,if (mte_allowed)sanitise_mte_tags (kvm, pfn, vma_pagesize);else if (kvm_has_mte_perm (kvm)) - prot |=KVM_PGTABLE_PROT_NORMAL_NOTAGACCESS ; + normal_memattr =KVM_PGTABLE_ATTR_NORMAL_NOTAGACCESS ;else { ret = -EFAULT ; goto out_unlock;
@@ -1681,7 +1682,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,if (device) {if (vfio_allow_any_uc) - prot |=KVM_PGTABLE_PROT_NORMAL_NC ; + normal_memattr =KVM_PGTABLE_ATTR_NORMAL_NC ;else prot |=KVM_PGTABLE_PROT_DEVICE ; } else if (cpus_have_final_cap(ARM64_HAS_CACHE_DIC) &&
@@ -1704,6 +1705,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, }else { ret =kvm_pgtable_stage2_map (pgt, fault_ipa, vma_pagesize,__pfn_to_phys (pfn), prot, + normal_memattr, memcache,KVM_PGTABLE_WALK_HANDLE_FAULT |KVM_PGTABLE_WALK_SHARED );
Signature · 1 line
2.43.0