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

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Jul 11, 2016 at 10:40:17AM -0400, Boris Ostrovsky wrote:
> On 07/11/2016 09:41 AM, Wei Liu wrote:
> > 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.
> >>>>>> 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.
> >>>>
> >>>> 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.
> >>
> > Maybe you misunderstood. Anthony's work will load the blobs into guest
> > memory -- all fields in hvm_start_info points to guest physical address
> > IIRC. Hvmloader might want to relocate them, but that's a different
> > matter.
> 
> What's the status on Anthony's series? I can rebase on top of his tree
> (I might indeed be able to reuse some of his code) but if this is way
> off the dependencies become problematic (because Shannon's series would
> also be delayed).
> 

His series is very close to get merged. I believe toolstack in his
series only require cosmetic changes and hvmloader patches are all
acked.

> >
> >> Having said that, I wonder whether we (both x86 and ARM) could use
> >> Anthony's xc_dom_image.full_acpi_module instead 
> 
> 
> (no full_acpi_module anymore, I was looking at an earlier series
> version. I guess it's system_firmware_module now)
> 
> 
> >> of acpitables_blob that
> >> 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).
> >>
> > Yes, that's along the line I'm thinking about.
> 
> So I am confused about xc_hvm_firmware_mode: is guest_addr_out meant to
> be guest's physical or virtual?
> 
> One one hand it looks like virtual:
> 
> in
> https://lists.xenproject.org/archives/html/xen-devel/2016-06/msg02901.html
>     +        module->guest_addr_out = seg.vstart;
> 
> but then in
> https://lists.xenproject.org/archives/html/xen-devel/2016-06/msg02902.html
> 
> +    modlist[index].paddr = module->guest_addr_out;
> 

It should be guest physical.

Wei.

> 
> -boris
> 
> 

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