| Message ID | 20191024114059.102802-8-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Add support for protected VMs | expand |
On 24.10.19 13:40, Janosch Frank wrote: > KSM will not work on secure pages, because when the kernel reads a > secure page, it will be encrypted and hence no two pages will look the > same. > > Let's mark the guest pages as unmergeable when we transition to secure > mode. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/include/asm/gmap.h | 1 + > arch/s390/kvm/kvm-s390.c | 6 ++++++ > arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- > 3 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h > index 6efc0b501227..eab6a2ec3599 100644 > --- a/arch/s390/include/asm/gmap.h > +++ b/arch/s390/include/asm/gmap.h > @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, > > void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], > unsigned long gaddr, unsigned long vmaddr); > +int gmap_mark_unmergeable(void); > #endif /* _ASM_S390_GMAP_H */ > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 924132d92782..d1ba12f857e7 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) > if (r) > break; > > + down_write(¤t->mm->mmap_sem); > + r = gmap_mark_unmergeable(); > + up_write(¤t->mm->mmap_sem); > + if (r) > + break; > + > mutex_lock(&kvm->lock); > kvm_s390_vcpu_block_all(kvm); > /* FMT 4 SIE needs esca */ > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index edcdca97e85e..bf365a09f900 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) > } > EXPORT_SYMBOL_GPL(s390_enable_sie); > > +int gmap_mark_unmergeable(void) > +{ > + struct mm_struct *mm = current->mm; > + struct vm_area_struct *vma; > + > + for (vma = mm->mmap; vma; vma = vma->vm_next) { > + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > + MADV_UNMERGEABLE, &vma->vm_flags)) { > + mm->context.uses_skeys = 0; That skey setting does not make too much sense when coming via kvm_s390_handle_pv(). handle that in the caller? > + return -ENOMEM; > + } > + } > + mm->def_flags &= ~VM_MERGEABLE; > + return 0; > +} > +EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); > + > /* > * Enable storage key handling from now on and initialize the storage > * keys with the default key. > @@ -2593,7 +2610,6 @@ static const struct mm_walk_ops enable_skey_walk_ops = { > int s390_enable_skey(void) > { > struct mm_struct *mm = current->mm; > - struct vm_area_struct *vma; > int rc = 0; > > down_write(&mm->mmap_sem); > @@ -2601,15 +2617,7 @@ int s390_enable_skey(void) > goto out_up; > > mm->context.uses_skeys = 1; > - for (vma = mm->mmap; vma; vma = vma->vm_next) { > - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > - MADV_UNMERGEABLE, &vma->vm_flags)) { > - mm->context.uses_skeys = 0; > - rc = -ENOMEM; > - goto out_up; > - } > - } > - mm->def_flags &= ~VM_MERGEABLE; > + gmap_mark_unmergeable(); > > walk_page_range(mm, 0, TASK_SIZE, &enable_skey_walk_ops, NULL); > >
On Thu, 24 Oct 2019 18:07:14 +0200 David Hildenbrand <david@redhat.com> wrote: > On 24.10.19 13:40, Janosch Frank wrote: > > KSM will not work on secure pages, because when the kernel reads a > > secure page, it will be encrypted and hence no two pages will look > > the same. > > > > Let's mark the guest pages as unmergeable when we transition to > > secure mode. > > > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > --- > > arch/s390/include/asm/gmap.h | 1 + > > arch/s390/kvm/kvm-s390.c | 6 ++++++ > > arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- > > 3 files changed, 25 insertions(+), 10 deletions(-) > > > > diff --git a/arch/s390/include/asm/gmap.h > > b/arch/s390/include/asm/gmap.h index 6efc0b501227..eab6a2ec3599 > > 100644 --- a/arch/s390/include/asm/gmap.h > > +++ b/arch/s390/include/asm/gmap.h > > @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, > > unsigned long start, > > void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long > > dirty_bitmap[4], unsigned long gaddr, unsigned long vmaddr); > > +int gmap_mark_unmergeable(void); > > #endif /* _ASM_S390_GMAP_H */ > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > > index 924132d92782..d1ba12f857e7 100644 > > --- a/arch/s390/kvm/kvm-s390.c > > +++ b/arch/s390/kvm/kvm-s390.c > > @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm > > *kvm, struct kvm_pv_cmd *cmd) if (r) > > break; > > > > + down_write(¤t->mm->mmap_sem); > > + r = gmap_mark_unmergeable(); > > + up_write(¤t->mm->mmap_sem); > > + if (r) > > + break; > > + > > mutex_lock(&kvm->lock); > > kvm_s390_vcpu_block_all(kvm); > > /* FMT 4 SIE needs esca */ > > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > > index edcdca97e85e..bf365a09f900 100644 > > --- a/arch/s390/mm/gmap.c > > +++ b/arch/s390/mm/gmap.c > > @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) > > } > > EXPORT_SYMBOL_GPL(s390_enable_sie); > > > > +int gmap_mark_unmergeable(void) > > +{ > > + struct mm_struct *mm = current->mm; > > + struct vm_area_struct *vma; > > + > > + for (vma = mm->mmap; vma; vma = vma->vm_next) { > > + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > > + MADV_UNMERGEABLE, &vma->vm_flags)) > > { > > + mm->context.uses_skeys = 0; > > That skey setting does not make too much sense when coming via > kvm_s390_handle_pv(). handle that in the caller? protected guests run keyless; any attempt to use keys in the guest will result in an exception in the guest. > > > + return -ENOMEM; > > + } > > + } > > + mm->def_flags &= ~VM_MERGEABLE; > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); > > + > > /* > > * Enable storage key handling from now on and initialize the > > storage > > * keys with the default key. > > @@ -2593,7 +2610,6 @@ static const struct mm_walk_ops > > enable_skey_walk_ops = { int s390_enable_skey(void) > > { > > struct mm_struct *mm = current->mm; > > - struct vm_area_struct *vma; > > int rc = 0; > > > > down_write(&mm->mmap_sem); > > @@ -2601,15 +2617,7 @@ int s390_enable_skey(void) > > goto out_up; > > > > mm->context.uses_skeys = 1; > > - for (vma = mm->mmap; vma; vma = vma->vm_next) { > > - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > > - MADV_UNMERGEABLE, &vma->vm_flags)) > > { > > - mm->context.uses_skeys = 0; > > - rc = -ENOMEM; > > - goto out_up; > > - } > > - } > > - mm->def_flags &= ~VM_MERGEABLE; > > + gmap_mark_unmergeable(); > > > > walk_page_range(mm, 0, TASK_SIZE, &enable_skey_walk_ops, > > NULL); > > > >
On 24.10.19 18:33, Claudio Imbrenda wrote: > On Thu, 24 Oct 2019 18:07:14 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> On 24.10.19 13:40, Janosch Frank wrote: >>> KSM will not work on secure pages, because when the kernel reads a >>> secure page, it will be encrypted and hence no two pages will look >>> the same. >>> >>> Let's mark the guest pages as unmergeable when we transition to >>> secure mode. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> arch/s390/include/asm/gmap.h | 1 + >>> arch/s390/kvm/kvm-s390.c | 6 ++++++ >>> arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- >>> 3 files changed, 25 insertions(+), 10 deletions(-) >>> >>> diff --git a/arch/s390/include/asm/gmap.h >>> b/arch/s390/include/asm/gmap.h index 6efc0b501227..eab6a2ec3599 >>> 100644 --- a/arch/s390/include/asm/gmap.h >>> +++ b/arch/s390/include/asm/gmap.h >>> @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, >>> unsigned long start, >>> void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long >>> dirty_bitmap[4], unsigned long gaddr, unsigned long vmaddr); >>> +int gmap_mark_unmergeable(void); >>> #endif /* _ASM_S390_GMAP_H */ >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>> index 924132d92782..d1ba12f857e7 100644 >>> --- a/arch/s390/kvm/kvm-s390.c >>> +++ b/arch/s390/kvm/kvm-s390.c >>> @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm >>> *kvm, struct kvm_pv_cmd *cmd) if (r) >>> break; >>> >>> + down_write(¤t->mm->mmap_sem); >>> + r = gmap_mark_unmergeable(); >>> + up_write(¤t->mm->mmap_sem); >>> + if (r) >>> + break; >>> + >>> mutex_lock(&kvm->lock); >>> kvm_s390_vcpu_block_all(kvm); >>> /* FMT 4 SIE needs esca */ >>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >>> index edcdca97e85e..bf365a09f900 100644 >>> --- a/arch/s390/mm/gmap.c >>> +++ b/arch/s390/mm/gmap.c >>> @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) >>> } >>> EXPORT_SYMBOL_GPL(s390_enable_sie); >>> >>> +int gmap_mark_unmergeable(void) >>> +{ >>> + struct mm_struct *mm = current->mm; >>> + struct vm_area_struct *vma; >>> + >>> + for (vma = mm->mmap; vma; vma = vma->vm_next) { >>> + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, >>> + MADV_UNMERGEABLE, &vma->vm_flags)) >>> { >>> + mm->context.uses_skeys = 0; >> >> That skey setting does not make too much sense when coming via >> kvm_s390_handle_pv(). handle that in the caller? > > protected guests run keyless; any attempt to use keys in the guest will > result in an exception in the guest. still, this is the recovery path for the "mm->context.uses_skeys = 1;" in enable_skey_walk_ops() and confuses reader (like me).
On 10/24/19 6:07 PM, David Hildenbrand wrote: > On 24.10.19 13:40, Janosch Frank wrote: >> KSM will not work on secure pages, because when the kernel reads a >> secure page, it will be encrypted and hence no two pages will look the >> same. >> >> Let's mark the guest pages as unmergeable when we transition to secure >> mode. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arch/s390/include/asm/gmap.h | 1 + >> arch/s390/kvm/kvm-s390.c | 6 ++++++ >> arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- >> 3 files changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h >> index 6efc0b501227..eab6a2ec3599 100644 >> --- a/arch/s390/include/asm/gmap.h >> +++ b/arch/s390/include/asm/gmap.h >> @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, >> >> void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], >> unsigned long gaddr, unsigned long vmaddr); >> +int gmap_mark_unmergeable(void); >> #endif /* _ASM_S390_GMAP_H */ >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index 924132d92782..d1ba12f857e7 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) >> if (r) >> break; >> >> + down_write(¤t->mm->mmap_sem); >> + r = gmap_mark_unmergeable(); >> + up_write(¤t->mm->mmap_sem); >> + if (r) >> + break; >> + >> mutex_lock(&kvm->lock); >> kvm_s390_vcpu_block_all(kvm); >> /* FMT 4 SIE needs esca */ >> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >> index edcdca97e85e..bf365a09f900 100644 >> --- a/arch/s390/mm/gmap.c >> +++ b/arch/s390/mm/gmap.c >> @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) >> } >> EXPORT_SYMBOL_GPL(s390_enable_sie); >> >> +int gmap_mark_unmergeable(void) >> +{ >> + struct mm_struct *mm = current->mm; >> + struct vm_area_struct *vma; >> + >> + for (vma = mm->mmap; vma; vma = vma->vm_next) { >> + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, >> + MADV_UNMERGEABLE, &vma->vm_flags)) { >> + mm->context.uses_skeys = 0; > > That skey setting does not make too much sense when coming via > kvm_s390_handle_pv(). handle that in the caller? Hmm, I think the name of that variable is just plain wrong. It should be "can_use_skeys" or "uses_unmergeable" (which would fit better into the mm context anyway) and then we could add a kvm->arch.uses_skeys to tell that we actually used them for migration checks, etc.. I had long discussions with Martin over these variable names a long time ago..
On 24.10.19 13:40, Janosch Frank wrote: > KSM will not work on secure pages, because when the kernel reads a > secure page, it will be encrypted and hence no two pages will look the > same. > > Let's mark the guest pages as unmergeable when we transition to secure > mode. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/include/asm/gmap.h | 1 + > arch/s390/kvm/kvm-s390.c | 6 ++++++ > arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- > 3 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h > index 6efc0b501227..eab6a2ec3599 100644 > --- a/arch/s390/include/asm/gmap.h > +++ b/arch/s390/include/asm/gmap.h > @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, > > void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], > unsigned long gaddr, unsigned long vmaddr); > +int gmap_mark_unmergeable(void); > #endif /* _ASM_S390_GMAP_H */ > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 924132d92782..d1ba12f857e7 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) > if (r) > break; > > + down_write(¤t->mm->mmap_sem); > + r = gmap_mark_unmergeable(); > + up_write(¤t->mm->mmap_sem); > + if (r) > + break; > + > mutex_lock(&kvm->lock); > kvm_s390_vcpu_block_all(kvm); > /* FMT 4 SIE needs esca */ > diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c > index edcdca97e85e..bf365a09f900 100644 > --- a/arch/s390/mm/gmap.c > +++ b/arch/s390/mm/gmap.c > @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) > } > EXPORT_SYMBOL_GPL(s390_enable_sie); > > +int gmap_mark_unmergeable(void) > +{ > + struct mm_struct *mm = current->mm; > + struct vm_area_struct *vma; > + > + for (vma = mm->mmap; vma; vma = vma->vm_next) { > + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > + MADV_UNMERGEABLE, &vma->vm_flags)) { > + mm->context.uses_skeys = 0; > + return -ENOMEM; > + } > + } > + mm->def_flags &= ~VM_MERGEABLE; > + return 0; > +} > +EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); > + > /* > * Enable storage key handling from now on and initialize the storage > * keys with the default key. > @@ -2593,7 +2610,6 @@ static const struct mm_walk_ops enable_skey_walk_ops = { > int s390_enable_skey(void) > { > struct mm_struct *mm = current->mm; > - struct vm_area_struct *vma; > int rc = 0; > > down_write(&mm->mmap_sem); > @@ -2601,15 +2617,7 @@ int s390_enable_skey(void) > goto out_up; > > mm->context.uses_skeys = 1; > - for (vma = mm->mmap; vma; vma = vma->vm_next) { > - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, > - MADV_UNMERGEABLE, &vma->vm_flags)) { > - mm->context.uses_skeys = 0; > - rc = -ENOMEM; > - goto out_up; > - } > - } > - mm->def_flags &= ~VM_MERGEABLE; > + gmap_mark_unmergeable(); also, here we are now ignoring errors? This looks broken.
On 25.10.19 09:18, Janosch Frank wrote: > On 10/24/19 6:07 PM, David Hildenbrand wrote: >> On 24.10.19 13:40, Janosch Frank wrote: >>> KSM will not work on secure pages, because when the kernel reads a >>> secure page, it will be encrypted and hence no two pages will look the >>> same. >>> >>> Let's mark the guest pages as unmergeable when we transition to secure >>> mode. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> arch/s390/include/asm/gmap.h | 1 + >>> arch/s390/kvm/kvm-s390.c | 6 ++++++ >>> arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- >>> 3 files changed, 25 insertions(+), 10 deletions(-) >>> >>> diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h >>> index 6efc0b501227..eab6a2ec3599 100644 >>> --- a/arch/s390/include/asm/gmap.h >>> +++ b/arch/s390/include/asm/gmap.h >>> @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, >>> >>> void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], >>> unsigned long gaddr, unsigned long vmaddr); >>> +int gmap_mark_unmergeable(void); >>> #endif /* _ASM_S390_GMAP_H */ >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>> index 924132d92782..d1ba12f857e7 100644 >>> --- a/arch/s390/kvm/kvm-s390.c >>> +++ b/arch/s390/kvm/kvm-s390.c >>> @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) >>> if (r) >>> break; >>> >>> + down_write(¤t->mm->mmap_sem); >>> + r = gmap_mark_unmergeable(); >>> + up_write(¤t->mm->mmap_sem); >>> + if (r) >>> + break; >>> + >>> mutex_lock(&kvm->lock); >>> kvm_s390_vcpu_block_all(kvm); >>> /* FMT 4 SIE needs esca */ >>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >>> index edcdca97e85e..bf365a09f900 100644 >>> --- a/arch/s390/mm/gmap.c >>> +++ b/arch/s390/mm/gmap.c >>> @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) >>> } >>> EXPORT_SYMBOL_GPL(s390_enable_sie); >>> >>> +int gmap_mark_unmergeable(void) >>> +{ >>> + struct mm_struct *mm = current->mm; >>> + struct vm_area_struct *vma; >>> + >>> + for (vma = mm->mmap; vma; vma = vma->vm_next) { >>> + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, >>> + MADV_UNMERGEABLE, &vma->vm_flags)) { >>> + mm->context.uses_skeys = 0; >> >> That skey setting does not make too much sense when coming via >> kvm_s390_handle_pv(). handle that in the caller? > > Hmm, I think the name of that variable is just plain wrong. > It should be "can_use_skeys" or "uses_unmergeable" (which would fit > better into the mm context anyway) and then we could add a > kvm->arch.uses_skeys to tell that we actually used them for migration > checks, etc.. > > I had long discussions with Martin over these variable names a long time > ago.. uses_skeys is set during s390_enable_skey(). that is used when we a) Call an skey instruction b) Migrate skeys So it should match "uses" or what am I missing? If you look at the users of "mm_uses_skeys(mm)" I think "uses_unmergeable" would actually be misleading. (e.g., pgste_set_key()). it really means "somebody used skeys". The unmergable is just a required side effect.
On 10/25/19 10:04 AM, David Hildenbrand wrote: > On 25.10.19 09:18, Janosch Frank wrote: >> On 10/24/19 6:07 PM, David Hildenbrand wrote: >>> On 24.10.19 13:40, Janosch Frank wrote: >>>> KSM will not work on secure pages, because when the kernel reads a >>>> secure page, it will be encrypted and hence no two pages will look the >>>> same. >>>> >>>> Let's mark the guest pages as unmergeable when we transition to secure >>>> mode. >>>> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> arch/s390/include/asm/gmap.h | 1 + >>>> arch/s390/kvm/kvm-s390.c | 6 ++++++ >>>> arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- >>>> 3 files changed, 25 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h >>>> index 6efc0b501227..eab6a2ec3599 100644 >>>> --- a/arch/s390/include/asm/gmap.h >>>> +++ b/arch/s390/include/asm/gmap.h >>>> @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, >>>> >>>> void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], >>>> unsigned long gaddr, unsigned long vmaddr); >>>> +int gmap_mark_unmergeable(void); >>>> #endif /* _ASM_S390_GMAP_H */ >>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>>> index 924132d92782..d1ba12f857e7 100644 >>>> --- a/arch/s390/kvm/kvm-s390.c >>>> +++ b/arch/s390/kvm/kvm-s390.c >>>> @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) >>>> if (r) >>>> break; >>>> >>>> + down_write(¤t->mm->mmap_sem); >>>> + r = gmap_mark_unmergeable(); >>>> + up_write(¤t->mm->mmap_sem); >>>> + if (r) >>>> + break; >>>> + >>>> mutex_lock(&kvm->lock); >>>> kvm_s390_vcpu_block_all(kvm); >>>> /* FMT 4 SIE needs esca */ >>>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c >>>> index edcdca97e85e..bf365a09f900 100644 >>>> --- a/arch/s390/mm/gmap.c >>>> +++ b/arch/s390/mm/gmap.c >>>> @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) >>>> } >>>> EXPORT_SYMBOL_GPL(s390_enable_sie); >>>> >>>> +int gmap_mark_unmergeable(void) >>>> +{ >>>> + struct mm_struct *mm = current->mm; >>>> + struct vm_area_struct *vma; >>>> + >>>> + for (vma = mm->mmap; vma; vma = vma->vm_next) { >>>> + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, >>>> + MADV_UNMERGEABLE, &vma->vm_flags)) { >>>> + mm->context.uses_skeys = 0; >>> >>> That skey setting does not make too much sense when coming via >>> kvm_s390_handle_pv(). handle that in the caller? >> >> Hmm, I think the name of that variable is just plain wrong. >> It should be "can_use_skeys" or "uses_unmergeable" (which would fit >> better into the mm context anyway) and then we could add a >> kvm->arch.uses_skeys to tell that we actually used them for migration >> checks, etc.. >> >> I had long discussions with Martin over these variable names a long time >> ago.. > > uses_skeys is set during s390_enable_skey(). that is used when we > > a) Call an skey instruction > b) Migrate skeys > > So it should match "uses" or what am I missing? > > If you look at the users of "mm_uses_skeys(mm)" I think > "uses_unmergeable" would actually be misleading. (e.g., > pgste_set_key()). it really means "somebody used skeys". The unmergable > is just a required side effect. Hmm, we couldn't check struct kvm from pgtable.c anyway. Oh well, I still don't like it very much but your arguments are better :-) Let's fix this.
diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h index 6efc0b501227..eab6a2ec3599 100644 --- a/arch/s390/include/asm/gmap.h +++ b/arch/s390/include/asm/gmap.h @@ -145,4 +145,5 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], unsigned long gaddr, unsigned long vmaddr); +int gmap_mark_unmergeable(void); #endif /* _ASM_S390_GMAP_H */ diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 924132d92782..d1ba12f857e7 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2176,6 +2176,12 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) if (r) break; + down_write(¤t->mm->mmap_sem); + r = gmap_mark_unmergeable(); + up_write(¤t->mm->mmap_sem); + if (r) + break; + mutex_lock(&kvm->lock); kvm_s390_vcpu_block_all(kvm); /* FMT 4 SIE needs esca */ diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index edcdca97e85e..bf365a09f900 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -2548,6 +2548,23 @@ int s390_enable_sie(void) } EXPORT_SYMBOL_GPL(s390_enable_sie); +int gmap_mark_unmergeable(void) +{ + struct mm_struct *mm = current->mm; + struct vm_area_struct *vma; + + for (vma = mm->mmap; vma; vma = vma->vm_next) { + if (ksm_madvise(vma, vma->vm_start, vma->vm_end, + MADV_UNMERGEABLE, &vma->vm_flags)) { + mm->context.uses_skeys = 0; + return -ENOMEM; + } + } + mm->def_flags &= ~VM_MERGEABLE; + return 0; +} +EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); + /* * Enable storage key handling from now on and initialize the storage * keys with the default key. @@ -2593,7 +2610,6 @@ static const struct mm_walk_ops enable_skey_walk_ops = { int s390_enable_skey(void) { struct mm_struct *mm = current->mm; - struct vm_area_struct *vma; int rc = 0; down_write(&mm->mmap_sem); @@ -2601,15 +2617,7 @@ int s390_enable_skey(void) goto out_up; mm->context.uses_skeys = 1; - for (vma = mm->mmap; vma; vma = vma->vm_next) { - if (ksm_madvise(vma, vma->vm_start, vma->vm_end, - MADV_UNMERGEABLE, &vma->vm_flags)) { - mm->context.uses_skeys = 0; - rc = -ENOMEM; - goto out_up; - } - } - mm->def_flags &= ~VM_MERGEABLE; + gmap_mark_unmergeable(); walk_page_range(mm, 0, TASK_SIZE, &enable_skey_walk_ops, NULL);
KSM will not work on secure pages, because when the kernel reads a secure page, it will be encrypted and hence no two pages will look the same. Let's mark the guest pages as unmergeable when we transition to secure mode. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- arch/s390/include/asm/gmap.h | 1 + arch/s390/kvm/kvm-s390.c | 6 ++++++ arch/s390/mm/gmap.c | 28 ++++++++++++++++++---------- 3 files changed, 25 insertions(+), 10 deletions(-)