On 06.07.2018 15:55, Janosch Frank wrote:
> Let's allow pmds to be linked into gmap for the upcoming s390 KVM huge
> page support.
>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
> arch/s390/include/asm/pgtable.h | 6 ++++--
> arch/s390/mm/gmap.c | 13 +++++++++----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 5ab636089c60..fe36b3bb2afd 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -268,8 +268,10 @@ static inline int is_module_addr(void *addr)
> #define _REGION_ENTRY_BITS_LARGE 0xffffffff8000fe2fUL
>
> /* Bits in the segment table entry */
> -#define _SEGMENT_ENTRY_BITS 0xfffffffffffffe33UL
> -#define _SEGMENT_ENTRY_BITS_LARGE 0xfffffffffff0ff33UL
> +#define _SEGMENT_ENTRY_BITS 0xfffffffffffffe33UL
> +#define _SEGMENT_ENTRY_BITS_LARGE 0xfffffffffff0ff33UL
> +#define _SEGMENT_ENTRY_HARDWARE_BITS 0xfffffffffffffe30UL
> +#define _SEGMENT_ENTRY_HARDWARE_BITS_LARGE 0xfffffffffff00730UL
> #define _SEGMENT_ENTRY_ORIGIN_LARGE ~0xfffffUL /* large page address */
> #define _SEGMENT_ENTRY_ORIGIN ~0x7ffUL/* page table origin */
> #define _SEGMENT_ENTRY_PROTECT 0x200 /* segment protection bit */
> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
> index 177419f01449..e31c365d4a2f 100644
> --- a/arch/s390/mm/gmap.c
> +++ b/arch/s390/mm/gmap.c
> @@ -599,10 +599,15 @@ int __gmap_link(struct gmap *gmap, unsigned long gaddr, unsigned long vmaddr)
> if (*table == _SEGMENT_ENTRY_EMPTY) {
> rc = radix_tree_insert(&gmap->host_to_guest,
> vmaddr >> PMD_SHIFT, table);
> - if (!rc)
> - *table = pmd_val(*pmd);
So until now we copied all bits (SW and HW), but now we want to only
copy the HW bits. You should add that to the patch description, because
it is also a change for 4k backed guests.
Something like "The GMAP tables should never copy SW defined bits from
process page tables that might be interpreted differently in GMAP
tables. So let's drop all SW defined bits."
> - } else
> - rc = 0;
> + if (!rc) {
> + if (pmd_large(*pmd)) {
> + *table = pmd_val(*pmd) &
> + _SEGMENT_ENTRY_HARDWARE_BITS_LARGE;
> + } else
> + *table = pmd_val(*pmd) &
> + _SEGMENT_ENTRY_HARDWARE_BITS;
> + }
> + }
> spin_unlock(&gmap->guest_table_lock);
> spin_unlock(ptl);
> radix_tree_preload_end();
>
I think this should be fine and we never actually accessed these SW bits
via the GMAP (would be wrong).
Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
--
Thanks,
David / dhildenb
