Re: [PATCH v1 02/27] xen/riscv: Implement construct_domain()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 4/9/26 3:39 PM, Oleksii Kurochko wrote:
> > > > > +    regs->sepc = kinfo->entry;
> > > > > +
> > > > > +    /* Guest boot cpuid = 0 */
> > > > > +    regs->a0 = 0;
> > > > > +    regs->a1 = kinfo->dtb_paddr;
> > > > > +
> > > > > +    for ( unsigned int i = 1; i < d->max_vcpus; i++ )
> > > > > +    {
> > > > > +        if ( vcpu_create(d, i) == NULL )
> > > > > +        {
> > > > > +            printk("Failed to allocate %pd v%d\n", d, i);
> > > > > +            break;
> > > > And no error is indicated to the caller?
> > > No, as generally it is enough to have only one vCPU0 to run domain, so
> > > we have to print that something went wrong with allocation of vCPU1...n
> > > but it is okay to me to continue domain construction.
> > Hmm, now that I look there, sched_setup_dom0_vcpus() ignores errors
> > and doesn't even emit a log message. Question is why neither Arm nor
> > RISC-V use that function, when we have it.
> I haven't seen this function, I will re-use it and in separate patch 
> suggest to re-use it for Arm.
> One thing I think we want to do then is to drop #ifdef x86 around 
> sched_setup_dom0_vcpus() and rename it to sched_setup_dom_vcpus().
> And maybe add dprintk()'s to provide some information about which vCPUs 
> were created and which not.
Arm doesn't re-use sched_setup_dom0_vcpus() because it has extra if() 
inside:
    for ( i = 1; i < d->max_vcpus; i++ )
    {
        if ( vcpu_create(d, i) == NULL )
        {
            printk("Failed to allocate d%dv%d\n", d->domain_id, i);
            break;
        }

        if ( is_64bit_domain(d) )
            vcpu_switch_to_aarch64_mode(d->vcpu[i]);
    }

    domain_update_node_affinity(d);

sched_setup_dom0_vcpus() still could be re-used but the an extra for() 
loop will be needed for:
        if ( is_64bit_domain(d) )
            vcpu_switch_to_aarch64_mode(d->vcpu[i]);

I think it doesn't then too much sense to re-use 
sched_setup_dom0_vcpus() for Arm.
~ Oleksii