Re: Design doc of adding ACPI support for arm64 on Xen - version 5

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/8/31 17:40, Jan Beulich wrote:
>>>> On 31.08.15 at 10:51, <zhaoshenglong@huawei.com> wrote:
>> On 2015/8/31 15:39, Jan Beulich wrote:
>>>>>> On 29.08.15 at 03:29, <shannon.zhao@linaro.org> wrote:
>>>> On 2015/8/28 23:06, Jan Beulich wrote:
>>>>>>>> On 28.08.15 at 11:45, <zhaoshenglong@huawei.com> wrote:
>>>>>> Create only one ConfigurationTable to store VendorGuid and VendorTable.
>>>>>
>>>>> What do you mean with "Create only one ..." - there is only one.
>>>>> DYM "Create one additional Configuration Table entry ...", implying
>>>>> that the Configuration Table will need to by copied too?
>>>>
>>>> Sorry for the misunderstanding. I mean that it doesn't copy the original
>>>> one and just creates a new ConfigurationTable.
>>>
>>> If you don't copy the original one, how does Dom0 learn of what is
>>> in the original one (ACPI being just one such element)? Right now I
>>> can't see why you wouldn't copy the entire table and simply append
>>> the one extra entry.
>>
>> The original System table contains a EFI configuration table which
>> stores the _host_ RSDP table address. But we create a new RSDP table and
>> the address is different. We create a new EFI configuration table to
>> store the new RSDP table address to VenderTable.
>> So if we supply both the host EFI configuration table and the new one to
>> Dom0, how do you expect Dom0 to get the right ACPI table?
> 
> There isn't even a way to supply both. You can only supply a
> clone. If you expect Linux to be able to deal with it with minimal
> change, the altered RSDP should still be provided using
> ACPI_20_TABLE_GUID.

That's what this design exactly does.

 But wait - I think I see where the confusion
> comes from: You say "store [...] to VendorTable", which I read as
> another kind of table, but I think you mean the VendorTable field
> of EFI_CONFIGURATION_TABLE. Since this isn't the first time
> imprecise wording has led to confusion - may I once again ask that
> you be very precise in the terms you use, so that tables, table
> entries, fields, etc can all be told apart easily (and namely without
> having to look up what a certain term used refers to)?
> 
>>>>>> d) Copy MADT table
>>>>>> It needs to change MADT table to restrict the number of vCPUs. We choose
>>>>>> to copy the first dom0_max_vcpus GICC entries of MADT to new created
>>>>>> MADT table when numa is not supported currently.
>>>>>
>>>>> Copy means you imply to have an original?
>>>>
>>>> So I'll change it to "create".
>>>>
>>>>> What if dom0_max_vcpus
>>>>> is larger than the physical CPU count?
>>>>
>>>> I think it only needs to care the cpu_interface_number, uid and mpidr
>>>> field of GICC entry and other fields could be same with the host GICC
>>>> entry. It could get the mpidr from the vCPU index.
>>>
>>> You again suggest to use data from host entries, i.e. you leave
>>> incompletely addressed the original question: "What source of
>>> information do you intend to use when the Dom0's vCPU count is
>>> higher than the host's pCPU count?"
>>>
>>
>> There are only the cpu_interface_number, uid and mpidr which need to
>> change. Other fields could copy from any one of the GICC entries from
>> host MADT table.
> 
> Hmm, okay, if _any one_ is indeed fine, then okay. But then please
> change to wording in your document to make this explicit (and to also
> make explicit that you consider this an okay thing to do in the first
> place, just to catch others' attention to double check it really is).
> 
>>>>>> g) Copy RSDP table
>>>>>> Change the value of xsdt_physical_address in RSDP table. As we create a
>>>>>> new XSDT table and the address of XSDT is changed, so it needs to update
>>>>>> the value of "xsdt_physical_address" in RSDP. So Dom0 could get the
>>>>>> right XSDT table rather than the old one. And it needs to update the
>>>>>> value of VendorTable in EFI Configuration Table which is created in
>>>>>> above step a).
>>>>>
>>>>> How is this last sentence related to the handling of RSDP?
>>>>
>>>> Because the ACPI root address(i.e. the address of RSDP table) is stored
>>>> in EFI Configuration Table.
>>>
>>> With this I can only see you to refer to everything _except_ the last
>>> sentence. The last sentence talks about VendorTable, which I continue
>>> to not see to have a relation to ACPI/RSDP.
>>>
>>
>> The value of VendorTable is the ACPI root address and Dom0 (or Linux)
>> get the ACPI root address from it when using UEFI with ACPI.
>>
>> (I wonder why you didn't get this if you have a glance at the booting
>> process. uefi_init(arch/arm64/kernel/efi.c of Linux) -->
>> efi_config_parse_tables --> match_config_table. It will save the
>> VenderTable to efi.acpi20 and when Linux call acpi_os_get_root_pointer
>> to get ACPI root address, it will return efi.acpi20)
> 
> See above - if I hadn't realized you, in every single place you use it,
> really mean "the VendorTable field of the Configuration Table entry
> using ACPI_20_TABLE_GUID", I would have complained here again.
> (Of course you don't need to spell it this way every time, but you
> should imo spell it this or a similar way at least once in each section.)
> 

So you can't get the meaning of "the value of VendorTable in EFI
Configuration Table"?

-- 
Shannon