Re: [PATCH v1 13/13] KVM: s390: remove the last user of page->index

[Janosch Frank <frankja@linux.ibm.com>]

On 1/8/25 7:14 PM, Claudio Imbrenda wrote:
> Shadow page tables use page->index to keep the g2 address of the guest
> page table being shadowed.
> 
> Instead of keeping the information in page->index, split the address
> and smear it over the 16-bit softbits areas of 4 PGSTEs.
> 
> This removes the last s390 user of page->index.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
>   arch/s390/include/asm/gmap.h    |  1 +
>   arch/s390/include/asm/pgtable.h | 15 +++++++++++++++
>   arch/s390/kvm/gaccess.c         |  6 ++++--
>   arch/s390/mm/gmap.c             | 22 ++++++++++++++++++++--
>   4 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h
> index 5ebc65ac78cc..28c5bf097268 100644
> --- a/arch/s390/include/asm/gmap.h
> +++ b/arch/s390/include/asm/gmap.h
> @@ -177,4 +177,5 @@ static inline int s390_uv_destroy_range_interruptible(struct mm_struct *mm, unsi
>   {
>   	return __s390_uv_destroy_range(mm, start, end, true);
>   }
> +

Stray \n

>   #endif /* _ASM_S390_GMAP_H */
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 151488bb9ed7..948100a8fa7e 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -419,6 +419,7 @@ static inline int is_module_addr(void *addr)
>   #define PGSTE_HC_BIT	0x0020000000000000UL
>   #define PGSTE_GR_BIT	0x0004000000000000UL
>   #define PGSTE_GC_BIT	0x0002000000000000UL
> +#define PGSTE_ST2_MASK	0x0000ffff00000000UL
>   #define PGSTE_UC_BIT	0x0000000000008000UL	/* user dirty (migration) */
>   #define PGSTE_IN_BIT	0x0000000000004000UL	/* IPTE notify bit */
>   #define PGSTE_VSIE_BIT	0x0000000000002000UL	/* ref'd in a shadow table */
> @@ -2001,4 +2002,18 @@ extern void s390_reset_cmma(struct mm_struct *mm);
>   #define pmd_pgtable(pmd) \
>   	((pgtable_t)__va(pmd_val(pmd) & -sizeof(pte_t)*PTRS_PER_PTE))
>   
> +static inline unsigned long gmap_pgste_get_index(unsigned long *pgt)
> +{
> +	unsigned long *pgstes, res;
> +
> +	pgstes = pgt + _PAGE_ENTRIES;
> +
> +	res = (pgstes[0] & PGSTE_ST2_MASK) << 16;
> +	res |= pgstes[1] & PGSTE_ST2_MASK;
> +	res |= (pgstes[2] & PGSTE_ST2_MASK) >> 16;
> +	res |= (pgstes[3] & PGSTE_ST2_MASK) >> 32;
> +
> +	return res;
> +}

I have to think about that change for a bit before I post an opinion.