Re: [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 06/27/2016 06:49 AM, Julien Grall wrote:
> (CC Boris)
>
> On 27/06/16 07:25, Shannon Zhao wrote:
>>
>>
>> On 2016/6/24 2:46, Julien Grall wrote:
>>>> +
>>>>   static int alloc_magic_pages(struct xc_dom_image *dom)
>>>>   {
>>>      >      int rc, i;
>>>> @@ -100,6 +141,16 @@ static int alloc_magic_pages(struct xc_dom_image
>>>> *dom)
>>>>       xc_hvm_param_set(dom->xch, dom->guest_domid, 
>>>> HVM_PARAM_STORE_EVTCHN,
>>>>               dom->xenstore_evtchn);
>>>>
>>>> +    if ( dom->acpitable_blob && dom->acpitable_size > 0 )
>>>> +    {
>>>> +        rc = xc_dom_copy_acpi(dom);
>>>> +        if ( rc != 0 )
>>>> +        {
>>>> +            DOMPRINTF("Unable to copy ACPI tables");
>>>> +            return rc;
>>>> +        }
>>>> +    }
>>>
>>> alloc_magic_pages looks the wrong place with this function. Any reason
>>> to not have a generic ACPI blob loading in xc_dom_core.c as we do for
>>> devicetree?
>> Looks like xc_dom_build_image is used for allocating pages in guest RAM
>> while ACPI blob is not put in guest RAM.
>
> The function xc_dom_build_image is used to load blob into the guest 
> memory and allocate others pages (such as magic pages) which may not 
> be part of the actual guest RAM.
>
> The callback alloc_magic_pages is used to allocate per-architecture 
> specific pages. However, the ACPI blob is pretty much generic. So I am 
> not sure why we would want to do it for ARM only.


FWIW, for PVH I don't plan, at least for now, to keep a pointer to ACPI 
stuff in xc_dom_image. I am building the tables and loading them into 
the guest right away.

(As a separate point, I noticed that this series adds 4th type of blob 
(in addition to kernel, ramdisk and devicetree) so I wonder whether 
introducing a struct blob might be useful.)

-boris


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