From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v5 2/4] xen/shadow: fix
 shadow_track_dirty_vram to work on hvm guests
Date: Fri, 8 May 2015 16:34:50 +0100
Message-ID: <554CD79A.4060707@citrix.com>
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>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Andrew.Cooper3@citrix.com>) id 1YqkIh-0002Vr-KG
	for xen-devel@lists.xenproject.org; Fri, 08 May 2015 15:34:55 +0000
In-Reply-To: <554CF24D02000078000786A4@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, Roger Pau Monne <roger.pau@citrix.com>, Tim Deegan <tim@xen.org>
Cc: xen-devel@lists.xenproject.org
List-Id: xen-devel@lists.xenproject.org

On 08/05/15 16:28, Jan Beulich wrote:
>>>> On 08.05.15 at 16:34, <roger.pau@citrix.com> wrote:
>> @@ -3668,21 +3671,19 @@ int shadow_track_dirty_vram(struct domain *d,
>>          if ( map_sl1p )
>>              sh_unmap_domain_page(map_sl1p);
>>  
>> -        rc = -EFAULT;
>> -        if ( copy_to_guest(dirty_bitmap, dirty_vram->dirty_bitmap, dirty_size) == 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.

~Andrew