Re: [XEN PATCH v2 2/5] xen: export get_free_port

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 26 Jan 2022, Julien Grall wrote:
> On 26/01/2022 07:30, Jan Beulich wrote:
> > On 26.01.2022 02:03, Stefano Stabellini wrote:
> > > Are you guys OK with something like this?
> > 
> > With proper proof that this isn't going to regress anything else, maybe.
> 
> That's why I sugested to also gate with system_state < SYS_STATE_boot so we
> reduce the potential regression (the use of hypercall should be limited at
> boot).
> 
> FWIW, there was a thread [1] to discuss the same issue as Stefano is facing
> (but in the context of Live-Update).
> 
> > But ...
> > 
> > > --- a/xen/include/xsm/dummy.h
> > > +++ b/xen/include/xsm/dummy.h
> > > @@ -92,7 +92,9 @@ static always_inline int xsm_default_action(
> > >               return 0;
> > >           /* fall through */
> > >       case XSM_PRIV:
> > > -        if ( is_control_domain(src) )
> > > +        if ( is_control_domain(src) ||
> > > +             src->domain_id == DOMID_IDLE ||
> > > +             src->domain_id == DOMID_XEN )
> > >               return 0;
> > 
> > ... my first question would be under what circumstances you might observe
> > DOMID_XEN here and hence why this check is there.

For simplicity I'll answer all the points here.

I added both DOMID_IDLE and DOMID_XEN because I thought it "made sense",
but there is no need for DOMID_XEN. We only need DOMID_IDLE. Also adding
a system_state <= SYS_STATE_boot is fine (but it needs to be <= instead
of <). The patch appended below works without issues.

Alternatively, I am also quite happy with Jan's suggestion of passing an
extra parameter to evtchn_alloc_unbound, it could be:

int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, bool disable_xsm);

So that XSM is enabled by default.

Adding the bool parameter to evtchn_alloc_unbound has the advantage of
having only a very minor impact. But either option is totally fine by
me, just let me know your preference.



diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b024119896..01996bd9d8 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -94,6 +94,8 @@ static always_inline int xsm_default_action(
     case XSM_PRIV:
         if ( is_control_domain(src) )
             return 0;
+        if ( system_state <= SYS_STATE_boot && src->domain_id == DOMID_IDLE )
+            return 0;
         return -EPERM;
     default:
         LINKER_BUG_ON(1);