From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: VMX status report. Xen:26323 & Dom0:3.7.1
Date: Mon, 14 Jan 2013 13:59:30 +0000
Message-ID: <50F40F42.5020807@citrix.com>
References: <1B4B44D9196EFF41AE41FDA404FC0A1024486E@SHSMSX101.ccr.corp.intel.com>	<50EE908602000078000B44CE@nat28.tlf.novell.com>	<D01C8DC1-D135-41E0-967A-FD2887E2604E@gridcentric.ca>	<50EFDC8802000078000B4AC2@nat28.tlf.novell.com>
	<750FD2DB-E7A5-4038-9274-2CBAF2B4027C@gridcentric.ca>
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: <750FD2DB-E7A5-4038-9274-2CBAF2B4027C@gridcentric.ca>
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: Andres Lagar-Cavilla <andreslc@gridcentric.ca>
Cc: Yongjie Ren <yongjie.ren@intel.com>, Ian Campbell <ian.campbell@citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, Mats Petersson <mats.petersson@citrix.com>, xen-devel <xen-devel@lists.xen.org>, Chao Zhou <chao.zhou@intel.com>, Jan Beulich <jbeulich@suse.com>, Yan Dai <yan.dai@intel.com>, YongweiX Xu <yongweix.xu@intel.com>, SongtaoX Liu <songtaox.liu@intel.com>, Andres Lagar-Cavilla <andres@lagarcavilla.org>
List-Id: xen-devel@lists.xenproject.org

On 14/01/13 04:29, Andres Lagar-Cavilla wrote:
> 
> Below you'll find pasted an RFC patch to fix this. I've expanded the
> cc line to add Mats Peterson, who is also looking into some improvements
> to privcmd (and IanC for general feedback).
> 
> The RFC patch cuts down code overall and cleans up logic too. I did
> change the behavior wrt classic implementations when it comes to
> handling errors & EFAULT. Instead of doing all the mapping work and then
> copying back to user, I copy back each individual mapping error as soon
> as it arises. And short-circuit and quit the whole operation as soon as
> the first EFAULT arises.

Which is broken.  Please just look at my v3 patch and implement that method.

> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index 3421f0d..9433396 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c 
[...]
> @@ -287,40 +285,35 @@ static int mmap_batch_fn(void *data, void *state)
[...]
> +           efault = __put_user(mfn_err, st->user_mfn++);
> +       } else { /* st->version == 2 */
> +           efault = __put_user(ret, st->user_err++);

You can't use __put_user() or any other function accessing user memory
while holding mmap_sem or you will occasionally deadlock in the page
fault handler (depending on whether the user page is currently present
or not).

David