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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.08.15 at 10:51, <zhaoshenglong@xxxxxxxxxx> wrote:
> On 2015/8/31 15:39, Jan Beulich wrote:
>>>>> On 29.08.15 at 03:29, <shannon.zhao@xxxxxxxxxx> wrote:
>>> On 2015/8/28 23:06, Jan Beulich wrote:
>>>>>>> On 28.08.15 at 11:45, <zhaoshenglong@xxxxxxxxxx> 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. 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.)

Jan

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