KVM: arm64: permit MAP_SHARED mappings with MTE enabled
Subsystems (9)
v5 cover KVM: arm64: permit MAP_SHARED mappings with MTE enabled
Hi,
This patch series allows VMMs to use shared mappings in MTE enabled guests. The first five patches were taken from Catalin's tree [1] which addressed some review feedback from when they were previously sent out as v3 of this series. The first patch from Catalin's tree makes room for an additional PG_arch_3 flag by making the newer PG_arch_* flags arch-dependent. The next four patches are based on a series that Catalin sent out prior to v3, whose cover letter [2] I quote from below:
Quoted 14 lines
This series aims to fix the races between initialising the tags on a page and setting the
PG_mte_taggedflag. Currently the flag is set either before or after that tag initialisation and this can lead to CoW copying stale tags. The first patch moves the flag setting after the tags have been initialised, solving the CoW issue. However, concurrentmprotect()on a shared mapping may (very rarely) lead to valid tags being zeroed.The second skips the
sanitise_mte_tags()call inkvm_set_spte_gfn(), deferring it touser_mem_abort(). The outcome is that nosanitise_mte_tags()can be simplified to skip thepfn_to_online_page()check and only rely onVM_MTE_ALLOWEDvma flag that can be checked inuser_mem_abort().
Quoted 7 lines
The third and fourth patches use
PG_arch_3as a lock for page tagging, based on Peter Collingbourne's idea of a two-bit lock.I think the first patch can be queued but the rest needs some in depth review and test. With this series (if correct) we could allos
MAP_SHAREDon KVM guest memory but this is to be discussed separately as there are some KVM ABI implications.
In this v5 I rebased Catalin's tree onto -next again. Please double check my rebase, which resolved the conflict with commit a8e5e5146ad0 ("arm64:
mte: Avoid setting PG_mte_tagged if no tags cleared or restored").
I now have Reviewed-by for all patches except for the last one, which adds the documentation. Thanks for the reviews so far, and please take a look!
I've tested it on QEMU as well as on MTE-capable hardware by booting a Linux kernel and userspace under a crosvm with MTE support [3].
[1] git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux devel/mte-pg-flags
[2] https://lore.kernel.org/all/[email protected]/
[3] https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3892141
Catalin Marinas (4):
mm: Do not enable PG_arch_2 for all 64-bit architectures
arm64: mte: Fix/clarify the PG_mte_tagged semantics
KVM: arm64: Simplify the sanitise_mte_tags() logic
arm64: mte: Lock a page for MTE tag initialisation
Peter Collingbourne (4):
mm: Add PG_arch_3 page flag
KVM: arm64: unify the tests for VMAs in memslots when MTE is enabled
KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE enabled
Documentation: document the ABI changes for KVM_CAP_ARM_MTE
Documentation/virt/kvm/api.rst | 5 ++- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/mte.h | 65 ++++++++++++++++++++++++++++++- arch/arm64/include/asm/pgtable.h | 4 +- arch/arm64/kernel/cpufeature.c | 4 +- arch/arm64/kernel/elfcore.c | 2 +- arch/arm64/kernel/hibernate.c | 2 +- arch/arm64/kernel/mte.c | 21 +++++----- arch/arm64/kvm/guest.c | 18 +++++---- arch/arm64/kvm/mmu.c | 55 +++++++++++--------------- arch/arm64/mm/copypage.c | 7 +++- arch/arm64/mm/fault.c | 4 +- arch/arm64/mm/mteswap.c | 16 +++----- fs/proc/page.c | 3 +- include/linux/kernel-page-flags.h | 1 + include/linux/page-flags.h | 3 +- include/trace/events/mmflags.h | 9 +++-- mm/Kconfig | 8 ++++ mm/huge_memory.c | 3 +- 19 files changed, 152 insertions(+), 79 deletions(-)
Signature · 1 line
2.38.1.431.g37b22c650d-goog
PATCH v5 1/8 mm: Do not enable PG_arch_2 for all 64-bit architectures
From: Catalin Marinas <[email protected]>
Commit 4beba9486abd ("mm: Add PG_arch_2 page flag") introduced a new page flag for all 64-bit architectures. However, even if an architecture is 64-bit, it may still have limited spare bits in the 'flags' member of 'struct page'. This may happen if an architecture enables SPARSEMEM without SPARSEMEM_VMEMMAP as is the case with the newly added loongarch. This architecture port needs 19 more bits for the sparsemem section information and, while it is currently fine with PG_arch_2, adding any more PG_arch_* flags will trigger build-time warnings.
Add a new CONFIG_ARCH_USES_PG_ARCH_X option which can be selected by architectures that need more PG_arch_* flags beyond PG_arch_1. Select it on arm64.
--- arch/arm64/Kconfig | 1 + fs/proc/page.c | 2 +- include/linux/page-flags.h | 2 +- include/trace/events/mmflags.h | 8 ++++---- mm/Kconfig | 8 ++++++++ mm/huge_memory.c | 2 +- 6 files changed, 16 insertions(+), 7 deletions(-)
arch/arm64/Kconfig +1 -0
@@ -1966,6 +1966,7 @@ config ARM64_MTE depends on ARM64_PAN select ARCH_HAS_SUBPAGE_FAULTS select ARCH_USES_HIGH_VMA_FLAGS + select ARCH_USES_PG_ARCH_X help Memory Tagging (part of the ARMv8.5 Extensions) provides architectural support for run-time, always-on detection of
fs/proc/page.c +1 -1
@@ -219,7 +219,7 @@ u64 stable_page_flags(struct page *page) u |=kpf_copy_bit (k,KPF_PRIVATE_2 , PG_private_2); u |=kpf_copy_bit (k,KPF_OWNER_PRIVATE , PG_owner_priv_1); u |=kpf_copy_bit (k,KPF_ARCH , PG_arch_1); -#ifdef CONFIG_64BIT +#ifdef CONFIG_ARCH_USES_PG_ARCH_X u |=kpf_copy_bit (k,KPF_ARCH_2 , PG_arch_2);#endif
include/linux/page-flags.h +1 -1
@@ -132,7 +132,7 @@ enum pageflags { PG_young, PG_idle,#endif -#ifdef CONFIG_64BIT +#ifdef CONFIG_ARCH_USES_PG_ARCH_X PG_arch_2,#endif #ifdef CONFIG_KASAN_HW_TAGS
include/trace/events/mmflags.h +4 -4
@@ -90,10 +90,10 @@#define IF_HAVE_PG_IDLE (flag,string)#endif -#ifdef CONFIG_64BIT -#define IF_HAVE_PG_ARCH_2 (flag,string) ,{1UL << flag, string} +#ifdef CONFIG_ARCH_USES_PG_ARCH_X +#define IF_HAVE_PG_ARCH_X (flag,string) ,{1UL << flag, string}#else -#define IF_HAVE_PG_ARCH_2 (flag,string) +#define IF_HAVE_PG_ARCH_X (flag,string)#endif #ifdef CONFIG_KASAN_HW_TAGS
@@ -129,7 +129,7 @@ IF_HAVE_PG_UNCACHED(PG_uncached, "uncached" ) \ IF_HAVE_PG_HWPOISON(PG_hwpoison, "hwpoison" ) \ IF_HAVE_PG_IDLE(PG_young, "young" ) \ IF_HAVE_PG_IDLE(PG_idle, "idle" ) \ -IF_HAVE_PG_ARCH_2(PG_arch_2, "arch_2" ) \ +IF_HAVE_PG_ARCH_X(PG_arch_2, "arch_2" ) \ IF_HAVE_PG_SKIP_KASAN_POISON(PG_skip_kasan_poison, "skip_kasan_poison")#define show_page_flags (flags) \
mm/Kconfig +8 -0
@@ -1005,6 +1005,14 @@ config ARCH_USES_HIGH_VMA_FLAGS config ARCH_HAS_PKEYS bool +config ARCH_USES_PG_ARCH_X + bool + help + Enable the definition of PG_arch_x page flags with x > 1. Only + suitable for 64-bit architectures with CONFIG_FLATMEM or + CONFIG_SPARSEMEM_VMEMMAP enabled, otherwise there may not be + enough room for additional bits in page->flags. + config VM_EVENT_COUNTERS default y bool "Enable VM event counters for /proc/vmstat" if EXPERT
mm/huge_memory.c +1 -1
@@ -2401,7 +2401,7 @@ static void __split_huge_page_tail(struct page *head, int tail, (1L << PG_workingset) | (1L << PG_locked) | (1L << PG_unevictable) | -#ifdef CONFIG_64BIT +#ifdef CONFIG_ARCH_USES_PG_ARCH_X (1L << PG_arch_2) | #endif (1L << PG_dirty) |
Signature · 1 line
2.38.1.431.g37b22c650d-goog
PATCH v5 2/8 arm64: mte: Fix/clarify the PG_mte_tagged semantics
In reply to Peter CollingbourneFrom: Catalin Marinas <[email protected]>
Currently the PG_mte_tagged page flag mostly means the page contains valid tags and it should be set after the tags have been cleared or restored. However, in mte_sync_tags() it is set before setting the tags to avoid, in theory, a race with concurrent mprotect(PROT_MTE) for shared pages. However, a concurrent mprotect(PROT_MTE) with a copy on write in another thread can cause the new page to have stale tags. Similarly, tag reading via ptrace() can read stale tags if the PG_mte_tagged flag is set before actually clearing/restoring the tags.
Fix the PG_mte_tagged semantics so that it is only set after the tags have been cleared or restored. This is safe for swap restoring into a MAP_SHARED or CoW page since the core code takes the page lock. Add two functions to test and set the PG_mte_tagged flag with acquire and release semantics. The downside is that concurrent mprotect(PROT_MTE) on a MAP_SHARED page may cause tag loss. This is already the case for KVM guests if a VMM changes the page protection while the guest triggers a user_mem_abort().
--- arch/arm64/include/asm/mte.h | 30 ++++++++++++++++++++++++++++++ arch/arm64/include/asm/pgtable.h | 2 +- arch/arm64/kernel/cpufeature.c | 4 +++- arch/arm64/kernel/elfcore.c | 2 +- arch/arm64/kernel/hibernate.c | 2 +- arch/arm64/kernel/mte.c | 17 +++++++++++------ arch/arm64/kvm/guest.c | 4 ++-- arch/arm64/kvm/mmu.c | 4 ++-- arch/arm64/mm/copypage.c | 5 +++-- arch/arm64/mm/fault.c | 2 +- arch/arm64/mm/mteswap.c | 2 +- 11 files changed, 56 insertions(+), 18 deletions(-)
arch/arm64/include/asm/mte.h +30 -0
@@ -37,6 +37,29 @@ void mte_free_tag_storage(char *storage);/* track which pages have valid allocation tags */ #define PG_mte_tagged PG_arch_2 +static inline void set_page_mte_tagged (struct page *page) +{ +/* +* Ensure that the tags written prior to this function are visible +* before the page flags update. +*/ +smp_wmb (); +set_bit (PG_mte_tagged, &page->flags); +} + +static inline bool page_mte_tagged (struct page *page) +{ +bool ret =test_bit (PG_mte_tagged, &page->flags); + +/* +* If the page is tagged, ensure ordering with a likely subsequent +* read of the tags. +*/ +if (ret) +smp_rmb (); +return ret; +} +void mte_zero_clear_page_tags (void *addr);void mte_sync_tags (pte_t old_pte,pte_t pte);void mte_copy_page_tags (void *kto,const void *kfrom);
@@ -56,6 +79,13 @@ size_t mte_probe_user_range(const char __user *uaddr, size_t size);/* unused if !CONFIG_ARM64_MTE, silence the compiler */ #define PG_mte_tagged 0 +static inline void set_page_mte_tagged (struct page *page) +{ +} +static inline bool page_mte_tagged (struct page *page) +{ +return false; +}static inline void mte_zero_clear_page_tags (void *addr) { }
arch/arm64/include/asm/pgtable.h +1 -1
@@ -1048,7 +1048,7 @@ static inline void arch_swap_invalidate_area(int type)static inline void arch_swap_restore (swp_entry_t entry,struct folio *folio) {if (system_supports_mte () &&mte_restore_tags (entry, &folio->page)) -set_bit (PG_mte_tagged, &folio->flags); +set_page_mte_tagged (&folio->page); }#endif /* CONFIG_ARM64_MTE */
arch/arm64/kernel/cpufeature.c +3 -1
@@ -2050,8 +2050,10 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)* Clear the tags in the zero page. This needs to be done via the * linear map which has the Tagged attribute. */ -if (!test_and_set_bit (PG_mte_tagged, &ZERO_PAGE (0 )->flags)) +if (!page_mte_tagged (ZERO_PAGE (0 ))) {mte_clear_page_tags (lm_alias (empty_zero_page)); +set_page_mte_tagged (ZERO_PAGE (0 )); + }kasan_init_hw_tags_cpu (); }
arch/arm64/kernel/elfcore.c +1 -1
@@ -47,7 +47,7 @@ static int mte_dump_tag_range(struct coredump_params *cprm,* Pages mapped in user space as !pte_access_permitted() (e.g. * PROT_EXEC only) may not have the PG_mte_tagged flag set. */ -if (!test_bit (PG_mte_tagged, &page->flags)) { +if (!page_mte_tagged (page)) {put_page (page);dump_skip (cprm,MTE_PAGE_TAG_STORAGE );continue ;
arch/arm64/kernel/hibernate.c +1 -1
@@ -271,7 +271,7 @@ static int swsusp_mte_save_tags(void)if (!page)continue ; -if (!test_bit (PG_mte_tagged, &page->flags)) +if (!page_mte_tagged (page))continue ; ret =save_tags (page, pfn);
arch/arm64/kernel/mte.c +11 -6
@@ -41,8 +41,10 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,if (check_swap &&is_swap_pte (old_pte)) {swp_entry_t entry =pte_to_swp_entry (old_pte); -if (!non_swap_entry (entry) &&mte_restore_tags (entry, page)) +if (!non_swap_entry (entry) &&mte_restore_tags (entry, page)) { +set_page_mte_tagged (page);return ; + } }if (!pte_is_tagged)
@@ -52,8 +54,10 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,* Test PG_mte_tagged again in case it was racing with another * set_pte_at(). */ -if (!test_and_set_bit (PG_mte_tagged, &page->flags)) +if (!page_mte_tagged (page)) {mte_clear_page_tags (page_address (page)); +set_page_mte_tagged (page); + } }void mte_sync_tags (pte_t old_pte,pte_t pte)
@@ -69,9 +73,11 @@ void mte_sync_tags(pte_t old_pte, pte_t pte)/* if PG_mte_tagged is set, tags have already been initialised */ for (i =0 ; i < nr_pages; i++, page++) { -if (!test_bit (PG_mte_tagged, &page->flags)) +if (!page_mte_tagged (page)) {mte_sync_page_tags (page, old_pte, check_swap, pte_is_tagged); +set_page_mte_tagged (page); + } }/* ensure the tags are visible before the PTE is set */
@@ -96,8 +102,7 @@ int memcmp_pages(struct page *page1, struct page *page2)* pages is tagged, set_pte_at() may zero or change the tags of the * other page via mte_sync_tags(). */ -if (test_bit (PG_mte_tagged, &page1->flags) || -test_bit (PG_mte_tagged, &page2->flags)) +if (page_mte_tagged (page1) ||page_mte_tagged (page2))return addr1 != addr2;return ret;
@@ -454,7 +459,7 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,put_page (page);break ; } -WARN_ON_ONCE (!test_bit (PG_mte_tagged, &page->flags)); +WARN_ON_ONCE (!page_mte_tagged (page));/* limit access to the end of the page */ offset =offset_in_page (addr);
arch/arm64/kvm/guest.c +2 -2
@@ -1059,7 +1059,7 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, maddr =page_address (page);if (!write) { -if (test_bit (PG_mte_tagged, &page->flags)) +if (page_mte_tagged (page)) num_tags =mte_copy_tags_to_user (tags, maddr,MTE_GRANULES_PER_PAGE );else
@@ -1076,7 +1076,7 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,* completed fully */ if (num_tags ==MTE_GRANULES_PER_PAGE ) -set_bit (PG_mte_tagged, &page->flags); +set_page_mte_tagged (page);kvm_release_pfn_dirty (pfn); }
arch/arm64/kvm/mmu.c +2 -2
@@ -1110,9 +1110,9 @@ static int sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,return -EFAULT ;for (i =0 ; i < nr_pages; i++, page++) { -if (!test_bit (PG_mte_tagged, &page->flags)) { +if (!page_mte_tagged (page)) {mte_clear_page_tags (page_address (page)); -set_bit (PG_mte_tagged, &page->flags); +set_page_mte_tagged (page); } }
arch/arm64/mm/copypage.c +3 -2
@@ -21,9 +21,10 @@ void copy_highpage(struct page *to, struct page *from)copy_page (kto, kfrom); -if (system_supports_mte () &&test_bit (PG_mte_tagged, &from->flags)) { -set_bit (PG_mte_tagged, &to->flags); +if (system_supports_mte () &&page_mte_tagged (from)) { +page_kasan_tag_reset (to);mte_copy_page_tags (kto, kfrom); +set_page_mte_tagged (to); } }EXPORT_SYMBOL (copy_highpage);
arch/arm64/mm/fault.c +1 -1
@@ -938,5 +938,5 @@ struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,void tag_clear_highpage (struct page *page) {mte_zero_clear_page_tags (page_address (page)); -set_bit (PG_mte_tagged, &page->flags); +set_page_mte_tagged (page); }
arch/arm64/mm/mteswap.c +1 -1
@@ -24,7 +24,7 @@ int mte_save_tags(struct page *page) {void *tag_storage, *ret; -if (!test_bit (PG_mte_tagged, &page->flags)) +if (!page_mte_tagged (page))return 0 ; tag_storage =mte_allocate_tag_storage ();
Signature · 1 line
2.38.1.431.g37b22c650d-goog
PATCH v5 3/8 KVM: arm64: Simplify the sanitise_mte_tags() logic
In reply to Peter CollingbourneFrom: Catalin Marinas <[email protected]>
Currently sanitise_mte_tags() checks if it's an online page before attempting to sanitise the tags. Such detection should be done in the caller via the VM_MTE_ALLOWED vma flag. Since kvm_set_spte_gfn() does not have the vma, leave the page unmapped if not already tagged. Tag initialisation will be done on a subsequent access fault in user_mem_abort().
--- arch/arm64/kvm/mmu.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-)
arch/arm64/kvm/mmu.c +15 -25
@@ -1091,23 +1091,14 @@ static int get_vma_page_shift(struct vm_area_struct *vma, unsigned long hva)* - mmap_lock protects between a VM faulting a page in and the VMM performing * an mprotect() to add VM_MTE */ -static int sanitise_mte_tags (struct kvm *kvm,kvm_pfn_t pfn, -unsigned long size) +static void sanitise_mte_tags (struct kvm *kvm,kvm_pfn_t pfn, +unsigned long size) {unsigned long i, nr_pages = size >>PAGE_SHIFT ; -struct page *page; +struct page *page =pfn_to_page (pfn);if (!kvm_has_mte (kvm)) -return 0 ; - -/* -* pfn_to_online_page() is used to reject ZONE_DEVICE pages -* that may not support tags. -*/ - page =pfn_to_online_page (pfn); - -if (!page) -return -EFAULT ; +return ;for (i =0 ; i < nr_pages; i++, page++) {if (!page_mte_tagged (page)) {
@@ -1115,8 +1106,6 @@ static int sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,set_page_mte_tagged (page); } } - -return 0 ; } static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
@@ -1127,7 +1116,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,bool write_fault, writable, force_pte = false;bool exec_fault;bool device = false; -bool shared;unsigned long mmu_seq;struct kvm *kvm = vcpu->kvm;struct kvm_mmu_memory_cache *memcache = &vcpu->arch.mmu_page_cache;
@@ -1177,8 +1165,6 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, vma_shift =get_vma_page_shift (vma, hva); } - shared = (vma->vm_flags &VM_SHARED ); -switch (vma_shift) {#ifndef __PAGETABLE_PMD_FOLDEDcase PUD_SHIFT :
@@ -1299,12 +1285,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,if (fault_status !=FSC_PERM && !device &&kvm_has_mte (kvm)) {/* Check the VMM hasn't introduced a new VM_SHARED VMA */ -if (!shared) - ret =sanitise_mte_tags (kvm, pfn, vma_pagesize); -else +if ((vma->vm_flags &VM_MTE_ALLOWED ) && + !(vma->vm_flags &VM_SHARED )) { +sanitise_mte_tags (kvm, pfn, vma_pagesize); + }else { ret = -EFAULT ; -if (ret) goto out_unlock; + } }if (writable)
@@ -1526,15 +1513,18 @@ bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)bool kvm_set_spte_gfn (struct kvm *kvm,struct kvm_gfn_range *range) {kvm_pfn_t pfn =pte_pfn (range->pte); -int ret;if (!kvm->arch.mmu.pgt)return false;WARN_ON (range->end - range->start !=1 ); - ret =sanitise_mte_tags (kvm, pfn,PAGE_SIZE ); -if (ret) +/* +* If the page isn't tagged, defer to user_mem_abort() for sanitising +* the MTE tags. The S2 pte should have been unmapped by +* mmu_notifier_invalidate_range_end(). +*/ +if (kvm_has_mte (kvm) && !page_mte_tagged (pfn_to_page (pfn)))return false;/*
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 3/8] KVM: arm64: Simplify the sanitise_mte_tags() logic
Quoted 16 lines · Peter Collingbourne
From: Catalin Marinas <[email protected]>
Currently
sanitise_mte_tags()checks if it's an online page before attempting to sanitise the tags. Such detection should be done in the caller via theVM_MTE_ALLOWEDvma flag. Sincekvm_set_spte_gfn()does not have the vma, leave the page unmapped if not already tagged. Tag initialisation will be done on a subsequent access fault inuser_mem_abort().Signed-off-by Catalin Marinas <[email protected]>[[email protected]: fix the page initializer]Signed-off-by Peter Collingbourne <[email protected]>Reviewed-by Steven Price <[email protected]>Cc Will Deacon <[email protected]>Cc Marc Zyngier <[email protected]>Cc Peter Collingbourne <[email protected]>
Quoted 3 lines
--- arch/arm64/kvm/mmu.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v5 4/8 mm: Add PG_arch_3 page flag
In reply to Peter CollingbourneAs with PG_arch_2, this flag is only allowed on 64-bit architectures due to the shortage of bits available. It will be used by the arm64 MTE code in subsequent patches.
--- fs/proc/page.c | 1 + include/linux/kernel-page-flags.h | 1 + include/linux/page-flags.h | 1 + include/trace/events/mmflags.h | 1 + mm/huge_memory.c | 1 + 5 files changed, 5 insertions(+)
fs/proc/page.c +1 -0
@@ -221,6 +221,7 @@ u64 stable_page_flags(struct page *page) u |=kpf_copy_bit (k,KPF_ARCH , PG_arch_1);#ifdef CONFIG_ARCH_USES_PG_ARCH_X u |=kpf_copy_bit (k,KPF_ARCH_2 , PG_arch_2); + u |=kpf_copy_bit (k,KPF_ARCH_3 , PG_arch_3);#endif return u;
include/linux/kernel-page-flags.h +1 -0
@@ -18,5 +18,6 @@#define KPF_UNCACHED 39#define KPF_SOFTDIRTY 40#define KPF_ARCH_2 41 +#define KPF_ARCH_3 42#endif /* LINUX_KERNEL_PAGE_FLAGS_H */
include/linux/page-flags.h +1 -0
@@ -134,6 +134,7 @@ enum pageflags {#endif #ifdef CONFIG_ARCH_USES_PG_ARCH_X PG_arch_2, + PG_arch_3,#endif #ifdef CONFIG_KASAN_HW_TAGS PG_skip_kasan_poison,
include/trace/events/mmflags.h +1 -0
@@ -130,6 +130,7 @@ IF_HAVE_PG_HWPOISON(PG_hwpoison, "hwpoison" ) \ IF_HAVE_PG_IDLE(PG_young, "young" ) \ IF_HAVE_PG_IDLE(PG_idle, "idle" ) \ IF_HAVE_PG_ARCH_X(PG_arch_2, "arch_2" ) \ +IF_HAVE_PG_ARCH_X(PG_arch_3, "arch_3" ) \ IF_HAVE_PG_SKIP_KASAN_POISON(PG_skip_kasan_poison, "skip_kasan_poison")#define show_page_flags (flags) \
mm/huge_memory.c +1 -0
@@ -2403,6 +2403,7 @@ static void __split_huge_page_tail(struct page *head, int tail, (1L << PG_unevictable) | #ifdef CONFIG_ARCH_USES_PG_ARCH_X (1L << PG_arch_2) | + (1L << PG_arch_3) | #endif (1L << PG_dirty) | LRU_GEN_MASK | LRU_REFS_MASK));
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v5 5/8 arm64: mte: Lock a page for MTE tag initialisation
In reply to Peter CollingbourneFrom: Catalin Marinas <[email protected]>
Initialising the tags and setting PG_mte_tagged flag for a page can race between multiple set_pte_at() on shared pages or setting the stage 2 pte via user_mem_abort(). Introduce a new PG_mte_lock flag as PG_arch_3 and set it before attempting page initialisation. Given that PG_mte_tagged is never cleared for a page, consider setting this flag to mean page unlocked and wait on this bit with acquire semantics if the page is locked:
- try_page_mte_tagging() - lock the page for tagging, return true if it can be tagged, false if already tagged. No acquire semantics if it returns true (PG_mte_tagged not set) as there is no serialisation with a previous set_page_mte_tagged().
- set_page_mte_tagged() - set PG_mte_tagged with release semantics.
The two-bit locking is based on Peter Collingbourne's idea.
--- arch/arm64/include/asm/mte.h | 35 +++++++++++++++++++++++++++++++- arch/arm64/include/asm/pgtable.h | 4 ++-- arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/mte.c | 12 +++-------- arch/arm64/kvm/guest.c | 16 +++++++++------ arch/arm64/kvm/mmu.c | 2 +- arch/arm64/mm/copypage.c | 2 ++ arch/arm64/mm/fault.c | 2 ++ arch/arm64/mm/mteswap.c | 14 +++++-------- 9 files changed, 60 insertions(+), 29 deletions(-)
arch/arm64/include/asm/mte.h +34 -1
@@ -25,7 +25,7 @@ unsigned long mte_copy_tags_to_user(void __user *to, void *from, unsigned long n);int mte_save_tags (struct page *page);void mte_save_page_tags (const void *page_addr,void *tag_storage); -bool mte_restore_tags (swp_entry_t entry,struct page *page); +void mte_restore_tags (swp_entry_t entry,struct page *page);void mte_restore_page_tags (void *page_addr,const void *tag_storage);void mte_invalidate_tags (int type,pgoff_t offset);void mte_invalidate_tags_area (int type);
@@ -36,6 +36,8 @@ void mte_free_tag_storage(char *storage);/* track which pages have valid allocation tags */ #define PG_mte_tagged PG_arch_2 +/* simple lock to avoid multiple threads tagging the same page */ +#define PG_mte_lock PG_arch_3static inline void set_page_mte_tagged (struct page *page) {
@@ -60,6 +62,33 @@ static inline bool page_mte_tagged(struct page *page)return ret; } +/* +* Lock the page for tagging and return 'true' if the page can be tagged, +* 'false' if already tagged. PG_mte_tagged is never cleared and therefore the +* locking only happens once for page initialisation. +* +* The page MTE lock state: +* +* Locked: PG_mte_lock && !PG_mte_tagged +* Unlocked: !PG_mte_lock || PG_mte_tagged +* +* Acquire semantics only if the page is tagged (returning 'false'). +*/ +static inline bool try_page_mte_tagging (struct page *page) +{ +if (!test_and_set_bit (PG_mte_lock, &page->flags)) +return true; + +/* +* The tags are either being initialised or may have been initialised +* already. Check if the PG_mte_tagged flag has been set or wait +* otherwise. +*/ +smp_cond_load_acquire (&page->flags,VAL & (1UL << PG_mte_tagged)); + +return false; +} +void mte_zero_clear_page_tags (void *addr);void mte_sync_tags (pte_t old_pte,pte_t pte);void mte_copy_page_tags (void *kto,const void *kfrom);
@@ -86,6 +115,10 @@ static inline bool page_mte_tagged(struct page *page) {return false; } +static inline bool try_page_mte_tagging (struct page *page) +{ +return false; +}static inline void mte_zero_clear_page_tags (void *addr) { }
arch/arm64/include/asm/pgtable.h +2 -2
@@ -1047,8 +1047,8 @@ static inline void arch_swap_invalidate_area(int type)#define __HAVE_ARCH_SWAP_RESTOREstatic inline void arch_swap_restore (swp_entry_t entry,struct folio *folio) { -if (system_supports_mte () &&mte_restore_tags (entry, &folio->page)) -set_page_mte_tagged (&folio->page); +if (system_supports_mte ()) +mte_restore_tags (entry, &folio->page); }#endif /* CONFIG_ARM64_MTE */
arch/arm64/kernel/cpufeature.c +1 -1
@@ -2050,7 +2050,7 @@ static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)* Clear the tags in the zero page. This needs to be done via the * linear map which has the Tagged attribute. */ -if (!page_mte_tagged (ZERO_PAGE (0 ))) { +if (try_page_mte_tagging (ZERO_PAGE (0 ))) {mte_clear_page_tags (lm_alias (empty_zero_page));set_page_mte_tagged (ZERO_PAGE (0 )); }
arch/arm64/kernel/mte.c +3 -9
@@ -41,20 +41,14 @@ static void mte_sync_page_tags(struct page *page, pte_t old_pte,if (check_swap &&is_swap_pte (old_pte)) {swp_entry_t entry =pte_to_swp_entry (old_pte); -if (!non_swap_entry (entry) &&mte_restore_tags (entry, page)) { -set_page_mte_tagged (page); -return ; - } +if (!non_swap_entry (entry)) +mte_restore_tags (entry, page); }if (!pte_is_tagged)return ; -/* -* Test PG_mte_tagged again in case it was racing with another -* set_pte_at(). -*/ -if (!page_mte_tagged (page)) { +if (try_page_mte_tagging (page)) {mte_clear_page_tags (page_address (page));set_page_mte_tagged (page); }
arch/arm64/kvm/guest.c +10 -6
@@ -1068,15 +1068,19 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,clear_user (tags,MTE_GRANULES_PER_PAGE );kvm_release_pfn_clean (pfn); }else { +/* +* Only locking to serialise with a concurrent +* set_pte_at() in the VMM but still overriding the +* tags, hence ignoring the return value. +*/ +try_page_mte_tagging (page); num_tags =mte_copy_tags_from_user (maddr, tags,MTE_GRANULES_PER_PAGE ); -/* -* Set the flag after checking the write -* completed fully -*/ -if (num_tags ==MTE_GRANULES_PER_PAGE ) -set_page_mte_tagged (page); +/* uaccess failed, don't leave stale tags */ +if (num_tags !=MTE_GRANULES_PER_PAGE ) +mte_clear_page_tags (page); +set_page_mte_tagged (page);kvm_release_pfn_dirty (pfn); }
arch/arm64/kvm/mmu.c +1 -1
@@ -1101,7 +1101,7 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,return ;for (i =0 ; i < nr_pages; i++, page++) { -if (!page_mte_tagged (page)) { +if (try_page_mte_tagging (page)) {mte_clear_page_tags (page_address (page));set_page_mte_tagged (page); }
arch/arm64/mm/copypage.c +2 -0
@@ -23,6 +23,8 @@ void copy_highpage(struct page *to, struct page *from)if (system_supports_mte () &&page_mte_tagged (from)) {page_kasan_tag_reset (to); +/* It's a new page, shouldn't have been tagged yet */ +WARN_ON_ONCE (!try_page_mte_tagging (to));mte_copy_page_tags (kto, kfrom);set_page_mte_tagged (to); }
arch/arm64/mm/fault.c +2 -0
@@ -937,6 +937,8 @@ struct page *alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,void tag_clear_highpage (struct page *page) { +/* Newly allocated page, shouldn't have been tagged yet */ +WARN_ON_ONCE (!try_page_mte_tagging (page));mte_zero_clear_page_tags (page_address (page));set_page_mte_tagged (page); }
arch/arm64/mm/mteswap.c +5 -9
@@ -46,21 +46,17 @@ int mte_save_tags(struct page *page)return 0 ; } -bool mte_restore_tags (swp_entry_t entry,struct page *page) +void mte_restore_tags (swp_entry_t entry,struct page *page) {void *tags =xa_load (&mte_pages, entry.val);if (!tags) -return false; +return ; -/* -* Test PG_mte_tagged again in case it was racing with another -* set_pte_at(). -*/ -if (!test_and_set_bit (PG_mte_tagged, &page->flags)) +if (try_page_mte_tagging (page)) {mte_restore_page_tags (page_address (page), tags); - -return true; +set_page_mte_tagged (page); + } }void mte_invalidate_tags (int type,pgoff_t offset)
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 5/8] arm64: mte: Lock a page for MTE tag initialisation
Quoted 25 lines · Peter Collingbourne
From: Catalin Marinas <[email protected]>
Initialising the tags and setting
PG_mte_taggedflag for a page can race between multipleset_pte_at()on shared pages or setting the stage 2 pte viauser_mem_abort(). Introduce a newPG_mte_lockflag asPG_arch_3and set it before attempting page initialisation. Given thatPG_mte_taggedis never cleared for a page, consider setting this flag to mean page unlocked and wait on this bit with acquire semantics if the page is locked:-
try_page_mte_tagging()- lock the page for tagging, return true if it can be tagged, false if already tagged. No acquire semantics if it returns true (PG_mte_taggednot set) as there is no serialisation with a previousset_page_mte_tagged().-
set_page_mte_tagged()- setPG_mte_taggedwith release semantics.The two-bit locking is based on Peter Collingbourne's idea.
Signed-off-by Catalin Marinas <[email protected]>Signed-off-by Peter Collingbourne <[email protected]>Reviewed-by Steven Price <[email protected]>Cc Will Deacon <[email protected]>Cc Marc Zyngier <[email protected]>Cc Peter Collingbourne <[email protected]>
Quoted 11 lines
--- arch/arm64/include/asm/mte.h | 35 +++++++++++++++++++++++++++++++- arch/arm64/include/asm/pgtable.h | 4 ++-- arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/mte.c | 12 +++-------- arch/arm64/kvm/guest.c | 16 +++++++++------ arch/arm64/kvm/mmu.c | 2 +- arch/arm64/mm/copypage.c | 2 ++ arch/arm64/mm/fault.c | 2 ++ arch/arm64/mm/mteswap.c | 14 +++++-------- 9 files changed, 60 insertions(+), 29 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v5 6/8 KVM: arm64: unify the tests for VMAs in memslots when MTE is enabled
In reply to Peter CollingbournePreviously we allowed creating a memslot containing a private mapping that was not VM_MTE_ALLOWED, but would later reject KVM_RUN with -EFAULT. Now we reject the memory region at memslot creation time.
Since this is a minor tweak to the ABI (a VMM that created one of these memslots would fail later anyway), no VMM to my knowledge has MTE support yet, and the hardware with the necessary features is not generally available, we can probably make this ABI change at this point.
--- arch/arm64/kvm/mmu.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
arch/arm64/kvm/mmu.c +16 -9
@@ -1108,6 +1108,19 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn, } } +static bool kvm_vma_mte_allowed (struct vm_area_struct *vma) +{ +/* +* VM_SHARED mappings are not allowed with MTE to avoid races +* when updating the PG_mte_tagged page flag, see +* sanitise_mte_tags for more details. +*/ +if (vma->vm_flags &VM_SHARED ) +return false; + +return vma->vm_flags &VM_MTE_ALLOWED ; +} +static int user_mem_abort (struct kvm_vcpu *vcpu,phys_addr_t fault_ipa,struct kvm_memory_slot *memslot,unsigned long hva,unsigned long fault_status)
@@ -1284,9 +1297,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, }if (fault_status !=FSC_PERM && !device &&kvm_has_mte (kvm)) { -/* Check the VMM hasn't introduced a new VM_SHARED VMA */ -if ((vma->vm_flags &VM_MTE_ALLOWED ) && - !(vma->vm_flags &VM_SHARED )) { +/* Check the VMM hasn't introduced a new disallowed VMA */ +if (kvm_vma_mte_allowed (vma)) {sanitise_mte_tags (kvm, pfn, vma_pagesize); }else { ret = -EFAULT ;
@@ -1730,12 +1742,7 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,if (!vma)break ; -/* -* VM_SHARED mappings are not allowed with MTE to avoid races -* when updating the PG_mte_tagged page flag, see -* sanitise_mte_tags for more details. -*/ -if (kvm_has_mte (kvm) && vma->vm_flags &VM_SHARED ) { +if (kvm_has_mte (kvm) && !kvm_vma_mte_allowed (vma)) { ret = -EINVAL ;break ; }
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 6/8] KVM: arm64: unify the tests for VMAs in memslots when MTE is enabled
Quoted 9 lines · Peter Collingbourne
Previously we allowed creating a memslot containing a private mapping that was not
VM_MTE_ALLOWED, but would later rejectKVM_RUNwith-EFAULT. Now we reject the memory region at memslot creation time.Since this is a minor tweak to the ABI (a VMM that created one of these memslots would fail later anyway), no VMM to my knowledge has MTE support yet, and the hardware with the necessary features is not generally available, we can probably make this ABI change at this point.
Quoted 6 lines
Signed-off-by Peter Collingbourne <[email protected]>Reviewed-by Catalin Marinas <[email protected]>Reviewed-by Steven Price <[email protected]>--- arch/arm64/kvm/mmu.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v5 7/8 KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE enabled
In reply to Peter CollingbourneCertain VMMs such as crosvm have features (e.g. sandboxing) that depend on being able to map guest memory as MAP_SHARED. The current restriction on sharing MAP_SHARED pages with the guest is preventing the use of those features with MTE. Now that the races between tasks concurrently clearing tags on the same page have been fixed, remove this restriction.
Note that this is a relaxation of the ABI.
--- arch/arm64/kvm/mmu.c | 8 -------- 1 file changed, 8 deletions(-)
arch/arm64/kvm/mmu.c +0 -8
@@ -1110,14 +1110,6 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,static bool kvm_vma_mte_allowed (struct vm_area_struct *vma) { -/* -* VM_SHARED mappings are not allowed with MTE to avoid races -* when updating the PG_mte_tagged page flag, see -* sanitise_mte_tags for more details. -*/ -if (vma->vm_flags &VM_SHARED ) -return false; -return vma->vm_flags &VM_MTE_ALLOWED ; }
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 7/8] KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE enabled
Quoted 6 lines · Peter Collingbourne
Certain VMMs such as crosvm have features (e.g. sandboxing) that depend on being able to map guest memory as
MAP_SHARED. The current restriction on sharingMAP_SHAREDpages with the guest is preventing the use of those features with MTE. Now that the races between tasks concurrently clearing tags on the same page have been fixed, remove this restriction.
Quoted 8 lines
Note that this is a relaxation of the ABI.
Signed-off-by Peter Collingbourne <[email protected]>Reviewed-by Catalin Marinas <[email protected]>Reviewed-by Steven Price <[email protected]>--- arch/arm64/kvm/mmu.c | 8 -------- 1 file changed, 8 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v5 8/8 Documentation: document the ABI changes for KVM_CAP_ARM_MTE
In reply to Peter CollingbourneDocument both the restriction on VM_MTE_ALLOWED mappings and the relaxation for shared mappings.
--- Documentation/virt/kvm/api.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Documentation/virt/kvm/api.rst +3 -2
@@ -7385,8 +7385,9 @@ hibernation of the host; however the VMM needs to manually save/restore the tags as appropriate if the VM is migrated. When this capability is enabled all memory in memslots must be mapped as -not-shareable (no MAP_SHARED), attempts to create a memslot with a -MAP_SHARED mmap will result in an -EINVAL return. +``MAP_ANONYMOUS`` or with a RAM-based file mapping (``tmpfs``, ``memfd``), +attempts to create a memslot with an invalid mmap will result in an +-EINVAL return. When enabled the VMM may make use of the ``KVM_ARM_MTE_COPY_TAGS`` ioctl to perform a bulk copy of tags to/from the guest.
Signature · 6 lines
2.38.1.431.g37b22c650d-goog _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v5 8/8] Documentation: document the ABI changes for KVM_CAP_ARM_MTE
Quoted 8 lines · Peter Collingbourne
Document both the restriction on
VM_MTE_ALLOWEDmappings and the relaxation for shared mappings.Signed-off-by Peter Collingbourne <[email protected]>Acked-by Catalin Marinas <[email protected]>--- Documentation/virt/kvm/api.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerQuoted 28 lines · Peter Collingbourne
Hi,
This patch series allows VMMs to use shared mappings in MTE enabled guests. The first five patches were taken from Catalin's tree [1] which addressed some review feedback from when they were previously sent out as v3 of this series. The first patch from Catalin's tree makes room for an additional
PG_arch_3flag by making the newer PG_arch_* flags arch-dependent. The next four patches are based on a series that Catalin sent out prior to v3, whose cover letter [2] I quote from below:Quoted 17 lines
This series aims to fix the races between initialising the tags on a page and setting the
PG_mte_taggedflag. Currently the flag is set either before or after that tag initialisation and this can lead to CoW copying stale tags. The first patch moves the flag setting after the tags have been initialised, solving the CoW issue. However, concurrentmprotect()on a shared mapping may (very rarely) lead to valid tags being zeroed.The second skips the
sanitise_mte_tags()call inkvm_set_spte_gfn(), deferring it touser_mem_abort(). The outcome is that nosanitise_mte_tags()can be simplified to skip thepfn_to_online_page()check and only rely onVM_MTE_ALLOWEDvma flag that can be checked inuser_mem_abort().The third and fourth patches use
PG_arch_3as a lock for page tagging, based on Peter Collingbourne's idea of a two-bit lock.
Quoted 6 lines
Quoted 4 lines
I think the first patch can be queued but the rest needs some in depth review and test. With this series (if correct) we could allos
MAP_SHAREDon KVM guest memory but this is to be discussed separately as there are some KVM ABI implications.In this v5 I rebased Catalin's tree onto -next again. Please double check
Please don't do use -next as a base. In-flight series should be based on a *stable* tag, either 6.0 or one of the early -RCs. If there is a known conflict with -next, do mention it in the cover letter and provide a resolution.
Quoted 2 lines
my rebase, which resolved the conflict with commit
a8e5e5146ad0("arm64:
mte: Avoid settingPG_mte_taggedif no tags cleared or restored").
This commit seems part of -rc1, so I guess the patches directly apply on top of that tag?
Quoted 2 lines
I now have Reviewed-by for all patches except for the last one, which adds the documentation. Thanks for the reviews so far, and please take a look!
I'd really like the MM folks (list now cc'd) to look at the relevant patches (1 and 5) and ack them before I take this.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Quoted 36 lines · Marc Zyngier
Quoted 33 lines · Peter Collingbourne
Hi,
This patch series allows VMMs to use shared mappings in MTE enabled guests. The first five patches were taken from Catalin's tree [1] which addressed some review feedback from when they were previously sent out as v3 of this series. The first patch from Catalin's tree makes room for an additional
PG_arch_3flag by making the newer PG_arch_* flags arch-dependent. The next four patches are based on a series that Catalin sent out prior to v3, whose cover letter [2] I quote from below:Quoted 21 lines
This series aims to fix the races between initialising the tags on a page and setting the
PG_mte_taggedflag. Currently the flag is set either before or after that tag initialisation and this can lead to CoW copying stale tags. The first patch moves the flag setting after the tags have been initialised, solving the CoW issue. However, concurrentmprotect()on a shared mapping may (very rarely) lead to valid tags being zeroed.The second skips the
sanitise_mte_tags()call inkvm_set_spte_gfn(), deferring it touser_mem_abort(). The outcome is that nosanitise_mte_tags()can be simplified to skip thepfn_to_online_page()check and only rely onVM_MTE_ALLOWEDvma flag that can be checked inuser_mem_abort().The third and fourth patches use
PG_arch_3as a lock for page tagging, based on Peter Collingbourne's idea of a two-bit lock.I think the first patch can be queued but the rest needs some in depth review and test. With this series (if correct) we could allos
MAP_SHAREDon KVM guest memory but this is to be discussed separately as there are some KVM ABI implications.
Quoted 6 lines
Quoted 1 line
In this v5 I rebased Catalin's tree onto -next again. Please double check
Please don't do use -next as a base. In-flight series should be based on a *stable* tag, either 6.0 or one of the early -RCs. If there is a known conflict with -next, do mention it in the cover letter and provide a resolution.
Okay, I will keep that in mind.
Quoted 5 lines
Quoted 2 lines
my rebase, which resolved the conflict with commit
a8e5e5146ad0("arm64:
mte: Avoid settingPG_mte_taggedif no tags cleared or restored").This commit seems part of -rc1, so I guess the patches directly apply on top of that tag?
Yes, sorry, this also applies cleanly to -rc1.
Quoted 5 lines
Quoted 2 lines
I now have Reviewed-by for all patches except for the last one, which adds the documentation. Thanks for the reviews so far, and please take a look!
I'd really like the MM folks (list now cc'd) to look at the relevant patches (1 and 5) and ack them before I take this.
Okay, here are the lore links for the convenience of the MM folks:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
Peter
Marc Zyngier
maintainerQuoted 58 lines · Peter Collingbourne
Quoted 36 lines · Marc Zyngier
Quoted 33 lines · Peter Collingbourne
Hi,
This patch series allows VMMs to use shared mappings in MTE enabled guests. The first five patches were taken from Catalin's tree [1] which addressed some review feedback from when they were previously sent out as v3 of this series. The first patch from Catalin's tree makes room for an additional
PG_arch_3flag by making the newer PG_arch_* flags arch-dependent. The next four patches are based on a series that Catalin sent out prior to v3, whose cover letter [2] I quote from below:Quoted 21 lines
This series aims to fix the races between initialising the tags on a page and setting the
PG_mte_taggedflag. Currently the flag is set either before or after that tag initialisation and this can lead to CoW copying stale tags. The first patch moves the flag setting after the tags have been initialised, solving the CoW issue. However, concurrentmprotect()on a shared mapping may (very rarely) lead to valid tags being zeroed.The second skips the
sanitise_mte_tags()call inkvm_set_spte_gfn(), deferring it touser_mem_abort(). The outcome is that nosanitise_mte_tags()can be simplified to skip thepfn_to_online_page()check and only rely onVM_MTE_ALLOWEDvma flag that can be checked inuser_mem_abort().The third and fourth patches use
PG_arch_3as a lock for page tagging, based on Peter Collingbourne's idea of a two-bit lock.I think the first patch can be queued but the rest needs some in depth review and test. With this series (if correct) we could allos
MAP_SHAREDon KVM guest memory but this is to be discussed separately as there are some KVM ABI implications.Quoted 6 lines
Quoted 1 line
In this v5 I rebased Catalin's tree onto -next again. Please double check
Please don't do use -next as a base. In-flight series should be based on a *stable* tag, either 6.0 or one of the early -RCs. If there is a known conflict with -next, do mention it in the cover letter and provide a resolution.
Okay, I will keep that in mind.
Quoted 5 lines
Quoted 2 lines
my rebase, which resolved the conflict with commit
a8e5e5146ad0("arm64:
mte: Avoid settingPG_mte_taggedif no tags cleared or restored").This commit seems part of -rc1, so I guess the patches directly apply on top of that tag?
Yes, sorry, this also applies cleanly to -rc1.
Quoted 3 lines
Quoted 2 lines
I now have Reviewed-by for all patches except for the last one, which adds the documentation. Thanks for the reviews so far, and please take a look!
Quoted 6 lines
Quoted 2 lines
I'd really like the MM folks (list now cc'd) to look at the relevant patches (1 and 5) and ack them before I take this.
Okay, here are the lore links for the convenience of the MM folks:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
I have not seen any Ack from the MM folks so far, and we're really running out of runway for this merge window.
Short of someone shouting now, I'll take the series into the kvmarm tree early next week.
Thanks,
Signature · 1 line
Without deviation from the norm, progress is not possible.
Quoted 9 lines · Peter Collingbourne
This patch series allows VMMs to use shared mappings in MTE enabled guests. The first five patches were taken from Catalin's tree [1] which addressed some review feedback from when they were previously sent out as v3 of this series. The first patch from Catalin's tree makes room for an additional
PG_arch_3flag by making the newer PG_arch_* flags arch-dependent. The next four patches are based on a series that Catalin sent out prior to v3, whose cover letter [2] I quote from below:[...]
No feedback has been received, so this code is obviously perfect.
Applied to next, thanks!
[1/8] mm: Do not enable PG_arch_2 for all 64-bit architectures
commit: b0284cd29a957e62d60c2886fd663be93c56f9c0
[2/8] arm64: mte: Fix/clarify the PG_mte_tagged semantics
commit: e059853d14ca4ed0f6a190d7109487918a22a976
[3/8] KVM: arm64: Simplify the sanitise_mte_tags() logic
commit: 2dbf12ae132cc78048615cfa19c9be64baaf0ced
[4/8] mm: Add PG_arch_3 page flag
commit: ef6458b1b6ca3fdb991ce4182e981a88d4c58c0f
[5/8] arm64: mte: Lock a page for MTE tag initialisation
commit: d77e59a8fccde7fb5dd8c57594ed147b4291c970
[6/8] KVM: arm64: unify the tests for VMAs in memslots when MTE is enabled
commit: d89585fbb30869011b326ef26c94c3137d228df9
[7/8] KVM: arm64: permit all VM_MTE_ALLOWED mappings with MTE enabled
commit: c911f0d4687947915f04024aa01803247fcf7f1a
[8/8] Documentation: document the ABI changes for KVM_CAP_ARM_MTE
commit: a4baf8d2639f24d4d31983ff67c01878e7a5393fCheers,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.