Re: [PATCH v3 2/5] xen: make evtchn_alloc_unbound public

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 25 Mar 2022, Julien Grall wrote:
> So to me, the idea of switching to a "fake" domain or bypassing the check is
> more appealing. I have a preference for the "fake" domain here.

As a maintainer, I am not opposed to the "fake"/"contructor" domain
concept.  It all depends on how many instances of this issue we are
going to have.  This is the only one on xen-devel so far. I don't think
it is worth adding a constructor domain for one instance only.  But I
agree with you and Daniel that if we end up with several instances, then
the constructor domain approach is probably the best option overall.


As a contributor, sadly I won't be able to spend a lot of time on this
in the following months. If a significant rework is required, I don't
think I'll be able to do it, at least not for this Xen release (and it
would be nice to have dom0less PV drivers in the coming release.) If
Daniel is willing, I could add his "idle_domain is_priv" patch to this
series.  Not as clean as a proper constructor domain but it would work
and it would be simple. It could be evolved into a nicer constructor
domain later.

This is not my preference but I could do that if Julien and Jan prefer
this approach and if Daniel is happy to share his patch.


> AFAIU, your proposal is to duplicate code. This brings other risks such as
> duplicated bug and more code to maintain.

Got it. I'll make one last attempt at a proposal that doesn't involve
the fake constructor domain. The goal is to avoid code duplication while
providing a safe way forward to make progress with only a small amount
of changes. What if we:

- rename evtchn_alloc_unbound to _evtchn_alloc_unbound (still static)
- add a skip_xsm parameter to _evtchn_alloc_unbound
- introduce a wrapper evtchn_alloc_unbound that always set skip_xsm to
  false (same interface as today's evtchn_alloc_unbound)
- introduce an __init early_evtchn_alloc_unbound public function that
  sets skip_xsm to true

This way:
- we have a single implementation in _evtchn_alloc_unbound (no code
  duplication)
- the only function exposed that skips the XSM check is __init
- evtchn_alloc_unbound continue to have the XSM check same as today


E.g.:
static int _evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, bool skip_xsm)
{
    ...
}

static int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
{
    return _evtchn_alloc_unbound(alloc, false);    
}

int __init early_evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
{
    return _evtchn_alloc_unbound(alloc, true);
}


Would this be good enough for now?