Re: [PATCH 2/3] tools/init-xenstore-domain: support xenstore pvh stubdom

[Wei Liu <wl@xxxxxxx>]

On Wed, Sep 23, 2020 at 08:45:40AM +0200, Juergen Gross wrote:
> Instead of creating the xenstore-stubdom domain first and parsing the
> kernel later do it the other way round. This enables to probe for the
> domain type supported by the xenstore-stubdom and to support both, pv
> and pvh type stubdoms.
> 
> Try to parse the stubdom image first for PV support, if this fails use
> HVM. Then create the domain with the appropriate type selected.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
[...]
> +    dom->container_type = XC_DOM_HVM_CONTAINER;
> +    rv = xc_dom_parse_image(dom);
> +    if ( rv )
> +    {
> +        dom->container_type = XC_DOM_PV_CONTAINER;
> +        rv = xc_dom_parse_image(dom);
> +        if ( rv )
> +        {
> +            fprintf(stderr, "xc_dom_parse_image failed\n");
> +            goto err;
> +        }
> +    }
> +    else
> +    {
> +        config.flags |= XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap;
> +        config.arch.emulation_flags = XEN_X86_EMU_LAPIC;
> +        dom->target_pages = mem_size >> XC_PAGE_SHIFT;
> +        dom->mmio_size = GB(4) - LAPIC_BASE_ADDRESS;
> +        dom->lowmem_end = (mem_size > LAPIC_BASE_ADDRESS) ?
> +                          LAPIC_BASE_ADDRESS : mem_size;
> +        dom->highmem_end = (mem_size > LAPIC_BASE_ADDRESS) ?
> +                           GB(4) + mem_size - LAPIC_BASE_ADDRESS : 0;
> +        dom->mmio_start = LAPIC_BASE_ADDRESS;
> +        dom->max_vcpus = 1;
> +        e820[0].addr = 0;
> +        e820[0].size = dom->lowmem_end;
> +        e820[0].type = E820_RAM;
> +        e820[1].addr = LAPIC_BASE_ADDRESS;
> +        e820[1].size = dom->mmio_size;
> +        e820[1].type = E820_RESERVED;
> +        e820[2].addr = GB(4);
> +        e820[2].size = dom->highmem_end - GB(4);

Do you not want to check if highmem_end is larger than GB(4) before
putting in this region?

> +        e820[2].type = E820_RAM;
> +    }

This hardcoded e820 map doesn't seem very flexible, but we
control the guest kernel anyway so I think this should be fine.

The rest of this patch looks okay to me.

Wei.