From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [BUGFIX][PATCH 4/4] XEN_DOMCTL_gdbsx_guestmemio: always do the copyback. Date: Tue, 07 Jan 2014 11:24:15 -0500 Message-ID: <52CC2A2F.7010700@terremark.com> References: <1388857936-664-1-git-send-email-dslutz@verizon.com> <1388857936-664-5-git-send-email-dslutz@verizon.com> <20140106175349.6cbd190b@mantra.us.oracle.com> <1389088824.31766.105.camel@kazak.uk.xensource.com> <1389088937.31766.107.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1389088937.31766.107.camel@kazak.uk.xensource.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: Ian Campbell , Mukesh Rathor Cc: Keir Fraser , Stefano Stabellini , George Dunlap , Ian Jackson , Don Slutz , xen-devel@lists.xen.org, Jan Beulich List-Id: xen-devel@lists.xenproject.org On 01/07/14 05:02, Ian Campbell wrote: > On Tue, 2014-01-07 at 10:00 +0000, Ian Campbell wrote: >> On Mon, 2014-01-06 at 17:53 -0800, Mukesh Rathor wrote: >>> On Sat, 4 Jan 2014 12:52:16 -0500 >>> Don Slutz wrote: >>> >>>> The gdbsx code expects that domctl->u.gdbsx_guest_memio.remain is >>>> returned. >>>> >>>> Without this gdb does not report an error. >>>> >>>> With this patch and using a 1G hvm domU: >>>> >>>> (gdb) x/1xh 0x6ae9168b >>>> 0x6ae9168b: Cannot access memory at address 0x6ae9168b >>>> >>>> Signed-off-by: Don Slutz >>>> --- >>>> xen/arch/x86/domctl.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c >>>> index ef6c140..4aa751f 100644 >>>> --- a/xen/arch/x86/domctl.c >>>> +++ b/xen/arch/x86/domctl.c >>>> @@ -997,8 +997,7 @@ long arch_do_domctl( >>>> domctl->u.gdbsx_guest_memio.len; >>>> >>>> ret = gdbsx_guest_mem_io(domctl->domain, >>>> &domctl->u.gdbsx_guest_memio); >>>> - if ( !ret ) >>>> - copyback = 1; >>>> + copyback = 1; >>>> } >>>> break; >>>> >>> Ooopsy... my thought was that an application should not even look at >>> remain if the hcall/syscall failed, but forgot when writing the >>> gdbsx itself :). Think of it this way, if the call didn't even make it to >>> xen, and some reason the ioctl returned non-zero rc, then remain would >>> still be zero. > Thinking about this for a second longer -- how does this interface deal > with partial success? > > It seems natural to me for it to return an error and also update > ->remain, but perhaps you have a different scheme in mind? > > I had assumed that this patch (which is not need to "fix" the bugs I found), was to be dropped in v2. However I will agree that currently there is no way to know about partial success. The untested change: diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index ef6c140..0add07e 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -43,7 +43,7 @@ static int gdbsx_guest_mem_io( iop->remain = dbg_rw_mem( (dbgva_t)iop->gva, (dbgbyte_t *)l_uva, iop->len, domid, iop->gwr, iop->pgd3val); - return (iop->remain ? -EFAULT : 0); + return 0; } long arch_do_domctl( Would appear to allow partial success to be reported and also meet with remain not to be looked at with an error. -Don Slutz