Re: [Xen-devel] [PATCH 8/8] x86/setup: lift dom0 creation out into create_dom0() function

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Fri, 2020-02-21 at 18:06 +0100, Jan Beulich wrote:
> On 01.02.2020 01:33, David Woodhouse wrote:
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -678,6 +678,92 @@ static unsigned int __init copy_bios_e820(struct 
> > e820entry *map, unsigned int li
> >      return n;
> >  }
> >  
> > +static struct domain * __init create_dom0(const module_t *image,
> > +                                          unsigned long headroom,
> > +                                          module_t *initrd, char *kextra,
> > +                                          char *loader)
> 
> Can any of these last three be pointer-to-const?

I suppose kextra can. The other two are passed on to construct_dom0(),
which could perhaps be changed to take const pointers but that's a
separate cleanup.

> > +{
> > +    struct xen_domctl_createdomain dom0_cfg = {
> > +        .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 
> > 0,
> > +        .max_evtchn_port = -1,
> > +        .max_grant_frames = -1,
> > +        .max_maptrack_frames = -1,
> > +    };
> > +    struct domain *d;
> > +    char *cmdline;
> > +
> > +    if ( opt_dom0_pvh )
> > +    {
> > +        dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
> > +                           ((hvm_hap_supported() && !opt_dom0_shadow) ?
> > +                            XEN_DOMCTL_CDF_hap : 0));
> > +
> > +        dom0_cfg.arch.emulation_flags |=
> > +            XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
> > +    }
> > +    dom0_cfg.max_vcpus = dom0_max_vcpus();
> 
> Can this not be part of the initializer now?

Yes, I suppose it can. Fixed.

> > +    if ( iommu_enabled )
> > +        dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> > +
> > +    /* Create initial domain 0. */
> > +    d = domain_create(get_initial_domain_id(), &dom0_cfg, !pv_shim);
> > +    if ( IS_ERR(d) || (alloc_dom0_vcpu0(d) == NULL) )
> > +        panic("Error creating domain 0\n");
> > +
> > +    /* Grab the DOM0 command line. */
> > +    cmdline = (char *)(image->string ? __va(image->string) : NULL);
> 
> Is this cast needed? (I know you're only moving the code, but some
> easy cleanup would be nice anyway.)
> 
> > +    if ( (cmdline != NULL) || (kextra != NULL) )
> 
> Similarly here you may want to consider shortening to
> 
>     if ( cmdline || kextra )
> 
> At least one more similar case further down.

Makes sense. Done too; thanks.

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel