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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.01.18 at 17:45, <roger.pau@xxxxxxxxxx> wrote:
> 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.

Oh, in that case I'm sorry for being the one being confused here.
I was certainly under the impression that this is the page shared
with the client domain.

>> >> > +    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).

Possible, but to be honest I'm not sure: If getting an event was
all that's wanted in Linux, I don't think it would need to be done
by issuing a hypercall. Otoh maybe that code just isn't optimally
written. IOW - as long as things work, I'm fine here.

Jan


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