Re: [v2 PATCH 1/2] hugetlb: arm64: add mte support

Yang Shi
2024-08-28T17:57:59Z

On 8/21/24 11:47 AM, Yang Shi wrote:
> Enable MTE support for hugetlb.
>
> The MTE page flags will be set on the head page only. When copying
> hugetlb folio, the tags for all tail pages will be copied when copying
> head page.
>
> When freeing hugetlb folio, the MTE flags will be cleared.
>
> Signed-off-by: Yang Shi <yang@os.amperecomputing.com>
> ---
> arch/arm64/include/asm/hugetlb.h | 11 ++++++++++-
> arch/arm64/include/asm/mman.h | 3 ++-
> arch/arm64/kernel/hibernate.c | 7 +++++++
> arch/arm64/kernel/mte.c | 25 +++++++++++++++++++++++--
> arch/arm64/kvm/guest.c | 16 +++++++++++++---
> arch/arm64/kvm/mmu.c | 11 +++++++++++
> arch/arm64/mm/copypage.c | 25 +++++++++++++++++++++++--
> fs/hugetlbfs/inode.c | 2 +-
> 8 files changed, 90 insertions(+), 10 deletions(-)
>
> v2: * Reimplemented the patch to fix the comments from Catalin.
> * Added test cases (patch #2) per Catalin.

Ping... Is there any comment on this version?

Thanks,
Yang

>
> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
> index 293f880865e8..00a1f75d40ee 100644
> --- a/arch/arm64/include/asm/hugetlb.h
> +++ b/arch/arm64/include/asm/hugetlb.h
> @@ -11,6 +11,7 @@
> #define __ASM_HUGETLB_H
>
> #include <asm/cacheflush.h>
> +#include <asm/mte.h>
> #include <asm/page.h>
>
> #ifdef CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION
> @@ -20,7 +21,15 @@ extern bool arch_hugetlb_migration_supported(struct hstate *h);
>
> static inline void arch_clear_hugetlb_flags(struct folio *folio)
> {
> - clear_bit(PG_dcache_clean, &folio->flags);
> + const unsigned long clear_flags = BIT(PG_dcache_clean) |
> + BIT(PG_mte_tagged) | BIT(PG_mte_lock);
> +
> + if (!system_supports_mte()) {
> + clear_bit(PG_dcache_clean, &folio->flags);
> + return;
> + }
> +
> + folio->flags &= ~clear_flags;
> }
> #define arch_clear_hugetlb_flags arch_clear_hugetlb_flags
>
> diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h
> index 5966ee4a6154..304dfc499e68 100644
> --- a/arch/arm64/include/asm/mman.h
> +++ b/arch/arm64/include/asm/mman.h
> @@ -28,7 +28,8 @@ static inline unsigned long arch_calc_vm_flag_bits(unsigned long flags)
> * backed by tags-capable memory. The vm_flags may be overridden by a
> * filesystem supporting MTE (RAM-based).
> */
> - if (system_supports_mte() && (flags & MAP_ANONYMOUS))
> + if (system_supports_mte() &&
> + (flags & (MAP_ANONYMOUS | MAP_HUGETLB)))
> return VM_MTE_ALLOWED;
>
> return 0;
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 02870beb271e..722e76f29141 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -266,10 +266,17 @@ static int swsusp_mte_save_tags(void)
> max_zone_pfn = zone_end_pfn(zone);
> for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
> struct page *page = pfn_to_online_page(pfn);
> + struct folio *folio;
>
> if (!page)
> continue;
>
> + folio = page_folio(page);
> +
> + if (folio_test_hugetlb(folio) &&
> + !page_mte_tagged(&folio->page))
> + continue;
> +
> if (!page_mte_tagged(page))
> continue;
>
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 6174671be7c1..b21f706018f7 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -38,7 +38,22 @@ EXPORT_SYMBOL_GPL(mte_async_or_asymm_mode);
> void mte_sync_tags(pte_t pte, unsigned int nr_pages)
> {
> struct page *page = pte_page(pte);
> - unsigned int i;
> + struct folio *folio = page_folio(page);
> + unsigned long i;
> +
> + if (folio_test_hugetlb(folio)) {
> + unsigned long nr = folio_nr_pages(folio);
> + /* Hugetlb MTE flags are set for head page only */
> + if (try_page_mte_tagging(&folio->page)) {
> + for (i = 0; i < nr; i++, page++)
> + mte_clear_page_tags(page_address(page));
> + set_page_mte_tagged(&folio->page);
> + }
> +
> + smp_wmb();
> +
> + return;
> + }
>
> /* if PG_mte_tagged is set, tags have already been initialised */
> for (i = 0; i < nr_pages; i++, page++) {
> @@ -410,6 +425,7 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,
> void *maddr;
> struct page *page = get_user_page_vma_remote(mm, addr,
> gup_flags, &vma);
> + struct folio *folio;
>
> if (IS_ERR(page)) {
> err = PTR_ERR(page);
> @@ -428,7 +444,12 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr,
> put_page(page);
> break;
> }
> - WARN_ON_ONCE(!page_mte_tagged(page));
> +
> + folio = page_folio(page);
> + if (folio_test_hugetlb(folio))
> + WARN_ON_ONCE(!page_mte_tagged(&folio->page));
> + else
> + 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 11098eb7eb44..77e181d96e97 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -1050,6 +1050,7 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
> void *maddr;
> unsigned long num_tags;
> struct page *page;
> + struct folio *folio;
>
> if (is_error_noslot_pfn(pfn)) {
> ret = -EFAULT;
> @@ -1062,10 +1063,13 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
> ret = -EFAULT;
> goto out;
> }
> + folio = page_folio(page);
> maddr = page_address(page);
>
> if (!write) {
> - if (page_mte_tagged(page))
> + if (page_mte_tagged(page) ||
> + (folio_test_hugetlb(folio) &&
> + page_mte_tagged(&folio->page)))
> num_tags = mte_copy_tags_to_user(tags, maddr,
> MTE_GRANULES_PER_PAGE);
> else
> @@ -1079,14 +1083,20 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
> * __set_ptes() in the VMM but still overriding the
> * tags, hence ignoring the return value.
> */
> - try_page_mte_tagging(page);
> + if (folio_test_hugetlb(folio))
> + try_page_mte_tagging(&folio->page);
> + else
> + try_page_mte_tagging(page);
> num_tags = mte_copy_tags_from_user(maddr, tags,
> MTE_GRANULES_PER_PAGE);
>
> /* uaccess failed, don't leave stale tags */
> if (num_tags != MTE_GRANULES_PER_PAGE)
> mte_clear_page_tags(maddr);
> - set_page_mte_tagged(page);
> + if (folio_test_hugetlb(folio))
> + set_page_mte_tagged(&folio->page);
> + else
> + 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 6981b1bc0946..1fa51ac4e3f4 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1401,10 +1401,21 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,
> {
> unsigned long i, nr_pages = size >> PAGE_SHIFT;
> struct page *page = pfn_to_page(pfn);
> + struct folio *folio = page_folio(page);
>
> if (!kvm_has_mte(kvm))
> return;
>
> + if (folio_test_hugetlb(folio)) {
> + /* Hugetlb has MTE flags set on head page only */
> + if (try_page_mte_tagging(&folio->page)) {
> + for (i = 0; i < nr_pages; i++, page++)
> + mte_clear_page_tags(page_address(page));
> + set_page_mte_tagged(&folio->page);
> + }
> + return;
> + }
> +
> for (i = 0; i < nr_pages; i++, page++) {
> if (try_page_mte_tagging(page)) {
> mte_clear_page_tags(page_address(page));
> diff --git a/arch/arm64/mm/copypage.c b/arch/arm64/mm/copypage.c
> index a7bb20055ce0..0f3b07d4a5cf 100644
> --- a/arch/arm64/mm/copypage.c
> +++ b/arch/arm64/mm/copypage.c
> @@ -18,6 +18,9 @@ void copy_highpage(struct page *to, struct page *from)
> {
> void *kto = page_address(to);
> void *kfrom = page_address(from);
> + struct folio *src = page_folio(from);
> + struct folio *dst = page_folio(to);
> + unsigned int i, nr_pages;
>
> copy_page(kto, kfrom);
>
> @@ -27,8 +30,26 @@ void copy_highpage(struct page *to, struct page *from)
> if (system_supports_mte() && page_mte_tagged(from)) {
> /* 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);
> +
> + /* Populate tags for all subpages if hugetlb */
> + if (folio_test_hugetlb(src)) {
> + /*
> + * MTE page flag is just set on the head page of
> + * hugetlb. If from has MTE flag set, it must be the
> + * head page.
> + */
> + VM_BUG_ON(!PageHead(from));
> + nr_pages = folio_nr_pages(src);
> + for (i = 0; i < nr_pages; i++, to++, from++) {
> + kto = page_address(to);
> + kfrom = page_address(from);
> + mte_copy_page_tags(kto, kfrom);
> + }
> + set_page_mte_tagged(&dst->page);
> + } else {
> + mte_copy_page_tags(kto, kfrom);
> + set_page_mte_tagged(to);
> + }
> }
> }
> EXPORT_SYMBOL(copy_highpage);
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 9f6cff356796..f944e8e7126b 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -110,7 +110,7 @@ static int hugetlbfs_file_mmap(struct file *file, struct vm_area_struct *vma)
> * way when do_mmap unwinds (may be important on powerpc
> * and ia64).
> */
> - vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND);
> + vm_flags_set(vma, VM_HUGETLB | VM_DONTEXPAND | VM_MTE_ALLOWED);
> vma->vm_ops = &hugetlb_vm_ops;
>
> ret = seal_check_write(info->seals, vma);