RE: [PATCH v1 10/18] x86: introduce the domain builder

["Smith, Jackson" <rsmith@xxxxxxxxxxxxxxxxxxxxx>]

Hi Daniel,

> -----Original Message-----
> Subject: [PATCH v1 10/18] x86: introduce the domain builder
> 
> This commit introduces the domain builder configuration FDT parser along
> with the domain builder core for domain creation. To enable domain builder
> to be a cross architecture internal API, a new arch domain creation call
is
> introduced for use by the domain builder.

> diff --git a/xen/common/domain-builder/core.c

> +void __init builder_init(struct boot_info *info) {
> +    struct boot_domain *d = NULL;
> +
> +    info->builder = &builder;
> +
> +    if ( IS_ENABLED(CONFIG_BUILDER_FDT) )
> +    {

> +    }
> +
> +    /*
> +     * No FDT config support or an FDT wasn't present, do an initial
> +     * domain construction
> +     */
> +    printk("Domain Builder: falling back to initial domain build\n");
> +    info->builder->nr_doms = 1;
> +    d = &info->builder->domains[0];
> +
> +    d->mode = opt_dom0_pvh ? 0 : BUILD_MODE_PARAVIRTUALIZED;
> +
> +    d->kernel = &info->mods[0];
> +    d->kernel->kind = BOOTMOD_KERNEL;
> +
> +    d->permissions = BUILD_PERMISSION_CONTROL |
> BUILD_PERMISSION_HARDWARE;
> +    d->functions = BUILD_FUNCTION_CONSOLE |
> BUILD_FUNCTION_XENSTORE |
> +                     BUILD_FUNCTION_INITIAL_DOM;
> +
> +    d->kernel->arch->headroom = bzimage_headroom(bootstrap_map(d-
> >kernel),
> +                                                   d->kernel->size);
> +    bootstrap_map(NULL);
> +
> +    if ( d->kernel->string.len )
> +        d->kernel->string.kind = BOOTSTR_CMDLINE; }

Forgive me if I'm incorrect, but I believe there is an issue with this
fallback logic for the case where no FDT was provided.

If dom0_mem is not supplied to the xen cmd line, then d->meminfo is never
initialized. (See dom0_compute_nr_pages/dom0_build.c:335)
This was giving me trouble because bd->meminfo.mem_max.nr_pages was left at
0, effectivity clamping dom0 to 0 pages of ram.

I'm not sure what the best solution is but one (easy) possibility is just
initializing meminfo to the dom0 defaults near the end of this function:
        d->meminfo.mem_size = dom0_size;
        d->meminfo.mem_min = dom0_min_size;
        d->meminfo.mem_max = dom0_max_size;

Thanks,
Jackson

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