All of lore.kernel.org
 help / color / mirror / Atom feed
* Design doc of adding ACPI support for arm64 on Xen - version 5
@ 2015-08-28  9:45 Shannon Zhao
  2015-08-28 12:55 ` Julien Grall
                   ` (2 more replies)
  0 siblings, 3 replies; 43+ messages in thread
From: Shannon Zhao @ 2015-08-28  9:45 UTC (permalink / raw)
  To: xen-devel, Christoffer Dall, Ian Campbell, Stefano Stabellini,
	Julien Grall, Stefano Stabellini, Jan Beulich, Parth Dixit,
	andrew, Boris Ostrovsky, David Vrabel, Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Shannon Zhao

This document is going to explain the design details of Xen booting with
ACPI on ARM. Maybe parts of it may not be appropriate. Any comments are
welcome.

Changes v4->v5:
* change the description of section 4 to make it more generic
* place EFI and ACPI tables at non-RAM space of Dom0

Changes v3->v4:
* add explanation for minimal DT and the properties
* drop "linux," prefix of the properties
* add explanation for the event channel flag
* create RSDP table since the "xsdt_physical_address" is changed
* since it uses hypervisor_id introduced by ACPI 6.0 to notify Dom0 the
  hypervisor ID, so it needs to limit minimum supported ACPI version for
  Xen on ARM to 6.0.

Changes v2->v3:
* remove the two HVM_PARAMs for grant table and let linux kernel use
  xlated_setup_gnttab_pages() to setup grant table.
* don't modify GTDT table
* add definition of event-channel interrupt flag
* state that route all Xen unused interrupt to Dom0
* state that reusing existing PCI bus_notifier for PCI devices MMIO
* mapping

To Xen itself booting with ACPI, this is similar to Linux kernel except
that Xen doesn't parse DSDT table. So I'll skip this part and focus on
how Xen prepares ACPI tables for Dom0 and how Xen passes them to Dom0.

1. Create minimal DT to pass required information to Dom0
----------------------------------------------------------
The UEFI stub is a feature that extends the Image/zImage into a valid
UEFI PE/COFF executable, including a loader application that makes it
possible to load the kernel directly from the UEFI shell, boot menu, or
one of the lightweight bootloaders like Gummiboot or rEFInd.
The kernel image built with stub support remains a valid kernel image
for booting in non-UEFI environments and the UEFI stub will be jumped
over for non-UEFI environments.

When booting in UEFI mode, the UEFI stub will create a minimal DT in
order to pass the command line and other informations (such as the EFI
memory table) to the kernel. And when booting with ACPI, kernel will get
command line, ACPI root table address and memory map information from
the minimal DT. Also, it will check if the DT contains only the /chosen
node to know whether it boots with DT or ACPI.

In addition, the current names of these properties with a "linux,"
prefix in the minimal DT are Linux specified. It needs to standardize
them so that other OS(such as FreeBSD) could reuse them in the future.
So we drop the "linux," prefix of UEFI parameters and change the names
in Linux kernel as well.

An example of the minimal DT:
/ {
    #address-cells = <2>;
    #size-cells = <1>;
    chosen {
        bootargs = "kernel=Image console=hvc0 earlycon=pl011,0x1c090000
root=/dev/vda2 rw rootfstype=ext4 init=/bin/sh acpi=force";
        linux,initrd-start = <0xXXXXXXXX>;
        linux,initrd-end = <0xXXXXXXXX>;
        uefi-system-table = <0xXXXXXXXX>;
        uefi-mmap-start = <0xXXXXXXXX>;
        uefi-mmap-size = <0xXXXXXXXX>;
        uefi-mmap-desc-size = <0xXXXXXXXX>;
        uefi-mmap-desc-ver = <0xXXXXXXXX>;
    };
};

For details loook at
https://github.com/torvalds/linux/blob/master/Documentation/arm/uefi.txt

2. Copy and change some EFI and ACPI tables
-------------------------------------------
a) Create EFI_SYSTEM_TABLE table
Copy the header from the origin and change the value of FirmwareVendor.
Create only one ConfigurationTable to store VendorGuid and VendorTable.
This EFI System Table is a made up one not copy of the host. The EFI
System Table will be passed to Dom0 through the property
"uefi-system-table" in the above minimal DT. So Dom0 could get ACPI root
table address through the ConfigurationTable.

b) Create EFI_MEMORY_DESCRIPTOR table
It needs to notify Dom0 where are the RAM regions. Add memory start and
size information of Dom0 in this table. It's passed to Dom0 through the
properties "uefi-mmap-start", "uefi-mmap-size", "uefi-mmap-desc-size"
and "uefi-mmap-desc-ver" of the minimal DT. Then Dom0 will get the
memory information through this EFI table.

c) Copy FADT table
Change the value of arm_boot_flags to enable PSCI and HVC.

Since there is no cpuid like x86 on ARM, here it's through the field
hypervisor_id of FADT table to tell Dom0 that the hypervisor is Xen.
Because hypervisor_id is introduced by ACPI 6.0, so it needs to limit
minimum supported ACPI version for Xen on ARM to 6.0.

Set the hypervisor_id to "XenVMM", then Dom0 could through HVM_PARAM to
get some informations for booting necessity, such as event-channel
interrupt.

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. If numa is supported in
the future, it would change the mpidr according to the cpu topology.
Also copy GICD as well.

e) Create STAO table
This table is a new added one that's used to define a list of ACPI
namespace names that are to be ignored by the OSPM in Dom0. Currently
we use it to tell OSPM that it should ignore UART defined in SPCR table.
Look at below url for details:
http://wiki.xenproject.org/mediawiki/images/0/02/Status-override-table.pdf

f) Copy XSDT table
Add a new table entry for STAO and change other table's entries.

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).

h) The rest of tables are not copied or changed. They are reused
including DSDT, SPCR, GTDT, etc. It doesn't mask EL2 resources for Dom0
because the Linux kernel would not use EL2 resources when it boots at
EL1.

All above tables will be mapped to Dom0 non-RAM space(e.g. the space
after Dom0 RAM).

3. Dom0 gets grant table and event channel irq information
-----------------------------------------------------------
The OS will have to find a place himself in the memory map for the grant
table region. For instance, Linux can make usage of
xlated_setup_gnttab_pages.

To event channel interrupt, reuse HVM_PARAM_CALLBACK_IRQ and add a new
delivery type to get it.
val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI (ARM and ARM64
only)
The definition of flag reusing the definition of xenv table. Bit 0
stands interrupt mode(1: edge triggered, 0: level triggered) and bit 1
stands interrupt polarity(1: active low, 0: active high).

As said above, we assign the hypervisor_id be "XenVMM" to tell Dom0 that
it runs on Xen hypervisor. Then Dom0 could get it through hypercall
HVMOP_get_param.

4. Map MMIO regions
-------------------
Add a new XENMAPSPACE "XENMAPSPACE_dev_mmio" for mapping mmio region.
Dom0 could use the hypercall XENMEM_add_to_physmap or
XENMEM_add_to_physmap_batch to map the mmio regions of devices. The
usage of the hypercall's parameters:
- domid: DOMID_SELF.
- space: XENMAPSPACE_dev_mmio.
- size: Number of pages to go through.
- idxs: native physical addresses.
- gpfns: guest physical addresses where the mapping should appear.

For example, Linux could register a bus_notifier for platform and amba
bus. Within the notifier, check if the device is newly added, if so call
the hypercall to map the mmio regions. And for PCI bus devices, Linux
could reuse the existing PCI bus_notifier like X86.

5. Route device interrupts to Dom0
----------------------------------
Route all the SPI interrupts to Dom0 before Dom0 booting, except the
interrupts used by Xen. For uart, it could get the interrupt from SPCR
table and exclude it. For SMMU, it could get the interrupts from IORT
table and exclude them as well.

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-28  9:45 Design doc of adding ACPI support for arm64 on Xen - version 5 Shannon Zhao
@ 2015-08-28 12:55 ` Julien Grall
  2015-08-29  1:00   ` Shannon Zhao
  2015-08-28 15:06 ` Jan Beulich
  2015-09-02 12:18 ` Ian Campbell
  2 siblings, 1 reply; 43+ messages in thread
From: Julien Grall @ 2015-08-28 12:55 UTC (permalink / raw)
  To: Shannon Zhao, xen-devel, Christoffer Dall, Ian Campbell,
	Stefano Stabellini, Stefano Stabellini, Jan Beulich, Parth Dixit,
	andrew, Boris Ostrovsky, David Vrabel, Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Shannon Zhao

Hi Shannon,

On 28/08/15 10:45, Shannon Zhao wrote:
> 2. Copy and change some EFI and ACPI tables
> -------------------------------------------

[..]

> All above tables will be mapped to Dom0 non-RAM space(e.g. the space
> after Dom0 RAM).

If I understand correctly what you are saying, you plan to put the
tables just after the Dom0 RAM bank. Although, there can be multiple
banks and how can you be so sure that there will be no MMIO at this place?


-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-28  9:45 Design doc of adding ACPI support for arm64 on Xen - version 5 Shannon Zhao
  2015-08-28 12:55 ` Julien Grall
@ 2015-08-28 15:06 ` Jan Beulich
  2015-08-29  1:29   ` Shannon Zhao
  2015-09-02 12:18 ` Ian Campbell
  2 siblings, 1 reply; 43+ messages in thread
From: Jan Beulich @ 2015-08-28 15:06 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, Shannon Zhao,
	andrew, Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPau Monne

>>> On 28.08.15 at 11:45, <zhaoshenglong@huawei.com> wrote:
> 2. Copy and change some EFI and ACPI tables
> -------------------------------------------
> a) Create EFI_SYSTEM_TABLE table
> Copy the header from the origin and change the value of FirmwareVendor.

Careful: The version in the header may imply that fields are there
which you aren't aware of. I think you need to adjust the version
field too. (Obviously along with FirmwareVendor you definitely also
want to change FirmwareRevision.)

> 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?

> 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? What if dom0_max_vcpus
is larger than the physical CPU count?

> 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?

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-28 12:55 ` Julien Grall
@ 2015-08-29  1:00   ` Shannon Zhao
  2015-08-31  7:33     ` Jan Beulich
  2015-08-31 11:44     ` Julien Grall
  0 siblings, 2 replies; 43+ messages in thread
From: Shannon Zhao @ 2015-08-29  1:00 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)

Hi Julien,

On 2015/8/28 20:55, Julien Grall wrote:
> Hi Shannon,
> 
> On 28/08/15 10:45, Shannon Zhao wrote:
>> 2. Copy and change some EFI and ACPI tables
>> -------------------------------------------
> 
> [..]
> 
>> All above tables will be mapped to Dom0 non-RAM space(e.g. the space
>> after Dom0 RAM).
> 
> If I understand correctly what you are saying, you plan to put the
> tables just after the Dom0 RAM bank. Although, there can be multiple
> banks and how can you be so sure that there will be no MMIO at this place?
> 
> 

Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
calculate the start address of non-RAM place. I'm not sure there will be
no MMIO at that place. Do you have any good idea to find such sure and
safe non-ram place?

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-28 15:06 ` Jan Beulich
@ 2015-08-29  1:29   ` Shannon Zhao
  2015-08-31  7:39     ` Jan Beulich
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-08-29  1:29 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPau Monne



On 2015/8/28 23:06, Jan Beulich wrote:
>>>> On 28.08.15 at 11:45, <zhaoshenglong@huawei.com> wrote:
>> 2. Copy and change some EFI and ACPI tables
>> -------------------------------------------
>> a) Create EFI_SYSTEM_TABLE table
>> Copy the header from the origin and change the value of FirmwareVendor.
> 
> Careful: The version in the header may imply that fields are there
> which you aren't aware of. I think you need to adjust the version
> field too. (Obviously along with FirmwareVendor you definitely also
> want to change FirmwareRevision.)
> 

Ok, will change the FirmwareRevision as well.

>> 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.

>> 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.

>> 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.

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-29  1:00   ` Shannon Zhao
@ 2015-08-31  7:33     ` Jan Beulich
  2015-08-31 11:44     ` Julien Grall
  1 sibling, 0 replies; 43+ messages in thread
From: Jan Beulich @ 2015-08-31  7:33 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, Roger Pau Monne

>>> On 29.08.15 at 03:00, <shannon.zhao@linaro.org> wrote:
> On 2015/8/28 20:55, Julien Grall wrote:
>> On 28/08/15 10:45, Shannon Zhao wrote:
>>> All above tables will be mapped to Dom0 non-RAM space(e.g. the space
>>> after Dom0 RAM).
>> 
>> If I understand correctly what you are saying, you plan to put the
>> tables just after the Dom0 RAM bank. Although, there can be multiple
>> banks and how can you be so sure that there will be no MMIO at this place?
> 
> Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
> calculate the start address of non-RAM place. I'm not sure there will be
> no MMIO at that place. Do you have any good idea to find such sure and
> safe non-ram place?

Assuming that MMIO ranges get mapped 1:1 to Dom0, and taking for
granted that not all of the RAM in the system can possible be handed
to Dom0, there should be ample host RAM address ranges void of any
RAM from Dom0's perspective where the tables could be put.

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-29  1:29   ` Shannon Zhao
@ 2015-08-31  7:39     ` Jan Beulich
  2015-08-31  8:51       ` Shannon Zhao
  0 siblings, 1 reply; 43+ messages in thread
From: Jan Beulich @ 2015-08-31  7:39 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPau Monne

>>> 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.

>>> 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?"

>>> 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.

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31  7:39     ` Jan Beulich
@ 2015-08-31  8:51       ` Shannon Zhao
  2015-08-31  9:40         ` Jan Beulich
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-08-31  8:51 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPau Monne

Hi Jan,

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?

>>>> 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.

>>>> 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)

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31  8:51       ` Shannon Zhao
@ 2015-08-31  9:40         ` Jan Beulich
  2015-08-31 11:31           ` Shannon Zhao
  2015-09-02 12:54           ` Ian Campbell
  0 siblings, 2 replies; 43+ messages in thread
From: Jan Beulich @ 2015-08-31  9:40 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne

>>> 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. 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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31  9:40         ` Jan Beulich
@ 2015-08-31 11:31           ` Shannon Zhao
  2015-08-31 11:46             ` Jan Beulich
  2015-09-02 12:54           ` Ian Campbell
  1 sibling, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-08-31 11:31 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne



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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-29  1:00   ` Shannon Zhao
  2015-08-31  7:33     ` Jan Beulich
@ 2015-08-31 11:44     ` Julien Grall
  2015-08-31 12:03       ` Shannon Zhao
  1 sibling, 1 reply; 43+ messages in thread
From: Julien Grall @ 2015-08-31 11:44 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)



On 29/08/2015 02:00, Shannon Zhao wrote:
> Hi Julien,
>
> On 2015/8/28 20:55, Julien Grall wrote:
>> Hi Shannon,
>>
>> On 28/08/15 10:45, Shannon Zhao wrote:
>>> 2. Copy and change some EFI and ACPI tables
>>> -------------------------------------------
>>
>> [..]
>>
>>> All above tables will be mapped to Dom0 non-RAM space(e.g. the space
>>> after Dom0 RAM).
>>
>> If I understand correctly what you are saying, you plan to put the
>> tables just after the Dom0 RAM bank. Although, there can be multiple
>> banks and how can you be so sure that there will be no MMIO at this place?
>>
>>
>
> Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
> calculate the start address of non-RAM place. I'm not sure there will be
> no MMIO at that place. Do you have any good idea to find such sure and
> safe non-ram place?

It's not a safe place because with the direct mapping for the RAM, DOM0 
RAM could live in the last bank. Furthermore, for the grant table 
region, we are re-using the RAM region used by Xen (find_gnttab_region), 
so it may clash with it too because Xen is relocated as high as possible.

Although, given that the grant table region will be found by Linux when 
ACPI is used (see your section 3.), we could be able to re-use the Xen 
region to store the new tables/structures.

Regards,
-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31 11:31           ` Shannon Zhao
@ 2015-08-31 11:46             ` Jan Beulich
  0 siblings, 0 replies; 43+ messages in thread
From: Jan Beulich @ 2015-08-31 11:46 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne

>>> On 31.08.15 at 13:31, <zhaoshenglong@huawei.com> wrote:
> On 2015/8/31 17:40, Jan Beulich wrote:
>>>>> On 31.08.15 at 10:51, <zhaoshenglong@huawei.com> wrote:
>>> (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"?

Without proper context, VendorTable to me refers to a table, not
the field of a particular table's slot.

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31 11:44     ` Julien Grall
@ 2015-08-31 12:03       ` Shannon Zhao
  2015-08-31 12:34         ` Julien Grall
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-08-31 12:03 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)



On 2015/8/31 19:44, Julien Grall wrote:
> 
> 
> On 29/08/2015 02:00, Shannon Zhao wrote:
>> Hi Julien,
>>
>> On 2015/8/28 20:55, Julien Grall wrote:
>>> Hi Shannon,
>>>
>>> On 28/08/15 10:45, Shannon Zhao wrote:
>>>> 2. Copy and change some EFI and ACPI tables
>>>> -------------------------------------------
>>>
>>> [..]
>>>
>>>> All above tables will be mapped to Dom0 non-RAM space(e.g. the space
>>>> after Dom0 RAM).
>>>
>>> If I understand correctly what you are saying, you plan to put the
>>> tables just after the Dom0 RAM bank. Although, there can be multiple
>>> banks and how can you be so sure that there will be no MMIO at this
>>> place?
>>>
>>>
>>
>> Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
>> calculate the start address of non-RAM place. I'm not sure there will be
>> no MMIO at that place. Do you have any good idea to find such sure and
>> safe non-ram place?
> 
> It's not a safe place because with the direct mapping for the RAM, DOM0
> RAM could live in the last bank. 

I put the tables after the last bank currently.

> Furthermore, for the grant table
> region, we are re-using the RAM region used by Xen (find_gnttab_region),
> so it may clash with it too because Xen is relocated as high as possible.
> 

Oh, I didn't realize this.

> Although, given that the grant table region will be found by Linux when
> ACPI is used (see your section 3.), we could be able to re-use the Xen
> region to store the new tables/structures.
> 

So you mean it's fine to put the tables after last bank of Dom0 RAM?

In addition, how does UEFI find the space to place the tables? Could we
use the same way?

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31 12:03       ` Shannon Zhao
@ 2015-08-31 12:34         ` Julien Grall
  2015-09-01  4:12           ` Shannon Zhao
  2015-09-02 12:58           ` Ian Campbell
  0 siblings, 2 replies; 43+ messages in thread
From: Julien Grall @ 2015-08-31 12:34 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)



On 31/08/2015 13:03, Shannon Zhao wrote:
>>> Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
>>> calculate the start address of non-RAM place. I'm not sure there will be
>>> no MMIO at that place. Do you have any good idea to find such sure and
>>> safe non-ram place?
>>
>> It's not a safe place because with the direct mapping for the RAM, DOM0
>> RAM could live in the last bank.
>
> I put the tables after the last bank currently.

I misread your suggestion and though you were planning to use the last 
host memory bank region for the UEFI tables.

So what you are suggesting is not safe at all, what does ensure you that 
there is no MMIO right after the last DOM0 bank? The DOM0 RAM region may 
lives at the edge of a host RAM bank.

>
>> Furthermore, for the grant table
>> region, we are re-using the RAM region used by Xen (find_gnttab_region),
>> so it may clash with it too because Xen is relocated as high as possible.
>>
>
> Oh, I didn't realize this.
>
>> Although, given that the grant table region will be found by Linux when
>> ACPI is used (see your section 3.), we could be able to re-use the Xen
>> region to store the new tables/structures.
>>
>
> So you mean it's fine to put the tables after last bank of Dom0 RAM?

No, see my answer above. I'm suggesting to re-use the same trick as we 
do for the grant table region. We know that this region will never be 
allocated in the DOM0 address space either because of the direct mapping 
or because it's very unlikely in the case of the non-direct mapping (Xen 
RAM region is very high).

>
> In addition, how does UEFI find the space to place the tables? Could we
> use the same way?

I think that those tables are living in the RAM and region used are 
marked as reserved.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31 12:34         ` Julien Grall
@ 2015-09-01  4:12           ` Shannon Zhao
  2015-09-01 11:28             ` Julien Grall
  2015-09-02 12:58           ` Ian Campbell
  1 sibling, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-01  4:12 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)



On 2015/8/31 20:34, Julien Grall wrote:
> 
> 
> On 31/08/2015 13:03, Shannon Zhao wrote:
>>>> Currently I use the last RAM bank(kinfo->mem.bank[nr_banks - 1]) to
>>>> calculate the start address of non-RAM place. I'm not sure there
>>>> will be
>>>> no MMIO at that place. Do you have any good idea to find such sure and
>>>> safe non-ram place?
>>>
>>> It's not a safe place because with the direct mapping for the RAM, DOM0
>>> RAM could live in the last bank.
>>
>> I put the tables after the last bank currently.
> 
> I misread your suggestion and though you were planning to use the last
> host memory bank region for the UEFI tables.
> 
> So what you are suggesting is not safe at all, what does ensure you that
> there is no MMIO right after the last DOM0 bank? The DOM0 RAM region may
> lives at the edge of a host RAM bank.
> 

Yes, you're right.

>>
>>> Furthermore, for the grant table
>>> region, we are re-using the RAM region used by Xen (find_gnttab_region),
>>> so it may clash with it too because Xen is relocated as high as
>>> possible.
>>>
>>
>> Oh, I didn't realize this.
>>
>>> Although, given that the grant table region will be found by Linux when
>>> ACPI is used (see your section 3.), we could be able to re-use the Xen
>>> region to store the new tables/structures.
>>>
>>
>> So you mean it's fine to put the tables after last bank of Dom0 RAM?
> 
> No, see my answer above. I'm suggesting to re-use the same trick as we
> do for the grant table region. We know that this region will never be
> allocated in the DOM0 address space either because of the direct mapping
> or because it's very unlikely in the case of the non-direct mapping (Xen
> RAM region is very high).
> 

I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
the address where these tables are mapped to Dom0. But the value of
gnttab_start is lower than the start of RAM, so Dom0 ingore these
regions and boot failed. see early_init_dt_add_memory_arch()

>>
>> In addition, how does UEFI find the space to place the tables? Could we
>> use the same way?
> 
> I think that those tables are living in the RAM and region used are
> marked as reserved.
> 

So can we use the same way for Dom0? I think the Linux will reserve the
regions for EFI in reserve_regions(). Therefore, Dom0 will not use these
reserved regions for other use.


-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01  4:12           ` Shannon Zhao
@ 2015-09-01 11:28             ` Julien Grall
  2015-09-01 12:35               ` Shannon Zhao
  0 siblings, 1 reply; 43+ messages in thread
From: Julien Grall @ 2015-09-01 11:28 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)

Hi Shannon,
On 01/09/15 05:12, Shannon Zhao wrote:
> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
> the address where these tables are mapped to Dom0. But the value of
> gnttab_start is lower than the start of RAM, so Dom0 ingore these
> regions and boot failed. see early_init_dt_add_memory_arch()

Can you elaborate? How Linux will fail? If this region is marked as
reserved in the UEFI memory map, Linux will mark the memory as reserved.

Furthermore, *ioremap is used in order to map the EFI tables so I don't
see a reason to fail.

>>>
>>> In addition, how does UEFI find the space to place the tables? Could we
>>> use the same way?
>>
>> I think that those tables are living in the RAM and region used are
>> marked as reserved.
>>
> 
> So can we use the same way for Dom0? I think the Linux will reserve the
> regions for EFI in reserve_regions(). Therefore, Dom0 will not use these
> reserved regions for other use.

Jan had some concerned about putting the EFI tables in RAM owned by DOM0
(see [1]).

Can you explain how Linux behave with EFI tables. I.e:
	- Where tables are expected to live (RAM, others...)?
	- Are thoses regions freed at some point to be re-use?
	- ...

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2015-08/msg02167.html

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01 11:28             ` Julien Grall
@ 2015-09-01 12:35               ` Shannon Zhao
  2015-09-01 13:40                 ` Julien Grall
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-01 12:35 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)



On 2015/9/1 19:28, Julien Grall wrote:
> Hi Shannon,
> On 01/09/15 05:12, Shannon Zhao wrote:
>> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
>> the address where these tables are mapped to Dom0. But the value of
>> gnttab_start is lower than the start of RAM, so Dom0 ingore these
>> regions and boot failed. see early_init_dt_add_memory_arch()
> 
> Can you elaborate? How Linux will fail? If this region is marked as
> reserved in the UEFI memory map, Linux will mark the memory as reserved.
> 
> Furthermore, *ioremap is used in order to map the EFI tables so I don't
> see a reason to fail.
> 

It's fine to parse EFI table but fails to parse ACPI table.

It doesn't add the memblock since it doesn't pass below check in
early_init_dt_add_memory_arch:
	if (base + size < phys_offset) {
		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
			   base, base + size);
		return;
	}

It's due to kinfo->gnttab_start (e.g. 0x87e00000) lower than the memory
start address (e.g. 0x90000000).

Then Linux will fail at parsing ACPI table.

ACPI: Interpreter enabled
ACPI: Using GIC for interrupt routing
Unhandled fault: alignment fault (0x96000021) at 0xffffff8000068184
Internal error: : 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc6+ #143
Hardware name: (null) (DT)
task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
PC is at acpi_get_phys_id+0x264/0x290
LR is at acpi_get_phys_id+0x178/0x290

>>>>
>>>> In addition, how does UEFI find the space to place the tables? Could we
>>>> use the same way?
>>>
>>> I think that those tables are living in the RAM and region used are
>>> marked as reserved.
>>>
>>
>> So can we use the same way for Dom0? I think the Linux will reserve the
>> regions for EFI in reserve_regions(). Therefore, Dom0 will not use these
>> reserved regions for other use.
> 
> Jan had some concerned about putting the EFI tables in RAM owned by DOM0
> (see [1]).
> 
> Can you explain how Linux behave with EFI tables. I.e:
> 	- Where tables are expected to live (RAM, others...)?
I'm not sure about this.

> 	- Are thoses regions freed at some point to be re-use?

I look at how Linux reserve these regions. See it in reserve_regions()
(arch/arm64/kernel/efi.c). It uses memblock_add() to add them to global
memblock and then uses memblock_reserve() to reserve them (like
allocating some memory) for certain use (here we use them to store the
tables). And I didn't see other places call memblock_free() to free them.

-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01 12:35               ` Shannon Zhao
@ 2015-09-01 13:40                 ` Julien Grall
  2015-09-01 14:03                   ` Jan Beulich
  2015-09-02  6:02                   ` Shannon Zhao
  0 siblings, 2 replies; 43+ messages in thread
From: Julien Grall @ 2015-09-01 13:40 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)

On 01/09/15 13:35, Shannon Zhao wrote:
> 
> 
> On 2015/9/1 19:28, Julien Grall wrote:
>> Hi Shannon,
>> On 01/09/15 05:12, Shannon Zhao wrote:
>>> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
>>> the address where these tables are mapped to Dom0. But the value of
>>> gnttab_start is lower than the start of RAM, so Dom0 ingore these
>>> regions and boot failed. see early_init_dt_add_memory_arch()
>>
>> Can you elaborate? How Linux will fail? If this region is marked as
>> reserved in the UEFI memory map, Linux will mark the memory as reserved.
>>
>> Furthermore, *ioremap is used in order to map the EFI tables so I don't
>> see a reason to fail.
>>
> 
> It's fine to parse EFI table but fails to parse ACPI table.
> 
> It doesn't add the memblock since it doesn't pass below check in
> early_init_dt_add_memory_arch:
> 	if (base + size < phys_offset) {
> 		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
> 			   base, base + size);
> 		return;
> 	}
> 
> It's due to kinfo->gnttab_start (e.g. 0x87e00000) lower than the memory
> start address (e.g. 0x90000000).
> 
> Then Linux will fail at parsing ACPI table.
> 
> ACPI: Interpreter enabled
> ACPI: Using GIC for interrupt routing
> Unhandled fault: alignment fault (0x96000021) at 0xffffff8000068184
> Internal error: : 96000021 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc6+ #143
> Hardware name: (null) (DT)
> task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
> PC is at acpi_get_phys_id+0x264/0x290
> LR is at acpi_get_phys_id+0x178/0x290

IIRC, this is because Linux will consider the region as non-RAM (see
acpi_os_ioremap in arch/arm64/include/asm/acpi.h).

IHMO this is not a problem in the design but a bug in Linux/Xen.

You need to see how to make Linux see the region as a RAM (either by
early_init_dt_add_memory_arch or memblock_reserve). This would mean
either change the way you describe the region in the UEFI memory map or
fix Linux.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01 13:40                 ` Julien Grall
@ 2015-09-01 14:03                   ` Jan Beulich
  2015-09-01 14:20                     ` Julien Grall
  2015-09-02  6:02                   ` Shannon Zhao
  1 sibling, 1 reply; 43+ messages in thread
From: Jan Beulich @ 2015-09-01 14:03 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Boris Ostrovsky, xen-devel,
	Parth Dixit, ChristofferDall, RogerPau Monne

>>> On 01.09.15 at 15:40, <julien.grall@citrix.com> wrote:
> On 01/09/15 13:35, Shannon Zhao wrote:

Shannon, Julien,

since the pattern continues and continues without anyone noticing:
Would you please stop sending at least detail discussions like what
has been going on for the last several rounds To everyone, instead
of just Cc-ing people to whom the original mail was addressed
(which by itself was already questionable)? I'm not sure about
others, but my incoming mail rules distinguish between mail sent to
me and mail I'm only being Cc-ed on. But regardless of that I think
it's bad practice (unless there are exceptional circumstances) to
have extremely wide To lists...

It's certainly not just you ignoring the distinction between To and
Cc, but the repeated occurrence finally forced me to point this out.

Thanks, Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01 14:03                   ` Jan Beulich
@ 2015-09-01 14:20                     ` Julien Grall
  0 siblings, 0 replies; 43+ messages in thread
From: Julien Grall @ 2015-09-01 14:20 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, Huangpeng (Peter),
	andrew, Stefano Stabellini, David Vrabel, Boris Ostrovsky,
	xen-devel, Parth Dixit, ChristofferDall, RogerPau Monne

On 01/09/15 15:03, Jan Beulich wrote:
>>>> On 01.09.15 at 15:40, <julien.grall@citrix.com> wrote:
>> On 01/09/15 13:35, Shannon Zhao wrote:
> 
> Shannon, Julien,
> 
> since the pattern continues and continues without anyone noticing:
> Would you please stop sending at least detail discussions like what
> has been going on for the last several rounds To everyone, instead
> of just Cc-ing people to whom the original mail was addressed
> (which by itself was already questionable)? I'm not sure about
> others, but my incoming mail rules distinguish between mail sent to
> me and mail I'm only being Cc-ed on. But regardless of that I think
> it's bad practice (unless there are exceptional circumstances) to
> have extremely wide To lists...

IHMO, cc-ing should contain the person least concern about this subject.
I.e having his point of view is not necessary.

In this case, most of the change are requested by you and the detail
related on how to do it concerns you as you may find another way to do it.

So it looks sensible to me having you in the "to".

Although, I agree that those details should be taken in a different
thread and not the design document.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-01 13:40                 ` Julien Grall
  2015-09-01 14:03                   ` Jan Beulich
@ 2015-09-02  6:02                   ` Shannon Zhao
  2015-09-02  8:41                     ` Jan Beulich
  2015-09-02 11:09                     ` Julien Grall
  1 sibling, 2 replies; 43+ messages in thread
From: Shannon Zhao @ 2015-09-02  6:02 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Jan Beulich



On 2015/9/1 21:40, Julien Grall wrote:
> On 01/09/15 13:35, Shannon Zhao wrote:
>>
>>
>> On 2015/9/1 19:28, Julien Grall wrote:
>>> Hi Shannon,
>>> On 01/09/15 05:12, Shannon Zhao wrote:
>>>> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
>>>> the address where these tables are mapped to Dom0. But the value of
>>>> gnttab_start is lower than the start of RAM, so Dom0 ingore these
>>>> regions and boot failed. see early_init_dt_add_memory_arch()
>>>
>>> Can you elaborate? How Linux will fail? If this region is marked as
>>> reserved in the UEFI memory map, Linux will mark the memory as reserved.
>>>
>>> Furthermore, *ioremap is used in order to map the EFI tables so I don't
>>> see a reason to fail.
>>>
>>
>> It's fine to parse EFI table but fails to parse ACPI table.
>>
>> It doesn't add the memblock since it doesn't pass below check in
>> early_init_dt_add_memory_arch:
>> 	if (base + size < phys_offset) {
>> 		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
>> 			   base, base + size);
>> 		return;
>> 	}
>>
>> It's due to kinfo->gnttab_start (e.g. 0x87e00000) lower than the memory
>> start address (e.g. 0x90000000).
>>
>> Then Linux will fail at parsing ACPI table.
>>
>> ACPI: Interpreter enabled
>> ACPI: Using GIC for interrupt routing
>> Unhandled fault: alignment fault (0x96000021) at 0xffffff8000068184
>> Internal error: : 96000021 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc6+ #143
>> Hardware name: (null) (DT)
>> task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
>> PC is at acpi_get_phys_id+0x264/0x290
>> LR is at acpi_get_phys_id+0x178/0x290
> 
> IIRC, this is because Linux will consider the region as non-RAM (see
> acpi_os_ioremap in arch/arm64/include/asm/acpi.h).
> 
> IHMO this is not a problem in the design but a bug in Linux/Xen.
> 
> You need to see how to make Linux see the region as a RAM (either by
> early_init_dt_add_memory_arch or memblock_reserve). This would mean
> either change the way you describe the region in the UEFI memory map or
> fix Linux.
> 
There are some descriptions in Documentation/arm64/booting.txt of Linux:

"The Image must be placed text_offset bytes from a 2MB aligned base
address near the start of usable system RAM and called there. Memory
below that base address is currently unusable by Linux, and therefore it
is strongly recommended that this location is the start of system RAM.
At least image_size bytes from the start of the image must be free for
use by the kernel."

>From this, it says "Memory below that base address is currently unusable
by Linux". So if we put these tables below Dom0 RAM address and even
describe these regions as RAM, the Linux could not use them.

Any thoughts about this?

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  6:02                   ` Shannon Zhao
@ 2015-09-02  8:41                     ` Jan Beulich
  2015-09-02  9:18                       ` Christoffer Dall
  2015-09-02  9:25                       ` Shannon Zhao
  2015-09-02 11:09                     ` Julien Grall
  1 sibling, 2 replies; 43+ messages in thread
From: Jan Beulich @ 2015-09-02  8:41 UTC (permalink / raw)
  To: zhaoshenglong
  Cc: hangaohuai, ian.campbell, stefano.stabellini, shannon.zhao,
	andrew, peter.huangpeng, julien.grall, stefano.stabellini,
	david.vrabel, boris.ostrovsky, xen-devel, parth.dixit,
	christoffer.dall, roger.pau

>>> Shannon Zhao <zhaoshenglong@huawei.com> 09/02/15 8:03 AM >>>
>There are some descriptions in Documentation/arm64/booting.txt of Linux:
>
>"The Image must be placed text_offset bytes from a 2MB aligned base
>address near the start of usable system RAM and called there. Memory
>below that base address is currently unusable by Linux, and therefore it
>is strongly recommended that this location is the start of system RAM.
>At least image_size bytes from the start of the image must be free for
>use by the kernel."
>
>From this, it says "Memory below that base address is currently unusable
>by Linux". So if we put these tables below Dom0 RAM address and even
>describe these regions as RAM, the Linux could not use them.

May I remind you that a design should not take specific guest OS
implementation details (which even for that one OS may change over time)
as the basis for decisions?

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  8:41                     ` Jan Beulich
@ 2015-09-02  9:18                       ` Christoffer Dall
  2015-09-02 11:15                         ` Julien Grall
  2015-09-02  9:25                       ` Shannon Zhao
  1 sibling, 1 reply; 43+ messages in thread
From: Christoffer Dall @ 2015-09-02  9:18 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, ian.campbell, stefano.stabellini, shannon.zhao,
	andrew, peter.huangpeng, julien.grall, stefano.stabellini,
	david.vrabel, zhaoshenglong, boris.ostrovsky, xen-devel,
	parth.dixit, roger.pau

Hi Jan,

On Wed, Sep 02, 2015 at 02:41:36AM -0600, Jan Beulich wrote:
> >>> Shannon Zhao <zhaoshenglong@huawei.com> 09/02/15 8:03 AM >>>
> >There are some descriptions in Documentation/arm64/booting.txt of Linux:
> >
> >"The Image must be placed text_offset bytes from a 2MB aligned base
> >address near the start of usable system RAM and called there. Memory
> >below that base address is currently unusable by Linux, and therefore it
> >is strongly recommended that this location is the start of system RAM.
> >At least image_size bytes from the start of the image must be free for
> >use by the kernel."
> >
> >From this, it says "Memory below that base address is currently unusable
> >by Linux". So if we put these tables below Dom0 RAM address and even
> >describe these regions as RAM, the Linux could not use them.
> 
> May I remind you that a design should not take specific guest OS
> implementation details (which even for that one OS may change over time)
> as the basis for decisions?
> 
While I agree that the guest behavior should not dictate an unfortunate
design, surely factoring in the behavior of the expected guests in the
design is a reasonable thing to do?

Changing the boot requirements of Linux for an architecture is a really
invasive change, IMHO, and should be avoided if possible.

Are there other acceptable solutions for placing the EFI tables
somewhere else that would work?

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  8:41                     ` Jan Beulich
  2015-09-02  9:18                       ` Christoffer Dall
@ 2015-09-02  9:25                       ` Shannon Zhao
  2015-09-02 12:30                         ` Jan Beulich
  1 sibling, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-02  9:25 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, ian.campbell, stefano.stabellini, shannon.zhao,
	andrew, peter.huangpeng, julien.grall, stefano.stabellini,
	david.vrabel, boris.ostrovsky, xen-devel, parth.dixit,
	christoffer.dall, roger.pau



On 2015/9/2 16:41, Jan Beulich wrote:
>>>> Shannon Zhao <zhaoshenglong@huawei.com> 09/02/15 8:03 AM >>>
>> >There are some descriptions in Documentation/arm64/booting.txt of Linux:
>> >
>> >"The Image must be placed text_offset bytes from a 2MB aligned base
>> >address near the start of usable system RAM and called there. Memory
>> >below that base address is currently unusable by Linux, and therefore it
>> >is strongly recommended that this location is the start of system RAM.
>> >At least image_size bytes from the start of the image must be free for
>> >use by the kernel."
>> >
>>From this, it says "Memory below that base address is currently unusable
>> >by Linux". So if we put these tables below Dom0 RAM address and even
>> >describe these regions as RAM, the Linux could not use them.
> May I remind you that a design should not take specific guest OS
> implementation details (which even for that one OS may change over time)
> as the basis for decisions?

Yeah, but I think it needs to evaluate the design to check if it's
feasible and make people believe this design could work by giving an
example. Otherwise, if the design could not work well with some guest
OSes or it needs to do a lot of changes, maybe we could choose another
way to do it with no or less changes.

-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  6:02                   ` Shannon Zhao
  2015-09-02  8:41                     ` Jan Beulich
@ 2015-09-02 11:09                     ` Julien Grall
  2015-09-02 12:02                       ` Shannon Zhao
  1 sibling, 1 reply; 43+ messages in thread
From: Julien Grall @ 2015-09-02 11:09 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel, Christoffer Dall,
	Ian Campbell, Stefano Stabellini, Stefano Stabellini,
	Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Jan Beulich

On 02/09/15 07:02, Shannon Zhao wrote:
> 
> 
> On 2015/9/1 21:40, Julien Grall wrote:
>> On 01/09/15 13:35, Shannon Zhao wrote:
>>>
>>>
>>> On 2015/9/1 19:28, Julien Grall wrote:
>>>> Hi Shannon,
>>>> On 01/09/15 05:12, Shannon Zhao wrote:
>>>>> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
>>>>> the address where these tables are mapped to Dom0. But the value of
>>>>> gnttab_start is lower than the start of RAM, so Dom0 ingore these
>>>>> regions and boot failed. see early_init_dt_add_memory_arch()
>>>>
>>>> Can you elaborate? How Linux will fail? If this region is marked as
>>>> reserved in the UEFI memory map, Linux will mark the memory as reserved.
>>>>
>>>> Furthermore, *ioremap is used in order to map the EFI tables so I don't
>>>> see a reason to fail.
>>>>
>>>
>>> It's fine to parse EFI table but fails to parse ACPI table.
>>>
>>> It doesn't add the memblock since it doesn't pass below check in
>>> early_init_dt_add_memory_arch:
>>> 	if (base + size < phys_offset) {
>>> 		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
>>> 			   base, base + size);
>>> 		return;
>>> 	}
>>>
>>> It's due to kinfo->gnttab_start (e.g. 0x87e00000) lower than the memory
>>> start address (e.g. 0x90000000).
>>>
>>> Then Linux will fail at parsing ACPI table.
>>>
>>> ACPI: Interpreter enabled
>>> ACPI: Using GIC for interrupt routing
>>> Unhandled fault: alignment fault (0x96000021) at 0xffffff8000068184
>>> Internal error: : 96000021 [#1] PREEMPT SMP
>>> Modules linked in:
>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc6+ #143
>>> Hardware name: (null) (DT)
>>> task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
>>> PC is at acpi_get_phys_id+0x264/0x290
>>> LR is at acpi_get_phys_id+0x178/0x290
>>
>> IIRC, this is because Linux will consider the region as non-RAM (see
>> acpi_os_ioremap in arch/arm64/include/asm/acpi.h).
>>
>> IHMO this is not a problem in the design but a bug in Linux/Xen.
>>
>> You need to see how to make Linux see the region as a RAM (either by
>> early_init_dt_add_memory_arch or memblock_reserve). This would mean
>> either change the way you describe the region in the UEFI memory map or
>> fix Linux.
>>
> There are some descriptions in Documentation/arm64/booting.txt of Linux:
> 
> "The Image must be placed text_offset bytes from a 2MB aligned base
> address near the start of usable system RAM and called there. Memory
> below that base address is currently unusable by Linux, and therefore it
> is strongly recommended that this location is the start of system RAM.
> At least image_size bytes from the start of the image must be free for
> use by the kernel."
> 
> From this, it says "Memory below that base address is currently unusable
> by Linux". So if we put these tables below Dom0 RAM address and even
> describe these regions as RAM, the Linux could not use them.
> 
> Any thoughts about this?

Hold on, this is about Linux able to use the memory for his own usage.
ACPI table are not part of this memory because they are marked reserved
by the firmware.

If we follow your logic, all ACPI tables always should be above the
kernel. I don't believe this is the case and it would be buggy on Xen
because of the DOM0 direct RAM mapping (i.e the first RAM bank can be
very high and the kernel too).

I think the problem is how you reserved this region in the EFI memory
table. From what I saw, you marked this new memory with EFI_MEMORY_WB
(which means that the region can be usable by Linux).

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  9:18                       ` Christoffer Dall
@ 2015-09-02 11:15                         ` Julien Grall
  0 siblings, 0 replies; 43+ messages in thread
From: Julien Grall @ 2015-09-02 11:15 UTC (permalink / raw)
  To: Christoffer Dall, Jan Beulich
  Cc: hangaohuai, ian.campbell, stefano.stabellini, shannon.zhao,
	andrew, peter.huangpeng, stefano.stabellini, david.vrabel,
	zhaoshenglong, boris.ostrovsky, xen-devel, parth.dixit,
	roger.pau

Hi Shannon,

On 02/09/15 10:18, Christoffer Dall wrote:
> On Wed, Sep 02, 2015 at 02:41:36AM -0600, Jan Beulich wrote:
>>>>> Shannon Zhao <zhaoshenglong@huawei.com> 09/02/15 8:03 AM >>>
>>> There are some descriptions in Documentation/arm64/booting.txt of Linux:
>>>
>>> "The Image must be placed text_offset bytes from a 2MB aligned base
>>> address near the start of usable system RAM and called there. Memory
>>> below that base address is currently unusable by Linux, and therefore it
>>> is strongly recommended that this location is the start of system RAM.
>>> At least image_size bytes from the start of the image must be free for
>>> use by the kernel."
>>>
>> >From this, it says "Memory below that base address is currently unusable
>>> by Linux". So if we put these tables below Dom0 RAM address and even
>>> describe these regions as RAM, the Linux could not use them.
>>
>> May I remind you that a design should not take specific guest OS
>> implementation details (which even for that one OS may change over time)
>> as the basis for decisions?
>>
> While I agree that the guest behavior should not dictate an unfortunate
> design, surely factoring in the behavior of the expected guests in the
> design is a reasonable thing to do?
> 
> Changing the boot requirements of Linux for an architecture is a really
> invasive change, IMHO, and should be avoided if possible.
> 
> Are there other acceptable solutions for placing the EFI tables
> somewhere else that would work?

Before speaking about other acceptable solutions, I'd like to have more
information about where the ACPI tables are living in memory on bare
metal.

Is it mandatory to have the ACPI tables living above the kernel Image
in the RAM?

As mentioned to Shannon, I would be surprised this is the case given
that ACPI should not live in memory usable by the kernel.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 11:09                     ` Julien Grall
@ 2015-09-02 12:02                       ` Shannon Zhao
  2015-09-02 12:52                         ` Julien Grall
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-02 12:02 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, xen-devel
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Jan Beulich, Boris Ostrovsky,
	Parth Dixit, Christoffer Dall, Roger Pau Monne



