Re: [RFC 07/37] KVM: s390: protvirt: Secure memory is not mergeable

[Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>]

On Thu, 24 Oct 2019 18:07:14 +0200
David Hildenbrand <david@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > ---
> >   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(&current->mm->mmap_sem);
> > +		r = gmap_mark_unmergeable();
> > +		up_write(&current->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); 
> >   
> 
>