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

[Jürgen Groß <jgross@xxxxxxxx>]

On 30.09.20 17:46, Wei Liu wrote:
> 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?
Oh, indeed I should.

> > +        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.
Thanks.


Juergen