From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC v2 2/3] xen/shadow: fix shadow_track_dirty_vram to work on hvm guests Date: Thu, 9 Apr 2015 13:45:24 +0100 Message-ID: <55267464.9060507@citrix.com> References: <1427970395-16203-1-git-send-email-roger.pau@citrix.com> <1427970395-16203-3-git-send-email-roger.pau@citrix.com> <551D933D.2030300@citrix.com> <20150409124152.GL17031@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YgC1Z-0001fH-2F for xen-devel@lists.xenproject.org; Thu, 09 Apr 2015 12:57:37 +0000 In-Reply-To: <20150409124152.GL17031@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: xen-devel@lists.xenproject.org, Jan Beulich , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org On 09/04/15 13:41, Tim Deegan wrote: > At 20:06 +0100 on 02 Apr (1428005197), Andrew Cooper wrote: >> On 02/04/15 11:26, Roger Pau Monne wrote: >>> Modify shadow_track_dirty_vram to use a local buffer and then flush to = the >>> guest without the paging_lock held. This is modeled after >>> hap_track_dirty_vram. >>> >>> Signed-off-by: Roger Pau Monn=E9 >>> Cc: Tim Deegan >>> Cc: Jan Beulich >>> Cc: Andrew Cooper >> Reviewed-by: Andrew Cooper , subject to two >> corrections and a suggestion. >> >>> --- >>> xen/arch/x86/mm/shadow/common.c | 18 +++++++++++++++--- >>> 1 file changed, 15 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/c= ommon.c >>> index 2e43d6d..8fff43a 100644 >>> --- a/xen/arch/x86/mm/shadow/common.c >>> +++ b/xen/arch/x86/mm/shadow/common.c >>> @@ -3516,7 +3516,7 @@ static void sh_clean_dirty_bitmap(struct domain *= d) >>> int shadow_track_dirty_vram(struct domain *d, >>> unsigned long begin_pfn, >>> unsigned long nr, >>> - XEN_GUEST_HANDLE_64(uint8) dirty_bitmap) >>> + XEN_GUEST_HANDLE_64(uint8) guest_dirty_bit= map) >>> { >>> int rc; >>> unsigned long end_pfn =3D begin_pfn + nr; >>> @@ -3526,6 +3526,7 @@ int shadow_track_dirty_vram(struct domain *d, >>> p2m_type_t t; >>> struct sh_dirty_vram *dirty_vram; >>> struct p2m_domain *p2m =3D p2m_get_hostp2m(d); >>> + uint8_t *dirty_bitmap =3D NULL; >>> = >>> if ( end_pfn < begin_pfn || end_pfn > p2m->max_mapped_pfn + 1 ) >>> return -EINVAL; >>> @@ -3554,6 +3555,12 @@ int shadow_track_dirty_vram(struct domain *d, >>> goto out; >>> } >>> = >>> + dirty_bitmap =3D xzalloc_bytes(dirty_size); >>> + if ( dirty_bitmap =3D=3D NULL ) >>> + { >>> + rc =3D -ENOMEM; >>> + goto out; >>> + } >>> /* This should happen seldomly (Video mode change), >>> * no need to be careful. */ >>> if ( !dirty_vram ) >>> @@ -3587,7 +3594,7 @@ int shadow_track_dirty_vram(struct domain *d, >>> { >>> /* still completely clean, just copy our empty bitmap */ >>> rc =3D -EFAULT; >>> - if ( copy_to_guest(dirty_bitmap, dirty_vram->dirty_bitmap, dir= ty_size) =3D=3D 0 ) >>> + if ( memcpy(dirty_bitmap, dirty_vram->dirty_bitmap, dirty_size= ) !=3D NULL ) >> memcpy() returns an int, not a pointer. > No, memcpy() returns a pointer, but it's always =3D=3D its first argument > so there's no need to check it at all. :) D'oh - I had mixed up memcmp() and memcpy(). You are completely correct. ~Andrew