[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 2/5] xen: make evtchn_alloc_unbound public
On Mon, 28 Mar 2022, Daniel P. Smith wrote: > On 3/25/22 17:05, Stefano Stabellini wrote: > > 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? > > Please see the RFC patch I just posted[1], IMHO I think this is a safer > approach for this specific instance. > > [1] > https://lore.kernel.org/xen-devel/20220328203622.30961-1-dpsmith@xxxxxxxxxxxxxxxxxxxx/T/#t I read it, the patch looks fine. I also tested it together with my series and it solves the problem. With [1], it is just a matter of making evtchn_alloc_unbound as is non-static. If the other maintainers also agree with [1], then I'll just rebase on it and limit my changes to exporting evtchn_alloc_unbound.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |