Re: xc_evtchn_status fails with EFAULT on HVM, the same on PV works

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2942 bytes --]

On Fri, Jan 13, 2017 at 06:15:35PM +0000, Andrew Cooper wrote:
> On 13/01/17 18:03, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 05:38:42PM +0000, Andrew Cooper wrote:
> >> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> >>> Hi,
> >>>
> >>> I have a strange problem - xc_evtchn_status fails when running in HVM,
> >>> while exactly the same code (same kernel, same application etc) works
> >>> fine in PV. I've narrowed it down to copy_from_guest call in
> >>> common/event_channel.c, but no idea why it fails there. Xen version is
> >>> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
> >> Which specific copy_from_guest() call?
> >>
> >> Copying data out of a PV guest is different to copying out of a HVM
> >> guest, but copy_from_guest() should cope properly with both.
> >>
> >> However, to progress, it would help to know exactly which piece of data
> >> is being requested.
> > This one:
> > https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104
> >
> >     case EVTCHNOP_status: {
> >         struct evtchn_status status;
> >         if ( copy_from_guest(&status, arg, 1) != 0 )
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >             return -EFAULT;
> >         rc = evtchn_status(&status);
> >         if ( !rc && __copy_to_guest(arg, &status, 1) )
> >             rc = -EFAULT;
> >         break;
> >
> > The evtchn_status structure in application is on stack (local variable),
> > but I think it shouldn't matter, as libxc copy it to a bounce buffer.
> >
> 
> The intent of bounce buffers is certainly to avoid this problem from
> happening.
> 
> Is this a 32bit HVM guest?  Compat argument translation does make the
> logic a little more complicated.

No, its 64bit guest.

> Can you get the result of this piece of debugging in the failure case?

I've got this:
** d4v0 CFG(24, 00007f794bd07004, 1) = 24

> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 638dc5e..ab5d82a 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -1101,8 +1101,13 @@ long do_event_channel_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case EVTCHNOP_status: {
>          struct evtchn_status status;
> -        if ( copy_from_guest(&status, arg, 1) != 0 )
> +        unsigned int res = copy_from_guest(&status, arg, 1);
> +        if ( res != 0 )
> +        {
> +            printk("** %pv CFG(%zu, %p, 1) = %u\n",
> +                   current, sizeof(status), _p(arg.p), res);
>              return -EFAULT;
> +        }
>          rc = evtchn_status(&status);
>          if ( !rc && __copy_to_guest(arg, &status, 1) )
>              rc = -EFAULT;

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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