Re: [Xen-devel] [PATCH RFC v1 55/74] xen/pvshim: forward evtchn ops between L0 Xen and L2 DomU

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jan 09, 2018 at 01:00:10AM -0700, Jan Beulich wrote:
> >>> On 08.01.18 at 17:22, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Jan 08, 2018 at 09:05:40AM -0700, Jan Beulich wrote:
> >> >>> On 04.01.18 at 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> >> > +        unsigned long evtchn = 
> >> > xchg(&XEN_shared_info->evtchn_pending[l1], 0);
> >> > +
> >> > +        __clear_bit(l1, &pending);
> >> > +        evtchn &= ~XEN_shared_info->evtchn_mask[l1];
> >> > +        while ( evtchn )
> >> > +        {
> >> > +            unsigned int port = ffsl(evtchn) - 1;
> >> > +
> >> > +            __clear_bit(port, &evtchn);
> >> > +            port += l1 * BITS_PER_LONG;
> >> 
> >> What about a 32-bit client? If that's not intended to be supported,
> >> building of such a guest should be prevented (in dom0_build.c).
> > 
> > 32bit client? You mean building a shim that runs in 32bit mode? If so
> > I haven't really through of it, but in any case BITS_PER_LOG would be
> > OK also in that case?
> 
> No, by "client" I mean the (sole) guest of the shim, in the 32-bit
> case of which you'd need to use BITS_PER_EVTCHN_WORD() here.
> But since 32-bit PV guests are not a problem wrt SP3, I can see
> why we wouldn't want/need to support that case. Yet if so, I'd
> prefer if we did that uniformly, by e.g. also avoiding the compat
> complications in the new grant table wrapper.

Hm, I'm afraid I'm not following. Xen is 64bits, and this is the
shared_info page of the shim (Xen), so the size it's BITS_PER_LONG.

32bit PV guests have been tested and seem to work fine. Whether
someone would want to convert them or not I don't know, but it's
almost no extra effort to provide a shim that works for both
bitness.

> >> > +    case EVTCHNOP_unmask: {
> >> > +        struct evtchn_unmask unmask;
> >> > +
> >> > +        if ( copy_from_guest(&unmask, arg, 1) != 0 )
> >> > +            return -EFAULT;
> >> > +
> >> > +        /* Unmask is handled in L1 */
> >> > +        rc = evtchn_unmask(unmask.port);
> >> > +
> >> > +        break;
> >> > +    }
> >> 
> >> Is this really sufficient, without handing anything through to L0?
> >> Perhaps it's fine as long as there's no pass-through support here.
> > 
> > For the unmask operation? I think so, if there was a pending event the
> > shim will already take care of injecting it to the guest.
> 
> Well, as the Linux code (evtchn_2l_unmask()) tells us certain
> unmasks have to go through the hypervisor. I would assume
> that in the case of the shim this means that L2 requests need
> to also be handed through to L0 whenever they're not being
> handled entirely locally to L1.

I'm not sure any L2 unmask needs to go through L0. If we perform the
unmask in L1 and there's an event pending L1 will already inject an
interrupt into L2, and AFAIK that's the point of using EVTCHNOP_unmask
(get an interrupt after unmask if an event is pending).

> >> > @@ -1030,6 +1055,11 @@ long do_event_channel_op(int cmd, 
> >> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >> >  {
> >> >      long rc;
> >> >  
> >> > +#ifdef CONFIG_X86
> >> > +    if ( pv_shim )
> >> > +        return pv_shim_event_channel_op(cmd, arg);
> >> > +#endif
> >> 
> >> Patch it right into the hypercall table instead?
> > 
> > That would only work if the shim is a compile time option, but not a
> > run time one, the hypercall table is ro.
> 
> Well, yes and no: See nmi_shootdown_cpus() for a precedent
> of how to do that without removing the r/o attribute. Not having
> the hook sit here would (I assume) allow to avoid compiling the
> entire do_event_channel_op() down the road in the shim-only
> case. The compiler may be able to partially do this (omitting the
> rest of the function), but my experience is that deferring to the
> compiler in this regard often means leaving some traces around.

I see, I could use the write_atomic + directmap trick, but I think I
will leave that for later, since doesn't seem crucial to me.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel