Re: [PATCH v5 3/4] xen/console: remove max_init_domid dependency

[dmkhn@xxxxxxxxx]

On Wed, Jun 04, 2025 at 02:55:40PM +0200, Jan Beulich wrote:
> On 31.05.2025 01:19, dmkhn@xxxxxxxxx wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -2461,6 +2461,39 @@ void domid_free(domid_t domid)
> >      spin_unlock(&domid_lock);
> >  }
> >
> > +/*
> > + * Find the ID of the next possible console owner domain.
> > + *
> > + * @return Domain ID: DOMID_XEN or non-system domain IDs within
> > + * the range of [0..DOMID_FIRST_RESERVED-1].
> > + */
> > +domid_t domid_find_with_input_allowed(domid_t hint)
> > +{
> > +    domid_t domid = DOMID_XEN;
> > +
> > +    if ( hint < DOMID_FIRST_RESERVED )
> > +    {
> > +        struct domain *d;
> > +
> > +        rcu_read_lock(&domlist_read_lock);
> > +
> > +        for ( d = domid_to_domain(hint);
> 
> If the domain with ID "hint" went away, what is being switched to changes
> compared to behavior prior to this patch, if I'm not mistaken. While this
> _may_ be acceptable, not saying so in the description is imo a no-go.

Will correct, thanks.

> 
> > +              d && get_domain(d) && d->domain_id < DOMID_FIRST_RESERVED;
> 
> What's the DOMID_FIRST_RESERVED check for? And where's the put_domain()
> for the get_domain() here?
> 
> > +              d = rcu_dereference(d->next_in_list) )
> > +        {
> > +            if ( d->console.input_allowed )
> > +            {
> > +                domid = d->domain_id;
> > +                break;
> > +            }
> > +        }
> > +
> > +        rcu_read_unlock(&domlist_read_lock);
> > +    }
> > +
> > +    return domid;
> > +}
> 
> My concern remains: With many domains, the loop here may take quite a few
> iterations. That's even more concerning because it regresses right away in
> environments where along with boot-time created domains (eligible for
> console focus) later further domains are created (non of which are eligible
> for console focus). That is, the step from last boot-time created back to
> DOMID_XEN may now take excessively long.

Julien in the other reply suggested to have a list of domains with console
input permission only. Will rework.

> 
> Jan