Re: [Xen-devel] [PATCH v3 38/62] arm/acpi: Add placeholder for efi and acpi load address

[Shannon Zhao <zhaoshenglong@xxxxxxxxxx>]

On 2015/11/17 22:23, Julien Grall wrote:
> Hi Shannon,
> 
> On 17/11/15 12:45, Shannon Zhao wrote:
>>
>>
>> On 2015/11/17 19:58, Julien Grall wrote:
>>> Hi Shannon,
>>>
>>> On 17/11/15 09:40, shannon.zhao@xxxxxxxxxx wrote:
>>>> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>>>
>>>> EFI table, memory description table and some of acpi tables will be
>>>> placed after DOM0 memory space. Add placeholder for the starting address
>>>> for loading in DOM0 and the size of new added tables. Also add a
>>>> placeholder to store the new created tables.
>>>>
>>>> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>>> ---
>>>>  xen/include/asm-arm/domain.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>>>> index 1e61f30..91272e5 100644
>>>> --- a/xen/include/asm-arm/domain.h
>>>> +++ b/xen/include/asm-arm/domain.h
>>>> @@ -125,6 +125,11 @@ struct arch_domain
>>>>      } vuart;
>>>>  
>>>>      unsigned int evtchn_irq;
>>>> +#ifdef CONFIG_ACPI
>>>> +    void *efi_acpi_table;
>>>> +    paddr_t efi_acpi_gpa;
>>>> +    paddr_t efi_acpi_len;
>>>> +#endif
>>>
>>> Why do you need this in the arch_domain? AFAICT those 3 variables should
>>> only be used while building DOM0, so a better place would be in struct
>>> kernel_info (see xen/arch/arm/kernel.h).
>>>
>>
>> Since I use efi_acpi_table to store the allocated pages pointer which
>> are used to store newly created ACPI and EFI tables.
>> See [PATCH v3 40/62]:
>> +    order = get_order_from_bytes(d->arch.efi_acpi_len);
>> +    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
>>
>> And it free these pages when destroying this domain:
>> +#ifdef CONFIG_ACPI
>> +    free_xenheap_pages(d->arch.efi_acpi_table,
>> +                       get_order_from_bytes(d->arch.efi_acpi_len));
>> +#endif
> 
> But this is wrong. You are assuming that DOM0 memory is direct mapped
> which may not be true in the future.
> 
I think we have discussed this when we discussed the design doc.

"All above tables will be mapped to Dom0 non-RAM space. Since when
booting through ACPI it doesn't need the grant table region(see below
section 3), it could use this region to store the tables or use the same
way to find one memory region to store them."

Firstly, as Jan suggested, these tables should not be in RAM space, so
we drop the previous way that copying these tables to Dom0 RAM.
Then I suggested map these tables to the space after the Dom0 RAM space,
but this not right because Dom0 RAM region might be at the edge of
physical RAM space and there might be device MMIO regions.
Then you suggest it could map these tables to the region which is used
for grant table(or the region found by the same way) while it's not used
when it boots with ACPI. These regions are not used by Xen and will not
be used by Dom0 either currently. But as you say, it will be wrong if
Dom0 memory is not 1:1 mapped.

So how about below idea:
We still copy these tables to Dom0 RAM space but when we create
EFI_MMAP_TABLE, we remove the space occupied by these tables from the
EfiConventionalMemory descriptor.

Thanks,
--
Shannon


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