On 2015/9/2 19:09, Julien Grall wrote:
> On 02/09/15 07:02, Shannon Zhao wrote:
>>
>>
>> On 2015/9/1 21:40, Julien Grall wrote:
>>> On 01/09/15 13:35, Shannon Zhao wrote:
>>>>
>>>>
>>>> On 2015/9/1 19:28, Julien Grall wrote:
>>>>> Hi Shannon,
>>>>> On 01/09/15 05:12, Shannon Zhao wrote:
>>>>>> I tried this. Directly use the "kinfo->gnttab_start = __pa(_stext)" as
>>>>>> the address where these tables are mapped to Dom0. But the value of
>>>>>> gnttab_start is lower than the start of RAM, so Dom0 ingore these
>>>>>> regions and boot failed. see early_init_dt_add_memory_arch()
>>>>>
>>>>> Can you elaborate? How Linux will fail? If this region is marked as
>>>>> reserved in the UEFI memory map, Linux will mark the memory as reserved.
>>>>>
>>>>> Furthermore, *ioremap is used in order to map the EFI tables so I don't
>>>>> see a reason to fail.
>>>>>
>>>>
>>>> It's fine to parse EFI table but fails to parse ACPI table.
>>>>
>>>> It doesn't add the memblock since it doesn't pass below check in
>>>> early_init_dt_add_memory_arch:
>>>> 	if (base + size < phys_offset) {
>>>> 		pr_warning("Ignoring memory block 0x%llx - 0x%llx\n",
>>>> 			   base, base + size);
>>>> 		return;
>>>> 	}
>>>>
>>>> It's due to kinfo->gnttab_start (e.g. 0x87e00000) lower than the memory
>>>> start address (e.g. 0x90000000).
>>>>
>>>> Then Linux will fail at parsing ACPI table.
>>>>
>>>> ACPI: Interpreter enabled
>>>> ACPI: Using GIC for interrupt routing
>>>> Unhandled fault: alignment fault (0x96000021) at 0xffffff8000068184
>>>> Internal error: : 96000021 [#1] PREEMPT SMP
>>>> Modules linked in:
>>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc6+ #143
>>>> Hardware name: (null) (DT)
>>>> task: ffffffc008870000 ti: ffffffc00884c000 task.ti: ffffffc00884c000
>>>> PC is at acpi_get_phys_id+0x264/0x290
>>>> LR is at acpi_get_phys_id+0x178/0x290
>>>
>>> IIRC, this is because Linux will consider the region as non-RAM (see
>>> acpi_os_ioremap in arch/arm64/include/asm/acpi.h).
>>>
>>> IHMO this is not a problem in the design but a bug in Linux/Xen.
>>>
>>> You need to see how to make Linux see the region as a RAM (either by
>>> early_init_dt_add_memory_arch or memblock_reserve). This would mean
>>> either change the way you describe the region in the UEFI memory map or
>>> fix Linux.
>>>
>> There are some descriptions in Documentation/arm64/booting.txt of Linux:
>>
>> "The Image must be placed text_offset bytes from a 2MB aligned base
>> address near the start of usable system RAM and called there. Memory
>> below that base address is currently unusable by Linux, and therefore it
>> is strongly recommended that this location is the start of system RAM.
>> At least image_size bytes from the start of the image must be free for
>> use by the kernel."
>>
>>  From this, it says "Memory below that base address is currently unusable
>> by Linux". So if we put these tables below Dom0 RAM address and even
>> describe these regions as RAM, the Linux could not use them.
>>
>> Any thoughts about this?
>
> Hold on, this is about Linux able to use the memory for his own usage.
> ACPI table are not part of this memory because they are marked reserved
> by the firmware.
>
> If we follow your logic, all ACPI tables always should be above the
> kernel. I don't believe this is the case and it would be buggy on Xen
> because of the DOM0 direct RAM mapping (i.e the first RAM bank can be
> very high and the kernel too).
>

It looks weird. But from the booting.txt, it says the memory below base 
address is unusable and from early_init_dt_add_memory_arch in Linux, it 
really ignores the memblock below the PAGE_OFFSET.

> I think the problem is how you reserved this region in the EFI memory
> table. From what I saw, you marked this new memory with EFI_MEMORY_WB
> (which means that the region can be usable by Linux).
>
Yes, I mark it with EFI_MEMORY_WB. Is this right?

-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-28  9:45 Design doc of adding ACPI support for arm64 on Xen - version 5 Shannon Zhao
  2015-08-28 12:55 ` Julien Grall
  2015-08-28 15:06 ` Jan Beulich
@ 2015-09-02 12:18 ` Ian Campbell
  2015-09-07  3:37   ` Shannon Zhao
  2 siblings, 1 reply; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 12:18 UTC (permalink / raw)
  To: Shannon Zhao, xen-devel, Christoffer Dall, Stefano Stabellini,
	Julien Grall, Stefano Stabellini, Jan Beulich, Parth Dixit,
	andrew, Boris Ostrovsky, David Vrabel, Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Shannon Zhao

On Fri, 2015-08-28 at 17:45 +0800, Shannon Zhao wrote:
> 
> 1. Create minimal DT to pass required information to Dom0
> ----------------------------------------------------------
> The UEFI stub is a feature that extends the Image/zImage into a valid
> UEFI PE/COFF executable, including a loader application that makes it
> possible to load the kernel directly from the UEFI shell, boot menu, or
> one of the lightweight bootloaders like Gummiboot or rEFInd.
> The kernel image built with stub support remains a valid kernel image
> for booting in non-UEFI environments and the UEFI stub will be jumped
> over for non-UEFI environments.
> 
> When booting in UEFI mode, the UEFI stub will create a minimal DT in
> order to pass the command line and other informations (such as the EFI
> memory table) to the kernel. And when booting with ACPI, kernel will get
> command line, ACPI root table address and memory map information from
> the minimal DT. Also, it will check if the DT contains only the /chosen
> node to know whether it boots with DT or ACPI.
> 
> In addition, the current names of these properties with a "linux,"
> prefix in the minimal DT are Linux specified. It needs to standardize
> them so that other OS(such as FreeBSD) could reuse them in the future.

I mentioned this just now in a reply to an older revision while I was
catching up on my mail backlog but I think it is important enough to
reiterate here on the currently latest version:

We need to discuss this possible standardisation of (some derivative of)
this Linux internal interfaces in the appropriate forums ASAP and come to a
wider agreement that it is acceptable than just here amongst us Xen people.

A large part of this design is predicated on this and we don't want to get
too far down this path only to discover the rest of the world says "No,
thanks".

See my earlier reply at 
http://lists.xen.org/archives/html/xen-devel/2015-09/msg00189.html for some
thoughts as to who we should be talking to.

> So we drop the "linux," prefix of UEFI parameters and change the names
> in Linux kernel as well.
> 
> An example of the minimal DT:
> / {
>     #address-cells = <2>;
>     #size-cells = <1>;
>     chosen {
>         bootargs = "kernel=Image console=hvc0 earlycon=pl011,0x1c090000
> root=/dev/vda2 rw rootfstype=ext4 init=/bin/sh acpi=force";
>         linux,initrd-start = <0xXXXXXXXX>;
>         linux,initrd-end = <0xXXXXXXXX>;
>         uefi-system-table = <0xXXXXXXXX>;
>         uefi-mmap-start = <0xXXXXXXXX>;
>         uefi-mmap-size = <0xXXXXXXXX>;
>         uefi-mmap-desc-size = <0xXXXXXXXX>;
>         uefi-mmap-desc-ver = <0xXXXXXXXX>;
>     };
> };
> 
> For details loook at
> > https://github.com/torvalds/linux/blob/master/Documentation/arm/uefi.txt

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02  9:25                       ` Shannon Zhao
@ 2015-09-02 12:30                         ` Jan Beulich
  0 siblings, 0 replies; 43+ messages in thread
From: Jan Beulich @ 2015-09-02 12:30 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, ian.campbell, stefano.stabellini, shannon.zhao,
	andrew, peter.huangpeng, julien.grall, stefano.stabellini,
	david.vrabel, boris.ostrovsky, xen-devel, parth.dixit,
	christoffer.dall, roger.pau

>>> On 02.09.15 at 11:25, <zhaoshenglong@huawei.com> wrote:

> 
> On 2015/9/2 16:41, Jan Beulich wrote:
>>>>> Shannon Zhao <zhaoshenglong@huawei.com> 09/02/15 8:03 AM >>>
>>> >There are some descriptions in Documentation/arm64/booting.txt of Linux:
>>> >
>>> >"The Image must be placed text_offset bytes from a 2MB aligned base
>>> >address near the start of usable system RAM and called there. Memory
>>> >below that base address is currently unusable by Linux, and therefore it
>>> >is strongly recommended that this location is the start of system RAM.
>>> >At least image_size bytes from the start of the image must be free for
>>> >use by the kernel."
>>> >
>>>From this, it says "Memory below that base address is currently unusable
>>> >by Linux". So if we put these tables below Dom0 RAM address and even
>>> >describe these regions as RAM, the Linux could not use them.
>> May I remind you that a design should not take specific guest OS
>> implementation details (which even for that one OS may change over time)
>> as the basis for decisions?
> 
> Yeah, but I think it needs to evaluate the design to check if it's
> feasible and make people believe this design could work by giving an
> example. Otherwise, if the design could not work well with some guest
> OSes or it needs to do a lot of changes, maybe we could choose another
> way to do it with no or less changes.

I don't mind (and indeed welcome) validating the design against
existing OSes, but what you said above sounded like far more
than that.

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 12:02                       ` Shannon Zhao
@ 2015-09-02 12:52                         ` Julien Grall
  2015-09-02 13:26                           ` Ian Campbell
  0 siblings, 1 reply; 43+ messages in thread
From: Julien Grall @ 2015-09-02 12:52 UTC (permalink / raw)
  To: Shannon Zhao, Shannon Zhao, xen-devel
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, andrew,
	Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Jan Beulich, Boris Ostrovsky,
	Parth Dixit, Christoffer Dall, Roger Pau Monne

On 02/09/15 13:02, Shannon Zhao wrote:
>> Hold on, this is about Linux able to use the memory for his own usage.
>> ACPI table are not part of this memory because they are marked reserved
>> by the firmware.
>>
>> If we follow your logic, all ACPI tables always should be above the
>> kernel. I don't believe this is the case and it would be buggy on Xen
>> because of the DOM0 direct RAM mapping (i.e the first RAM bank can be
>> very high and the kernel too).
>>
> 
> It looks weird. But from the booting.txt, it says the memory below base
> address is unusable and from early_init_dt_add_memory_arch in Linux, it
> really ignores the memblock below the PAGE_OFFSET.

It's unusable in the sense that Linux can't use them to store its own
data. Having the ACPI table outside of the System RAM is valid because
they are marked as reserved and ioremap will be used to map them.

> 
>> I think the problem is how you reserved this region in the EFI memory
>> table. From what I saw, you marked this new memory with EFI_MEMORY_WB
>> (which means that the region can be usable by Linux).
>>
> Yes, I mark it with EFI_MEMORY_WB. Is this right?

I would say no, but it's only because I looked at the kernel code quickly.

You have to looks how ACPI region/UEFI tables are described in the host
EFI memory map and mimicking for the DOM0 EFI memory map.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31  9:40         ` Jan Beulich
  2015-08-31 11:31           ` Shannon Zhao
@ 2015-09-02 12:54           ` Ian Campbell
  2015-09-02 13:59             ` Shannon Zhao
  1 sibling, 1 reply; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 12:54 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao, Shannon Zhao
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne

On Mon, 2015-08-31 at 03:40 -0600, Jan Beulich wrote:
> > 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).

In the DT world we completely ignore the host /cpus/* nodes when creating
dom0's /cpus/ and instead create our own directly. It would seem logical to
me to do the same for the MADR in ACPI world as well.

Above the cpu_interface_number, uid and mpidr are called out as things we
we know how to create/determine ourselves.

For the other fields you propose copying from a host entry in the MADT, but
it would be better where possible to create our own. Looking at ACPI 6.0 I
see for the MADT table and the various ARM related structures I'm not
immediately seeing any fields which Xen shouldn't be able to decide for
itself.

When creating the node we may of course be relying on things (such as d
->arch.vgic or maybe in some small cases gic_hw_ops) which are initialised
(elsewhere) by values from the host, but form the PoV of creating the MADT
this is abstracted away.

So to give some examples:

      * The GIC redistributor addresses come from d
        ->arch.vgic.rdist_regions[i].
      * the "Parking Protocol Version" ought to be set to reflect Xen's
        support for PSCI, not whatever the host says.
      * "Processor Power Efficiency Class" ought to be some number made up
        by Xen for all processors (probably 0), since the power efficiency
        of a VCPU has nothing to do with any particular PCPU and we assume
        (today) that all VCPUs are equivalent.
      * The GICD version field comes from d->arch.vgic.version

What, if any, fields do you now think should be copied directly from the
host MADT?

Ian.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-08-31 12:34         ` Julien Grall
  2015-09-01  4:12           ` Shannon Zhao
@ 2015-09-02 12:58           ` Ian Campbell
  1 sibling, 0 replies; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 12:58 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, Shannon Zhao, xen-devel,
	Christoffer Dall, Stefano Stabellini, Stefano Stabellini,
	Jan Beulich, Parth Dixit, andrew, Boris Ostrovsky, David Vrabel,
	Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter)

On Mon, 2015-08-31 at 13:34 +0100, Julien Grall wrote:
> 
> No, see my answer above. I'm suggesting to re-use the same trick as we 
> do for the grant table region. We know that this region will never be 
> allocated in the DOM0 address space either because of the direct mapping 
> or because it's very unlikely in the case of the non-direct mapping (Xen 
> RAM region is very high).

Slight aside: If/when we ever manage to find a system which doesn't need
1:1 for dom0 RAM and therefore can support this latter case we will
probably need to revisit the selection of the memory region for both the
gnttab and these tables rather than hope it is unlikely they will clash.

But for now using the same mechanism for these tables as we do for the
gnttab region makes sense, and if/when things change the answer will likely
be the same.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 12:52                         ` Julien Grall
@ 2015-09-02 13:26                           ` Ian Campbell
  2015-09-02 13:48                             ` Julien Grall
  0 siblings, 1 reply; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 13:26 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, Shannon Zhao, xen-devel
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Jan Beulich, Boris Ostrovsky,
	Parth Dixit, Christoffer Dall, Roger Pau Monne

On Wed, 2015-09-02 at 13:52 +0100, Julien Grall wrote:
> On 02/09/15 13:02, Shannon Zhao wrote:
> > > Hold on, this is about Linux able to use the memory for his own 
> > > usage.
> > > ACPI table are not part of this memory because they are marked 
> > > reserved
> > > by the firmware.
> > > 
> > > If we follow your logic, all ACPI tables always should be above the
> > > kernel. I don't believe this is the case and it would be buggy on Xen
> > > because of the DOM0 direct RAM mapping (i.e the first RAM bank can be
> > > very high and the kernel too).
> > > 
> > 
> > It looks weird. But from the booting.txt, it says the memory below base
> > address is unusable and from early_init_dt_add_memory_arch in Linux, it
> > really ignores the memblock below the PAGE_OFFSET.
> 
> It's unusable in the sense that Linux can't use them to store its own
> data. Having the ACPI table outside of the System RAM is valid because
> they are marked as reserved and ioremap will be used to map them.

I think a useful way to think of this might be that the the tables are not
stored in something which has, to the OS, RAM semantics.

In that context Linux's statement that "RAM below the kernel image is
unusable" is irrelevant because the tables are not in RAM.

The fact that under the hood the tables may indeed technically be stored in
DRAM, even in the same DIMM as actual "RAM" is somewhat irrelevant, in
theory it could be a ROM, or anything else which can be read by the CPU.

> 
> > 
> > > I think the problem is how you reserved this region in the EFI memory
> > > table. From what I saw, you marked this new memory with EFI_MEMORY_WB
> > > (which means that the region can be usable by Linux).
> > > 
> > Yes, I mark it with EFI_MEMORY_WB. Is this right?
> 
> I would say no, but it's only because I looked at the kernel code 
> quickly.
> 
> You have to looks how ACPI region/UEFI tables are described in the host
> EFI memory map and mimicking for the DOM0 EFI memory map.

Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and not
the mapping attributes which should control whether an OS considers a
region usable? At least until the OS is done parsing tables neither of
those are usable (which implies we want NVS as our type, unless the memory
is intended to be reclaimed by dom0, implying it should own it).

Ian.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:26                           ` Ian Campbell
@ 2015-09-02 13:48                             ` Julien Grall
  2015-09-02 13:54                               ` Ian Campbell
  2015-09-02 14:00                               ` Jan Beulich
  0 siblings, 2 replies; 43+ messages in thread
From: Julien Grall @ 2015-09-02 13:48 UTC (permalink / raw)
  To: Ian Campbell, Shannon Zhao, Shannon Zhao, xen-devel
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Jan Beulich, Boris Ostrovsky,
	Parth Dixit, Christoffer Dall, Roger Pau Monne

On 02/09/15 14:26, Ian Campbell wrote:
>>>> I think the problem is how you reserved this region in the EFI memory
>>>> table. From what I saw, you marked this new memory with EFI_MEMORY_WB
>>>> (which means that the region can be usable by Linux).
>>>>
>>> Yes, I mark it with EFI_MEMORY_WB. Is this right?
>>
>> I would say no, but it's only because I looked at the kernel code 
>> quickly.
>>
>> You have to looks how ACPI region/UEFI tables are described in the host
>> EFI memory map and mimicking for the DOM0 EFI memory map.
> 
> Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and not
> the mapping attributes which should control whether an OS considers a
> region usable? At least until the OS is done parsing tables neither of
> those are usable (which implies we want NVS as our type, unless the memory
> is intended to be reclaimed by dom0, implying it should own it).

It looks like that Linux on ARM64 is considering any region with
EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
reserve_regions in arch/arm64/kernel/efi.c).

At the same time, having WB set for a region that should be read-only
looks like wrong to me.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:48                             ` Julien Grall
@ 2015-09-02 13:54                               ` Ian Campbell
  2015-09-02 13:57                                 ` Christoffer Dall
  2015-09-02 14:00                               ` Jan Beulich
  1 sibling, 1 reply; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 13:54 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao, Shannon Zhao, xen-devel
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Stefano Stabellini, David Vrabel, Jan Beulich, Boris Ostrovsky,
	Parth Dixit, Christoffer Dall, Roger Pau Monne

On Wed, 2015-09-02 at 14:48 +0100, Julien Grall wrote:
> On 02/09/15 14:26, Ian Campbell wrote:
> > > > > I think the problem is how you reserved this region in the EFI 
> > > > > memory
> > > > > table. From what I saw, you marked this new memory with 
> > > > > EFI_MEMORY_WB
> > > > > (which means that the region can be usable by Linux).
> > > > > 
> > > > Yes, I mark it with EFI_MEMORY_WB. Is this right?
> > > 
> > > I would say no, but it's only because I looked at the kernel code 
> > > quickly.
> > > 
> > > You have to looks how ACPI region/UEFI tables are described in the 
> > > host
> > > EFI memory map and mimicking for the DOM0 EFI memory map.
> > 
> > Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and 
> > not
> > the mapping attributes which should control whether an OS considers a
> > region usable? At least until the OS is done parsing tables neither of
> > those are usable (which implies we want NVS as our type, unless the 
> > memory
> > is intended to be reclaimed by dom0, implying it should own it).
> 
> It looks like that Linux on ARM64 is considering any region with
> EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
> reserve_regions in arch/arm64/kernel/efi.c).

It's hard to believe this isn't a bug... It's probably worth asking the
Linux maintainers about this.

On first glance it seems to me that the is_reserve_region check ought to be
before the is_normal_ram one, not vice versa. But what do I know...

> At the same time, having WB set for a region that should be read-only
> looks like wrong to me.

it does seem a bit meaningless at least, if not wrong.

> 
> Regards,
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:54                               ` Ian Campbell
@ 2015-09-02 13:57                                 ` Christoffer Dall
  2015-09-02 15:27                                   ` Leif Lindholm
  0 siblings, 1 reply; 43+ messages in thread
From: Christoffer Dall @ 2015-09-02 13:57 UTC (permalink / raw)
  To: Ian Campbell, Leif Lindholm
  Cc: Huangpeng (Peter),
	Hangaohuai, Stefano Stabellini, andrew, xen-devel, Julien Grall,
	Stefano Stabellini, Shannon Zhao, Jan Beulich, Shannon Zhao,
	Boris Ostrovsky, Roger Pau Monne, Parth Dixit, David Vrabel

On Wed, Sep 2, 2015 at 3:54 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Wed, 2015-09-02 at 14:48 +0100, Julien Grall wrote:
>> On 02/09/15 14:26, Ian Campbell wrote:
>> > > > > I think the problem is how you reserved this region in the EFI
>> > > > > memory
>> > > > > table. From what I saw, you marked this new memory with
>> > > > > EFI_MEMORY_WB
>> > > > > (which means that the region can be usable by Linux).
>> > > > >
>> > > > Yes, I mark it with EFI_MEMORY_WB. Is this right?
>> > >
>> > > I would say no, but it's only because I looked at the kernel code
>> > > quickly.
>> > >
>> > > You have to looks how ACPI region/UEFI tables are described in the
>> > > host
>> > > EFI memory map and mimicking for the DOM0 EFI memory map.
>> >
>> > Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and
>> > not
>> > the mapping attributes which should control whether an OS considers a
>> > region usable? At least until the OS is done parsing tables neither of
>> > those are usable (which implies we want NVS as our type, unless the
>> > memory
>> > is intended to be reclaimed by dom0, implying it should own it).
>>
>> It looks like that Linux on ARM64 is considering any region with
>> EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
>> reserve_regions in arch/arm64/kernel/efi.c).
>
> It's hard to believe this isn't a bug... It's probably worth asking the
> Linux maintainers about this.
>

wasn't this that whole workaround to make sure Linux maps the data as
regular RAM, because otherwise architecture generic code would map it
as IO memory, and generic routines such as memcpy would fault on
unaligned accesses, or am I confusing ACPI with EFI here?

Leif (added to the to-field) had some insight on this earlier on.

