Message ID | 20220722015034.809663-2-pcc@google.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: arm64: permit MAP_SHARED mappings with MTE enabled | expand |
Hi Peter, I love your patch! Yet something to improve: [auto build test ERROR on arm64/for-next/core] [also build test ERROR on next-20220722] [cannot apply to kvmarm/next arm/for-next soc/for-next xilinx-xlnx/master linus/master v5.19-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Peter-Collingbourne/KVM-arm64-permit-MAP_SHARED-mappings-with-MTE-enabled/20220722-095300 base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core config: arm64-randconfig-s032-20220721 (https://download.01.org/0day-ci/archive/20220723/202207230959.jvdok5UK-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 12.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.4-39-gce1a6720-dirty # https://github.com/intel-lab-lkp/linux/commit/3323e416892d6b5326503b9afc2ee835162b819b git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Collingbourne/KVM-arm64-permit-MAP_SHARED-mappings-with-MTE-enabled/20220722-095300 git checkout 3323e416892d6b5326503b9afc2ee835162b819b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 prepare If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from arch/arm64/include/asm/pgtable.h:12, from include/linux/pgtable.h:6, from include/linux/kasan.h:33, from include/linux/slab.h:140, from include/linux/resource_ext.h:11, from include/linux/acpi.h:14, from include/acpi/apei.h:9, from include/acpi/ghes.h:5, from include/linux/arm_sdei.h:8, from arch/arm64/kernel/asm-offsets.c:10: >> arch/arm64/include/asm/mte.h:80:15: error: return type defaults to 'int' [-Werror=implicit-int] 80 | static inline set_page_mte_tagged(struct page *page) | ^~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors make[2]: *** [scripts/Makefile.build:117: arch/arm64/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1200: prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [Makefile:219: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +/int +80 arch/arm64/include/asm/mte.h 79 > 80 static inline set_page_mte_tagged(struct page *page) 81 { 82 } 83 static inline bool page_mte_tagged(struct page *page) 84 { 85 return false; 86 } 87 static inline void mte_zero_clear_page_tags(void *addr) 88 { 89 } 90 static inline void mte_sync_tags(pte_t old_pte, pte_t pte) 91 { 92 } 93 static inline void mte_copy_page_tags(void *kto, const void *kfrom) 94 { 95 } 96 static inline void mte_thread_init_user(void) 97 { 98 } 99 static inline void mte_thread_switch(struct task_struct *next) 100 { 101 } 102 static inline void mte_suspend_enter(void) 103 { 104 } 105 static inline long set_mte_ctrl(struct task_struct *task, unsigned long arg) 106 { 107 return 0; 108 } 109 static inline long get_mte_ctrl(struct task_struct *task) 110 { 111 return 0; 112 } 113 static inline int mte_ptrace_copy_tags(struct task_struct *child, 114 long request, unsigned long addr, 115 unsigned long data) 116 { 117 return -EIO; 118 } 119
Hi Peter, I love your patch! Yet something to improve: [auto build test ERROR on arm64/for-next/core] [also build test ERROR on next-20220722] [cannot apply to kvmarm/next arm/for-next soc/for-next xilinx-xlnx/master linus/master v5.19-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Peter-Collingbourne/KVM-arm64-permit-MAP_SHARED-mappings-with-MTE-enabled/20220722-095300 base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core config: arm64-buildonly-randconfig-r004-20220722 (https://download.01.org/0day-ci/archive/20220724/202207240907.aR4gvNkj-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 72686d68c137551cce816416190a18d45b4d4e2a) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/3323e416892d6b5326503b9afc2ee835162b819b git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Collingbourne/KVM-arm64-permit-MAP_SHARED-mappings-with-MTE-enabled/20220722-095300 git checkout 3323e416892d6b5326503b9afc2ee835162b819b # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 prepare If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from arch/arm64/kernel/asm-offsets.c:10: In file included from include/linux/arm_sdei.h:8: In file included from include/acpi/ghes.h:5: In file included from include/acpi/apei.h:9: In file included from include/linux/acpi.h:15: In file included from include/linux/device.h:32: In file included from include/linux/device/driver.h:21: In file included from include/linux/module.h:19: In file included from include/linux/elf.h:6: In file included from arch/arm64/include/asm/elf.h:141: In file included from include/linux/fs.h:33: In file included from include/linux/percpu-rwsem.h:7: In file included from include/linux/rcuwait.h:6: In file included from include/linux/sched/signal.h:9: In file included from include/linux/sched/task.h:11: In file included from include/linux/uaccess.h:11: In file included from arch/arm64/include/asm/uaccess.h:24: >> arch/arm64/include/asm/mte.h:80:15: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int] static inline set_page_mte_tagged(struct page *page) ~~~~~~~~~~~~~ ^ int In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:456:22: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds] case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:456:10: warning: array index 7 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:456:42: warning: array index 6 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:456:53: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds] case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:458:22: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds] case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:458:10: warning: array index 5 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:458:42: warning: array index 4 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:458:53: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds] case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:460:22: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds] case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ In file included from arch/arm64/kernel/asm-offsets.c:12: In file included from include/linux/kexec.h:31: include/linux/compat.h:460:10: warning: array index 3 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:460:42: warning: array index 2 is past the end of the array (which contains 2 elements) [-Warray-bounds] case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1]; ^ ~ include/linux/compat.h:131:2: note: array 'sig' declared here compat_sigset_word sig[_COMPAT_NSIG_WORDS]; ^ include/linux/compat.h:460:53: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds] case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1]; ^ ~ include/uapi/asm-generic/signal.h:62:2: note: array 'sig' declared here unsigned long sig[_NSIG_WORDS]; ^ 12 warnings and 1 error generated. make[2]: *** [scripts/Makefile.build:117: arch/arm64/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1200: prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [Makefile:219: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +/int +80 arch/arm64/include/asm/mte.h 79 > 80 static inline set_page_mte_tagged(struct page *page) 81 { 82 } 83 static inline bool page_mte_tagged(struct page *page) 84 { 85 return false; 86 } 87 static inline void mte_zero_clear_page_tags(void *addr) 88 { 89 } 90 static inline void mte_sync_tags(pte_t old_pte, pte_t pte) 91 { 92 } 93 static inline void mte_copy_page_tags(void *kto, const void *kfrom) 94 { 95 } 96 static inline void mte_thread_init_user(void) 97 { 98 } 99 static inline void mte_thread_switch(struct task_struct *next) 100 { 101 } 102 static inline void mte_suspend_enter(void) 103 { 104 } 105 static inline long set_mte_ctrl(struct task_struct *task, unsigned long arg) 106 { 107 return 0; 108 } 109 static inline long get_mte_ctrl(struct task_struct *task) 110 { 111 return 0; 112 } 113 static inline int mte_ptrace_copy_tags(struct task_struct *child, 114 long request, unsigned long addr, 115 unsigned long data) 116 { 117 return -EIO; 118 } 119
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h index aa523591a44e..c69218c56980 100644 --- a/arch/arm64/include/asm/mte.h +++ b/arch/arm64/include/asm/mte.h @@ -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); @@ -54,6 +77,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 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) { } diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index b5df82aa99e6..82719fa42c0e 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -1050,7 +1050,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 */ diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index fae4c7a785d8..c66f0ffaaf47 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -2020,8 +2020,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(); } diff --git a/arch/arm64/kernel/elfcore.c b/arch/arm64/kernel/elfcore.c index 27ef7ad3ffd2..353009d7f307 100644 --- a/arch/arm64/kernel/elfcore.c +++ b/arch/arm64/kernel/elfcore.c @@ -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; diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c index af5df48ba915..788597a6b6a2 100644 --- a/arch/arm64/kernel/hibernate.c +++ b/arch/arm64/kernel/hibernate.c @@ -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); diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c index b2b730233274..2287316639f3 100644 --- a/arch/arm64/kernel/mte.c +++ b/arch/arm64/kernel/mte.c @@ -41,14 +41,17 @@ 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) return; mte_clear_page_tags(page_address(page)); + set_page_mte_tagged(page); } void mte_sync_tags(pte_t old_pte, pte_t pte) @@ -64,7 +67,7 @@ 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_and_set_bit(PG_mte_tagged, &page->flags)) + if (!page_mte_tagged(page)) mte_sync_page_tags(page, old_pte, check_swap, pte_is_tagged); } @@ -91,8 +94,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; @@ -398,7 +400,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); diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 8c607199cad1..3b04e69006b4 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -1058,7 +1058,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 @@ -1075,7 +1075,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); } diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index 87f1cd0df36e..c9012707f69c 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -1075,9 +1075,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); } } diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c index 24913271e898..4223389b6180 100644 --- a/arch/arm64/mm/copypage.c +++ b/arch/arm64/mm/copypage.c @@ -21,9 +21,9 @@ 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)) { mte_copy_page_tags(kto, kfrom); + set_page_mte_tagged(to); } } EXPORT_SYMBOL(copy_highpage); diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index c33f1fad2745..d095bfa16771 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -931,5 +931,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); } diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c index 4334dec93bd4..a78c1db23c68 100644 --- a/arch/arm64/mm/mteswap.c +++ b/arch/arm64/mm/mteswap.c @@ -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();