On 27.06.2018 15:55, Janosch Frank wrote:
> From: Dominik Dingel <dingel@xxxxxxxxxxxxxxxxxx>
>
> When a guest starts using storage keys, we trap and set a default one
> for its whole valid address space. With this patch we are now able to
> do that for large pages.
>
> To speed up the storage key insertion, we use
> __storage_key_init_range, which in-turn will use sske_frame to set
> multiple storage keys with one instruction. As it has been previously
> used for debuging we have to get rid of the default key check and make
> it quiescing.
>
> Signed-off-by: Dominik Dingel <dingel@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
> [replaced page_set_storage_key loop with __storage_key_init_range]
> Reviewed-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> ---
> arch/s390/mm/gmap.c | 26 +++++++++++++++++++++++---
> arch/s390/mm/pageattr.c | 6 ++----
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
> index 3da4f85ec330..af0a87eeede0 100644
> --- a/arch/s390/mm/gmap.c
> +++ b/arch/s390/mm/gmap.c
> @@ -2761,17 +2761,37 @@ EXPORT_SYMBOL_GPL(s390_enable_sie);
> * Enable storage key handling from now on and initialize the storage
> * keys with the default key.
> */
> -static int __s390_enable_skey(pte_t *pte, unsigned long addr,
> - unsigned long next, struct mm_walk *walk)
> +static int __s390_enable_skey_pte(pte_t *pte, unsigned long addr,
> + unsigned long next, struct mm_walk *walk)
> {
> /* Clear storage key */
> ptep_zap_key(walk->mm, addr, pte);
> return 0;
> }
>
> +static int __s390_enable_skey_hugetlb(pte_t *pte, unsigned long addr,
> + unsigned long hmask, unsigned long next,
> + struct mm_walk *walk)
> +{
> + pmd_t *pmd = (pmd_t *)pte;
> + unsigned long start, end;
> +
> + if (pmd_val(*pmd) & _SEGMENT_ENTRY_INVALID
> + || !(pmd_val(*pmd) & _SEGMENT_ENTRY_WRITE))
In this file, we usually have the "||"/"&&" at the end of the previous line.
Silly, question: why do we have to handle invalid entries here and why
do we have to skip !write? Can both even happen? Should we simply not
initialize storage keys here?
> + return 0;
> +
> + start = pmd_val(*pmd) & HPAGE_MASK;
> + end = start + HPAGE_SIZE - 1;
> + __storage_key_init_range(start, end);
> + return 0;
> +}
> +
> int s390_enable_skey(void)
> {
> - struct mm_walk walk = { .pte_entry = __s390_enable_skey };
> + struct mm_walk walk = {
> + .hugetlb_entry = __s390_enable_skey_hugetlb,
> + .pte_entry = __s390_enable_skey_pte,
> + };
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma;
> int rc = 0;
> diff --git a/arch/s390/mm/pageattr.c b/arch/s390/mm/pageattr.c
> index c44171588d08..f8c6faab41f4 100644
> --- a/arch/s390/mm/pageattr.c
> +++ b/arch/s390/mm/pageattr.c
> @@ -14,7 +14,7 @@
>
> static inline unsigned long sske_frame(unsigned long addr, unsigned char skey)
> {
> - asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],9,0"
> + asm volatile(".insn rrf,0xb22b0000,%[skey],%[addr],1,0"
> : [addr] "+a" (addr) : [skey] "d" (skey));
> return addr;
> }
> @@ -23,8 +23,6 @@ void __storage_key_init_range(unsigned long start, unsigned long end)
> {
> unsigned long boundary, size;
>
> - if (!PAGE_DEFAULT_KEY)
> - return;
> while (start < end) {
> if (MACHINE_HAS_EDAT1) {
> /* set storage keys for a 1MB frame */
> @@ -37,7 +35,7 @@ void __storage_key_init_range(unsigned long start, unsigned long end)
> continue;
> }
> }
> - page_set_storage_key(start, PAGE_DEFAULT_KEY, 0);
> + page_set_storage_key(start, PAGE_DEFAULT_KEY, 1);
Why do we have to change quiescing here? And why in sske_frame? (in
other words: why did it work previously?)
> start += PAGE_SIZE;
> }
> }
>
--
Thanks,
David / dhildenb
