Re: [RFC/PATCH v2 19/22] s390/mm: Split huge pages if granular protection is needed

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

[-- Attachment #1.1: Type: text/plain, Size: 3165 bytes --]

On 13.12.2017 13:53, Janosch Frank wrote:
> A guest can put DAT tables for a lower level guest in the same huge
> segment as one of its prefixes or a g3 page. This would make it
> necessary for the segment to be unprotected (because of the prefix)
> and protected (because of the shadowing) at the same time. This is not
> possible in this universe.
> 
> Hence we split the affected huge segment, so we can protect on a
> per-page basis. Such gmap segments are special and get a new software
> bit, that helps us handling this edge case.
> 
> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
>  arch/s390/include/asm/gmap.h    |  13 ++
>  arch/s390/include/asm/pgtable.h |   7 +-
>  arch/s390/mm/fault.c            |  10 +-
>  arch/s390/mm/gmap.c             | 256 ++++++++++++++++++++++++++++++++++++----
>  arch/s390/mm/pgtable.c          |  51 ++++++++
>  5 files changed, 313 insertions(+), 24 deletions(-)

> @@ -1081,20 +1189,27 @@ static int gmap_protect_range(struct gmap *gmap, unsigned long gaddr,
>  	spinlock_t *ptl;
>  	unsigned long vmaddr, dist;
>  	pmd_t *pmdp, *hpmdp;
> -	int rc;
> +	int rc = 0;
> 
>  	while (len) {
>  		rc = -EAGAIN;
>  		vmaddr = __gmap_translate(gmap, gaddr);
>  		hpmdp = (pmd_t *)huge_pte_offset(gmap->mm, vmaddr, HPAGE_SIZE);
> +		if (!hpmdp)
> +			BUG();
>  		/* Do we need tests here? */
>  		ptl = pmd_lock(gmap->mm, hpmdp);
> 
>  		pmdp = gmap_pmd_op_walk(gmap, gaddr);
>  		if (pmdp) {
>  			if (!pmd_large(*pmdp)) {
> -				rc = gmap_protect_pte(gmap, gaddr, pmdp, prot,
> -						      bits);
> +				if (gmap_pmd_is_split(pmdp) &&
> +				    (bits & GMAP_NOTIFY_MPROT)) {
> +					pmd_val(*pmdp) |= _SEGMENT_ENTRY_GMAP_IN;
> +				}

@David:
This currently breaks my brain. There *was* a reason why I put this
there and I was quite insistent that we needed it. Something about
notification areas on splits, but I absolutely can't remember it. Sigh,
should've made a comment.

This might be a leftover from earlier versions, but could also keep us
from doing mprot notification on pte's.

> +
> +				rc = gmap_protect_pte(gmap, gaddr, vmaddr,
> +						      pmdp, hpmdp, prot, bits);
>  				if (!rc) {
>  					len -= PAGE_SIZE;
>  					gaddr += PAGE_SIZE;
> @@ -1111,7 +1226,9 @@ static int gmap_protect_range(struct gmap *gmap, unsigned long gaddr,

[...]

> @@ -2774,6 +2977,8 @@ void gmap_pmdp_idte_global(struct mm_struct *mm, unsigned long vmaddr)
>  					    IDTE_GLOBAL);
>  			else
>  				__pmdp_csp(pmdp);
> +
> +			gmap_pmd_split_free(pmdp);
>  			*entry = _SEGMENT_ENTRY_EMPTY;
>  		}
>  		spin_unlock(&gmap->guest_table_lock);
> @@ -2852,6 +3057,7 @@ void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long bitmap[4],
>  	pmd_t *pmdp, *hpmdp;
>  	spinlock_t *ptl;
> 
> +	/* Protection against gmap_link vsie unprotection. */
>  	hpmdp = (pmd_t *)huge_pte_offset(gmap->mm, vmaddr, HPAGE_SIZE);
>  	if (!hpmdp)
>  		return;
> @@ -2867,9 +3073,17 @@ void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long bitmap[4],
>  						      gaddr, vmaddr))
>  			memset(bitmap, 0xFF, 32);

s/0xFF/0xff/



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]