Re: [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Anthony PERARD <anthony.perard@citrix.com>]

On Mon, Jul 11, 2016 at 09:33:21AM -0400, Boris Ostrovsky wrote:
> On 07/11/2016 06:47 AM, Wei Liu wrote:
> > On Fri, Jul 08, 2016 at 01:20:46PM -0400, Boris Ostrovsky wrote:
> >> On 07/08/2016 12:07 PM, Wei Liu wrote:
> >>> On Fri, Jul 08, 2016 at 10:48:52AM -0400, Boris Ostrovsky wrote:
> >>>> On 07/08/2016 06:55 AM, Wei Liu wrote:
> >>>>>> +
> >>>>>> +    /* Map page that will hold RSDP */
> >>>>>> +    extent = RSDP_ADDRESS >> ctxt.page_shift;
> >>>>>> +    rc = populate_acpi_pages(dom, &extent, 1, &ctxt);
> >>>>>> +    if (rc) {
> >>>>>> +        LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
> >>>>>> +            __FUNCTION__, rc);
> >>>>>> +        goto out;
> >>>>>> +    }
> >>>>>> +    config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
> >>>>>> +                                                      ctxt.page_size,
> >>>>>> +                                                      PROT_READ | PROT_WRITE,
> >>>>>> +                                                      RSDP_ADDRESS >> ctxt.page_shift);
> >>>>> I think with Anthony's on-going work you should be more flexible for all
> >>>>> you tables.

If the tool stack already knows where it can (or should) load the rsdp,
there is not much reason to be flexible.

> >>>> Not sure I understand what you mean here. You want the address
> >>>> (RSDP_ADDRESS) to be a variable as opposed to a macro?
> >>>>
> >>> I'm still trying to wrap my head around the possible interaction between
> >>> Anthony's work and your work.
> >>>
> >>> Anthony's work allows dynamically loading of firmware blobs. If you use
> >>> a fixed address, theoretically it can clash with firmware blobs among
> >>> other things libxc can load. The high address is a safe bet so that
> >>> probably won't happen in practice.
> >>>
> >>> Anthony's work allows loading arbitrary blobs actually. Can you take
> >>> advantage of that mechanism as well? That is, to specify all your tables
> >>> as modules and let libxc handle actually loading them  into guest
> >>> memory.
> >>>
> >>> Does this make sense?
> >>>
> >>> Also CC Anthony here.
> >>
> >> My understanding of Anthony's series is that its goal was to provide an
> >> interface to pass DSDT (and maybe some other tables) from the toolstack
> >> to hvmloader.

Not anymore. The only new functionality provided by the patch series is
to load the BIOS (or OVMF) from the filesystem (instead of having this
blob embedded into hvmloader).

It does also change the way an extra acpi tables or a smbios is loaded
into guest memory for consumption by hvmloader. But that just make the
libxc code a bit cleaner.

> >> Here we don't have hvmloader, we are loading the tables directly into
> >> guest's memory.
> >>
> > Do you use the same hvm_start_info structure? I don't think that
> > structure is restricted to hvmloader.
> 
> 
> Yes, we do. However, in PVH(v2) case it will be seen next by the guest
> who will expect the tables to already be in memory. I.e. there is no
> intermediate Xen component, such as hvmloader, who can load the blobs.
> 
> Having said that, I wonder whether we (both x86 and ARM) could use
> Anthony's xc_dom_image.full_acpi_module instead of acpitables_blob that

I don't have full_acpi_module anymore in my patch series. But that does
not prevent you from introducing one.

> Shannon's series added. (even if we can't, I think
> xc_hvm_firmware_module is the right datastructure to store the blob
> since it has both toolstack's virtual and guest's physical addresses).

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel