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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 26 Jan 2012, Keir Fraser wrote:
> On 26/01/2012 15: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.
> 
> It's nasty that pirq_eoi_gmfn has the side effect. I suggest add a PHYSDEVOP
> to explicitly enable/disable unmask-on-eoi (i.e., the command accepts a
> boolean parameter). Once it is explicitly enabled/disabled in this way,
> pirq_eoi_gmfn no longer has the side effect (regardless of whether it is
> called before or after the explicit setting). So e.g., pv_domain.auto_unmask
> becomes an int where 0/1 means no/yes, and -1 means default (i.e., old
> behavour where it depends on whether PHYSDEVOP_pirq_eoi_gmfn has been
> called).
> 
> This seems to me to move a bad interface in a better direction.

The problem with this approach is that by default we have an hypercall
(PHYSDEVOP_pirq_eoi_gmfn) changing the behaviour of another one
(PHYSDEVOP_eoi). Not only this but we have an hypercall
(PHYSDEVOP_pirq_eoi_gmfn) violating the public interface of shared_info
as documented in public/xen.h.

Introducing a new hypercall with the same name
(PHYSDEVOP_pirq_eoi_gmfn_new) is the first step in admitting that the
old hypercall was a mistake and should not be used.

I don't think we should ever change the semantics of PHYSDEVOP_eoi with
another hypercall. If we want a PHYSDEVOP that eoi and unmask and event
channel let's introduce PHYSDEVOP_eoi_unmask.

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