From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH v5 2/4] xen/shadow: fix shadow_track_dirty_vram to work on hvm guests Date: Mon, 11 May 2015 14:51:57 +0100 Message-ID: <20150511135157.GB83668@deinos.phlegethon.org> References: <1431095651-25935-1-git-send-email-roger.pau@citrix.com> <1431095651-25935-3-git-send-email-roger.pau@citrix.com> <554CF24D02000078000786A4@mail.emea.novell.com> <554CD79A.4060707@citrix.com> <5550B2A8.30906@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Yro7m-00018w-7w for xen-devel@lists.xenproject.org; Mon, 11 May 2015 13:52:02 +0000 Content-Disposition: inline In-Reply-To: <5550B2A8.30906@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: Andrew Cooper , Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org At 15:46 +0200 on 11 May (1431359176), Roger Pau Monn=E9 wrote: > El 08/05/15 a les 17.34, Andrew Cooper ha escrit: > > On 08/05/15 16:28, Jan Beulich wrote: > >>>>> On 08.05.15 at 16:34, wrote: > >>> @@ -3668,21 +3671,19 @@ int shadow_track_dirty_vram(struct domain *d, > >>> if ( map_sl1p ) > >>> sh_unmap_domain_page(map_sl1p); > >>> = > >>> - rc =3D -EFAULT; > >>> - if ( copy_to_guest(dirty_bitmap, dirty_vram->dirty_bitmap, d= irty_size) =3D=3D 0 ) { > >>> - memset(dirty_vram->dirty_bitmap, 0, dirty_size); > >>> - if (dirty_vram->last_dirty + SECONDS(2) < NOW()) > >>> + memcpy(dirty_bitmap, dirty_vram->dirty_bitmap, dirty_size); > >>> + memset(dirty_vram->dirty_bitmap, 0, dirty_size); > >> This is certainly a behavioral change; I'm only uncertain whether it's > >> acceptable. Previously the memset() was done only when the copying > >> to guest memory succeeded, while now it happens unconditionally. > > = > > On the one hand, if the toolstack logdirty buffer suffers an EFAULT, > > most bets are probably off. > > = > > However, it would better if Xen didn't then clobber the dirty bitmap, in > > case the toolstack's kernel is doing some particularly funky memory > > management which would succeed on a retry. > = > A possible workaround to this would be to do acquire the paging_lock > again if copy_to_guest fails and set the dirty_bitmap to 0xff, although > it's not very elegant. Equivalently, you could set a bit that makes the _next_ pass over the bitmap return all 0xff. IIRC, this is what happens when the vram area changes. I think it's a reasobnable thing to do if the toolstack messed up its hypercall buffers. Tim.