-Christoffer

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 12:54           ` Ian Campbell
@ 2015-09-02 13:59             ` Shannon Zhao
  2015-09-02 14:24               ` Ian Campbell
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-02 13:59 UTC (permalink / raw)
  To: Ian Campbell, Jan Beulich, Shannon Zhao
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne

Hi Ian,

On 2015/9/2 20:54, Ian Campbell wrote:
> On Mon, 2015-08-31 at 03:40 -0600, Jan Beulich wrote:
>>> 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).
>
> In the DT world we completely ignore the host /cpus/* nodes when creating
> dom0's /cpus/ and instead create our own directly. It would seem logical to
> me to do the same for the MADR in ACPI world as well.
>
> Above the cpu_interface_number, uid and mpidr are called out as things we
> we know how to create/determine ourselves.
>
> For the other fields you propose copying from a host entry in the MADT, but
> it would be better where possible to create our own. Looking at ACPI 6.0 I
> see for the MADT table and the various ARM related structures I'm not
> immediately seeing any fields which Xen shouldn't be able to decide for
> itself.
>
> When creating the node we may of course be relying on things (such as d
> ->arch.vgic or maybe in some small cases gic_hw_ops) which are initialised
> (elsewhere) by values from the host, but form the PoV of creating the MADT
> this is abstracted away.
>
> So to give some examples:
>
>        * The GIC redistributor addresses come from d
>          ->arch.vgic.rdist_regions[i].
>        * the "Parking Protocol Version" ought to be set to reflect Xen's
>          support for PSCI, not whatever the host says.
>        * "Processor Power Efficiency Class" ought to be some number made up
>          by Xen for all processors (probably 0), since the power efficiency
>          of a VCPU has nothing to do with any particular PCPU and we assume
>          (today) that all VCPUs are equivalent.
>        * The GICD version field comes from d->arch.vgic.version
>
> What, if any, fields do you now think should be copied directly from the
> host MADT?
>
Of course we could create the entries according to the information of 
d->arch.vgic. But the contents of d->arch.vgic are also get from the 
host MADT table. It really has a big difference between the two ways?

In addition, I don't object to the way you suggest. Both are fine to me.

Thanks,
-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:48                             ` Julien Grall
  2015-09-02 13:54                               ` Ian Campbell
@ 2015-09-02 14:00                               ` Jan Beulich
  1 sibling, 0 replies; 43+ messages in thread
From: Jan Beulich @ 2015-09-02 14:00 UTC (permalink / raw)
  To: Julien Grall
  Cc: Hangaohuai, Ian Campbell, Stefano Stabellini, Shannon Zhao,
	andrew, Huangpeng (Peter),
	Stefano Stabellini, DavidVrabel, Shannon Zhao, Boris Ostrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, Roger Pau Monne

>>> On 02.09.15 at 15:48, <julien.grall@citrix.com> wrote:
> On 02/09/15 14:26, Ian Campbell wrote:
>>>>> I think the problem is how you reserved this region in the EFI memory
>>>>> table. From what I saw, you marked this new memory with EFI_MEMORY_WB
>>>>> (which means that the region can be usable by Linux).
>>>>>
>>>> Yes, I mark it with EFI_MEMORY_WB. Is this right?
>>>
>>> I would say no, but it's only because I looked at the kernel code 
>>> quickly.
>>>
>>> You have to looks how ACPI region/UEFI tables are described in the host
>>> EFI memory map and mimicking for the DOM0 EFI memory map.
>> 
>> Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and not
>> the mapping attributes which should control whether an OS considers a
>> region usable? At least until the OS is done parsing tables neither of
>> those are usable (which implies we want NVS as our type, unless the memory
>> is intended to be reclaimed by dom0, implying it should own it).
> 
> It looks like that Linux on ARM64 is considering any region with
> EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
> reserve_regions in arch/arm64/kernel/efi.c).

This should be corrected, as it's certainly not in line with the spec.

Jan

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:59             ` Shannon Zhao
@ 2015-09-02 14:24               ` Ian Campbell
  0 siblings, 0 replies; 43+ messages in thread
From: Ian Campbell @ 2015-09-02 14:24 UTC (permalink / raw)
  To: Shannon Zhao, Jan Beulich, Shannon Zhao
  Cc: Hangaohuai, Stefano Stabellini, andrew, Huangpeng (Peter),
	Julien Grall, Stefano Stabellini, David Vrabel, BorisOstrovsky,
	xen-devel, Parth Dixit, Christoffer Dall, RogerPauMonne

