[PATCH 00/13] Add ACPI support for Xen Dom0 on ARM64

[PATCH 00/13] Add ACPI support for Xen Dom0 on ARM64

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

This patch set adds ACPI support for Xen Dom0 on ARM64. The relevant Xen
ACPI on ARM64 design document could be found from [1].

This patch set adds a new FDT node "uefi" under /hypervisor to pass UEFI
information. Introduce a bus notifier of AMBA and Platform bus to map
the new added device's MMIO space. Make Xen domain use
xlated_setup_gnttab_pages to setup grant table and a new hypercall to
get event-channel irq.

Regarding the initialization flow of Linux kernel, it needs to move
xen_early_init() before efi_init(). Then xen_early_init() will check
whether it runs on Xen through the /hypervisor node and efi_init() will
call a new function fdt_find_xen_uefi_params(), to parse those
xen,uefi-* parameters just like the existing efi_get_fdt_params().

And in arm64_enable_runtime_services() it will check whether it runs on
Xen and call another new function xen_efi_runtime_setup() to setup
runtime service instead of efi_native_runtime_setup(). The
xen_efi_runtime_setup() will assign the runtime function pointers with
the functions of driver/xen/efi.c.

And since we pass a /hypervisor node and a /chosen node to Dom0, it
needs to check whether the DTS only contains a /hypervisor node and a
/chosen node in acpi_boot_table_init().

Patches are tested on FVP base model. This patch set can be found from
[2] and the corresponding Xen patches can be found from [3].

Thanks,
Shannon

[1] http://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html
[2] http://git.linaro.org/people/shannon.zhao/linux-mainline.git  ACPI_XEN_ARM_Linux_V1
[3] http://git.linaro.org/people/shannon.zhao/xen.git  ACPI_XEN_ARM_V3

Shannon Zhao (13):
  Xen : Hide UART used by Xen
  xen/grant-table: Move xlated_setup_gnttab_pages to common place
  arm/xen: Use xlated_setup_gnttab_pages to setup grant table
  xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio
  Xen: ARM: Add support for mapping platform device mmio
  Xen: ARM: Add support for mapping amba device mmio
  ARM: Xen: Document UEFI support on Xen ARM virtual platforms
  Xen: EFI: Parse DT parameters for Xen specific UEFI
  ARM: Xen: Initialize Xen specific UEFI runtime services
  ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI
  xen/hvm/params: Add a new dilivery type for event-channel in
    HVM_PARAM_CALLBACK_IRQ
  arm/xen: Get event-channel irq through HVM_PARAM when booting with
    ACPI
  ARM: XEN: Move xen_early_init() before efi_init()

 Documentation/devicetree/bindings/arm/xen.txt |  25 +++++++
 arch/arm/include/asm/xen/hypercall.h          |   1 +
 arch/arm/xen/enlighten.c                      |  96 +++++++++++++++++-------
 arch/arm/xen/hypercall.S                      |   1 +
 arch/arm64/kernel/acpi.c                      |  12 ++-
 arch/arm64/kernel/efi.c                       |  20 +++--
 arch/arm64/kernel/setup.c                     |   2 +-
 arch/arm64/xen/hypercall.S                    |   1 +
 arch/x86/xen/grant-table.c                    |  51 -------------
 drivers/acpi/bus.c                            |  22 ++++++
 drivers/firmware/efi/efi.c                    |  67 +++++++++++++++--
 drivers/xen/Kconfig                           |   2 +-
 drivers/xen/Makefile                          |   2 +
 drivers/xen/amba.c                            |  99 ++++++++++++++++++++++++
 drivers/xen/efi.c                             |  22 ++++++
 drivers/xen/grant-table.c                     |  49 ++++++++++++
 drivers/xen/platform.c                        | 104 ++++++++++++++++++++++++++
 include/xen/grant_table.h                     |   1 +
 include/xen/interface/hvm/params.h            |   5 ++
 include/xen/interface/memory.h                |   1 +
 include/xen/xen-ops.h                         |  10 +++
 21 files changed, 498 insertions(+), 95 deletions(-)
 create mode 100644 drivers/xen/amba.c
 create mode 100644 drivers/xen/platform.c

-- 
2.1.0

[PATCH 01/13] Xen : Hide UART used by Xen

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

According to the STAO table, hide UART used by Xen from Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 drivers/acpi/bus.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a212cef..e93806b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -46,6 +46,7 @@ ACPI_MODULE_NAME("bus");
 struct acpi_device *acpi_root;
 struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
+static u64 uart_addr;
 
 #ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -93,6 +94,16 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
 {
 	acpi_status status;
 
+	if (uart_addr) {
+		u64 addr;
+		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
+					       &addr);
+		if (ACPI_SUCCESS(status) && (addr == uart_addr)) {
+			*sta = 0;
+			return AE_OK;
+		}
+	}
+
 	status = acpi_evaluate_integer(handle, "_STA", NULL, sta);
 	if (ACPI_SUCCESS(status))
 		return AE_OK;
@@ -1069,6 +1080,7 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
 static int __init acpi_init(void)
 {
 	int result;
+	struct acpi_table_stao *stao_ptr;
 
 	if (acpi_disabled) {
 		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
@@ -1081,6 +1093,16 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	acpi_get_table(ACPI_SIG_STAO, 1,
+		       (struct acpi_table_header **)&stao_ptr);
+	if(stao_ptr->ignore_uart)
+	{
+		struct acpi_table_spcr *spcr_ptr;
+		acpi_get_table(ACPI_SIG_SPCR, 1,
+			       (struct acpi_table_header **)&spcr_ptr);
+		uart_addr = spcr_ptr->serial_port.address;
+	}
+
 	init_acpi_device_notify();
 	result = acpi_bus_init();
 	if (result) {
-- 
2.1.0

[PATCH 01/13] Xen : Hide UART used by Xen

[Stefano Stabellini]

CC'ing ACPI maintainers.

Please use ./scripts/get_maintainer.pl to retrieve a list of maintainers
to CC for each patch.


On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> According to the STAO table, hide UART used by Xen from Dom0.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  drivers/acpi/bus.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cef..e93806b 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -46,6 +46,7 @@ ACPI_MODULE_NAME("bus");
>  struct acpi_device *acpi_root;
>  struct proc_dir_entry *acpi_root_dir;
>  EXPORT_SYMBOL(acpi_root_dir);
> +static u64 uart_addr;

I would call this spcr_addr or spcr_uart_addr because there are many
uarts on the system but only one spcr. It might make sense to initialize
this to 0xffffffffffffffff rather than 0, which could actually be a
correct uart address.


>  #ifdef CONFIG_X86
>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> @@ -93,6 +94,16 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
>  {
>  	acpi_status status;
>
> +	if (uart_addr) {
> +		u64 addr;
> +		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
> +					       &addr);
> +		if (ACPI_SUCCESS(status) && (addr == uart_addr)) {
> +			*sta = 0;
> +			return AE_OK;
> +		}
> +	}
> +
>  	status = acpi_evaluate_integer(handle, "_STA", NULL, sta);
>  	if (ACPI_SUCCESS(status))
>  		return AE_OK;
> @@ -1069,6 +1080,7 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
>  static int __init acpi_init(void)
>  {
>  	int result;
> +	struct acpi_table_stao *stao_ptr;
>  
>  	if (acpi_disabled) {
>  		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
> @@ -1081,6 +1093,16 @@ static int __init acpi_init(void)
>  		acpi_kobj = NULL;
>  	}
>  
> +	acpi_get_table(ACPI_SIG_STAO, 1,
> +		       (struct acpi_table_header **)&stao_ptr);
> +	if(stao_ptr->ignore_uart)
> +	{

Please follow the linux coding style. You can use
./scripts/checkpatch.pl to check for coding style errors.


> +		struct acpi_table_spcr *spcr_ptr;
> +		acpi_get_table(ACPI_SIG_SPCR, 1,
> +			       (struct acpi_table_header **)&spcr_ptr);
> +		uart_addr = spcr_ptr->serial_port.address;
> +	}
> +
>  	init_acpi_device_notify();
>  	result = acpi_bus_init();
>  	if (result) {
> -- 
> 2.1.0
> 

[PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Move xlated_setup_gnttab_pages to common place, so it can be reused by
ARM to setup grant table when booting with ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/x86/xen/grant-table.c | 51 ----------------------------------------------
 drivers/xen/grant-table.c  | 49 ++++++++++++++++++++++++++++++++++++++++++++
 include/xen/grant_table.h  |  1 +
 3 files changed, 50 insertions(+), 51 deletions(-)

diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
index 1580e7a..2a99a9a 100644
--- a/arch/x86/xen/grant-table.c
+++ b/arch/x86/xen/grant-table.c
@@ -111,57 +111,6 @@ int arch_gnttab_init(unsigned long nr_shared)
 }
 
 #ifdef CONFIG_XEN_PVH
-#include <xen/balloon.h>
-#include <xen/events.h>
-#include <linux/slab.h>
-static int __init xlated_setup_gnttab_pages(void)
-{
-	struct page **pages;
-	xen_pfn_t *pfns;
-	void *vaddr;
-	int rc;
-	unsigned int i;
-	unsigned long nr_grant_frames = gnttab_max_grant_frames();
-
-	BUG_ON(nr_grant_frames == 0);
-	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
-	if (!pages)
-		return -ENOMEM;
-
-	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
-	if (!pfns) {
-		kfree(pages);
-		return -ENOMEM;
-	}
-	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
-	if (rc) {
-		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
-			nr_grant_frames, rc);
-		kfree(pages);
-		kfree(pfns);
-		return rc;
-	}
-	for (i = 0; i < nr_grant_frames; i++)
-		pfns[i] = page_to_pfn(pages[i]);
-
-	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
-	if (!vaddr) {
-		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
-			nr_grant_frames, rc);
-		free_xenballooned_pages(nr_grant_frames, pages);
-		kfree(pages);
-		kfree(pfns);
-		return -ENOMEM;
-	}
-	kfree(pages);
-
-	xen_auto_xlat_grant_frames.pfn = pfns;
-	xen_auto_xlat_grant_frames.count = nr_grant_frames;
-	xen_auto_xlat_grant_frames.vaddr = vaddr;
-
-	return 0;
-}
-
 static int __init xen_pvh_gnttab_setup(void)
 {
 	if (!xen_pvh_domain())
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 62f591f..c084ba6 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
 }
 EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
 
+int __init xlated_setup_gnttab_pages(void)
+{
+	struct page **pages;
+	xen_pfn_t *pfns;
+	void *vaddr;
+	int rc;
+	unsigned int i;
+	unsigned long nr_grant_frames = gnttab_max_grant_frames();
+
+	BUG_ON(nr_grant_frames == 0);
+	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
+	if (!pages)
+		return -ENOMEM;
+
+	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
+	if (!pfns) {
+		kfree(pages);
+		return -ENOMEM;
+	}
+	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
+	if (rc) {
+		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
+			nr_grant_frames, rc);
+		kfree(pages);
+		kfree(pfns);
+		return rc;
+	}
+	for (i = 0; i < nr_grant_frames; i++)
+		pfns[i] = page_to_pfn(pages[i]);
+
+	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
+	if (!vaddr) {
+		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
+			nr_grant_frames, rc);
+		free_xenballooned_pages(nr_grant_frames, pages);
+		kfree(pages);
+		kfree(pfns);
+		return -ENOMEM;
+	}
+	kfree(pages);
+
+	xen_auto_xlat_grant_frames.pfn = pfns;
+	xen_auto_xlat_grant_frames.count = nr_grant_frames;
+	xen_auto_xlat_grant_frames.vaddr = vaddr;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xlated_setup_gnttab_pages);
+
 void gnttab_free_auto_xlat_frames(void)
 {
 	if (!xen_auto_xlat_grant_frames.count)
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 4478f4b..fa31e66 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -177,6 +177,7 @@ struct grant_frames {
 extern struct grant_frames xen_auto_xlat_grant_frames;
 unsigned int gnttab_max_grant_frames(void);
 int gnttab_setup_auto_xlat_frames(phys_addr_t addr);
+int xlated_setup_gnttab_pages(void);
 void gnttab_free_auto_xlat_frames(void);
 
 #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
-- 
2.1.0

[Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[David Vrabel]

On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move xlated_setup_gnttab_pages to common place, so it can be reused by
> ARM to setup grant table when booting with ACPI.
[...]
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>  }
>  EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>  
> +int __init xlated_setup_gnttab_pages(void)
> +{
> +	struct page **pages;
> +	xen_pfn_t *pfns;
> +	void *vaddr;
> +	int rc;
> +	unsigned int i;
> +	unsigned long nr_grant_frames = gnttab_max_grant_frames();
> +
> +	BUG_ON(nr_grant_frames == 0);
> +	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
> +	if (!pages)
> +		return -ENOMEM;
> +
> +	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
> +	if (!pfns) {
> +		kfree(pages);
> +		return -ENOMEM;
> +	}
> +	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
> +	if (rc) {
> +		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
> +			nr_grant_frames, rc);
> +		kfree(pages);
> +		kfree(pfns);
> +		return rc;
> +	}
> +	for (i = 0; i < nr_grant_frames; i++)
> +		pfns[i] = page_to_pfn(pages[i]);
> +
> +	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
> +	if (!vaddr) {
> +		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
> +			nr_grant_frames, rc);
> +		free_xenballooned_pages(nr_grant_frames, pages);
> +		kfree(pages);
> +		kfree(pfns);
> +		return -ENOMEM;
> +	}
> +	kfree(pages);

Can you move most of this function into drivers/xen/xlate_mmu.c in a
function like:

/**
 * xen_xlate_map_ballooned_pages - map a new set of ballooned pages
 * count: number of GFNs.
 * pages: returns the array of (now) ballooned pages.
 * gfns: returns the array of corresponding GFNs.
 * virt: returns the virtual address of the mapped region.
 *
 * This allocate a set of ballooned pages and maps them into the
 * kernel's address space.
 */
void *xen_xlate_map_ballooned_pages(
    unsigned int count,
    struct page **pages,
    xen_pfn_t **gfns);

David

[Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[Shannon Zhao]

On 2015/11/18 0:02, David Vrabel wrote:
> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Move xlated_setup_gnttab_pages to common place, so it can be reused by
>> ARM to setup grant table when booting with ACPI.
> [...]
>> --- a/drivers/xen/grant-table.c
>> +++ b/drivers/xen/grant-table.c
>> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>>  }
>>  EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>>  
>> +int __init xlated_setup_gnttab_pages(void)
>> +{
>> +	struct page **pages;
>> +	xen_pfn_t *pfns;
>> +	void *vaddr;
>> +	int rc;
>> +	unsigned int i;
>> +	unsigned long nr_grant_frames = gnttab_max_grant_frames();
>> +
>> +	BUG_ON(nr_grant_frames == 0);
>> +	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
>> +	if (!pages)
>> +		return -ENOMEM;
>> +
>> +	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
>> +	if (!pfns) {
>> +		kfree(pages);
>> +		return -ENOMEM;
>> +	}
>> +	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
>> +	if (rc) {
>> +		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
>> +			nr_grant_frames, rc);
>> +		kfree(pages);
>> +		kfree(pfns);
>> +		return rc;
>> +	}
>> +	for (i = 0; i < nr_grant_frames; i++)
>> +		pfns[i] = page_to_pfn(pages[i]);
>> +
>> +	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
>> +	if (!vaddr) {
>> +		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
>> +			nr_grant_frames, rc);
>> +		free_xenballooned_pages(nr_grant_frames, pages);
>> +		kfree(pages);
>> +		kfree(pfns);
>> +		return -ENOMEM;
>> +	}
>> +	kfree(pages);
> 
> Can you move most of this function into drivers/xen/xlate_mmu.c in a
> function like:
> 
> /**
>  * xen_xlate_map_ballooned_pages - map a new set of ballooned pages
>  * count: number of GFNs.
>  * pages: returns the array of (now) ballooned pages.
>  * gfns: returns the array of corresponding GFNs.
>  * virt: returns the virtual address of the mapped region.
>  *
>  * This allocate a set of ballooned pages and maps them into the
>  * kernel's address space.
>  */
> void *xen_xlate_map_ballooned_pages(
>     unsigned int count,
>     struct page **pages,
>     xen_pfn_t **gfns);
> 
Sure, but the parameter "pages" is not necessary I think, since the
pages has been freed in the function and it doesn't need in
xen_auto_xlat_grant_frames either. Right?

Thanks,
-- 
Shannon

[Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[David Vrabel]

On 18/11/15 04:32, Shannon Zhao wrote:
> On 2015/11/18 0:02, David Vrabel wrote:
>> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Move xlated_setup_gnttab_pages to common place, so it can be reused by
>>> ARM to setup grant table when booting with ACPI.
>> [...]
>>> --- a/drivers/xen/grant-table.c
>>> +++ b/drivers/xen/grant-table.c
>>> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>>>  }
>>>  EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>>>  
>>> +int __init xlated_setup_gnttab_pages(void)
>>> +{
>>> +	struct page **pages;
>>> +	xen_pfn_t *pfns;
>>> +	void *vaddr;
>>> +	int rc;
>>> +	unsigned int i;
>>> +	unsigned long nr_grant_frames = gnttab_max_grant_frames();
>>> +
>>> +	BUG_ON(nr_grant_frames == 0);
>>> +	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
>>> +	if (!pages)
>>> +		return -ENOMEM;
>>> +
>>> +	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
>>> +	if (!pfns) {
>>> +		kfree(pages);
>>> +		return -ENOMEM;
>>> +	}
>>> +	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
>>> +	if (rc) {
>>> +		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
>>> +			nr_grant_frames, rc);
>>> +		kfree(pages);
>>> +		kfree(pfns);
>>> +		return rc;
>>> +	}
>>> +	for (i = 0; i < nr_grant_frames; i++)
>>> +		pfns[i] = page_to_pfn(pages[i]);
>>> +
>>> +	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
>>> +	if (!vaddr) {
>>> +		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
>>> +			nr_grant_frames, rc);
>>> +		free_xenballooned_pages(nr_grant_frames, pages);
>>> +		kfree(pages);
>>> +		kfree(pfns);
>>> +		return -ENOMEM;
>>> +	}
>>> +	kfree(pages);
>>
>> Can you move most of this function into drivers/xen/xlate_mmu.c in a
>> function like:
>>
>> /**
>>  * xen_xlate_map_ballooned_pages - map a new set of ballooned pages
>>  * count: number of GFNs.
>>  * pages: returns the array of (now) ballooned pages.
>>  * gfns: returns the array of corresponding GFNs.
>>  * virt: returns the virtual address of the mapped region.
>>  *
>>  * This allocate a set of ballooned pages and maps them into the
>>  * kernel's address space.
>>  */
>> void *xen_xlate_map_ballooned_pages(
>>     unsigned int count,
>>     struct page **pages,
>>     xen_pfn_t **gfns);
>>
> Sure, but the parameter "pages" is not necessary I think, since the
> pages has been freed in the function and it doesn't need in
> xen_auto_xlat_grant_frames either. Right?

