Re: VMX status report. Xen:26323 & Dom0:3.7.1

[David Vrabel <david.vrabel@citrix.com>]

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