On Wed, 2015-09-02 at 21:59 +0800, Shannon Zhao wrote:
> Hi Ian,
> 
> On 2015/9/2 20:54, Ian Campbell wrote:
> > On Mon, 2015-08-31 at 03:40 -0600, Jan Beulich wrote:
> > > > 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).
> > 
> > In the DT world we completely ignore the host /cpus/* nodes when 
> > creating
> > dom0's /cpus/ and instead create our own directly. It would seem 
> > logical to
> > me to do the same for the MADR in ACPI world as well.
> > 
> > Above the cpu_interface_number, uid and mpidr are called out as things 
> > we
> > we know how to create/determine ourselves.
> > 
> > For the other fields you propose copying from a host entry in the MADT, 
> > but
> > it would be better where possible to create our own. Looking at ACPI 
> > 6.0 I
> > see for the MADT table and the various ARM related structures I'm not
> > immediately seeing any fields which Xen shouldn't be able to decide for
> > itself.
> > 
> > When creating the node we may of course be relying on things (such as d
> > ->arch.vgic or maybe in some small cases gic_hw_ops) which are 
> > initialised
> > (elsewhere) by values from the host, but form the PoV of creating the 
> > MADT
> > this is abstracted away.
> > 
> > So to give some examples:
> > 
> >        * The GIC redistributor addresses come from d
> >          ->arch.vgic.rdist_regions[i].
> >        * the "Parking Protocol Version" ought to be set to reflect 
> > Xen's
> >          support for PSCI, not whatever the host says.
> >        * "Processor Power Efficiency Class" ought to be some number 
> > made up
> >          by Xen for all processors (probably 0), since the power 
> > efficiency
> >          of a VCPU has nothing to do with any particular PCPU and we 
> > assume
> >          (today) that all VCPUs are equivalent.
> >        * The GICD version field comes from d->arch.vgic.version
> > 
> > What, if any, fields do you now think should be copied directly from 
> > the
> > host MADT?
> > 
> Of course we could create the entries according to the information of 
> d->arch.vgic. But the contents of d->arch.vgic are also get from the 
> host MADT table. It really has a big difference between the two ways?

Yes, the difference is in the use of the correct abstractions. Going direct
to the host MADT in the code which builds the dom0 MADT goes around the
abstraction.

> In addition, I don't object to the way you suggest. Both are fine to me.
> 
> Thanks,

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 13:57                                 ` Christoffer Dall
@ 2015-09-02 15:27                                   ` Leif Lindholm
  2015-09-02 15:37                                     ` Ard Biesheuvel
  0 siblings, 1 reply; 43+ messages in thread
From: Leif Lindholm @ 2015-09-02 15:27 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: Huangpeng (Peter),
	Hangaohuai, Ian Campbell, Stefano Stabellini, Ard Biesheuvel,
	andrew, xen-devel, Julien Grall, Stefano Stabellini,
	Shannon Zhao, Jan Beulich, Shannon Zhao, Boris Ostrovsky,
	Roger Pau Monne, Parth Dixit, David Vrabel

On Wed, Sep 02, 2015 at 03:57:51PM +0200, Christoffer Dall wrote:
> On Wed, Sep 2, 2015 at 3:54 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> > On Wed, 2015-09-02 at 14:48 +0100, Julien Grall wrote:
> >> On 02/09/15 14:26, Ian Campbell wrote:
> >> > > > > I think the problem is how you reserved this region in the EFI
> >> > > > > memory
> >> > > > > table. From what I saw, you marked this new memory with
> >> > > > > EFI_MEMORY_WB
> >> > > > > (which means that the region can be usable by Linux).
> >> > > > >
> >> > > > Yes, I mark it with EFI_MEMORY_WB. Is this right?
> >> > >
> >> > > I would say no, but it's only because I looked at the kernel code
> >> > > quickly.
> >> > >
> >> > > You have to looks how ACPI region/UEFI tables are described in the
> >> > > host
> >> > > EFI memory map and mimicking for the DOM0 EFI memory map.
> >> >
> >> > Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and
> >> > not
> >> > the mapping attributes which should control whether an OS considers a
> >> > region usable? At least until the OS is done parsing tables neither of
> >> > those are usable (which implies we want NVS as our type, unless the
> >> > memory
> >> > is intended to be reclaimed by dom0, implying it should own it).

The mapping attributes are checked to see whether a page _could_ be
used as generic RAM or not. is_reserve_region() determines whether it
should.

> >> It looks like that Linux on ARM64 is considering any region with
> >> EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
> >> reserve_regions in arch/arm64/kernel/efi.c).

The current state of things ends up being basically:
    if (EFI_MEMORY_WB)
       memblock_add()
    if (EFI_MEMORY_WB && !reclaimable_region)
       memblock_reserve()

That is, apart from counterintuitive, a bug.
It should be using memblock_remove() instead.

> > It's hard to believe this isn't a bug... It's probably worth asking the
> > Linux maintainers about this.
> 
> wasn't this that whole workaround to make sure Linux maps the data as
> regular RAM, because otherwise architecture generic code would map it
> as IO memory,

I hope not.

> and generic routines such as memcpy would fault on
> unaligned accesses, or am I confusing ACPI with EFI here?

Even ACPI tables should need to be in Normal memory in order to work
as expected.

> Leif (added to the to-field) had some insight on this earlier on.

(Adding Ard as well.)
Ard wrote a series end of last year to clean much of this up, but it's
not been merged:
http://thread.gmane.org/gmane.linux.kernel.efi/5133

We should probably push for this to go in as a bugfix, and those
interested in seeing this can weigh in in public.

/
    Leif

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 15:27                                   ` Leif Lindholm
@ 2015-09-02 15:37                                     ` Ard Biesheuvel
  0 siblings, 0 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2015-09-02 15:37 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Huangpeng (Peter),
	Hangaohuai, Ian Campbell, Stefano Stabellini, andrew, xen-devel,
	Julien Grall, Stefano Stabellini, Shannon Zhao, Jan Beulich,
	Shannon Zhao, Boris Ostrovsky, Roger Pau Monne, Parth Dixit,
	Christoffer Dall, David Vrabel

On 2 September 2015 at 17:27, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Sep 02, 2015 at 03:57:51PM +0200, Christoffer Dall wrote:
>> On Wed, Sep 2, 2015 at 3:54 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
>> > On Wed, 2015-09-02 at 14:48 +0100, Julien Grall wrote:
>> >> On 02/09/15 14:26, Ian Campbell wrote:
>> >> > > > > I think the problem is how you reserved this region in the EFI
>> >> > > > > memory
>> >> > > > > table. From what I saw, you marked this new memory with
>> >> > > > > EFI_MEMORY_WB
>> >> > > > > (which means that the region can be usable by Linux).
>> >> > > > >
>> >> > > > Yes, I mark it with EFI_MEMORY_WB. Is this right?
>> >> > >
>> >> > > I would say no, but it's only because I looked at the kernel code
>> >> > > quickly.
>> >> > >
>> >> > > You have to looks how ACPI region/UEFI tables are described in the
>> >> > > host
>> >> > > EFI memory map and mimicking for the DOM0 EFI memory map.
>> >> >
>> >> > Surely it is the type (EfiACPIReclaimMemory, EfiACPIMemoryNVS etc) and
>> >> > not
>> >> > the mapping attributes which should control whether an OS considers a
>> >> > region usable? At least until the OS is done parsing tables neither of
>> >> > those are usable (which implies we want NVS as our type, unless the
>> >> > memory
>> >> > is intended to be reclaimed by dom0, implying it should own it).
>
> The mapping attributes are checked to see whether a page _could_ be
> used as generic RAM or not. is_reserve_region() determines whether it
> should.
>

Only the following regions are ever considered to be usable as normal memory:

EFI_LOADER_CODE:
EFI_LOADER_DATA:
EFI_BOOT_SERVICES_CODE:
EFI_BOOT_SERVICES_DATA:
EFI_CONVENTIONAL_MEMORY:
EFI_PERSISTENT_MEMORY:

The code in is_reserve_region() reads a bit backwards, but the only
reason for the is_normal_ram() in there is to prevent non-memory
regions (i.e., MMIO regions for the RTC or NOR flash the UEFI uses) to
be memblock_reserve()d.

>> >> It looks like that Linux on ARM64 is considering any region with
>> >> EFI_MEMORY_WB set as normal RAM and will try to add as System RAM (see
>> >> reserve_regions in arch/arm64/kernel/efi.c).
>
> The current state of things ends up being basically:
>     if (EFI_MEMORY_WB)
>        memblock_add()
>     if (EFI_MEMORY_WB && !reclaimable_region)
>        memblock_reserve()
>
> That is, apart from counterintuitive, a bug.
> It should be using memblock_remove() instead.
>

Indeed. It would be much better to never add reserved memory to the
linear mapping this way, but the problem is that we lose all
annotation of this region being backed by memory. I.e., pfn_valid()
will return false, and the ACPI code will map it as a device,
resulting in alignment faults when traversing ACPI tables it contains.

So in the series Leif refers to below, I use the physmem memblock
table to record all regions, use the memory memblock table to only
record the regions available to the OS. That way, we can keep track of
which regions are backed by RAM but not covered by the linear mapping.

>> > It's hard to believe this isn't a bug... It's probably worth asking the
>> > Linux maintainers about this.
>>
>> wasn't this that whole workaround to make sure Linux maps the data as
>> regular RAM, because otherwise architecture generic code would map it
>> as IO memory,
>
> I hope not.
>

Indeed.

>> and generic routines such as memcpy would fault on
>> unaligned accesses, or am I confusing ACPI with EFI here?
>
> Even ACPI tables should need to be in Normal memory in order to work
> as expected.
>
>> Leif (added to the to-field) had some insight on this earlier on.
>
> (Adding Ard as well.)
> Ard wrote a series end of last year to clean much of this up, but it's
> not been merged:
> http://thread.gmane.org/gmane.linux.kernel.efi/5133
>
> We should probably push for this to go in as a bugfix, and those
> interested in seeing this can weigh in in public.
>

I will brush up the series and repost.

-- 
Ard.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-02 12:18 ` Ian Campbell
@ 2015-09-07  3:37   ` Shannon Zhao
  2015-09-07 10:47     ` Stefano Stabellini
  0 siblings, 1 reply; 43+ messages in thread
From: Shannon Zhao @ 2015-09-07  3:37 UTC (permalink / raw)
  To: Ian Campbell, xen-devel, Christoffer Dall, Stefano Stabellini,
	Julien Grall, Stefano Stabellini, Jan Beulich, Parth Dixit,
	andrew, Boris Ostrovsky, David Vrabel, Roger Pau Monne
  Cc: Hangaohuai, Huangpeng (Peter), Shannon Zhao



On 2015/9/2 20:18, Ian Campbell wrote:
> On Fri, 2015-08-28 at 17:45 +0800, Shannon Zhao wrote:
>>
>> 1. Create minimal DT to pass required information to Dom0
>> ----------------------------------------------------------
>> The UEFI stub is a feature that extends the Image/zImage into a valid
>> UEFI PE/COFF executable, including a loader application that makes it
>> possible to load the kernel directly from the UEFI shell, boot menu, or
>> one of the lightweight bootloaders like Gummiboot or rEFInd.
>> The kernel image built with stub support remains a valid kernel image
>> for booting in non-UEFI environments and the UEFI stub will be jumped
>> over for non-UEFI environments.
>>
>> When booting in UEFI mode, the UEFI stub will create a minimal DT in
>> order to pass the command line and other informations (such as the EFI
>> memory table) to the kernel. And when booting with ACPI, kernel will get
>> command line, ACPI root table address and memory map information from
>> the minimal DT. Also, it will check if the DT contains only the /chosen
>> node to know whether it boots with DT or ACPI.
>>
>> In addition, the current names of these properties with a "linux,"
>> prefix in the minimal DT are Linux specified. It needs to standardize
>> them so that other OS(such as FreeBSD) could reuse them in the future.
> 
> I mentioned this just now in a reply to an older revision while I was
> catching up on my mail backlog but I think it is important enough to
> reiterate here on the currently latest version:
> 
> We need to discuss this possible standardisation of (some derivative of)
> this Linux internal interfaces in the appropriate forums ASAP and come to a
> wider agreement that it is acceptable than just here amongst us Xen people.
> 
> A large part of this design is predicated on this and we don't want to get
> too far down this path only to discover the rest of the world says "No,
> thanks".
> 
> See my earlier reply at 
> http://lists.xen.org/archives/html/xen-devel/2015-09/msg00189.html for some
> thoughts as to who we should be talking to.
> 

Can we start with the patch for dropping the prefix "linux," and send it
to Linux kernel ML, Linux arm kernel ML, devicetree-spec and BSD ML?
Explain why it needs to do the change and standardization. Base on this,
we could discuss it with other people from the related fields.

>> So we drop the "linux," prefix of UEFI parameters and change the names
>> in Linux kernel as well.
>>
>> An example of the minimal DT:
>> / {
>>     #address-cells = <2>;
>>     #size-cells = <1>;
>>     chosen {
>>         bootargs = "kernel=Image console=hvc0 earlycon=pl011,0x1c090000
>> root=/dev/vda2 rw rootfstype=ext4 init=/bin/sh acpi=force";
>>         linux,initrd-start = <0xXXXXXXXX>;
>>         linux,initrd-end = <0xXXXXXXXX>;
>>         uefi-system-table = <0xXXXXXXXX>;
>>         uefi-mmap-start = <0xXXXXXXXX>;
>>         uefi-mmap-size = <0xXXXXXXXX>;
>>         uefi-mmap-desc-size = <0xXXXXXXXX>;
>>         uefi-mmap-desc-ver = <0xXXXXXXXX>;
>>     };
>> };
>>
>> For details loook at
>>> https://github.com/torvalds/linux/blob/master/Documentation/arm/uefi.txt
> 
> 
> .
> 

-- 
Shannon

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Design doc of adding ACPI support for arm64 on Xen - version 5
  2015-09-07  3:37   ` Shannon Zhao
@ 2015-09-07 10:47     ` Stefano Stabellini
  0 siblings, 0 replies; 43+ messages in thread
From: Stefano Stabellini @ 2015-09-07 10:47 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: Huangpeng (Peter),
	Hangaohuai, Ian Campbell, Stefano Stabellini, Shannon Zhao,
	andrew, xen-devel, Julien Grall, Stefano Stabellini,
	David Vrabel, Jan Beulich, Boris Ostrovsky, Parth Dixit,
	Christoffer Dall, Roger Pau Monne

On Mon, 7 Sep 2015, Shannon Zhao wrote:
> On 2015/9/2 20:18, Ian Campbell wrote:
> > On Fri, 2015-08-28 at 17:45 +0800, Shannon Zhao wrote:
> >>
> >> 1. Create minimal DT to pass required information to Dom0
> >> ----------------------------------------------------------
> >> The UEFI stub is a feature that extends the Image/zImage into a valid
> >> UEFI PE/COFF executable, including a loader application that makes it
> >> possible to load the kernel directly from the UEFI shell, boot menu, or
> >> one of the lightweight bootloaders like Gummiboot or rEFInd.
> >> The kernel image built with stub support remains a valid kernel image
> >> for booting in non-UEFI environments and the UEFI stub will be jumped
> >> over for non-UEFI environments.
> >>
> >> When booting in UEFI mode, the UEFI stub will create a minimal DT in
> >> order to pass the command line and other informations (such as the EFI
> >> memory table) to the kernel. And when booting with ACPI, kernel will get
> >> command line, ACPI root table address and memory map information from
> >> the minimal DT. Also, it will check if the DT contains only the /chosen
> >> node to know whether it boots with DT or ACPI.
> >>
> >> In addition, the current names of these properties with a "linux,"
> >> prefix in the minimal DT are Linux specified. It needs to standardize
> >> them so that other OS(such as FreeBSD) could reuse them in the future.
> > 
> > I mentioned this just now in a reply to an older revision while I was
> > catching up on my mail backlog but I think it is important enough to
> > reiterate here on the currently latest version:
> > 
> > We need to discuss this possible standardisation of (some derivative of)
> > this Linux internal interfaces in the appropriate forums ASAP and come to a
> > wider agreement that it is acceptable than just here amongst us Xen people.
> > 
> > A large part of this design is predicated on this and we don't want to get
> > too far down this path only to discover the rest of the world says "No,
> > thanks".
> > 
> > See my earlier reply at 
> > http://lists.xen.org/archives/html/xen-devel/2015-09/msg00189.html for some
> > thoughts as to who we should be talking to.
> > 
> 
> Can we start with the patch for dropping the prefix "linux," and send it
> to Linux kernel ML, Linux arm kernel ML, devicetree-spec and BSD ML?
> Explain why it needs to do the change and standardization. Base on this,
> we could discuss it with other people from the related fields.

It looks like a good way forward to me

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2015-09-07 10:47 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-28  9:45 Design doc of adding ACPI support for arm64 on Xen - version 5 Shannon Zhao
2015-08-28 12:55 ` Julien Grall
2015-08-29  1:00   ` Shannon Zhao
2015-08-31  7:33     ` Jan Beulich
2015-08-31 11:44     ` Julien Grall
2015-08-31 12:03       ` Shannon Zhao
2015-08-31 12:34         ` Julien Grall
2015-09-01  4:12           ` Shannon Zhao
2015-09-01 11:28             ` Julien Grall
2015-09-01 12:35               ` Shannon Zhao
2015-09-01 13:40                 ` Julien Grall
2015-09-01 14:03                   ` Jan Beulich
2015-09-01 14:20                     ` Julien Grall
2015-09-02  6:02                   ` Shannon Zhao
2015-09-02  8:41                     ` Jan Beulich
2015-09-02  9:18                       ` Christoffer Dall
2015-09-02 11:15                         ` Julien Grall
2015-09-02  9:25                       ` Shannon Zhao
2015-09-02 12:30                         ` Jan Beulich
2015-09-02 11:09                     ` Julien Grall
2015-09-02 12:02                       ` Shannon Zhao
2015-09-02 12:52                         ` Julien Grall
2015-09-02 13:26                           ` Ian Campbell
2015-09-02 13:48                             ` Julien Grall
2015-09-02 13:54                               ` Ian Campbell
2015-09-02 13:57                                 ` Christoffer Dall
2015-09-02 15:27                                   ` Leif Lindholm
2015-09-02 15:37                                     ` Ard Biesheuvel
2015-09-02 14:00                               ` Jan Beulich
2015-09-02 12:58           ` Ian Campbell
2015-08-28 15:06 ` Jan Beulich
2015-08-29  1:29   ` Shannon Zhao
2015-08-31  7:39     ` Jan Beulich
2015-08-31  8:51       ` Shannon Zhao
2015-08-31  9:40         ` Jan Beulich
2015-08-31 11:31           ` Shannon Zhao
2015-08-31 11:46             ` Jan Beulich
2015-09-02 12:54           ` Ian Campbell
2015-09-02 13:59             ` Shannon Zhao
2015-09-02 14:24               ` Ian Campbell
2015-09-02 12:18 ` Ian Campbell
2015-09-07  3:37   ` Shannon Zhao
2015-09-07 10:47     ` Stefano Stabellini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.