From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [PATCH 2/18 V2]: PVH xen: add
 XENMEM_add_to_physmap_range
Date: Wed, 20 Mar 2013 18:01:39 -0700
Message-ID: <20130320180139.7d65354b@mantra.us.oracle.com>
References: <20130315172018.395bbbe3@mantra.us.oracle.com>
	<51470ACC02000078000C64CB@nat28.tlf.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>
In-Reply-To: <51470ACC02000078000C64CB@nat28.tlf.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>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On Mon, 18 Mar 2013 11:38:35 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 16.03.13 at 01:20, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > +        struct xen_add_to_physmap_range xatpr;
> > +        struct domain *d;
> > +
> > +        if ( copy_from_guest(&xatpr, arg, 1) )
> > +            return -EFAULT;
> > +
> > +        rc = rcu_lock_target_domain_by_id(xatpr.domid, &d);
> > +        if ( rc != 0 )
> > +            return rc;
> > +
> > +        rc = xenmem_add_to_physmap_range(d, &xatpr);
> > +
> > +        rcu_unlock_domain(d);
> > +
> > +        if ( rc && copy_to_guest(arg, &xatpr, 1) )
> 
> For one, shouldn't this be "!rc"?
> 
> And then you update ->size, but that one is specified to be only
> and IN field. And considering that "errs" is the only OUT one, yet
> that isn't even formally correct (because the field itself is an IN,
> its what it points to where the output goes), I don't see why you
> would need to copy back any part of the structure.

Ah, I see the struct got updated. Konrad, do you have updated version
of the struct with following added to the end:

struct xen_add_to_physmap_range {
....
    /* OUT */
        
    /* Per index error code. */
    XEN_GUEST_HANDLE(int) errs;

in the latest linux tree?

thanks,
Mukesh