From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH 2/2] x86/shadow: avoid extra local array variable Date: Thu, 10 Mar 2016 03:13:40 -0700 Message-ID: <56E156E402000078000DB2BE@prv-mh.provo.novell.com> References: <56E1555002000078000DB293@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part5364E9C4.0__=" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1adxbE-0002Kh-ED for xen-devel@lists.xenproject.org; Thu, 10 Mar 2016 10:13:44 +0000 In-Reply-To: <56E1555002000078000DB293@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: xen-devel Cc: Tim Deegan List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part5364E9C4.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline mfns[2] was there just because struct sh_emulate_ctxt's two MFN values can't be used to hand to vmap(). Making the structure fields an array avoids the extra copying. Signed-off-by: Jan Beulich --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -1746,11 +1746,11 @@ void *sh_emulate_map_dest(struct vcpu *v struct domain *d =3D v->domain; void *map; =20 - sh_ctxt->mfn1 =3D emulate_gva_to_mfn(v, vaddr, sh_ctxt); - if ( !mfn_valid(sh_ctxt->mfn1) ) - return ((mfn_x(sh_ctxt->mfn1) =3D=3D BAD_GVA_TO_GFN) ? + sh_ctxt->mfn[0] =3D emulate_gva_to_mfn(v, vaddr, sh_ctxt); + if ( !mfn_valid(sh_ctxt->mfn[0]) ) + return ((mfn_x(sh_ctxt->mfn[0]) =3D=3D BAD_GVA_TO_GFN) ? MAPPING_EXCEPTION : - (mfn_x(sh_ctxt->mfn1) =3D=3D READONLY_GFN) ? + (mfn_x(sh_ctxt->mfn[0]) =3D=3D READONLY_GFN) ? MAPPING_SILENT_FAIL : MAPPING_UNHANDLEABLE); =20 #ifndef NDEBUG @@ -1767,39 +1767,36 @@ void *sh_emulate_map_dest(struct vcpu *v =20 /* Unaligned writes mean probably this isn't a pagetable. */ if ( vaddr & (bytes - 1) ) - sh_remove_shadows(d, sh_ctxt->mfn1, 0, 0 /* Slow, can fail. */ ); + sh_remove_shadows(d, sh_ctxt->mfn[0], 0, 0 /* Slow, can fail. */ = ); =20 if ( likely(((vaddr + bytes - 1) & PAGE_MASK) =3D=3D (vaddr & = PAGE_MASK)) ) { /* Whole write fits on a single page. */ - sh_ctxt->mfn2 =3D _mfn(INVALID_MFN); - map =3D map_domain_page(sh_ctxt->mfn1) + (vaddr & ~PAGE_MASK); + sh_ctxt->mfn[1] =3D _mfn(INVALID_MFN); + map =3D map_domain_page(sh_ctxt->mfn[0]) + (vaddr & ~PAGE_MASK); } - else + else if ( !is_hvm_domain(d) ) { - mfn_t mfns[2]; - /* * Cross-page emulated writes are only supported for HVM guests; * PV guests ought to know better. */ - if ( !is_hvm_domain(d) ) - return MAPPING_UNHANDLEABLE; - + return MAPPING_UNHANDLEABLE; + } + else + { /* This write crosses a page boundary. Translate the second page. = */ - sh_ctxt->mfn2 =3D emulate_gva_to_mfn(v, vaddr + bytes, sh_ctxt); - if ( !mfn_valid(sh_ctxt->mfn2) ) - return ((mfn_x(sh_ctxt->mfn2) =3D=3D BAD_GVA_TO_GFN) ? + sh_ctxt->mfn[1] =3D emulate_gva_to_mfn(v, vaddr + bytes, = sh_ctxt); + if ( !mfn_valid(sh_ctxt->mfn[1]) ) + return ((mfn_x(sh_ctxt->mfn[1]) =3D=3D BAD_GVA_TO_GFN) ? MAPPING_EXCEPTION : - (mfn_x(sh_ctxt->mfn2) =3D=3D READONLY_GFN) ? + (mfn_x(sh_ctxt->mfn[1]) =3D=3D READONLY_GFN) ? MAPPING_SILENT_FAIL : MAPPING_UNHANDLEABLE); =20 /* Cross-page writes mean probably not a pagetable. */ - sh_remove_shadows(d, sh_ctxt->mfn2, 0, 0 /* Slow, can fail. */ ); + sh_remove_shadows(d, sh_ctxt->mfn[1], 0, 0 /* Slow, can fail. */ = ); =20 - mfns[0] =3D sh_ctxt->mfn1; - mfns[1] =3D sh_ctxt->mfn2; - map =3D vmap(mfns, 2); + map =3D vmap(sh_ctxt->mfn, 2); if ( !map ) return MAPPING_UNHANDLEABLE; map +=3D (vaddr & ~PAGE_MASK); @@ -1831,7 +1828,7 @@ void sh_emulate_unmap_dest(struct vcpu * * - it was aligned to the PTE boundaries; and * - _PAGE_PRESENT was clear before and after the write. */ - shflags =3D mfn_to_page(sh_ctxt->mfn1)->shadow_flags; + shflags =3D mfn_to_page(sh_ctxt->mfn[0])->shadow_flags; #if (SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VERIFY) if ( sh_ctxt->low_bit_was_clear && !(*(u8 *)addr & _PAGE_PRESENT) @@ -1852,12 +1849,12 @@ void sh_emulate_unmap_dest(struct vcpu * && bytes <=3D 4)) ) { /* Writes with this alignment constraint can't possibly cross = pages. */ - ASSERT(!mfn_valid(sh_ctxt->mfn2)); + ASSERT(!mfn_valid(sh_ctxt->mfn[1])); } else #endif /* SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VERIFY */ { - if ( unlikely(mfn_valid(sh_ctxt->mfn2)) ) + if ( unlikely(mfn_valid(sh_ctxt->mfn[1])) ) { /* Validate as two writes, one to each page. */ b1 =3D PAGE_SIZE - (((unsigned long)addr) & ~PAGE_MASK); @@ -1865,16 +1862,16 @@ void sh_emulate_unmap_dest(struct vcpu * ASSERT(b2 < bytes); } if ( likely(b1 > 0) ) - sh_validate_guest_pt_write(v, sh_ctxt->mfn1, addr, b1); + sh_validate_guest_pt_write(v, sh_ctxt->mfn[0], addr, b1); if ( unlikely(b2 > 0) ) - sh_validate_guest_pt_write(v, sh_ctxt->mfn2, addr + b1, b2); + sh_validate_guest_pt_write(v, sh_ctxt->mfn[1], addr + b1, = b2); } =20 - paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn1)); + paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn[0])); =20 - if ( unlikely(mfn_valid(sh_ctxt->mfn2)) ) + if ( unlikely(mfn_valid(sh_ctxt->mfn[1])) ) { - paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn2)); + paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn[1])); vunmap((void *)((unsigned long)addr & PAGE_MASK)); } else --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -4615,13 +4615,13 @@ static void emulate_unmap_dest(struct vc u32 bytes, struct sh_emulate_ctxt *sh_ctxt) { - ASSERT(mfn_valid(sh_ctxt->mfn1)); + ASSERT(mfn_valid(sh_ctxt->mfn[0])); =20 /* If we are writing lots of PTE-aligned zeros, might want to = unshadow */ if ( likely(bytes >=3D 4) && (*(u32 *)addr =3D=3D 0) ) { if ( ((unsigned long) addr & ((sizeof (guest_intpte_t)) - 1)) = =3D=3D 0 ) - check_for_early_unshadow(v, sh_ctxt->mfn1); + check_for_early_unshadow(v, sh_ctxt->mfn[0]); /* Don't reset the heuristic if we're writing zeros at non-aligned= * addresses, otherwise it doesn't catch REP MOVSD on PAE guests = */ } --- a/xen/arch/x86/mm/shadow/private.h +++ b/xen/arch/x86/mm/shadow/private.h @@ -727,7 +727,7 @@ struct sh_emulate_ctxt { struct segment_register seg_reg[6]; =20 /* MFNs being written to in write/cmpxchg callbacks */ - mfn_t mfn1, mfn2; + mfn_t mfn[2]; =20 #if (SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VERIFY) /* Special case for avoiding having to verify writes: remember --=__Part5364E9C4.0__= Content-Type: text/plain; name="x86-sh-mfn-array.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="x86-sh-mfn-array.patch" x86/shadow: avoid extra local array variable=0A=0Amfns[2] was there just = because struct sh_emulate_ctxt's two MFN values=0Acan't be used to hand to = vmap(). Making the structure fields an array=0Aavoids the extra copying.=0A= =0ASigned-off-by: Jan Beulich =0A=0A--- a/xen/arch/x86/m= m/shadow/common.c=0A+++ b/xen/arch/x86/mm/shadow/common.c=0A@@ -1746,11 = +1746,11 @@ void *sh_emulate_map_dest(struct vcpu *v=0A struct domain = *d =3D v->domain;=0A void *map;=0A =0A- sh_ctxt->mfn1 =3D emulate_gv= a_to_mfn(v, vaddr, sh_ctxt);=0A- if ( !mfn_valid(sh_ctxt->mfn1) )=0A- = return ((mfn_x(sh_ctxt->mfn1) =3D=3D BAD_GVA_TO_GFN) ?=0A+ = sh_ctxt->mfn[0] =3D emulate_gva_to_mfn(v, vaddr, sh_ctxt);=0A+ if ( = !mfn_valid(sh_ctxt->mfn[0]) )=0A+ return ((mfn_x(sh_ctxt->mfn[0]) = =3D=3D BAD_GVA_TO_GFN) ?=0A MAPPING_EXCEPTION :=0A- = (mfn_x(sh_ctxt->mfn1) =3D=3D READONLY_GFN) ?=0A+ = (mfn_x(sh_ctxt->mfn[0]) =3D=3D READONLY_GFN) ?=0A = MAPPING_SILENT_FAIL : MAPPING_UNHANDLEABLE);=0A =0A #ifndef NDEBUG=0A@@ = -1767,39 +1767,36 @@ void *sh_emulate_map_dest(struct vcpu *v=0A =0A = /* Unaligned writes mean probably this isn't a pagetable. */=0A if ( = vaddr & (bytes - 1) )=0A- sh_remove_shadows(d, sh_ctxt->mfn1, 0, 0 = /* Slow, can fail. */ );=0A+ sh_remove_shadows(d, sh_ctxt->mfn[0], = 0, 0 /* Slow, can fail. */ );=0A =0A if ( likely(((vaddr + bytes - 1) = & PAGE_MASK) =3D=3D (vaddr & PAGE_MASK)) )=0A {=0A /* Whole = write fits on a single page. */=0A- sh_ctxt->mfn2 =3D _mfn(INVALID_M= FN);=0A- map =3D map_domain_page(sh_ctxt->mfn1) + (vaddr & = ~PAGE_MASK);=0A+ sh_ctxt->mfn[1] =3D _mfn(INVALID_MFN);=0A+ = map =3D map_domain_page(sh_ctxt->mfn[0]) + (vaddr & ~PAGE_MASK);=0A = }=0A- else=0A+ else if ( !is_hvm_domain(d) )=0A {=0A- = mfn_t mfns[2];=0A-=0A /*=0A * Cross-page emulated writes = are only supported for HVM guests;=0A * PV guests ought to know = better.=0A */=0A- if ( !is_hvm_domain(d) )=0A- = return MAPPING_UNHANDLEABLE;=0A-=0A+ return MAPPING_UNHANDLEABLE;=0A= + }=0A+ else=0A+ {=0A /* This write crosses a page = boundary. Translate the second page. */=0A- sh_ctxt->mfn2 =3D = emulate_gva_to_mfn(v, vaddr + bytes, sh_ctxt);=0A- if ( !mfn_valid(s= h_ctxt->mfn2) )=0A- return ((mfn_x(sh_ctxt->mfn2) =3D=3D = BAD_GVA_TO_GFN) ?=0A+ sh_ctxt->mfn[1] =3D emulate_gva_to_mfn(v, = vaddr + bytes, sh_ctxt);=0A+ if ( !mfn_valid(sh_ctxt->mfn[1]) )=0A+ = return ((mfn_x(sh_ctxt->mfn[1]) =3D=3D BAD_GVA_TO_GFN) ?=0A = MAPPING_EXCEPTION :=0A- (mfn_x(sh_ctxt->= mfn2) =3D=3D READONLY_GFN) ?=0A+ (mfn_x(sh_ctxt->mfn[1])= =3D=3D READONLY_GFN) ?=0A MAPPING_SILENT_FAIL : = MAPPING_UNHANDLEABLE);=0A =0A /* Cross-page writes mean probably = not a pagetable. */=0A- sh_remove_shadows(d, sh_ctxt->mfn2, 0, 0 /* = Slow, can fail. */ );=0A+ sh_remove_shadows(d, sh_ctxt->mfn[1], 0, = 0 /* Slow, can fail. */ );=0A =0A- mfns[0] =3D sh_ctxt->mfn1;=0A- = mfns[1] =3D sh_ctxt->mfn2;=0A- map =3D vmap(mfns, 2);=0A+ = map =3D vmap(sh_ctxt->mfn, 2);=0A if ( !map )=0A = return MAPPING_UNHANDLEABLE;=0A map +=3D (vaddr & ~PAGE_MASK);=0A@@= -1831,7 +1828,7 @@ void sh_emulate_unmap_dest(struct vcpu *=0A * - = it was aligned to the PTE boundaries; and=0A * - _PAGE_PRESENT was = clear before and after the write.=0A */=0A- shflags =3D mfn_to_page= (sh_ctxt->mfn1)->shadow_flags;=0A+ shflags =3D mfn_to_page(sh_ctxt->mfn[= 0])->shadow_flags;=0A #if (SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VERIFY)=0A = if ( sh_ctxt->low_bit_was_clear=0A && !(*(u8 *)addr & _PAGE_PRESE= NT)=0A@@ -1852,12 +1849,12 @@ void sh_emulate_unmap_dest(struct vcpu *=0A = && bytes <=3D 4)) )=0A {=0A /* Writes with this = alignment constraint can't possibly cross pages. */=0A- ASSERT(!mfn_= valid(sh_ctxt->mfn2));=0A+ ASSERT(!mfn_valid(sh_ctxt->mfn[1]));=0A = }=0A else=0A #endif /* SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VERIFY = */=0A {=0A- if ( unlikely(mfn_valid(sh_ctxt->mfn2)) )=0A+ = if ( unlikely(mfn_valid(sh_ctxt->mfn[1])) )=0A {=0A = /* Validate as two writes, one to each page. */=0A b1 =3D = PAGE_SIZE - (((unsigned long)addr) & ~PAGE_MASK);=0A@@ -1865,16 +1862,16 = @@ void sh_emulate_unmap_dest(struct vcpu *=0A ASSERT(b2 < = bytes);=0A }=0A if ( likely(b1 > 0) )=0A- = sh_validate_guest_pt_write(v, sh_ctxt->mfn1, addr, b1);=0A+ = sh_validate_guest_pt_write(v, sh_ctxt->mfn[0], addr, b1);=0A if ( = unlikely(b2 > 0) )=0A- sh_validate_guest_pt_write(v, sh_ctxt->mf= n2, addr + b1, b2);=0A+ sh_validate_guest_pt_write(v, sh_ctxt->m= fn[1], addr + b1, b2);=0A }=0A =0A- paging_mark_dirty(v->domain, = mfn_x(sh_ctxt->mfn1));=0A+ paging_mark_dirty(v->domain, mfn_x(sh_ctxt->m= fn[0]));=0A =0A- if ( unlikely(mfn_valid(sh_ctxt->mfn2)) )=0A+ if ( = unlikely(mfn_valid(sh_ctxt->mfn[1])) )=0A {=0A- paging_mark_dirt= y(v->domain, mfn_x(sh_ctxt->mfn2));=0A+ paging_mark_dirty(v->domain,= mfn_x(sh_ctxt->mfn[1]));=0A vunmap((void *)((unsigned long)addr & = PAGE_MASK));=0A }=0A else=0A--- a/xen/arch/x86/mm/shadow/multi.c=0A= +++ b/xen/arch/x86/mm/shadow/multi.c=0A@@ -4615,13 +4615,13 @@ static void = emulate_unmap_dest(struct vc=0A u32 = bytes,=0A struct sh_emulate_ctxt = *sh_ctxt)=0A {=0A- ASSERT(mfn_valid(sh_ctxt->mfn1));=0A+ ASSERT(mfn_v= alid(sh_ctxt->mfn[0]));=0A =0A /* If we are writing lots of PTE-aligned= zeros, might want to unshadow */=0A if ( likely(bytes >=3D 4) && = (*(u32 *)addr =3D=3D 0) )=0A {=0A if ( ((unsigned long) addr & = ((sizeof (guest_intpte_t)) - 1)) =3D=3D 0 )=0A- check_for_early_= unshadow(v, sh_ctxt->mfn1);=0A+ check_for_early_unshadow(v, = sh_ctxt->mfn[0]);=0A /* Don't reset the heuristic if we're writing = zeros at non-aligned=0A * addresses, otherwise it doesn't catch = REP MOVSD on PAE guests */=0A }=0A--- a/xen/arch/x86/mm/shadow/private.= h=0A+++ b/xen/arch/x86/mm/shadow/private.h=0A@@ -727,7 +727,7 @@ struct = sh_emulate_ctxt {=0A struct segment_register seg_reg[6];=0A =0A /* = MFNs being written to in write/cmpxchg callbacks */=0A- mfn_t mfn1, = mfn2;=0A+ mfn_t mfn[2];=0A =0A #if (SHADOW_OPTIMIZATIONS & SHOPT_SKIP_VE= RIFY)=0A /* Special case for avoiding having to verify writes: = remember=0A --=__Part5364E9C4.0__= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --=__Part5364E9C4.0__=--