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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 07/06/2016 01:03 PM, Julien Grall wrote:
>
>
> On 06/07/16 17:30, Boris Ostrovsky wrote:
>> On 07/06/2016 12:04 PM, Julien Grall wrote:
>>> Hi Boris,
>>>
>>> On 06/07/16 16:50, Boris Ostrovsky wrote:
>>>> On 07/06/2016 07:05 AM, Julien Grall wrote:
>>>>>> +static int populate_acpi_pages(struct xc_dom_image *dom,
>>>>>> +                               xen_pfn_t *extents,
>>>>>> +                               unsigned int num_pages,
>>>>>> +                               struct acpi_ctxt *ctxt)
>>>>>> +{
>>>>>> +    int rc;
>>>>>> +    xc_interface *xch = dom->xch;
>>>>>> +    uint32_t domid = dom->guest_domid;
>>>>>> +    unsigned long idx, first_high_idx = (1ull << (32 -
>>>>>> ctxt->page_shift));
>>>>>> +
>>>>>> +    for (; num_pages; num_pages--, extents++) {
>>>>>> +
>>>>>> +        if (xc_domain_populate_physmap(xch, domid, 1, 0, 0,
>>>>>> extents)
>>>>>> == 1)
>>>>>
>>>>> It looks like this is working because libxl is setting the maximum
>>>>> size of the domain with some slack (1MB). You might get in trouble if
>>>>> the slack is reduced or used by someone or the ACPI blob is
>>>>> increasing.
>>>>
>>>>
>>>> I saw your conversation about slack with Stefano and I am not sure I
>>>> understood what it was about. If this was about padding guest's memory
>>>> to be able to put there some additional data (such ACPI tables) then
>>>> this is not my intention here: if I can't populate (because it is
>>>> already populated, I guess) then I try to move memory around with code
>>>> below. That's what mem_hole_populate_ram() does as well.
>>>
>>> The maximum amount of memory that could be assigned to a domain is
>>> fixed per-domain. This maximum amount does not take into account the
>>> size of the ACPI blob. So you may end up to fail because all the
>>> memory was assigned somewhere else.
>>
>> Why shouldn't max amount of guest's memory include ACPI tables? I think
>> we should fail and not rely on this slack if no more memory is
>> available.
>>
>> ...
>
> Because, at least for ARM, the ACPI memory region is not part of the
> "real" RAM. So this value is not taken into account into the current
> max mem.


Hmm.. I've always assumed it is part of memory but marked as a special
type in e820 map on x86. Maybe Andrew or Jan can comment on this.


> My point is we need to add the size of the ACPI blob to max mem to
> avoid any error here.

So you (or Shannon) plan on doing this for ARM, right (it's not done
with the current version of patches)?

>
>>
>>>
>>>>
>>>>> However, as mentioned in the ACPI thread [1], all the blobs are
>>>>> generally loaded by libxc and not libxl. This is more true on ARM
>>>>> because the guest address space is controlled by libxc (the position
>>>>> of all the blob are decided by it).
>>>>
>>>> The difference is that I not only load the tables here but also build
>>>> them. Which may or may not be the right thing to do in libxc.
>>>>
>>>> I suppose I can defer loading (and then keep pointer to tables in
>>>> acpitable_blob) but the then I need to keep RSDP descriptor somewhere
>>>> else (it is not part of the blob since it lives in lower MB of the
>>>> guest).
>>>
>>> The device tree for ARM are built in libxl and loaded for libxc. IHMO,
>>> it would be strange to have a different pattern for ACPI.
>>
>> Is RSDP part of the ACPI blob for ARM? If not, how do you load it?
>
> RSDP is part of the ACPI blob for ARM.


And it's not for x86. So to separate building and loading the tables
would require two blobs.

I suppose we cal loop over blobs in xc_dom_load_acpi in
https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00309.html


-boris




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