Re: [for-4.22][PATCH] xen/arm: Fail domain construction if a secondary vCPU cannot be created

["Halder, Ayan Kumar" <ayankuma@xxxxxxx>]

On 08/07/2026 12:20, Orzel, Michal wrote:
> On 08-Jul-26 11:06, Halder, Ayan Kumar wrote:
> > Hi MIchal,
> >
> > Nice catch. Few questions.
> >
> > On 08/07/2026 08:49, Michal Orzel wrote:
> > > construct_domain() creates the secondary vCPUs in a loop, but on a
> > > vcpu_create() failure it only prints a message and breaks out of the
> > > loop returning success. As a result the domain can be constructed
> > > with fewer vCPUs than d->max_vcpus, leaving NULL holes in d->vcpu[]
> > > below max_vcpus.
> > >
> > > When the guest probes the redistributor of a vCPU that was never created,
> > Shouldn't the guest check how many vCPUs were created and probe the ones
> > that were created ?
> See below about DTB.
>
> > > get_vcpu_from_rdist() only checks vcpu_id against d->max_vcpus and then
> > > dereferences the NULL d->vcpu[vcpu_id], resulting in a data abort.
> > >
> > > Return an error instead of breaking out of the loop. Both callers
> > > (construct_domU() and construct_hwdom()) already propagate a negative
> > > return value and fail domain construction, which is the correct
> > > behaviour: a domain that cannot provide the requested number of vCPUs
> > > should not be brought up.
> > I see your reasoning.
> >
> > Alternatively it can be a design choice. Xen does not commit to create
> > the max_vcpus that was requested.
> Everything can be a design choice but this one wouldn't be wise, would it?
> All in all, we (Arm maintainers) aim at following the contract to fail as soon
> as possible if the user request cannot be satisfied.
Can we remove the redundant DTB node so that the contract is maintained 
? IMHO , stopping a domain creation seems severe if the other vCPUs were 
created.
> > If Xen is unable to create any vCPU, it should abort domain creation.
> >
> > If Xen creates lesser number of vCPUs than requested by max_vcpus, it
> > can just print a warning and carry on.
> No. Xen creates domain DTB before creating vCPUs, so Xen advertises something
> that is not true. Here, Xen would create a DTB with e.g. 2 vCPUs while only 1
> was created.
I agree that this is a problem.

- Ayan

> ~Michal
>
> > In that case it should be the guest's responsibility to check the number
> > of CPUs that it has.
> >
> > - Ayan
> >
> > > Fixes: 6b0e8e43348a ("xen/arm: allocate secondaries dom0 vcpus")
> > > Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> > > ---
> > >    xen/arch/arm/domain_build.c | 2 +-
> > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index 550617f152bb..b46574fd32aa 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -1847,7 +1847,7 @@ int __init construct_domain(struct domain *d, struct 
> > > kernel_info *kinfo)
> > >            if ( vcpu_create(d, i) == NULL )
> > >            {
> > >                printk("Failed to allocate d%dv%d\n", d->domain_id, i);
> > > -            break;
> > > +            return -EINVAL;
> > >            }
> > >    
> > >            if ( is_64bit_domain(d) )