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 09:50:14AM -0800, Anthony Liguori wrote:
> On Mon, Jan 8, 2018 at 8:05 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>>> On 04.01.18 at 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> >> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> >>
> >> Note that the unmask and the virq operations are handled by the shim
> >> itself, and that FIFO event channels are not exposed to the guest.
> >>
> >> Signed-off-by: Anthony Liguori <aliguori@xxxxxxxxxx>
> >> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
> >
> > In RFC state this certainly doesn't matter yet, but generally I'd
> > expect From: to match the first S-o-b.
> >
> >> @@ -155,11 +156,31 @@ static void set_vcpu_id(void)
> >>  static void xen_evtchn_upcall(struct cpu_user_regs *regs)
> >>  {
> >>      struct vcpu_info *vcpu_info = this_cpu(vcpu_info);
> >> +    unsigned long pending;
> >>
> >>      vcpu_info->evtchn_upcall_pending = 0;
> >> -    xchg(&vcpu_info->evtchn_pending_sel, 0);
> >> +    pending = xchg(&vcpu_info->evtchn_pending_sel, 0);
> >>
> >> -    pv_console_rx(regs);
> >> +    while ( pending )
> >> +    {
> >> +        unsigned int l1 = ffsl(pending) - 1;
> >
> > find_first_set_bit() would look to be the better match here (and
> > below), not the least because it translates (on capable hardware)
> > to TZCNT instead of BSF.
> >
> >> +        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).
> 
> Note that we discarded this approach in the Vixen series because it
> wasn't working reliably for injecting remote event channel
> notifications.

I have to admit I haven't found issues with this implementation, and
AFAICT it looks correct albeit simplistic.

The lack if issues might be because the shim only supports
HVMOP_set_evtchn_upcall_vector as the event channel injection
mechanism ATM. I guess we will revising this if required once more
event channel injection mechanisms are added.

Thanks, Roger.

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