Re: [Xen-devel] [PATCH 1/2] xen: Introduce PHYSDEVOP_pirq_eoi_gmfn_new

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 26 Jan 2012, Jan Beulich wrote:
> >>> On 26.01.12 at 16:49, Stefano Stabellini 
> >>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > PHYSDEVOP_pirq_eoi_gmfn changes the semantics of PHYSDEVOP_eoi.
> > Introduce PHYSDEVOP_pirq_eoi_gmfn_new, that is like
> > PHYSDEVOP_pirq_eoi_gmfn but it doesn't modify the behaviour of another
> > hypercall.
> 
> I keep forgetting why you think the auto-unmasking does any harm.
> It was done that way to avoid an extra hypercall.

We let Linux mask/unmask the event channel whenever it would make/unmask
the irq, so an automatic unmask is equivalent to unmasking an irq
without Linux knowing or wanting to do so.


> > @@ -531,6 +532,8 @@ long do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> >          }
> >  
> >          current->domain->arch.pirq_eoi_map = mfn_to_virt(mfn);
> > +           if ( cmd == PHYSDEVOP_pirq_eoi_gmfn )
> > +                   current->domain->arch.auto_unmask = 1;
> 
> Indentation.,
>
> >          ret = 0;
> >          break;
> >      }
> > --- a/xen/arch/x86/physdev.c
> > +++ b/xen/arch/x86/physdev.c
> > @@ -271,7 +271,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> >              break;
> >          }
> >          if ( !is_hvm_domain(v->domain) &&
> > -             v->domain->arch.pv_domain.pirq_eoi_map )
> > +             v->domain->arch.pv_domain.pirq_eoi_map &&
> > +             v->domain->arch.pv_domain.auto_unmask )
> 
> Could you not avoid the checking of v->domain->arch.pv_domain.pirq_eoi_map
> by making sure v->domain->arch.pv_domain.auto_unmask gets cleared
> when the map gets destroyed (not sure if that is permitted at all).

I think I can, I'll change it that way.


> >              evtchn_unmask(pirq->evtchn);
> >          if ( !is_hvm_domain(v->domain) ||
> >               domain_pirq_to_irq(v->domain, eoi.irq) > 0 )
> > --- a/xen/include/asm-x86/domain.h
> > +++ b/xen/include/asm-x86/domain.h
> > @@ -231,6 +231,9 @@ struct pv_domain
> >      /* Shared page for notifying that explicit PIRQ EOI is required. */
> >      unsigned long *pirq_eoi_map;
> >      unsigned long pirq_eoi_map_mfn;
> > +    /* set auto_unmask to 1 if you want PHYSDEVOP_eoi to automatically
> > +     * unmask the event channel */
> > +    unsigned int auto_unmask;
> 
> bool_t?

good idea


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel