[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [BUGFIX][PATCH 4/4] XEN_DOMCTL_gdbsx_guestmemio: always do the copyback.



On Tue, 07 Jan 2014 11:24:15 -0500
Don Slutz <dslutz@xxxxxxxxxxx> wrote:

> 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 <dslutz@xxxxxxxxxxx> wrote:
> >>>
> >>>> The gdbsx code expects that domctl->u.gdbsx_guest_memio.remain is
> >>>> returned.
> >>>>

..... 

> 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.

No, the partial success is relevant in other cases, like EAGAIN,
but not EFAULT. If we make it pre-emptible in future to return
EAGAIN, we'd need to make sure remain was honored. Again, think of it
this way, if the first copyin failed, remain would have not been 
initialized.

So, because now the only cause of unfinished copy from dbg_rw_mem is 
EFAULT, we should leave above xen code alone, and just change gdbsx.

thanks
mukesh

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.