Re: [PATCH v2] s390/mm: Allow hugepage migration for s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] s390/mm: guest storage key loss on 1M/2G hugetlbfs page
  migration
- [Medium] s390/mm: loss of soft-dirty bit in __pte_to_rste() and
  __rste_to_pte() on hugetlb migration
--

commit d03e5331e4e0b88aa75246fc7dc6aef18e7c8cdd
Author: Gerald Schaefer <gerald.schaefer@linux.ibm.com>

s390/mm: Allow hugepage migration for s390

This commit selects ARCH_ENABLE_HUGEPAGE_MIGRATION on s390 and defines
arch_hugetlb_migration_supported() for 1M and 2G hugepages so hugetlbfs
pages can be migrated and allocated from ZONE_MOVABLE.

> diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
> index f84aa9265430f..2c6e94888bd06 100644
> --- a/arch/s390/mm/hugetlbpage.c
> +++ b/arch/s390/mm/hugetlbpage.c
> @@ -224,7 +224,7 @@ pte_t *huge_pte_offset(struct mm_struct *mm,
>  	return (pte_t *) pmdp;
>  }
>  
> -bool __init arch_hugetlb_valid_size(unsigned long size)
> +static bool __hugetlb_valid_size(unsigned long size)
>  {
>  	if (cpu_has_edat1() && size == PMD_SIZE)
>  		return true;
> @@ -241,3 +241,15 @@ unsigned int __init arch_hugetlb_cma_order(void)
>  
>  	return 0;
>  }
> +
> +bool __init arch_hugetlb_valid_size(unsigned long size)
> +{
> +	return __hugetlb_valid_size(size);
> +}
> +
> +#if IS_ENABLED(CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION)
> +bool arch_hugetlb_migration_supported(struct hstate *h)
> +{
> +	return __hugetlb_valid_size(huge_page_size(h));
> +}
> +#endif

[Severity: High]
Does migrating a 1M or 2G hugetlbfs folio backing a KVM guest lose the
guest's hardware storage keys?

For 1M and 2G mappings (KVM_CAP_S390_HPAGE_1M / KVM_CAP_S390_HPAGE_2G),
the KVM gmap uses large leaf CRSTEs without PGSTEs, and guest storage keys
live only in the physical page frames.  When unmap_and_move_hugetlb_folio()
unmaps the folio via mmu_notifier_invalidate_range_start(),
_gmap_unmap_crste() clears the large CRSTE to _CRSTE_EMPTY without saving
or splitting the hardware storage keys:

arch/s390/kvm/gmap/gmap.c:_gmap_unmap_crste() {
	...
	if (!old.h.fc)
		return 0;

	if (old.s.fc1.pr && test_bit(GMAP_FLAG_EXPORT_ON_UNMAP, &priv->gmap->flags))
		folio = phys_to_folio(crste_origin_large(old));
	ok = gmap_crstep_xchg_atomic(priv->gmap, crstep, old, _CRSTE_EMPTY(old.h.tt), gfn);
	...
}

folio_copy() -> copy_highpage() -> copy_page() then copies only the 4K
page contents via mvcl and does not copy the hardware storage keys to the
destination frames, leaving the guest with stale storage keys and causing
guest protection exceptions or memory corruption.

[Severity: Medium]
Does migrating a hugetlbfs page also drop its soft-dirty bit in
__pte_to_rste() and __rste_to_pte()?

When try_to_migrate_one() installs a hugetlb migration entry, it sets
_PAGE_SOFT_DIRTY via pte_swp_mksoft_dirty() and calls set_huge_pte_at().
However, the swap entry branches in __pte_to_rste() and __rste_to_pte() do
not transfer _PAGE_SOFT_DIRTY to and from _SEGMENT_ENTRY_SOFT_DIRTY:

arch/s390/mm/hugetlbpage.c:__pte_to_rste() {
	...
	} else if (!pte_none(pte)) {
		/* swap pte */
		arch_entry = __pte_to_swp_entry(pte);
		rste = mk_swap_rste(__swp_type(arch_entry), __swp_offset(arch_entry));
	...
}

arch/s390/mm/hugetlbpage.c:__rste_to_pte() {
	...
	} else if (!none) {
		/* swap rste */
		arch_entry = __rste_to_swp_entry(rste);
		pte = mk_swap_pte(__swp_type_rste(arch_entry), __swp_offset_rste(arch_entry));
		pteval = pte_val(pte);
	...
}

As a result, remove_migration_pte() reads back old_pte via huge_ptep_get()
with pte_swp_soft_dirty(old_pte) == 0 and clears _PAGE_SOFT_DIRTY on the
restored hugepage PTE, breaking soft-dirty tracking via /proc/pid/pagemap.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111182944.4154805-1-gerald.schaefer@linux.ibm.com?part=1