Yes.

David

[Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[Julien Grall]

Hi Shannon,

On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 62f591f..c084ba6 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>  }
>  EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>  
> +int __init xlated_setup_gnttab_pages(void)
> +{
> +	struct page **pages;
> +	xen_pfn_t *pfns;
> +	void *vaddr;
> +	int rc;
> +	unsigned int i;
> +	unsigned long nr_grant_frames = gnttab_max_grant_frames();
> +
> +	BUG_ON(nr_grant_frames == 0);
> +	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
> +	if (!pages)
> +		return -ENOMEM;
> +
> +	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
> +	if (!pfns) {
> +		kfree(pages);
> +		return -ENOMEM;
> +	}
> +	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);

Can you rebase your patch on top of linux/master? The 3rd parameters has
been dropped for alloc_xenballooned_pages.

> +	if (rc) {
> +		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
> +			nr_grant_frames, rc);
> +		kfree(pages);
> +		kfree(pfns);
> +		return rc;
> +	}
> +	for (i = 0; i < nr_grant_frames; i++)
> +		pfns[i] = page_to_pfn(pages[i]);

This code needs to be adapt for 64KB page granularity. Please use
page_to_xen_pfn here.

> +
> +	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
> +	if (!vaddr) {
> +		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
> +			nr_grant_frames, rc);
> +		free_xenballooned_pages(nr_grant_frames, pages);
> +		kfree(pages);
> +		kfree(pfns);
> +		return -ENOMEM;
> +	}
> +	kfree(pages);
> +
> +	xen_auto_xlat_grant_frames.pfn = pfns;
> +	xen_auto_xlat_grant_frames.count = nr_grant_frames;
> +	xen_auto_xlat_grant_frames.vaddr = vaddr;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xlated_setup_gnttab_pages);
> +
>  void gnttab_free_auto_xlat_frames(void)
>  {
>  	if (!xen_auto_xlat_grant_frames.count)
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index 4478f4b..fa31e66 100644
> --- a/include/xen/grant_table.h
> +++ b/include/xen/grant_table.h
> @@ -177,6 +177,7 @@ struct grant_frames {
>  extern struct grant_frames xen_auto_xlat_grant_frames;
>  unsigned int gnttab_max_grant_frames(void);
>  int gnttab_setup_auto_xlat_frames(phys_addr_t addr);
> +int xlated_setup_gnttab_pages(void);
>  void gnttab_free_auto_xlat_frames(void);
>  
>  #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
> 

Regards,


-- 
Julien Grall

[Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[Shannon Zhao]

On 2015/11/18 20:25, Julien Grall wrote:
> Hi Shannon,
>
> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
>> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
>> index 62f591f..c084ba6 100644
>> --- a/drivers/xen/grant-table.c
>> +++ b/drivers/xen/grant-table.c
>> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>>   }
>>   EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>>
>> +int __init xlated_setup_gnttab_pages(void)
>> +{
>> +	struct page **pages;
>> +	xen_pfn_t *pfns;
>> +	void *vaddr;
>> +	int rc;
>> +	unsigned int i;
>> +	unsigned long nr_grant_frames = gnttab_max_grant_frames();
>> +
>> +	BUG_ON(nr_grant_frames == 0);
>> +	pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
>> +	if (!pages)
>> +		return -ENOMEM;
>> +
>> +	pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
>> +	if (!pfns) {
>> +		kfree(pages);
>> +		return -ENOMEM;
>> +	}
>> +	rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
>
> Can you rebase your patch on top of linux/master? The 3rd parameters has
> been dropped for alloc_xenballooned_pages.
>
Sure, I rebase this patchset on Linux 4.3 and will rebase on master at 
next version.
>> +	if (rc) {
>> +		pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
>> +			nr_grant_frames, rc);
>> +		kfree(pages);
>> +		kfree(pfns);
>> +		return rc;
>> +	}
>> +	for (i = 0; i < nr_grant_frames; i++)
>> +		pfns[i] = page_to_pfn(pages[i]);
>
> This code needs to be adapt for 64KB page granularity. Please use
> page_to_xen_pfn here.
>
Ok.

>> +
>> +	vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
>> +	if (!vaddr) {
>> +		pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
>> +			nr_grant_frames, rc);
>> +		free_xenballooned_pages(nr_grant_frames, pages);
>> +		kfree(pages);
>> +		kfree(pfns);
>> +		return -ENOMEM;
>> +	}
>> +	kfree(pages);
>> +
>> +	xen_auto_xlat_grant_frames.pfn = pfns;
>> +	xen_auto_xlat_grant_frames.count = nr_grant_frames;
>> +	xen_auto_xlat_grant_frames.vaddr = vaddr;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xlated_setup_gnttab_pages);
>> +
>>   void gnttab_free_auto_xlat_frames(void)
>>   {
>>   	if (!xen_auto_xlat_grant_frames.count)
>> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
>> index 4478f4b..fa31e66 100644
>> --- a/include/xen/grant_table.h
>> +++ b/include/xen/grant_table.h
>> @@ -177,6 +177,7 @@ struct grant_frames {
>>   extern struct grant_frames xen_auto_xlat_grant_frames;
>>   unsigned int gnttab_max_grant_frames(void);
>>   int gnttab_setup_auto_xlat_frames(phys_addr_t addr);
>> +int xlated_setup_gnttab_pages(void);
>>   void gnttab_free_auto_xlat_frames(void);
>>
>>   #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
>>
>
> Regards,
>
>

-- 
Shannon

[PATCH 03/13] arm/xen: Use xlated_setup_gnttab_pages to setup grant table

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Use xlated_setup_gnttab_pages to setup grant table. Then it doesn't rely
on DT or ACPI to pass the start address and size of grant table.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm/xen/enlighten.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index eeeab07..7cb82f7 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -186,18 +186,10 @@ static int __init xen_guest_init(void)
 {
 	struct xen_add_to_physmap xatp;
 	struct shared_info *shared_info_page = NULL;
-	struct resource res;
-	phys_addr_t grant_frames;
 
 	if (!xen_domain())
 		return 0;
 
-	if (of_address_to_resource(xen_node, GRANT_TABLE_PHYSADDR, &res)) {
-		pr_err("Xen grant table base address not found\n");
-		return -ENODEV;
-	}
-	grant_frames = res.start;
-
 	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
 	if (!xen_events_irq) {
 		pr_err("Xen event channel interrupt not found\n");
@@ -232,7 +224,7 @@ static int __init xen_guest_init(void)
 	if (xen_vcpu_info == NULL)
 		return -ENOMEM;
 
-	if (gnttab_setup_auto_xlat_frames(grant_frames)) {
+	if (xlated_setup_gnttab_pages()) {
 		free_percpu(xen_vcpu_info);
 		return -ENOMEM;
 	}
-- 
2.1.0

[PATCH 03/13] arm/xen: Use xlated_setup_gnttab_pages to setup grant table

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Use xlated_setup_gnttab_pages to setup grant table. Then it doesn't rely
> on DT or ACPI to pass the start address and size of grant table.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

It looks good. I'll wait for the final version of
xlated_setup_gnttab_pages before acking it, but it is fine in principle.


>  arch/arm/xen/enlighten.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index eeeab07..7cb82f7 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -186,18 +186,10 @@ static int __init xen_guest_init(void)
>  {
>  	struct xen_add_to_physmap xatp;
>  	struct shared_info *shared_info_page = NULL;
> -	struct resource res;
> -	phys_addr_t grant_frames;
>  
>  	if (!xen_domain())
>  		return 0;
>  
> -	if (of_address_to_resource(xen_node, GRANT_TABLE_PHYSADDR, &res)) {
> -		pr_err("Xen grant table base address not found\n");
> -		return -ENODEV;
> -	}
> -	grant_frames = res.start;
> -
>  	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>  	if (!xen_events_irq) {
>  		pr_err("Xen event channel interrupt not found\n");
> @@ -232,7 +224,7 @@ static int __init xen_guest_init(void)
>  	if (xen_vcpu_info == NULL)
>  		return -ENOMEM;
>  
> -	if (gnttab_setup_auto_xlat_frames(grant_frames)) {
> +	if (xlated_setup_gnttab_pages()) {
>  		free_percpu(xen_vcpu_info);
>  		return -ENOMEM;
>  	}
> -- 
> 2.1.0
> 

[PATCH 04/13] xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 include/xen/interface/memory.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
index 2ecfe4f..9aa8988 100644
--- a/include/xen/interface/memory.h
+++ b/include/xen/interface/memory.h
@@ -160,6 +160,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t);
 #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
 				    * XENMEM_add_to_physmap_range only.
 				    */
+#define XENMAPSPACE_dev_mmio     5 /* device mmio region */
 
 /*
  * Sets the GPFN at which a particular page appears in the specified guest's
-- 
2.1.0

[PATCH 04/13] xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio

[Ard Biesheuvel]

On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>

No empty commit logs please.

> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  include/xen/interface/memory.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
> index 2ecfe4f..9aa8988 100644
> --- a/include/xen/interface/memory.h
> +++ b/include/xen/interface/memory.h
> @@ -160,6 +160,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t);
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
>                                     * XENMEM_add_to_physmap_range only.
>                                     */
> +#define XENMAPSPACE_dev_mmio     5 /* device mmio region */
>
>  /*
>   * Sets the GPFN at which a particular page appears in the specified guest's
> --
> 2.1.0
>

[PATCH 05/13] Xen: ARM: Add support for mapping platform device mmio

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a bus_notifier for platform bus device in order to map the device
mmio regions when DOM0 booting with ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 drivers/xen/Makefile   |   1 +
 drivers/xen/platform.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 drivers/xen/platform.c

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index e293bc5..2f867e7 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -11,6 +11,7 @@ CFLAGS_features.o			:= $(nostackp)
 
 CFLAGS_efi.o				+= -fshort-wchar
 
+dom0-$(CONFIG_ARM64) += platform.o
 dom0-$(CONFIG_PCI) += pci.o
 dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
 dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
diff --git a/drivers/xen/platform.c b/drivers/xen/platform.c
new file mode 100644
index 0000000..0f95e7f
--- /dev/null
+++ b/drivers/xen/platform.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2015, Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * Author: Shannon Zhao <shannon.zhao@linaro.org>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/acpi.h>
+#include <xen/xen.h>
+#include <xen/interface/memory.h>
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+
+static int xen_map_platform_device_mmio(struct platform_device *pdev)
+{
+	int i, rc = 0;
+
+	for (i = 0; i < pdev->num_resources; i++)
+	{
+		struct resource *r = &pdev->resource[i];
+
+		if (resource_type(r) == IORESOURCE_MEM)
+		{
+			int j;
+			int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
+			xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr,
+						   GFP_KERNEL);
+			xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
+						    GFP_KERNEL);
+			int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);
+			struct xen_add_to_physmap_range xatp;
+
+			for (j = 0; j < nr; j++) {
+				gpfns[j] = (r->start >> PAGE_SHIFT) + j;
+				idxs[j] = (r->start >> PAGE_SHIFT) + j;
+				errs[j] = 0;
+			}
+
+			xatp.domid = DOMID_SELF;
+			xatp.size = nr;
+			xatp.space = XENMAPSPACE_dev_mmio;
+
+			set_xen_guest_handle(xatp.gpfns, gpfns);
+			set_xen_guest_handle(xatp.idxs, idxs);
+			set_xen_guest_handle(xatp.errs, errs);
+
+			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range,
+						  &xatp);
+			kfree(gpfns);
+			kfree(idxs);
+			kfree(errs);
+			if (rc != 0)
+				return rc;
+		}
+	}
+
+	return rc;
+}
+
+static int xen_platform_notifier(struct notifier_block *nb,
+				 unsigned long action, void *data)
+{
+	struct platform_device *pdev = to_platform_device(data);
+	int r = 0;
+
+	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
+		r = xen_map_platform_device_mmio(pdev);
+
+	if (r)
+	{
+		dev_err(&pdev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
+			pdev->name);
+		return NOTIFY_OK;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block platform_device_nb = {
+	.notifier_call = xen_platform_notifier,
+};
+
+static int __init register_xen_platform_notifier(void)
+{
+	if (!xen_initial_domain())
+		return 0;
+
+	return bus_register_notifier(&platform_bus_type, &platform_device_nb);
+}
+
+arch_initcall(register_xen_platform_notifier);
-- 
2.1.0

[Xen-devel] [PATCH 05/13] Xen: ARM: Add support for mapping platform device mmio

[David Vrabel]

On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a bus_notifier for platform bus device in order to map the device
> mmio regions when DOM0 booting with ACPI.
[...]
> --- /dev/null
> +++ b/drivers/xen/platform.c
> @@ -0,0 +1,104 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.

"You should have received a copy of the GNU General Public License
along with this program.  If not, see <http://www.gnu.org/licenses/>."

> + *
> + * Author: Shannon Zhao <shannon.zhao@linaro.org>

Omit this since it just becomes stale and git will know the correct
authorship.

> +static int xen_platform_notifier(struct notifier_block *nb,
> +				 unsigned long action, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(data);
> +	int r = 0;
> +
> +	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
> +		r = xen_map_platform_device_mmio(pdev);

Why even register the notifier if acpi_disabled?

> +
> +	if (r)
> +	{
> +		dev_err(&pdev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
> +			pdev->name);
> +		return NOTIFY_OK;

Return NOTIFY_BAD here?  The device will be unusable.

> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block platform_device_nb = {
> +	.notifier_call = xen_platform_notifier,
> +};
> +
> +static int __init register_xen_platform_notifier(void)
> +{
> +	if (!xen_initial_domain())
> +		return 0;
> +
> +	return bus_register_notifier(&platform_bus_type, &platform_device_nb);

Why only platform busses?  What about PCI devices?  Where will they get
added to dom0's physmap?

David

[Xen-devel] [PATCH 05/13] Xen: ARM: Add support for mapping platform device mmio

[Shannon Zhao]

On 2015/11/18 0:32, David Vrabel wrote:
> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a bus_notifier for platform bus device in order to map the device
>> mmio regions when DOM0 booting with ACPI.
> [...]
>> --- /dev/null
>> +++ b/drivers/xen/platform.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright (c) 2015, Linaro Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
>> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> 
> "You should have received a copy of the GNU General Public License
> along with this program.  If not, see <http://www.gnu.org/licenses/>."
> 
>> + *
>> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Omit this since it just becomes stale and git will know the correct
> authorship.
> 
>> +static int xen_platform_notifier(struct notifier_block *nb,
>> +				 unsigned long action, void *data)
>> +{
>> +	struct platform_device *pdev = to_platform_device(data);
>> +	int r = 0;
>> +
>> +	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
>> +		r = xen_map_platform_device_mmio(pdev);
> 
> Why even register the notifier if acpi_disabled?
> 
>> +
>> +	if (r)
>> +	{
>> +		dev_err(&pdev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
>> +			pdev->name);
>> +		return NOTIFY_OK;
> 
> Return NOTIFY_BAD here?  The device will be unusable.
> 
>> +	}
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static struct notifier_block platform_device_nb = {
>> +	.notifier_call = xen_platform_notifier,
>> +};
>> +
>> +static int __init register_xen_platform_notifier(void)
>> +{
>> +	if (!xen_initial_domain())
>> +		return 0;
>> +
>> +	return bus_register_notifier(&platform_bus_type, &platform_device_nb);
> 
> Why only platform busses?  What about PCI devices?  Where will they get
> added to dom0's physmap?
> 
I've said in the design doc[1] that "And for PCI bus devices, Linux
could reuse the existing PCI bus_notifier like X86."

Since currently Xen on ARM doesn't support PCI (IIRC) and if it adds the
support in the future, we could make the codes in drivers/xen/pci.c
common to ARM and x86 at that moment.

Thanks,

[1] http://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html

-- 
Shannon

[Xen-devel] [PATCH 05/13] Xen: ARM: Add support for mapping platform device mmio

[Shannon Zhao]

On 2015/11/17 22:38, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 17, 2015 at 05:57:03PM +0800, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a bus_notifier for platform bus device in order to map the device
>> mmio regions when DOM0 booting with ACPI.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  drivers/xen/Makefile   |   1 +
>>  drivers/xen/platform.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 105 insertions(+)
>>  create mode 100644 drivers/xen/platform.c
>>
>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>> index e293bc5..2f867e7 100644
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -11,6 +11,7 @@ CFLAGS_features.o			:= $(nostackp)
>>  
>>  CFLAGS_efi.o				+= -fshort-wchar
>>  
>> +dom0-$(CONFIG_ARM64) += platform.o
>>  dom0-$(CONFIG_PCI) += pci.o
>>  dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
>>  dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
>> diff --git a/drivers/xen/platform.c b/drivers/xen/platform.c
>> new file mode 100644
>> index 0000000..0f95e7f
>> --- /dev/null
>> +++ b/drivers/xen/platform.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright (c) 2015, Linaro Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
>> + * Place - Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/acpi.h>
>> +#include <xen/xen.h>
>> +#include <xen/interface/memory.h>
>> +#include <asm/xen/hypervisor.h>
>> +#include <asm/xen/hypercall.h>
>> +
>> +static int xen_map_platform_device_mmio(struct platform_device *pdev)
>> +{
>> +	int i, rc = 0;
> 
> unsigned int i;
>> +
>> +	for (i = 0; i < pdev->num_resources; i++)
>> +	{
>> +		struct resource *r = &pdev->resource[i];
>> +
>> +		if (resource_type(r) == IORESOURCE_MEM)
> 
> Make this:
> 
> 	if (resource_type(r) != IORESOURCE_MEM)
> 		continue;
> 
>> +		{
> 
> And then you don't have to move all of this code to the right.
> 
>> +			int j;
> unsigned int
>> +			int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
> 
> unsigned int
>> +			xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr,
>> +						   GFP_KERNEL);
>> +			xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
>> +						    GFP_KERNEL);
>> +			int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);
> 
> You can use kzalloc.
>> +			struct xen_add_to_physmap_range xatp;
> 
> This declaration of variables and then setting does not look like that
> pretty to my mind. (And it is a headache to debug if for example 'nr' ends up
> being zero for example). Or worst -1U since you had it as 'int'.
> 
Yes, a little. Maybe I can add a check for nr. Would that be fine?

Thanks,
-- 
Shannon

[PATCH 05/13] Xen: ARM: Add support for mapping platform device mmio

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a bus_notifier for platform bus device in order to map the device
> mmio regions when DOM0 booting with ACPI.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  drivers/xen/Makefile   |   1 +
>  drivers/xen/platform.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 105 insertions(+)
>  create mode 100644 drivers/xen/platform.c
> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index e293bc5..2f867e7 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -11,6 +11,7 @@ CFLAGS_features.o			:= $(nostackp)
>  
>  CFLAGS_efi.o				+= -fshort-wchar
>  
> +dom0-$(CONFIG_ARM64) += platform.o
>  dom0-$(CONFIG_PCI) += pci.o
>  dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
>  dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
> diff --git a/drivers/xen/platform.c b/drivers/xen/platform.c
> new file mode 100644
> index 0000000..0f95e7f
> --- /dev/null
> +++ b/drivers/xen/platform.c
> @@ -0,0 +1,104 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <xen/xen.h>
> +#include <xen/interface/memory.h>
> +#include <asm/xen/hypervisor.h>
> +#include <asm/xen/hypercall.h>
> +
> +static int xen_map_platform_device_mmio(struct platform_device *pdev)
> +{
> +	int i, rc = 0;
> +
> +	for (i = 0; i < pdev->num_resources; i++)
> +	{
> +		struct resource *r = &pdev->resource[i];
> +
> +		if (resource_type(r) == IORESOURCE_MEM)
> +		{
> +			int j;
> +			int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
> +			xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr,
> +						   GFP_KERNEL);
> +			xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
> +						    GFP_KERNEL);
> +			int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);

please check for kmalloc returning NULL and return -ENOMEM


> +			struct xen_add_to_physmap_range xatp;
> +
> +			for (j = 0; j < nr; j++) {
> +				gpfns[j] = (r->start >> PAGE_SHIFT) + j;
> +				idxs[j] = (r->start >> PAGE_SHIFT) + j;

XEN_PFN_DOWN


> +				errs[j] = 0;
> +			}
> +
> +			xatp.domid = DOMID_SELF;
> +			xatp.size = nr;
> +			xatp.space = XENMAPSPACE_dev_mmio;
> +
> +			set_xen_guest_handle(xatp.gpfns, gpfns);
> +			set_xen_guest_handle(xatp.idxs, idxs);
> +			set_xen_guest_handle(xatp.errs, errs);
> +
> +			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range,
> +						  &xatp);
> +			kfree(gpfns);
> +			kfree(idxs);
> +			kfree(errs);
> +			if (rc != 0)
> +				return rc;
> +		}
> +	}
> +
> +	return rc;
> +}
> +
> +static int xen_platform_notifier(struct notifier_block *nb,
> +				 unsigned long action, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(data);
> +	int r = 0;
> +
> +	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
> +		r = xen_map_platform_device_mmio(pdev);
> +
> +	if (r)
> +	{
> +		dev_err(&pdev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
> +			pdev->name);
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block platform_device_nb = {
> +	.notifier_call = xen_platform_notifier,
> +};
> +
> +static int __init register_xen_platform_notifier(void)
> +{
> +	if (!xen_initial_domain())
> +		return 0;
> +
> +	return bus_register_notifier(&platform_bus_type, &platform_device_nb);
> +}
> +
> +arch_initcall(register_xen_platform_notifier);
> -- 
> 2.1.0
> 

[PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a bus_notifier for AMBA bus device in order to map the device
mmio regions when DOM0 booting with ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 drivers/xen/Makefile |  1 +
 drivers/xen/amba.c   | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 drivers/xen/amba.c

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 2f867e7..139bd0b 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -12,6 +12,7 @@ CFLAGS_features.o			:= $(nostackp)
 CFLAGS_efi.o				+= -fshort-wchar
 
 dom0-$(CONFIG_ARM64) += platform.o
+dom0-$(CONFIG_ARM_AMBA) += amba.o
 dom0-$(CONFIG_PCI) += pci.o
 dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
 dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
diff --git a/drivers/xen/amba.c b/drivers/xen/amba.c
new file mode 100644
index 0000000..e491c8e
--- /dev/null
+++ b/drivers/xen/amba.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2015, Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * Author: Shannon Zhao <shannon.zhao@linaro.org>
+ */
+
+#include <linux/amba/bus.h>
+#include <linux/acpi.h>
+#include <xen/xen.h>
+#include <xen/interface/memory.h>
+#include <asm/xen/hypervisor.h>
+#include <asm/xen/hypercall.h>
+
+static int xen_map_amba_device_mmio(struct amba_device *adev)
+{
+	int rc = 0;
+	struct resource *r = &adev->res;
+
+	if (resource_type(r) == IORESOURCE_MEM)
+	{
+		int j;
+		int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
+		xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr, GFP_KERNEL);
+		xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
+					    GFP_KERNEL);
+		int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);
+		struct xen_add_to_physmap_range xatp;
+
+		for (j = 0; j < nr; j++) {
+			gpfns[j] = (r->start >> PAGE_SHIFT) + j;
+			idxs[j] = (r->start >> PAGE_SHIFT) + j;
+			errs[j] = 0;
+		}
+
+		xatp.domid = DOMID_SELF;
+		xatp.size = nr;
+		xatp.space = XENMAPSPACE_dev_mmio;
+
+		set_xen_guest_handle(xatp.gpfns, gpfns);
+		set_xen_guest_handle(xatp.idxs, idxs);
+		set_xen_guest_handle(xatp.errs, errs);
+
+		rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
+
+		kfree(gpfns);
+		kfree(idxs);
+		kfree(errs);
+		if (rc != 0)
+			return rc;
+	}
+
+	return rc;
+}
+
+static int xen_amba_notifier(struct notifier_block *nb,
+			     unsigned long action, void *data)
+{
+	struct amba_device *adev = to_amba_device(data);
+	int r = 0;
+
+	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
+		r = xen_map_amba_device_mmio(adev);
+
+	if (r)
+	{
+		dev_err(&adev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
+			adev->dev.init_name);
+		return NOTIFY_OK;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block amba_device_nb = {
+	.notifier_call = xen_amba_notifier,
+};
+
+static int __init register_xen_amba_notifier(void)
+{
+	if (!xen_initial_domain())
+		return 0;
+
+	return bus_register_notifier(&amba_bustype, &amba_device_nb);
+}
+
+arch_initcall(register_xen_amba_notifier);
-- 
2.1.0

[Xen-devel] [PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[David Vrabel]

On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a bus_notifier for AMBA bus device in order to map the device
> mmio regions when DOM0 booting with ACPI.
[...]
> +static int xen_map_amba_device_mmio(struct amba_device *adev)
> +{
> +	int rc = 0;
> +	struct resource *r = &adev->res;
> +
> +	if (resource_type(r) == IORESOURCE_MEM)
> +	{

Haven't I seen something like this before...?

This is (almost) identical to the code added for platform devices.  This
needs to be generic.

David

[Xen-devel] [PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[Shannon Zhao]

On 2015/11/17 22:40, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 17, 2015 at 05:57:04PM +0800, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a bus_notifier for AMBA bus device in order to map the device
>> mmio regions when DOM0 booting with ACPI.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  drivers/xen/Makefile |  1 +
>>  drivers/xen/amba.c   | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 100 insertions(+)
>>  create mode 100644 drivers/xen/amba.c
>>
>> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
>> index 2f867e7..139bd0b 100644
>> --- a/drivers/xen/Makefile
>> +++ b/drivers/xen/Makefile
>> @@ -12,6 +12,7 @@ CFLAGS_features.o			:= $(nostackp)
>>  CFLAGS_efi.o				+= -fshort-wchar
>>  
>>  dom0-$(CONFIG_ARM64) += platform.o
>> +dom0-$(CONFIG_ARM_AMBA) += amba.o
>>  dom0-$(CONFIG_PCI) += pci.o
>>  dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
>>  dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
>> diff --git a/drivers/xen/amba.c b/drivers/xen/amba.c
>> new file mode 100644
>> index 0000000..e491c8e
>> --- /dev/null
>> +++ b/drivers/xen/amba.c
>> @@ -0,0 +1,99 @@
>> +/*
>> + * Copyright (c) 2015, Linaro Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
>> + * Place - Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
>> + */
>> +
>> +#include <linux/amba/bus.h>
>> +#include <linux/acpi.h>
>> +#include <xen/xen.h>
>> +#include <xen/interface/memory.h>
>> +#include <asm/xen/hypervisor.h>
>> +#include <asm/xen/hypercall.h>
>> +
>> +static int xen_map_amba_device_mmio(struct amba_device *adev)
>> +{
>> +	int rc = 0;
>> +	struct resource *r = &adev->res;
>> +
>> +	if (resource_type(r) == IORESOURCE_MEM)
> 
> This is like the 'xen_map_platform_device_mmio' code. Why don't you call
> that? Or at least make the 'xen_map_platform_device_mmio' internal parts be
> visible to both of these functions.
> 
Ok, will do.

>> +	{
>> +		int j;
>> +		int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
>> +		xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr, GFP_KERNEL);
>> +		xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
>> +					    GFP_KERNEL);
>> +		int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);
>> +		struct xen_add_to_physmap_range xatp;
>> +
>> +		for (j = 0; j < nr; j++) {
>> +			gpfns[j] = (r->start >> PAGE_SHIFT) + j;
>> +			idxs[j] = (r->start >> PAGE_SHIFT) + j;
>> +			errs[j] = 0;
>> +		}
>> +
>> +		xatp.domid = DOMID_SELF;
>> +		xatp.size = nr;
>> +		xatp.space = XENMAPSPACE_dev_mmio;
>> +
>> +		set_xen_guest_handle(xatp.gpfns, gpfns);
>> +		set_xen_guest_handle(xatp.idxs, idxs);
>> +		set_xen_guest_handle(xatp.errs, errs);
>> +
>> +		rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>> +
>> +		kfree(gpfns);
>> +		kfree(idxs);
>> +		kfree(errs);
>> +		if (rc != 0)
>> +			return rc;
>> +	}
>> +
>> +	return rc;
>> +}
>> +
>> +static int xen_amba_notifier(struct notifier_block *nb,
>> +			     unsigned long action, void *data)
>> +{
>> +	struct amba_device *adev = to_amba_device(data);
>> +	int r = 0;
>> +
>> +	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
>> +		r = xen_map_amba_device_mmio(adev);
>> +
> 
> What about the removal of a bus device? No need to handle that?
> 
I have thought about removal before. I think there is little(or no)
chance for AMBA and platform bus devices to be removed. It's not like
the PCI devices which will be hot-unplug.
Maybe I'm not right. If so, it's fine to add a case to handle the removal.

Thanks,
-- 
Shannon

[Xen-devel] [PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[Julien Grall]

On 18/11/15 06:03, Shannon Zhao wrote:
>>
>> What about the removal of a bus device? No need to handle that?
>>
> I have thought about removal before. I think there is little(or no)
> chance for AMBA and platform bus devices to be removed. It's not like
> the PCI devices which will be hot-unplug.
> Maybe I'm not right. If so, it's fine to add a case to handle the removal.

What about the platform device passthrough case?

In anycase, if the AMBA platform provides a removal method, there is
harmless to implement it.

Regards,

-- 
Julien Grall

[Xen-devel] [PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[Shannon Zhao]

On 2015/11/18 20:27, Julien Grall wrote:
> On 18/11/15 06:03, Shannon Zhao wrote:
>>>
>>> What about the removal of a bus device? No need to handle that?
>>>
>> I have thought about removal before. I think there is little(or no)
>> chance for AMBA and platform bus devices to be removed. It's not like
>> the PCI devices which will be hot-unplug.
>> Maybe I'm not right. If so, it's fine to add a case to handle the removal.
>
> What about the platform device passthrough case?
>
No, it will add the platform passthrough device at booting step while 
it's only removed when Dom0 shutdowns unless it supports platform device 
hot-unplug.

> In anycase, if the AMBA platform provides a removal method, there is
> harmless to implement it.
>
Sure, as you said, it's harmless to add.

Thanks,
-- 
Shannon

[PATCH 06/13] Xen: ARM: Add support for mapping amba device mmio

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a bus_notifier for AMBA bus device in order to map the device
> mmio regions when DOM0 booting with ACPI.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Please use scripts/checkpatch.pl to check the coding style


>  drivers/xen/Makefile |  1 +
>  drivers/xen/amba.c   | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 100 insertions(+)
>  create mode 100644 drivers/xen/amba.c
> 
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index 2f867e7..139bd0b 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -12,6 +12,7 @@ CFLAGS_features.o			:= $(nostackp)
>  CFLAGS_efi.o				+= -fshort-wchar
>  
>  dom0-$(CONFIG_ARM64) += platform.o
> +dom0-$(CONFIG_ARM_AMBA) += amba.o
>  dom0-$(CONFIG_PCI) += pci.o
>  dom0-$(CONFIG_USB_SUPPORT) += dbgp.o
>  dom0-$(CONFIG_XEN_ACPI) += acpi.o $(xen-pad-y)
> diff --git a/drivers/xen/amba.c b/drivers/xen/amba.c
> new file mode 100644
> index 0000000..e491c8e
> --- /dev/null
> +++ b/drivers/xen/amba.c
> @@ -0,0 +1,99 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
> + */
> +
> +#include <linux/amba/bus.h>
> +#include <linux/acpi.h>
> +#include <xen/xen.h>
> +#include <xen/interface/memory.h>
> +#include <asm/xen/hypervisor.h>
> +#include <asm/xen/hypercall.h>
> +
> +static int xen_map_amba_device_mmio(struct amba_device *adev)
> +{
> +	int rc = 0;
> +	struct resource *r = &adev->res;
> +
> +	if (resource_type(r) == IORESOURCE_MEM)
> +	{
> +		int j;
> +		int nr = DIV_ROUND_UP(resource_size(r), PAGE_SIZE);
> +		xen_pfn_t *gpfns = kmalloc(sizeof(xen_pfn_t) * nr, GFP_KERNEL);
> +		xen_ulong_t *idxs = kmalloc(sizeof(xen_ulong_t) * nr,
> +					    GFP_KERNEL);
> +		int *errs = kmalloc(sizeof(int) * nr, GFP_KERNEL);
> +		struct xen_add_to_physmap_range xatp;
> +
> +		for (j = 0; j < nr; j++) {
> +			gpfns[j] = (r->start >> PAGE_SHIFT) + j;
> +			idxs[j] = (r->start >> PAGE_SHIFT) + j;
> +			errs[j] = 0;

same comments as before: use XEN_PFN_DOWN and check for return errors
from kmalloc


> +		}
> +
> +		xatp.domid = DOMID_SELF;
> +		xatp.size = nr;
> +		xatp.space = XENMAPSPACE_dev_mmio;
> +
> +		set_xen_guest_handle(xatp.gpfns, gpfns);
> +		set_xen_guest_handle(xatp.idxs, idxs);
> +		set_xen_guest_handle(xatp.errs, errs);
> +
> +		rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> +
> +		kfree(gpfns);
> +		kfree(idxs);
> +		kfree(errs);
> +		if (rc != 0)
> +			return rc;
> +	}
> +
> +	return rc;
> +}
> +
> +static int xen_amba_notifier(struct notifier_block *nb,
> +			     unsigned long action, void *data)
> +{
> +	struct amba_device *adev = to_amba_device(data);
> +	int r = 0;
> +
> +	if (!acpi_disabled && (action == BUS_NOTIFY_ADD_DEVICE))
> +		r = xen_map_amba_device_mmio(adev);
> +
> +	if (r)
> +	{
> +		dev_err(&adev->dev, "Failed to add_to_physmap device (%s) mmio!\n",
> +			adev->dev.init_name);
> +		return NOTIFY_OK;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block amba_device_nb = {
> +	.notifier_call = xen_amba_notifier,
> +};
> +
> +static int __init register_xen_amba_notifier(void)
> +{
> +	if (!xen_initial_domain())
> +		return 0;
> +
> +	return bus_register_notifier(&amba_bustype, &amba_device_nb);
> +}
> +
> +arch_initcall(register_xen_amba_notifier);
> -- 
> 2.1.0
> 

[PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
scan this to get the UEFI information.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 Documentation/devicetree/bindings/arm/xen.txt | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
index 0f7b9c2..c375d67 100644
--- a/Documentation/devicetree/bindings/arm/xen.txt
+++ b/Documentation/devicetree/bindings/arm/xen.txt
@@ -15,6 +15,24 @@ the following properties:
 - interrupts: the interrupt used by Xen to inject event notifications.
   A GIC node is also required.
 
+To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
+under /hypervisor with following parameters:
+
+________________________________________________________________________________
+Name                      | Size   | Description
+================================================================================
+xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
+                          |        | pointed to in previous entry.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
+                          |        | memory map.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
+--------------------------------------------------------------------------------
 
 Example (assuming #address-cells = <2> and #size-cells = <2>):
 
@@ -22,4 +40,11 @@ hypervisor {
 	compatible = "xen,xen-4.3", "xen,xen";
 	reg = <0 0xb0000000 0 0x20000>;
 	interrupts = <1 15 0xf08>;
+	uefi {
+		xen,uefi-system-table = <0xXXXXXXXX>;
+		xen,uefi-mmap-start = <0xXXXXXXXX>;
+		xen,uefi-mmap-size = <0xXXXXXXXX>;
+		xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
+		xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
+        };
 };
-- 
2.1.0

[Xen-devel] [PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[David Vrabel]

On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
> scan this to get the UEFI information.
[...]
> --- a/Documentation/devicetree/bindings/arm/xen.txt
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -15,6 +15,24 @@ the following properties:
>  - interrupts: the interrupt used by Xen to inject event notifications.
>    A GIC node is also required.
>  
> +To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
> +under /hypervisor with following parameters:

It's not really clear why this is under /hypervisor and why xen needs to
define its own nodes.

The handling of the existing linux standard nodes can be made
Xen-specific by the presence of the /hypervisor/compatible == "xen" node
(or similar), right?

> +________________________________________________________________________________
> +Name                      | Size   | Description
> +================================================================================
> +xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
> +--------------------------------------------------------------------------------

I would probably say "Guest physical address..." just to be clear.

> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
> +                          |        | pointed to in previous entry.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
> +                          |        | memory map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
> +--------------------------------------------------------------------------------

Need a reference to the mmap descriptor format here.

David

[Xen-devel] [PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[Shannon Zhao]

On 2015/11/18 0:46, David Vrabel wrote:
> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
>> scan this to get the UEFI information.
> [...]
>> --- a/Documentation/devicetree/bindings/arm/xen.txt
>> +++ b/Documentation/devicetree/bindings/arm/xen.txt
>> @@ -15,6 +15,24 @@ the following properties:
>>  - interrupts: the interrupt used by Xen to inject event notifications.
>>    A GIC node is also required.
>>  
>> +To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
>> +under /hypervisor with following parameters:
> 
> It's not really clear why this is under /hypervisor and why xen needs to
> define its own nodes.
> 
> The handling of the existing linux standard nodes can be made
> Xen-specific by the presence of the /hypervisor/compatible == "xen" node
> (or similar), right?
> 
Right. The reason is that we want to separate this Xen specific UEFI
from the normal UEFI. Don't make them mixed together.

>> +________________________________________________________________________________
>> +Name                      | Size   | Description
>> +================================================================================
>> +xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
>> +--------------------------------------------------------------------------------
> 
> I would probably say "Guest physical address..." just to be clear.
> 
>> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
>> +                          |        | pointed to in previous entry.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
>> +                          |        | memory map.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
>> +--------------------------------------------------------------------------------
> 
> Need a reference to the mmap descriptor format here.
> 
> David
> 
> .
> 

-- 
Shannon

[Xen-devel] [PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[Stefano Stabellini]

On Tue, 17 Nov 2015, David Vrabel wrote:
> On 17/11/15 09:57, shannon.zhao at linaro.org wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
> > scan this to get the UEFI information.
> [...]
> > --- a/Documentation/devicetree/bindings/arm/xen.txt
> > +++ b/Documentation/devicetree/bindings/arm/xen.txt
> > @@ -15,6 +15,24 @@ the following properties:
> >  - interrupts: the interrupt used by Xen to inject event notifications.
> >    A GIC node is also required.
> >  
> > +To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
> > +under /hypervisor with following parameters:
> 
> It's not really clear why this is under /hypervisor and why xen needs to
> define its own nodes.

There was a very long discussion about this, see http://marc.info/?l=xen-devel&m=144321258920213


> The handling of the existing linux standard nodes can be made
> Xen-specific by the presence of the /hypervisor/compatible == "xen" node
> (or similar), right?
> 
> > +________________________________________________________________________________
> > +Name                      | Size   | Description
> > +================================================================================
> > +xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
> > +--------------------------------------------------------------------------------
> > +xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
> > +--------------------------------------------------------------------------------
> 
> I would probably say "Guest physical address..." just to be clear.
> 
> > +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
> > +                          |        | pointed to in previous entry.
> > +--------------------------------------------------------------------------------
> > +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
> > +                          |        | memory map.
> > +--------------------------------------------------------------------------------
> > +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
> > +--------------------------------------------------------------------------------
> 
> Need a reference to the mmap descriptor format here.
> 
> David
> 

[PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[Rob Herring]

On Tue, Nov 17, 2015 at 05:57:05PM +0800, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
> scan this to get the UEFI information.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/xen.txt | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
> index 0f7b9c2..c375d67 100644
> --- a/Documentation/devicetree/bindings/arm/xen.txt
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -15,6 +15,24 @@ the following properties:
>  - interrupts: the interrupt used by Xen to inject event notifications.
>    A GIC node is also required.
>  
> +To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
> +under /hypervisor with following parameters:
> +
> +________________________________________________________________________________
> +Name                      | Size   | Description
> +================================================================================
> +xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
> +                          |        | pointed to in previous entry.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
> +                          |        | memory map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
> +--------------------------------------------------------------------------------

Why aren't you using what is already populated by UEFI as defined in 
Documentation/arm/uefi.txt? Seems to be about the same thing.

Rob

[PATCH 07/13] ARM: Xen: Document UEFI support on Xen ARM virtual platforms

[Shannon Zhao]

On 2015/11/18 4:44, Rob Herring wrote:
> On Tue, Nov 17, 2015 at 05:57:05PM +0800, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a "uefi" node under /hypervisor node in FDT, then Linux kernel could
>> scan this to get the UEFI information.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/arm/xen.txt | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
>> index 0f7b9c2..c375d67 100644
>> --- a/Documentation/devicetree/bindings/arm/xen.txt
>> +++ b/Documentation/devicetree/bindings/arm/xen.txt
>> @@ -15,6 +15,24 @@ the following properties:
>>  - interrupts: the interrupt used by Xen to inject event notifications.
>>    A GIC node is also required.
>>  
>> +To support UEFI on Xen ARM virtual platforms, Xen pupulates the FDT "uefi" node
>> +under /hypervisor with following parameters:
>> +
>> +________________________________________________________________________________
>> +Name                      | Size   | Description
>> +================================================================================
>> +xen,uefi-system-table     | 64-bit | Physical address of the UEFI System Table.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-start       | 64-bit | Physical address of the UEFI memory map.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
>> +                          |        | pointed to in previous entry.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
>> +                          |        | memory map.
>> +--------------------------------------------------------------------------------
>> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
>> +--------------------------------------------------------------------------------
> 
> Why aren't you using what is already populated by UEFI as defined in 
> Documentation/arm/uefi.txt? Seems to be about the same thing.
> 

Yes, the parameters are similar. But since we can't find a good way to
use normal UEFI for Xen Dom0 on ARM(there are some aspects which are not
consistent with the UEFI spec), we have to introduce a Xen specific
UEFI. Then we want to separate this from the normal UEFI completely and
don't affect the normal use. So we add these parameter under Xenia
specific node /hypervisor.

For details of the previous discussion, look at below link:

http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg03295.html

Thanks,
-- 
Shannon

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a new function to parse DT parameters for Xen specific UEFI just
like the way for normal UEFI. Then it could reuse the existing codes.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 62 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d6144e3..629bd06 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -24,6 +24,7 @@
 #include <linux/of_fdt.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <xen/xen.h>
 
 struct efi __read_mostly efi = {
 	.mps        = EFI_INVALID_TABLE_ADDR,
@@ -488,12 +489,60 @@ static __initdata struct {
 	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
 };
 
+static __initdata struct {
+	const char name[32];
+	const char propname[32];
+	int offset;
+	int size;
+} xen_dt_params[] = {
+	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
+	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
+	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
+	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
+	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
+};
+
 struct param_info {
 	int verbose;
 	int found;
 	void *params;
 };
 
+static int __init fdt_find_xen_uefi_params(unsigned long node,
+					   const char *uname, int depth,
+					   void *data)
+{
+	struct param_info *info = data;
+	const void *prop;
+	void *dest;
+	u64 val;
+	int i, len;
+
+	if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") != 0))
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {
+		prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,
+					   &len);
+		if (!prop)
+			return 0;
+		dest = info->params + xen_dt_params[i].offset;
+		info->found++;
+
+		val = of_read_number(prop, len / sizeof(u32));
+
+		if (dt_params[i].size == sizeof(u32))
+			*(u32 *)dest = val;
+		else
+			*(u64 *)dest = val;
+
+		if (info->verbose)
+			pr_info("  %s: 0x%0*llx\n", xen_dt_params[i].name,
+				xen_dt_params[i].size * 2, val);
+	}
+
+	return 1;
+}
 static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
 				       int depth, void *data)
 {
@@ -538,12 +587,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
 	info.found = 0;
 	info.params = params;
 
-	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
-	if (!info.found)
+	if (xen_initial_domain())
+		ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);
+	else
+		ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
+	if (!info.found) {
 		pr_info("UEFI not found.\n");
-	else if (!ret)
-		pr_err("Can't find '%s' in device tree!\n",
-		       dt_params[info.found].name);
+	} else if (!ret) {
+		if (xen_initial_domain())
+			pr_err("Can't find '%s' in device tree!\n",
+			       xen_dt_params[info.found].name);
+		else
+			pr_err("Can't find '%s' in device tree!\n",
+			       xen_dt_params[info.found].name);
+	}
 
 	return ret;
 }
-- 
2.1.0

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Ard Biesheuvel]

On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add a new function to parse DT parameters for Xen specific UEFI just
> like the way for normal UEFI. Then it could reuse the existing codes.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d6144e3..629bd06 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -24,6 +24,7 @@
>  #include <linux/of_fdt.h>
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
> +#include <xen/xen.h>
>
>  struct efi __read_mostly efi = {
>         .mps        = EFI_INVALID_TABLE_ADDR,
> @@ -488,12 +489,60 @@ static __initdata struct {
>         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
>  };
>
> +static __initdata struct {
> +       const char name[32];
> +       const char propname[32];
> +       int offset;
> +       int size;
> +} xen_dt_params[] = {
> +       UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> +       UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> +       UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> +       UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> +       UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> +};
> +

We discussed (and agreed afair) that dropping the "linux," prefix from
the DT properties was an acceptable change. If we do that, and reuse
the same names in the xen version (i.e., drop the "xen," prefix), we
could make this change a *lot* simpler.

>  struct param_info {
>         int verbose;
>         int found;
>         void *params;
>  };
>
> +static int __init fdt_find_xen_uefi_params(unsigned long node,
> +                                          const char *uname, int depth,
> +                                          void *data)
> +{
> +       struct param_info *info = data;
> +       const void *prop;
> +       void *dest;
> +       u64 val;
> +       int i, len;
> +
> +       if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") != 0))
> +               return 0;
> +
> +       for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {
> +               prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,
> +                                          &len);
> +               if (!prop)
> +                       return 0;
> +               dest = info->params + xen_dt_params[i].offset;
> +               info->found++;
> +
> +               val = of_read_number(prop, len / sizeof(u32));
> +
> +               if (dt_params[i].size == sizeof(u32))
> +                       *(u32 *)dest = val;
> +               else
> +                       *(u64 *)dest = val;
> +
> +               if (info->verbose)
> +                       pr_info("  %s: 0x%0*llx\n", xen_dt_params[i].name,
> +                               xen_dt_params[i].size * 2, val);
> +       }
> +
> +       return 1;
> +}
>  static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
>                                        int depth, void *data)
>  {
> @@ -538,12 +587,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
>         info.found = 0;
>         info.params = params;
>
> -       ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> -       if (!info.found)
> +       if (xen_initial_domain())
> +               ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);
> +       else
> +               ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> +       if (!info.found) {
>                 pr_info("UEFI not found.\n");
> -       else if (!ret)
> -               pr_err("Can't find '%s' in device tree!\n",
> -                      dt_params[info.found].name);
> +       } else if (!ret) {
> +               if (xen_initial_domain())
> +                       pr_err("Can't find '%s' in device tree!\n",
> +                              xen_dt_params[info.found].name);
> +               else
> +                       pr_err("Can't find '%s' in device tree!\n",
> +                              xen_dt_params[info.found].name);

Wrong array here

> +       }
>
>         return ret;
>  }
> --
> 2.1.0
>

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Mark Rutland]

On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
> On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >
> > Add a new function to parse DT parameters for Xen specific UEFI just
> > like the way for normal UEFI. Then it could reuse the existing codes.
> >
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 62 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index d6144e3..629bd06 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/of_fdt.h>
> >  #include <linux/io.h>
> >  #include <linux/platform_device.h>
> > +#include <xen/xen.h>
> >
> >  struct efi __read_mostly efi = {
> >         .mps        = EFI_INVALID_TABLE_ADDR,
> > @@ -488,12 +489,60 @@ static __initdata struct {
> >         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> >  };
> >
> > +static __initdata struct {
> > +       const char name[32];
> > +       const char propname[32];
> > +       int offset;
> > +       int size;
> > +} xen_dt_params[] = {
> > +       UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> > +       UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> > +       UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> > +       UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> > +       UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> > +};
> > +
> 
> We discussed (and agreed afair) that dropping the "linux," prefix from
> the DT properties was an acceptable change. If we do that, and reuse
> the same names in the xen version (i.e., drop the "xen," prefix), we
> could make this change a *lot* simpler.

Regardless of if we drop the "linux," prefix from the existing strings,
I think we need the "xen," prefix here.

The xen EFI interface comes with additional caveats, and we need to
treat it separately.

Unless you're suggesting that /hypervisor/uefi-* is handled differently
to /chosen/uefi-*?

I think I'd still prefer the "xen," prefix regardless.

Thanks,
Mark.

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Ard Biesheuvel]

On 17 November 2015 at 12:37, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
>> On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> >
>> > Add a new function to parse DT parameters for Xen specific UEFI just
>> > like the way for normal UEFI. Then it could reuse the existing codes.
>> >
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> >  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
>> >  1 file changed, 62 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> > index d6144e3..629bd06 100644
>> > --- a/drivers/firmware/efi/efi.c
>> > +++ b/drivers/firmware/efi/efi.c
>> > @@ -24,6 +24,7 @@
>> >  #include <linux/of_fdt.h>
>> >  #include <linux/io.h>
>> >  #include <linux/platform_device.h>
>> > +#include <xen/xen.h>
>> >
>> >  struct efi __read_mostly efi = {
>> >         .mps        = EFI_INVALID_TABLE_ADDR,
>> > @@ -488,12 +489,60 @@ static __initdata struct {
>> >         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
>> >  };
>> >
>> > +static __initdata struct {
>> > +       const char name[32];
>> > +       const char propname[32];
>> > +       int offset;
>> > +       int size;
>> > +} xen_dt_params[] = {
>> > +       UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
>> > +       UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
>> > +       UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
>> > +       UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
>> > +       UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
>> > +};
>> > +
>>
>> We discussed (and agreed afair) that dropping the "linux," prefix from
>> the DT properties was an acceptable change. If we do that, and reuse
>> the same names in the xen version (i.e., drop the "xen," prefix), we
>> could make this change a *lot* simpler.
>
> Regardless of if we drop the "linux," prefix from the existing strings,
> I think we need the "xen," prefix here.
>
> The xen EFI interface comes with additional caveats, and we need to
> treat it separately.
>
> Unless you're suggesting that /hypervisor/uefi-* is handled differently
> to /chosen/uefi-*?
>
> I think I'd still prefer the "xen," prefix regardless.
>

Well, we should still be able to reuse more of the existing code
rather than duplicating a bunch of code in fdt_find_xen_uefi_params()

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Shannon Zhao]

On 2015/11/17 20:17, Ard Biesheuvel wrote:
> On 17 November 2015 at 12:37, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
>>> On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Add a new function to parse DT parameters for Xen specific UEFI just
>>>> like the way for normal UEFI. Then it could reuse the existing codes.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> ---
>>>>  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
>>>>  1 file changed, 62 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>>>> index d6144e3..629bd06 100644
>>>> --- a/drivers/firmware/efi/efi.c
>>>> +++ b/drivers/firmware/efi/efi.c
>>>> @@ -24,6 +24,7 @@
>>>>  #include <linux/of_fdt.h>
>>>>  #include <linux/io.h>
>>>>  #include <linux/platform_device.h>
>>>> +#include <xen/xen.h>
>>>>
>>>>  struct efi __read_mostly efi = {
>>>>         .mps        = EFI_INVALID_TABLE_ADDR,
>>>> @@ -488,12 +489,60 @@ static __initdata struct {
>>>>         UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
>>>>  };
>>>>
>>>> +static __initdata struct {
>>>> +       const char name[32];
>>>> +       const char propname[32];
>>>> +       int offset;
>>>> +       int size;
>>>> +} xen_dt_params[] = {
>>>> +       UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
>>>> +       UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
>>>> +       UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
>>>> +       UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
>>>> +       UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
>>>> +};
>>>> +
>>>
>>> We discussed (and agreed afair) that dropping the "linux," prefix from
>>> the DT properties was an acceptable change. If we do that, and reuse
>>> the same names in the xen version (i.e., drop the "xen," prefix), we
>>> could make this change a *lot* simpler.
>>
>> Regardless of if we drop the "linux," prefix from the existing strings,
>> I think we need the "xen," prefix here.
>>
>> The xen EFI interface comes with additional caveats, and we need to
>> treat it separately.
>>
>> Unless you're suggesting that /hypervisor/uefi-* is handled differently
>> to /chosen/uefi-*?
>>
>> I think I'd still prefer the "xen," prefix regardless.
>>
> 
> Well, we should still be able to reuse more of the existing code
> rather than duplicating a bunch of code in fdt_find_xen_uefi_params()

Sure, will try to reuse the codes.

Thanks,
-- 
Shannon

[PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a new function to parse DT parameters for Xen specific UEFI just
> like the way for normal UEFI. Then it could reuse the existing codes.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  drivers/firmware/efi/efi.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 62 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d6144e3..629bd06 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -24,6 +24,7 @@
>  #include <linux/of_fdt.h>
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
> +#include <xen/xen.h>
>  
>  struct efi __read_mostly efi = {
>  	.mps        = EFI_INVALID_TABLE_ADDR,
> @@ -488,12 +489,60 @@ static __initdata struct {
>  	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
>  };
>  
> +static __initdata struct {
> +	const char name[32];
> +	const char propname[32];
> +	int offset;
> +	int size;
> +} xen_dt_params[] = {
> +	UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> +	UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> +	UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> +	UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> +	UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> +};
> +
>  struct param_info {
>  	int verbose;
>  	int found;
>  	void *params;
>  };
>  
> +static int __init fdt_find_xen_uefi_params(unsigned long node,
> +					   const char *uname, int depth,
> +					   void *data)
> +{
> +	struct param_info *info = data;
> +	const void *prop;
> +	void *dest;
> +	u64 val;
> +	int i, len;
> +
> +	if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") != 0))
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {
> +		prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,
> +					   &len);
> +		if (!prop)
> +			return 0;
> +		dest = info->params + xen_dt_params[i].offset;
> +		info->found++;
> +
> +		val = of_read_number(prop, len / sizeof(u32));
> +
> +		if (dt_params[i].size == sizeof(u32))
> +			*(u32 *)dest = val;
> +		else
> +			*(u64 *)dest = val;
> +
> +		if (info->verbose)
> +			pr_info("  %s: 0x%0*llx\n", xen_dt_params[i].name,
> +				xen_dt_params[i].size * 2, val);
> +	}
> +
> +	return 1;
> +}
>  static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
>  				       int depth, void *data)
>  {
> @@ -538,12 +587,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params *params, int verbose)
>  	info.found = 0;
>  	info.params = params;
>  
> -	ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> -	if (!info.found)
> +	if (xen_initial_domain())
> +		ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);
> +	else
> +		ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> +	if (!info.found) {
>  		pr_info("UEFI not found.\n");
> -	else if (!ret)
> -		pr_err("Can't find '%s' in device tree!\n",
> -		       dt_params[info.found].name);
> +	} else if (!ret) {
> +		if (xen_initial_domain())
> +			pr_err("Can't find '%s' in device tree!\n",
> +			       xen_dt_params[info.found].name);
> +		else
> +			pr_err("Can't find '%s' in device tree!\n",
> +			       xen_dt_params[info.found].name);

xen_dt_params is obviously wrong here

[PATCH 09/13] ARM: Xen: Initialize Xen specific UEFI runtime services

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

When running on Xen hypervisor, runtime services are supported through
hypercall. So call Xen specific function to initialize runtime services.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm/include/asm/xen/hypercall.h |  1 +
 arch/arm/xen/enlighten.c             |  1 +
 arch/arm/xen/hypercall.S             |  1 +
 arch/arm64/kernel/efi.c              | 20 ++++++++++++++------
 arch/arm64/xen/hypercall.S           |  1 +
 drivers/xen/Kconfig                  |  2 +-
 drivers/xen/efi.c                    | 22 ++++++++++++++++++++++
 include/xen/xen-ops.h                | 10 ++++++++++
 8 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
index 712b50e..0de6074 100644
--- a/arch/arm/include/asm/xen/hypercall.h
+++ b/arch/arm/include/asm/xen/hypercall.h
@@ -50,6 +50,7 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
 int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
+int HYPERVISOR_dom0_op(void *arg);
 
 static inline int
 HYPERVISOR_suspend(unsigned long start_info_mfn)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 7cb82f7..1373d6d 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -289,3 +289,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
 EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(HYPERVISOR_dom0_op);
diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
index 10fd99c..16fc153 100644
--- a/arch/arm/xen/hypercall.S
+++ b/arch/arm/xen/hypercall.S
@@ -90,6 +90,7 @@ HYPERCALL2(physdev_op);
 HYPERCALL3(vcpu_op);
 HYPERCALL1(tmem_op);
 HYPERCALL2(multicall);
+HYPERCALL1(dom0_op);
 
 ENTRY(privcmd_call)
 	stmdb sp!, {r4}
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 13671a9..ab1c9e9 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -34,6 +34,9 @@
 #include <asm/mmu.h>
 #include <asm/pgtable.h>
 
+#include <xen/xen.h>
+#include <xen/xen-ops.h>
+
 struct efi_memory_map memmap;
 
 static u64 efi_system_table;
@@ -308,13 +311,18 @@ static int __init arm64_enable_runtime_services(void)
 	}
 	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
 
-	if (!efi_virtmap_init()) {
-		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
-		return -1;
-	}
+	if (!xen_initial_domain()) {
+		if (!efi_virtmap_init()) {
+			pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
+			return -1;
+		}
 
-	/* Set up runtime services function pointers */
-	efi_native_runtime_setup();
+		/* Set up runtime services function pointers */
+		efi_native_runtime_setup();
+	} else {
+		/* Set up runtime services function pointers for Xen UEFI */
+		xen_efi_runtime_setup();
+	}
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
 	efi.runtime_version = efi.systab->hdr.revision;
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 8bbe940..f6e15aa 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -81,6 +81,7 @@ HYPERCALL2(physdev_op);
 HYPERCALL3(vcpu_op);
 HYPERCALL1(tmem_op);
 HYPERCALL2(multicall);
+HYPERCALL1(dom0_op);
 
 ENTRY(privcmd_call)
 	mov x16, x0
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 73708ac..59aec8b 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -268,7 +268,7 @@ config XEN_HAVE_PVMMU
 
 config XEN_EFI
 	def_bool y
-	depends on X86_64 && EFI
+	depends on ARM64 || X86_64 && EFI
 
 config XEN_AUTO_XLATE
 	def_bool y
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index f745db2..5246d89 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -33,6 +33,7 @@
 
 #define INIT_EFI_OP(name) \
 	{.cmd = XENPF_efi_runtime_call, \
+	 .interface_version = XENPF_INTERFACE_VERSION, \
 	 .u.efi_runtime_call.function = XEN_EFI_##name, \
 	 .u.efi_runtime_call.misc = 0}
 
@@ -261,6 +262,7 @@ static efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
 	return efi_data(op).status;
 }
 
+#ifdef CONFIG_x86
 static efi_char16_t vendor[100] __initdata;
 
 static efi_system_table_t efi_systab_xen __initdata = {
@@ -319,7 +321,26 @@ static const struct efi efi_xen __initconst = {
 	.memmap                   = NULL, /* Not used under Xen. */
 	.flags			  = 0     /* Initialized later. */
 };
+#endif
 
+#ifdef CONFIG_ARM64
+void xen_efi_runtime_setup(void)
+{
+	efi.get_time = xen_efi_get_time;
+	efi.set_time = xen_efi_set_time;
+	efi.get_wakeup_time = xen_efi_get_wakeup_time;
+	efi.set_wakeup_time = xen_efi_set_wakeup_time;
+	efi.get_variable = xen_efi_get_variable;
+	efi.get_next_variable = xen_efi_get_next_variable;
+	efi.set_variable = xen_efi_set_variable;
+	efi.query_variable_info = xen_efi_query_variable_info;
+	efi.update_capsule = xen_efi_update_capsule;
+	efi.query_capsule_caps = xen_efi_query_capsule_caps;
+	efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+}
+#endif
+
+#ifdef CONFIG_X86
 efi_system_table_t __init *xen_efi_probe(void)
 {
 	struct xen_platform_op op = {
@@ -369,3 +390,4 @@ efi_system_table_t __init *xen_efi_probe(void)
 
 	return &efi_systab_xen;
 }
+#endif
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index e4e214a..a6c26ad 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -84,13 +84,23 @@ int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
 bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
 
 #ifdef CONFIG_XEN_EFI
+#ifdef CONFIG_X86
 extern efi_system_table_t *xen_efi_probe(void);
+#endif
+#ifdef CONFIG_ARM64
+extern void xen_efi_runtime_setup(void);
+#endif
 #else
+#ifdef CONFIG_X86
 static inline efi_system_table_t __init *xen_efi_probe(void)
 {
 	return NULL;
 }
 #endif
+#ifdef CONFIG_ARM64
+static inline void xen_efi_runtime_setup(void) { }
+#endif
+#endif
 
 #ifdef CONFIG_PREEMPT
 
-- 
2.1.0

[PATCH 09/13] ARM: Xen: Initialize Xen specific UEFI runtime services

[Ard Biesheuvel]

On 17 November 2015 at 10:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> When running on Xen hypervisor, runtime services are supported through
> hypercall. So call Xen specific function to initialize runtime services.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  arch/arm/include/asm/xen/hypercall.h |  1 +
>  arch/arm/xen/enlighten.c             |  1 +
>  arch/arm/xen/hypercall.S             |  1 +
>  arch/arm64/kernel/efi.c              | 20 ++++++++++++++------
>  arch/arm64/xen/hypercall.S           |  1 +
>  drivers/xen/Kconfig                  |  2 +-
>  drivers/xen/efi.c                    | 22 ++++++++++++++++++++++
>  include/xen/xen-ops.h                | 10 ++++++++++
>  8 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
> index 712b50e..0de6074 100644
> --- a/arch/arm/include/asm/xen/hypercall.h
> +++ b/arch/arm/include/asm/xen/hypercall.h
> @@ -50,6 +50,7 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
>  int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
>  int HYPERVISOR_tmem_op(void *arg);
>  int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
> +int HYPERVISOR_dom0_op(void *arg);
>
>  static inline int
>  HYPERVISOR_suspend(unsigned long start_info_mfn)
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 7cb82f7..1373d6d 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -289,3 +289,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
>  EXPORT_SYMBOL_GPL(privcmd_call);
> +EXPORT_SYMBOL_GPL(HYPERVISOR_dom0_op);
> diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> index 10fd99c..16fc153 100644
> --- a/arch/arm/xen/hypercall.S
> +++ b/arch/arm/xen/hypercall.S
> @@ -90,6 +90,7 @@ HYPERCALL2(physdev_op);
>  HYPERCALL3(vcpu_op);
>  HYPERCALL1(tmem_op);
>  HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>
>  ENTRY(privcmd_call)
>         stmdb sp!, {r4}
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 13671a9..ab1c9e9 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -34,6 +34,9 @@
>  #include <asm/mmu.h>
>  #include <asm/pgtable.h>
>
> +#include <xen/xen.h>
> +#include <xen/xen-ops.h>
> +
>  struct efi_memory_map memmap;
>
>  static u64 efi_system_table;
> @@ -308,13 +311,18 @@ static int __init arm64_enable_runtime_services(void)
>         }
>         set_bit(EFI_SYSTEM_TABLES, &efi.flags);
>
> -       if (!efi_virtmap_init()) {
> -               pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
> -               return -1;
> -       }
> +       if (!xen_initial_domain()) {
> +               if (!efi_virtmap_init()) {
> +                       pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
> +                       return -1;
> +               }
>

I'd prefer it if we could separate the code logically, rather than
putting xen_initial_domain() tests in all the code paths.

For instance, we could re-use the EFI_PARAVIRT flag, and set if from
the Xen init code. Then, we could simply test it here, and bail early.
That way, you can have a Xen specific alternative (which does not use
the virtmap etc anyway) in a xen source file.

> -       /* Set up runtime services function pointers */
> -       efi_native_runtime_setup();
> +               /* Set up runtime services function pointers */
> +               efi_native_runtime_setup();
> +       } else {
> +               /* Set up runtime services function pointers for Xen UEFI */
> +               xen_efi_runtime_setup();
> +       }
>         set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>
>         efi.runtime_version = efi.systab->hdr.revision;
> diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> index 8bbe940..f6e15aa 100644
> --- a/arch/arm64/xen/hypercall.S
> +++ b/arch/arm64/xen/hypercall.S
> @@ -81,6 +81,7 @@ HYPERCALL2(physdev_op);
>  HYPERCALL3(vcpu_op);
>  HYPERCALL1(tmem_op);
>  HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>
>  ENTRY(privcmd_call)
>         mov x16, x0
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 73708ac..59aec8b 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -268,7 +268,7 @@ config XEN_HAVE_PVMMU
>
>  config XEN_EFI
>         def_bool y
> -       depends on X86_64 && EFI
> +       depends on ARM64 || X86_64 && EFI
>
>  config XEN_AUTO_XLATE
>         def_bool y
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index f745db2..5246d89 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -33,6 +33,7 @@
>
>  #define INIT_EFI_OP(name) \
>         {.cmd = XENPF_efi_runtime_call, \
> +        .interface_version = XENPF_INTERFACE_VERSION, \
>          .u.efi_runtime_call.function = XEN_EFI_##name, \
>          .u.efi_runtime_call.misc = 0}
>
> @@ -261,6 +262,7 @@ static efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
>         return efi_data(op).status;
>  }
>
> +#ifdef CONFIG_x86
>  static efi_char16_t vendor[100] __initdata;
>
>  static efi_system_table_t efi_systab_xen __initdata = {
> @@ -319,7 +321,26 @@ static const struct efi efi_xen __initconst = {
>         .memmap                   = NULL, /* Not used under Xen. */
>         .flags                    = 0     /* Initialized later. */
>  };
> +#endif
>
> +#ifdef CONFIG_ARM64
> +void xen_efi_runtime_setup(void)
> +{
> +       efi.get_time = xen_efi_get_time;
> +       efi.set_time = xen_efi_set_time;
> +       efi.get_wakeup_time = xen_efi_get_wakeup_time;
> +       efi.set_wakeup_time = xen_efi_set_wakeup_time;
> +       efi.get_variable = xen_efi_get_variable;
> +       efi.get_next_variable = xen_efi_get_next_variable;
> +       efi.set_variable = xen_efi_set_variable;
> +       efi.query_variable_info = xen_efi_query_variable_info;
> +       efi.update_capsule = xen_efi_update_capsule;
> +       efi.query_capsule_caps = xen_efi_query_capsule_caps;
> +       efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> +}
> +#endif
> +
> +#ifdef CONFIG_X86
>  efi_system_table_t __init *xen_efi_probe(void)
>  {
>         struct xen_platform_op op = {
> @@ -369,3 +390,4 @@ efi_system_table_t __init *xen_efi_probe(void)
>
>         return &efi_systab_xen;
>  }
> +#endif
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index e4e214a..a6c26ad 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -84,13 +84,23 @@ int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
>  bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
>
>  #ifdef CONFIG_XEN_EFI
> +#ifdef CONFIG_X86
>  extern efi_system_table_t *xen_efi_probe(void);
> +#endif
> +#ifdef CONFIG_ARM64
> +extern void xen_efi_runtime_setup(void);
> +#endif
>  #else
> +#ifdef CONFIG_X86
>  static inline efi_system_table_t __init *xen_efi_probe(void)
>  {
>         return NULL;
>  }
>  #endif
> +#ifdef CONFIG_ARM64
> +static inline void xen_efi_runtime_setup(void) { }
> +#endif
> +#endif
>
>  #ifdef CONFIG_PREEMPT
>
> --
> 2.1.0
>

[PATCH 09/13] ARM: Xen: Initialize Xen specific UEFI runtime services

[Shannon Zhao]

On 2015/11/17 19:28, Ard Biesheuvel wrote:
>> @@ -308,13 +311,18 @@ static int __init arm64_enable_runtime_services(void)
>> >         }
>> >         set_bit(EFI_SYSTEM_TABLES, &efi.flags);
>> >
>> > -       if (!efi_virtmap_init()) {
>> > -               pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
>> > -               return -1;
>> > -       }
>> > +       if (!xen_initial_domain()) {
>> > +               if (!efi_virtmap_init()) {
>> > +                       pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
>> > +                       return -1;
>> > +               }
>> >
> I'd prefer it if we could separate the code logically, rather than
> putting xen_initial_domain() tests in all the code paths.
> 
> For instance, we could re-use the EFI_PARAVIRT flag, and set if from
> the Xen init code. Then, we could simply test it here, and bail early.
> That way, you can have a Xen specific alternative (which does not use
> the virtmap etc anyway) in a xen source file.
> 
Ok, will do. Thanks.

-- 
Shannon

[PATCH 09/13] ARM: Xen: Initialize Xen specific UEFI runtime services

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> When running on Xen hypervisor, runtime services are supported through
> hypercall. So call Xen specific function to initialize runtime services.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

You can drop the dom0_op part of the patch and based on top of

http://marc.info/?l=xen-devel&m=144802905409775&w=2


>  arch/arm/include/asm/xen/hypercall.h |  1 +
>  arch/arm/xen/enlighten.c             |  1 +
>  arch/arm/xen/hypercall.S             |  1 +
>  arch/arm64/kernel/efi.c              | 20 ++++++++++++++------
>  arch/arm64/xen/hypercall.S           |  1 +
>  drivers/xen/Kconfig                  |  2 +-
>  drivers/xen/efi.c                    | 22 ++++++++++++++++++++++
>  include/xen/xen-ops.h                | 10 ++++++++++
>  8 files changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/include/asm/xen/hypercall.h b/arch/arm/include/asm/xen/hypercall.h
> index 712b50e..0de6074 100644
> --- a/arch/arm/include/asm/xen/hypercall.h
> +++ b/arch/arm/include/asm/xen/hypercall.h
> @@ -50,6 +50,7 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
>  int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
>  int HYPERVISOR_tmem_op(void *arg);
>  int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
> +int HYPERVISOR_dom0_op(void *arg);
>  
>  static inline int
>  HYPERVISOR_suspend(unsigned long start_info_mfn)
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 7cb82f7..1373d6d 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -289,3 +289,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
>  EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
>  EXPORT_SYMBOL_GPL(privcmd_call);
> +EXPORT_SYMBOL_GPL(HYPERVISOR_dom0_op);
> diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> index 10fd99c..16fc153 100644
> --- a/arch/arm/xen/hypercall.S
> +++ b/arch/arm/xen/hypercall.S
> @@ -90,6 +90,7 @@ HYPERCALL2(physdev_op);
>  HYPERCALL3(vcpu_op);
>  HYPERCALL1(tmem_op);
>  HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>  
>  ENTRY(privcmd_call)
>  	stmdb sp!, {r4}
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 13671a9..ab1c9e9 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -34,6 +34,9 @@
>  #include <asm/mmu.h>
>  #include <asm/pgtable.h>
>  
> +#include <xen/xen.h>
> +#include <xen/xen-ops.h>
> +
>  struct efi_memory_map memmap;
>  
>  static u64 efi_system_table;
> @@ -308,13 +311,18 @@ static int __init arm64_enable_runtime_services(void)
>  	}
>  	set_bit(EFI_SYSTEM_TABLES, &efi.flags);
>  
> -	if (!efi_virtmap_init()) {
> -		pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
> -		return -1;
> -	}
> +	if (!xen_initial_domain()) {
> +		if (!efi_virtmap_init()) {
> +			pr_err("No UEFI virtual mapping was installed -- runtime services will not be available\n");
> +			return -1;
> +		}
>  
> -	/* Set up runtime services function pointers */
> -	efi_native_runtime_setup();
> +		/* Set up runtime services function pointers */
> +		efi_native_runtime_setup();
> +	} else {
> +		/* Set up runtime services function pointers for Xen UEFI */
> +		xen_efi_runtime_setup();
> +	}
>  	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>  
>  	efi.runtime_version = efi.systab->hdr.revision;
> diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> index 8bbe940..f6e15aa 100644
> --- a/arch/arm64/xen/hypercall.S
> +++ b/arch/arm64/xen/hypercall.S
> @@ -81,6 +81,7 @@ HYPERCALL2(physdev_op);
>  HYPERCALL3(vcpu_op);
>  HYPERCALL1(tmem_op);
>  HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>  
>  ENTRY(privcmd_call)
>  	mov x16, x0
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 73708ac..59aec8b 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -268,7 +268,7 @@ config XEN_HAVE_PVMMU
>  
>  config XEN_EFI
>  	def_bool y
> -	depends on X86_64 && EFI
> +	depends on ARM64 || X86_64 && EFI

Please use brackets


>  config XEN_AUTO_XLATE
>  	def_bool y
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index f745db2..5246d89 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -33,6 +33,7 @@
>  
>  #define INIT_EFI_OP(name) \
>  	{.cmd = XENPF_efi_runtime_call, \
> +	 .interface_version = XENPF_INTERFACE_VERSION, \
>  	 .u.efi_runtime_call.function = XEN_EFI_##name, \
>  	 .u.efi_runtime_call.misc = 0}

This change won't be needed anymore if you rebase on 

http://marc.info/?l=xen-devel&m=144802905409775&w=2


> @@ -261,6 +262,7 @@ static efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
>  	return efi_data(op).status;
>  }
>  
> +#ifdef CONFIG_x86
>  static efi_char16_t vendor[100] __initdata;
>  
>  static efi_system_table_t efi_systab_xen __initdata = {
> @@ -319,7 +321,26 @@ static const struct efi efi_xen __initconst = {
>  	.memmap                   = NULL, /* Not used under Xen. */
>  	.flags			  = 0     /* Initialized later. */
>  };
> +#endif
>  
> +#ifdef CONFIG_ARM64
> +void xen_efi_runtime_setup(void)
> +{
> +	efi.get_time = xen_efi_get_time;
> +	efi.set_time = xen_efi_set_time;
> +	efi.get_wakeup_time = xen_efi_get_wakeup_time;
> +	efi.set_wakeup_time = xen_efi_set_wakeup_time;
> +	efi.get_variable = xen_efi_get_variable;
> +	efi.get_next_variable = xen_efi_get_next_variable;
> +	efi.set_variable = xen_efi_set_variable;
> +	efi.query_variable_info = xen_efi_query_variable_info;
> +	efi.update_capsule = xen_efi_update_capsule;
> +	efi.query_capsule_caps = xen_efi_query_capsule_caps;
> +	efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> +}
> +#endif
> +
> +#ifdef CONFIG_X86
>  efi_system_table_t __init *xen_efi_probe(void)
>  {
>  	struct xen_platform_op op = {
> @@ -369,3 +390,4 @@ efi_system_table_t __init *xen_efi_probe(void)
>  
>  	return &efi_systab_xen;
>  }
> +#endif

Instead of using #ifdef's I would move the x86 specific code into
arch/x86/xen/efi.c. In particular xen_efi_probe, efi_xen, and
efi_systab_xen. I think it is OK for the initialization to be arch
specific.


> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index e4e214a..a6c26ad 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -84,13 +84,23 @@ int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
>  bool xen_running_on_version_or_later(unsigned int major, unsigned int minor);
>  
>  #ifdef CONFIG_XEN_EFI
> +#ifdef CONFIG_X86
>  extern efi_system_table_t *xen_efi_probe(void);
> +#endif
> +#ifdef CONFIG_ARM64
> +extern void xen_efi_runtime_setup(void);
> +#endif
>  #else
> +#ifdef CONFIG_X86
>  static inline efi_system_table_t __init *xen_efi_probe(void)
>  {
>  	return NULL;
>  }
>  #endif
> +#ifdef CONFIG_ARM64
> +static inline void xen_efi_runtime_setup(void) { }
> +#endif
> +#endif
>  
>  #ifdef CONFIG_PREEMPT

I would get rid of this mess and instead I would export the functions
defined in drivers/xen/efi.c. Then I would use them in
arch/{arm,x86}/xen/efi.c.

[PATCH 10/13] ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

When it's a Xen domain0 booting with ACPI, it will supply a /chosen and
a /hypervisor node in DT. So check if it needs to enable ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm64/kernel/acpi.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 19de753..7b67426 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -62,10 +62,13 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
 {
 	/*
 	 * Return 1 as soon as we encounter a node at depth 1 that is
-	 * not the /chosen node.
+	 * not the /chosen node or to Xen initial domain that is not
+	 * either /chosen or /hypervisor node.
 	 */
 	if (depth == 1 && (strcmp(uname, "chosen") != 0))
-		return 1;
+		if (!xen_initial_domain() || (strcmp(uname, "hypervisor") != 0))
+			return 1;
+
 	return 0;
 }
 
@@ -179,8 +182,9 @@ void __init acpi_boot_table_init(void)
 	/*
 	 * Enable ACPI instead of device tree unless
 	 * - ACPI has been disabled explicitly (acpi=off), or
-	 * - the device tree is not empty (it has more than just a /chosen node)
-	 *   and ACPI has not been force enabled (acpi=force)
+	 * - the device tree is not empty (it has more than just a /chosen node
+	 *   or to Xen initial domain it has more than a /chosen node and
+	 *   /hypervisor node) and ACPI has not been force enabled (acpi=force)
 	 */
 	if (param_acpi_off ||
 	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
-- 
2.1.0

[PATCH 10/13] ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> When it's a Xen domain0 booting with ACPI, it will supply a /chosen and
> a /hypervisor node in DT. So check if it needs to enable ACPI.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  arch/arm64/kernel/acpi.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 19de753..7b67426 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -62,10 +62,13 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
>  {
>  	/*
>  	 * Return 1 as soon as we encounter a node at depth 1 that is
> -	 * not the /chosen node.
> +	 * not the /chosen node or to Xen initial domain that is not
> +	 * either /chosen or /hypervisor node.

* not the /chosen node, or /hypervisor node when running on Xen.

>  	 */
>  	if (depth == 1 && (strcmp(uname, "chosen") != 0))
> -		return 1;
> +		if (!xen_initial_domain() || (strcmp(uname, "hypervisor") != 0))
> +			return 1;

Please add { } around the inner if statement.


>  	return 0;
>  }
>  
> @@ -179,8 +182,9 @@ void __init acpi_boot_table_init(void)
>  	/*
>  	 * Enable ACPI instead of device tree unless
>  	 * - ACPI has been disabled explicitly (acpi=off), or
> -	 * - the device tree is not empty (it has more than just a /chosen node)
> -	 *   and ACPI has not been force enabled (acpi=force)
> +	 * - the device tree is not empty (it has more than just a /chosen node
> +	 *   or to Xen initial domain it has more than a /chosen node and
> +	 *   /hypervisor node) and ACPI has not been force enabled (acpi=force)

the device tree is not empty (it has more than just a /chosen node, and
an /hypervisor node when running on Xen


>  	 */
>  	if (param_acpi_off ||
>  	    (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
> -- 
> 2.1.0
> 

[PATCH 11/13] xen/hvm/params: Add a new dilivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add a new dilivery type:
val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
To the flag, bit 0 stands the interrupt mode is edge(1) or level(0) and
bit 1 stands the interrupt polarity is active low(1) or high(0).

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 include/xen/interface/hvm/params.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
index a6c7991..550688a 100644
--- a/include/xen/interface/hvm/params.h
+++ b/include/xen/interface/hvm/params.h
@@ -34,6 +34,11 @@
  *                  Domain = val[47:32], Bus  = val[31:16],
  *                  DevFn  = val[15: 8], IntX = val[ 1: 0]
  * val[63:56] == 2: val[7:0] is a vector number.
+ * val[63:56] == 3: val[15:8] is flag of event-channel interrupt:
+ *                      bit 0: interrupt is edge(1) or level(0) triggered
+ *                      bit 1: interrupt is active low(1) or high(0)
+ *                  val[7:0] is PPI number used by event-channel.
+ *                  This is only used by ARM/ARM64.
  * If val == 0 then CPU0 event-channel notifications are not delivered.
  */
 #define HVM_PARAM_CALLBACK_IRQ 0
-- 
2.1.0

[PATCH 11/13] xen/hvm/params: Add a new dilivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a new dilivery type:

delivery

> val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> To the flag, bit 0 stands the interrupt mode is edge(1) or level(0) and
> bit 1 stands the interrupt polarity is active low(1) or high(0).
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  include/xen/interface/hvm/params.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
> index a6c7991..550688a 100644
> --- a/include/xen/interface/hvm/params.h
> +++ b/include/xen/interface/hvm/params.h
> @@ -34,6 +34,11 @@
>   *                  Domain = val[47:32], Bus  = val[31:16],
>   *                  DevFn  = val[15: 8], IntX = val[ 1: 0]
>   * val[63:56] == 2: val[7:0] is a vector number.
> + * val[63:56] == 3: val[15:8] is flag of event-channel interrupt:
> + *                      bit 0: interrupt is edge(1) or level(0) triggered
> + *                      bit 1: interrupt is active low(1) or high(0)
> + *                  val[7:0] is PPI number used by event-channel.
> + *                  This is only used by ARM/ARM64.
>   * If val == 0 then CPU0 event-channel notifications are not delivered.
>   */
>  #define HVM_PARAM_CALLBACK_IRQ 0
> -- 
> 2.1.0
> 

[Xen-devel] [PATCH 11/13] xen/hvm/params: Add a new dilivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

[Andrew Cooper]

On 20/11/15 17:07, Stefano Stabellini wrote:
> On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add a new dilivery type:
> delivery
>
>> val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
>> To the flag, bit 0 stands the interrupt mode is edge(1) or level(0) and
>> bit 1 stands the interrupt polarity is active low(1) or high(0).
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  include/xen/interface/hvm/params.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
>> index a6c7991..550688a 100644
>> --- a/include/xen/interface/hvm/params.h
>> +++ b/include/xen/interface/hvm/params.h
>> @@ -34,6 +34,11 @@
>>   *                  Domain = val[47:32], Bus  = val[31:16],
>>   *                  DevFn  = val[15: 8], IntX = val[ 1: 0]
>>   * val[63:56] == 2: val[7:0] is a vector number.
>> + * val[63:56] == 3: val[15:8] is flag of event-channel interrupt:
>> + *                      bit 0: interrupt is edge(1) or level(0) triggered
>> + *                      bit 1: interrupt is active low(1) or high(0)
>> + *                  val[7:0] is PPI number used by event-channel.
>> + *                  This is only used by ARM/ARM64.
>>   * If val == 0 then CPU0 event-channel notifications are not delivered.
>>   */
>>  #define HVM_PARAM_CALLBACK_IRQ 0

Sadly NACK in this form.

This is not your fault, but this particular field has hidden ABI which
the original submitter neglected to put into public API.

Bits 63:56 of this field currently have a hidden ABI with:

xen/include/asm-x86/hvm/irq.h
enum {
    HVMIRQ_callback_none,
    HVMIRQ_callback_gsi,
    HVMIRQ_callback_pci_intx,
    HVMIRQ_callback_vector
} callback_via_type;

I will submit a patch to fix this this properly.

~Andrew

[Xen-devel] [PATCH 11/13] xen/hvm/params: Add a new dilivery type for event-channel in HVM_PARAM_CALLBACK_IRQ

[Stefano Stabellini]

On Fri, 20 Nov 2015, Andrew Cooper wrote:
> On 20/11/15 17:07, Stefano Stabellini wrote:
> > On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> >> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>
> >> Add a new dilivery type:
> > delivery
> >
> >> val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> >> To the flag, bit 0 stands the interrupt mode is edge(1) or level(0) and
> >> bit 1 stands the interrupt polarity is active low(1) or high(0).
> >>
> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> ---
> >>  include/xen/interface/hvm/params.h | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/include/xen/interface/hvm/params.h b/include/xen/interface/hvm/params.h
> >> index a6c7991..550688a 100644
> >> --- a/include/xen/interface/hvm/params.h
> >> +++ b/include/xen/interface/hvm/params.h
> >> @@ -34,6 +34,11 @@
> >>   *                  Domain = val[47:32], Bus  = val[31:16],
> >>   *                  DevFn  = val[15: 8], IntX = val[ 1: 0]
> >>   * val[63:56] == 2: val[7:0] is a vector number.
> >> + * val[63:56] == 3: val[15:8] is flag of event-channel interrupt:
> >> + *                      bit 0: interrupt is edge(1) or level(0) triggered
> >> + *                      bit 1: interrupt is active low(1) or high(0)
> >> + *                  val[7:0] is PPI number used by event-channel.
> >> + *                  This is only used by ARM/ARM64.
> >>   * If val == 0 then CPU0 event-channel notifications are not delivered.
> >>   */
> >>  #define HVM_PARAM_CALLBACK_IRQ 0
> 
> Sadly NACK in this form.
> 
> This is not your fault, but this particular field has hidden ABI which
> the original submitter neglected to put into public API.
> 
> Bits 63:56 of this field currently have a hidden ABI with:
> 
> xen/include/asm-x86/hvm/irq.h
> enum {
>     HVMIRQ_callback_none,
>     HVMIRQ_callback_gsi,
>     HVMIRQ_callback_pci_intx,
>     HVMIRQ_callback_vector
> } callback_via_type;
 
I take the value should be "4" then?


> I will submit a patch to fix this this properly.

[PATCH 12/13] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

When booting with ACPI, it could get the event-channel irq through
HVM_PARAM_CALLBACK_IRQ.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm/xen/enlighten.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1373d6d..b8e9db8 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -25,6 +25,7 @@
 #include <linux/cpufreq.h>
 #include <linux/cpu.h>
 #include <linux/console.h>
+#include <linux/acpi.h>
 
 #include <linux/mm.h>
 
@@ -190,10 +191,24 @@ static int __init xen_guest_init(void)
 	if (!xen_domain())
 		return 0;
 
-	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
-	if (!xen_events_irq) {
-		pr_err("Xen event channel interrupt not found\n");
-		return -ENODEV;
+	if (!acpi_disabled) {
+		struct xen_hvm_param a;
+
+		a.domid = DOMID_SELF;
+		a.index = HVM_PARAM_CALLBACK_IRQ;
+		if (HYPERVISOR_hvm_op(HVMOP_get_param, &a)) {
+			if ((a.value >> 56) != 3) {
+				pr_err("Can't get Xen event-channel irq\n");
+				return -ENODEV;
+			}
+		}
+		xen_events_irq = a.value & 0xff;
+	} else {
+		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
+		if (!xen_events_irq) {
+			pr_err("Xen event channel interrupt not found\n");
+			return -ENODEV;
+		}
 	}
 
 	shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
-- 
2.1.0

[PATCH 12/13] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> When booting with ACPI, it could get the event-channel irq through
> HVM_PARAM_CALLBACK_IRQ.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  arch/arm/xen/enlighten.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 1373d6d..b8e9db8 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -25,6 +25,7 @@
>  #include <linux/cpufreq.h>
>  #include <linux/cpu.h>
>  #include <linux/console.h>
> +#include <linux/acpi.h>
>  
>  #include <linux/mm.h>
>  
> @@ -190,10 +191,24 @@ static int __init xen_guest_init(void)
>  	if (!xen_domain())
>  		return 0;
>  
> -	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
> -	if (!xen_events_irq) {
> -		pr_err("Xen event channel interrupt not found\n");
> -		return -ENODEV;
> +	if (!acpi_disabled) {
> +		struct xen_hvm_param a;
> +
> +		a.domid = DOMID_SELF;
> +		a.index = HVM_PARAM_CALLBACK_IRQ;
> +		if (HYPERVISOR_hvm_op(HVMOP_get_param, &a)) {

If HYPERVISOR_hvm_op returns an error (ret != 0), I wouldn't bother
checking for the returned value and simply return error.

On the other hand, if HYPERVISOR_hvm_op is successful (ret == 0), then
it might make sense to check for (a.value >> 56) == 3.


> +			if ((a.value >> 56) != 3) {
> +				pr_err("Can't get Xen event-channel irq\n");
> +				return -ENODEV;
> +			}
> +		}
> +		xen_events_irq = a.value & 0xff;
> +	} else {
> +		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
> +		if (!xen_events_irq) {
> +			pr_err("Xen event channel interrupt not found\n");
> +			return -ENODEV;

Please make the error message common and move it out of the if/else.


> +		}
>  	}
>  
>  	shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
> -- 
> 2.1.0
> 

[PATCH 12/13] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI

[Shannon Zhao]

On 2015/11/21 1:11, Stefano Stabellini wrote:
> On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> When booting with ACPI, it could get the event-channel irq through
>> HVM_PARAM_CALLBACK_IRQ.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  arch/arm/xen/enlighten.c | 23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 1373d6d..b8e9db8 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/cpufreq.h>
>>  #include <linux/cpu.h>
>>  #include <linux/console.h>
>> +#include <linux/acpi.h>
>>  
>>  #include <linux/mm.h>
>>  
>> @@ -190,10 +191,24 @@ static int __init xen_guest_init(void)
>>  	if (!xen_domain())
>>  		return 0;
>>  
>> -	xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>> -	if (!xen_events_irq) {
>> -		pr_err("Xen event channel interrupt not found\n");
>> -		return -ENODEV;
>> +	if (!acpi_disabled) {
>> +		struct xen_hvm_param a;
>> +
>> +		a.domid = DOMID_SELF;
>> +		a.index = HVM_PARAM_CALLBACK_IRQ;
>> +		if (HYPERVISOR_hvm_op(HVMOP_get_param, &a)) {
> 
> If HYPERVISOR_hvm_op returns an error (ret != 0), I wouldn't bother
> checking for the returned value and simply return error.
> 
> On the other hand, if HYPERVISOR_hvm_op is successful (ret == 0), then
> it might make sense to check for (a.value >> 56) == 3.
> 
Ok.
> 
>> +			if ((a.value >> 56) != 3) {
>> +				pr_err("Can't get Xen event-channel irq\n");
>> +				return -ENODEV;
>> +			}
>> +		}
>> +		xen_events_irq = a.value & 0xff;
>> +	} else {
>> +		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>> +		if (!xen_events_irq) {
>> +			pr_err("Xen event channel interrupt not found\n");
>> +			return -ENODEV;
> 
> Please make the error message common and move it out of the if/else.
> 
Sure. Thanks.
> 
>> +		}
>>  	}
>>  
>>  	shared_info_page = (struct shared_info *)get_zeroed_page(GFP_KERNEL);
>> -- 
>> 2.1.0
>>
> 
> .
> 

-- 
Shannon

[PATCH 13/13] ARM: XEN: Move xen_early_init() before efi_init()

[shannon.zhao at linaro.org]

From: Shannon Zhao <shannon.zhao@linaro.org>

Move xen_early_init() before efi_init(), then when calling efi_init()
could initialize Xen specific UEFI.

Check if it runs on Xen hypervisor through the flat dts.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 arch/arm/xen/enlighten.c  | 62 ++++++++++++++++++++++++++++++++++++-----------
 arch/arm64/kernel/setup.c |  2 +-
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index b8e9db8..d4f884c 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -19,6 +19,7 @@
 #include <linux/irqreturn.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_fdt.h>
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/cpuidle.h>
@@ -48,8 +49,6 @@ struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
 
 static __read_mostly unsigned int xen_events_irq;
 
-static __initdata struct device_node *xen_node;
-
 int xen_remap_domain_gfn_array(struct vm_area_struct *vma,
 			       unsigned long addr,
 			       xen_pfn_t *gfn, int nr,
@@ -142,6 +141,34 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+struct xen_node_info {
+	const char *compat;
+	const char *prefix;
+	const char *version;
+	bool found;
+};
+
+static int __init fdt_find_xen_node(unsigned long node, const char *uname,
+				    int depth, void *data)
+{
+	struct xen_node_info *info = data;
+	const void *s = NULL;
+	int len;
+
+	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
+		return 0;
+
+	if (of_flat_dt_is_compatible(node, info->compat))
+		info->found = true;
+
+	s = of_get_flat_dt_prop(node, "compatible", &len);
+	if (strlen(info->prefix) + 3  < len &&
+                        !strncmp(info->prefix, s, strlen(info->prefix)))
+                info->version = s + strlen(info->prefix);
+
+	return 0;
+}
+
 /*
  * see Documentation/devicetree/bindings/arm/xen.txt for the
  * documentation of the Xen Device Tree format.
@@ -149,26 +176,25 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
 #define GRANT_TABLE_PHYSADDR 0
 void __init xen_early_init(void)
 {
-	int len;
-	const char *s = NULL;
-	const char *version = NULL;
-	const char *xen_prefix = "xen,xen-";
+	struct xen_node_info info;
+
+	info.compat = "xen,xen";
+	info.prefix = "xen,xen-";
+	info.version = NULL;
+	info.found = false;
 
-	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
-	if (!xen_node) {
+	of_scan_flat_dt(fdt_find_xen_node, &info);
+	if (!info.found) {
 		pr_debug("No Xen support\n");
 		return;
 	}
-	s = of_get_property(xen_node, "compatible", &len);
-	if (strlen(xen_prefix) + 3  < len &&
-			!strncmp(xen_prefix, s, strlen(xen_prefix)))
-		version = s + strlen(xen_prefix);
-	if (version == NULL) {
+
+	if (info.version == NULL) {
 		pr_debug("Xen version not found\n");
 		return;
 	}
 
-	pr_info("Xen %s support found\n", version);
+	pr_info("Xen %s support found\n", info.version);
 
 	xen_domain_type = XEN_HVM_DOMAIN;
 
@@ -204,6 +230,14 @@ static int __init xen_guest_init(void)
 		}
 		xen_events_irq = a.value & 0xff;
 	} else {
+		struct device_node *xen_node;
+
+		xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
+		if (!xen_node) {
+			pr_debug("No Xen support\n");
+			return -ENODEV;
+		}
+
 		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
 		if (!xen_events_irq) {
 			pr_err("Xen event channel interrupt not found\n");
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 2322479..ee95593 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -428,6 +428,7 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	local_async_enable();
 
+	xen_early_init();
 	efi_init();
 	arm64_memblock_init();
 
@@ -446,7 +447,6 @@ void __init setup_arch(char **cmdline_p)
 	} else {
 		psci_acpi_init();
 	}
-	xen_early_init();
 
 	cpu_read_bootcpu_ops();
 	smp_init_cpus();
-- 
2.1.0

[PATCH 13/13] ARM: XEN: Move xen_early_init() before efi_init()

[Stefano Stabellini]

On Tue, 17 Nov 2015, shannon.zhao at linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move xen_early_init() before efi_init(), then when calling efi_init()
> could initialize Xen specific UEFI.
> 
> Check if it runs on Xen hypervisor through the flat dts.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  arch/arm/xen/enlighten.c  | 62 ++++++++++++++++++++++++++++++++++++-----------
>  arch/arm64/kernel/setup.c |  2 +-
>  2 files changed, 49 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index b8e9db8..d4f884c 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -19,6 +19,7 @@
>  #include <linux/irqreturn.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_fdt.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_address.h>
>  #include <linux/cpuidle.h>
> @@ -48,8 +49,6 @@ struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
>  
>  static __read_mostly unsigned int xen_events_irq;
>  
> -static __initdata struct device_node *xen_node;
> -
>  int xen_remap_domain_gfn_array(struct vm_area_struct *vma,
>  			       unsigned long addr,
>  			       xen_pfn_t *gfn, int nr,
> @@ -142,6 +141,34 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +struct xen_node_info {
> +	const char *compat;
> +	const char *prefix;
> +	const char *version;
> +	bool found;
> +};
> +
> +static int __init fdt_find_xen_node(unsigned long node, const char *uname,
> +				    int depth, void *data)
> +{
> +	struct xen_node_info *info = data;
> +	const void *s = NULL;
> +	int len;
> +
> +	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
> +		return 0;
> +
> +	if (of_flat_dt_is_compatible(node, info->compat))
> +		info->found = true;
> +
> +	s = of_get_flat_dt_prop(node, "compatible", &len);
> +	if (strlen(info->prefix) + 3  < len &&
> +                        !strncmp(info->prefix, s, strlen(info->prefix)))
> +                info->version = s + strlen(info->prefix);
> +	return 0;
> +}
> +
>  /*
>   * see Documentation/devicetree/bindings/arm/xen.txt for the
>   * documentation of the Xen Device Tree format.
> @@ -149,26 +176,25 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
>  #define GRANT_TABLE_PHYSADDR 0
>  void __init xen_early_init(void)
>  {
> -	int len;
> -	const char *s = NULL;
> -	const char *version = NULL;
> -	const char *xen_prefix = "xen,xen-";
> +	struct xen_node_info info;
> +
> +	info.compat = "xen,xen";
> +	info.prefix = "xen,xen-";
> +	info.version = NULL;
> +	info.found = false;

Can you initialize the fields directly when you define xen_node_info and
make it static?

The rest looks good.


> -	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
> -	if (!xen_node) {
> +	of_scan_flat_dt(fdt_find_xen_node, &info);
> +	if (!info.found) {
>  		pr_debug("No Xen support\n");
>  		return;
>  	}
> -	s = of_get_property(xen_node, "compatible", &len);
> -	if (strlen(xen_prefix) + 3  < len &&
> -			!strncmp(xen_prefix, s, strlen(xen_prefix)))
> -		version = s + strlen(xen_prefix);
>
> -	if (version == NULL) {
> +
> +	if (info.version == NULL) {
>  		pr_debug("Xen version not found\n");
>  		return;
>  	}
>  
> -	pr_info("Xen %s support found\n", version);
> +	pr_info("Xen %s support found\n", info.version);
>  
>  	xen_domain_type = XEN_HVM_DOMAIN;
>  
> @@ -204,6 +230,14 @@ static int __init xen_guest_init(void)
>  		}
>  		xen_events_irq = a.value & 0xff;
>  	} else {
> +		struct device_node *xen_node;
> +
> +		xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
> +		if (!xen_node) {
> +			pr_debug("No Xen support\n");
> +			return -ENODEV;
> +		}
> +
>  		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>  		if (!xen_events_irq) {
>  			pr_err("Xen event channel interrupt not found\n");
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 2322479..ee95593 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -428,6 +428,7 @@ void __init setup_arch(char **cmdline_p)
>  	 */
>  	local_async_enable();
>  
> +	xen_early_init();
>  	efi_init();
>  	arm64_memblock_init();
>  
> @@ -446,7 +447,6 @@ void __init setup_arch(char **cmdline_p)
>  	} else {
>  		psci_acpi_init();
>  	}
> -	xen_early_init();
>  
>  	cpu_read_bootcpu_ops();
>  	smp_init_cpus();
> -- 
> 2.1.0
> 

[PATCH 13/13] ARM: XEN: Move xen_early_init() before efi_init()

[Shannon Zhao]

On 2015/11/21 1:30, Stefano Stabellini wrote:
>> +struct xen_node_info {
>> > +	const char *compat;
>> > +	const char *prefix;
>> > +	const char *version;
>> > +	bool found;
>> > +};
>> > +
>> > +static int __init fdt_find_xen_node(unsigned long node, const char *uname,
>> > +				    int depth, void *data)
>> > +{
>> > +	struct xen_node_info *info = data;
>> > +	const void *s = NULL;
>> > +	int len;
>> > +
>> > +	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
>> > +		return 0;
>> > +
>> > +	if (of_flat_dt_is_compatible(node, info->compat))
>> > +		info->found = true;
>> > +
>> > +	s = of_get_flat_dt_prop(node, "compatible", &len);
>> > +	if (strlen(info->prefix) + 3  < len &&
>> > +                        !strncmp(info->prefix, s, strlen(info->prefix)))
>> > +                info->version = s + strlen(info->prefix);
>> > +	return 0;
>> > +}
>> > +
>> >  /*
>> >   * see Documentation/devicetree/bindings/arm/xen.txt for the
>> >   * documentation of the Xen Device Tree format.
>> > @@ -149,26 +176,25 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
>> >  #define GRANT_TABLE_PHYSADDR 0
>> >  void __init xen_early_init(void)
>> >  {
>> > -	int len;
>> > -	const char *s = NULL;
>> > -	const char *version = NULL;
>> > -	const char *xen_prefix = "xen,xen-";
>> > +	struct xen_node_info info;
>> > +
>> > +	info.compat = "xen,xen";
>> > +	info.prefix = "xen,xen-";
>> > +	info.version = NULL;
>> > +	info.found = false;
> Can you initialize the fields directly when you define xen_node_info and
> make it static?
Sure.

Thanks,
-- 
Shannon