On Tuesday, April 8th, 2025 at 7:26 AM, Jan Beulich <jbeul...@suse.com> wrote:
>
>
> On 01.04.2025 01:05, dm...@proton.me wrote:
>
> > From: Denis Mukhin dmuk...@ford.com
> >
> > Move domain ID allocation during domain creation to a dedicated
> > function domid_alloc().
> >
> > Allocation algorithm:
> > - If an explicit domain ID is provided, verify its availability and
> > use it if ID is unused;
> > - Otherwise, perform an exhaustive search for the first available ID
> > within the [0..DOMID_FIRST_RESERVED) range, excluding hardware_domid.
> >
> > This minimizes the use of max_init_domid in the code and, thus, is a
> > prerequisite change for enabling console input rotation across domains
> > with console input permission on x86 platforms (which currently is
> > limited to dom0, PV shim and Xen).
>
>
> By removing the updating of max_init_domid you do - afaict - break the
> remaining use site(s) of the variable.
Unfortunately, this patch should go together with the next patch in
this series:
xen/domain: introduce domid_top()
I mentioned dependency in the cover letter.
>
> > @@ -1003,6 +1004,12 @@ static struct domain *__init create_dom0(struct
> > boot_info *bi)
> >
> > image = &bi->mods[idx];
> >
> > + rc = domid_alloc(get_initial_domain_id());
> > + if ( rc < 0 )
> > + panic("Cannot use domain ID %d (rc = %d)\n",
> > + get_initial_domain_id(), rc);
> > + domid = rc;
> > +
> > if ( opt_dom0_pvh )
> > {
> > dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
>
>
> Why does this need to move up, ...
>
> > @@ -1017,7 +1024,6 @@ static struct domain *__init create_dom0(struct
> > boot_info *bi)
> > dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> >
> > /* Create initial domain. Not d0 for pvshim. */
> > - domid = get_initial_domain_id();
>
>
> ... disconnecting the logic from the comment that is relevant there, ...
>
> > d = domain_create(domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
>
>
> ... and not so much here?
So that in case domid_alloc() fails, create_dom0() will reach error state
quickly. I will re-group it as suggested.
>
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -37,6 +37,9 @@ void arch_get_domain_info(const struct domain *d,
> >
> > domid_t get_initial_domain_id(void);
> >
> > +#define DOMID_AUTO (-1)
> > +int domid_alloc(int hint);
>
>
> Imo it would be better to use e.g. DOMID_INVALID as the "give me whatever
> is available" indicator, allowing the function parameter to properly be
> domid_t.
Ack.
>
> But first of all - can we please take a step back and re-evaluate whether
> all of this re-arrangement that you're doing (not just in the patch here)
> is really needed? It seems to me that it must be possible to do whatever
> you ultimately want to do without re-writing quite a few pretty central
> pieces that have been serving us fine for a long time. That is, rather
> than make our code fit your desires, make your plans fit within the code
> base we have.
Thanks for the review and feedback!
My plan is to deliver the NS16550 emulator for x86 because that saves some
time during bring up of an embedded system.
Current xen console driver allows physical input pass-through to the domain.
This is implemented differently between arm and x86. Currently, the logic
of switching input depends on the global variable `max_init_domid`, which is
advanced on arm only (and never decreased, which makes sense for embedded
designs).
One of the planned emulator features is allowing guest OS to accept
physical input over the emulated UART. On x86, that includes dom0,
PV shim and, with emulator in place, also includes all domains with
NS16550 emulator enabled.
So I need to solve the problem of switching console input to another domain
on x86.
I see there are two ways to solve it:
(a) iterate through the list of known domains and check "UART emulator
present" property. AFAIU, that will require maintaining "current console
domain" pointer for a domain list walk.
(b) calculate the max known domain ID and then check whether domain ID
corresponds to a domain with "UART emulator present" property by iterating
through the range of domain IDs. This is current approach in the code.
So, this patch series solves problem of switching console input to another
domain
on x86 using (b); as looked as less changes to me comparing to maintaining a
domain pointer.
re: code re-arrangements: while working on the emulator, getting more feedback,
this and other console-related cleanup series appeared and I think the code
which
served fine for a long time, got a bit cleaner, easier to follow and, I hope,
a bit easier to maintain.
>
> Jan
Thanks,
Denis
