Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1309909 > unrolled thread

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

Started byShannon Zhao <zhaoshenglong@huawei.com>
First post2016-01-15 08:00 +0100
Last post2016-01-19 09:30 +0100
Articles 20 on this page of 28 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 00/16] Add ACPI support for Xen Dom0 on ARM64 Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:00 +0100
    [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:00 +0100
      Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-15 16:50 +0100
      Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen Mark Rutland <mark.rutland@arm.com> - 2016-01-18 11:40 +0100
        Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-18 12:40 +0100
    [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:00 +0100
      Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI  runtime services Mark Rutland <mark.rutland@arm.com> - 2016-01-18 12:10 +0100
        Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime  services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-18 18:50 +0100
          Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI  runtime services Mark Rutland <mark.rutland@arm.com> - 2016-01-18 19:30 +0100
            Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime  services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-19 13:10 +0100
              Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI  runtime services Mark Rutland <mark.rutland@arm.com> - 2016-01-19 14:10 +0100
                Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime  services Shannon Zhao <shannon.zhao@linaro.org> - 2016-01-19 14:40 +0100
                  Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime  services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-19 15:30 +0100
      Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific  UEFI runtime services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-18 18:10 +0100
        Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific  UEFI runtime services Shannon Zhao <shannon.zhao@linaro.org> - 2016-01-19 14:20 +0100
          Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific  UEFI runtime services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-19 15:30 +0100
        Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific  UEFI runtime services Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-22 05:00 +0100
          Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific  UEFI runtime services Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-22 12:10 +0100
    [PATCH v2 10/16] ARM: XEN: Move xen_early_init() before efi_init() Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:00 +0100
      Re: [Xen-devel] [PATCH v2 10/16] ARM: XEN: Move xen_early_init()  before efi_init() Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-18 16:10 +0100
        Re: [Xen-devel] [PATCH v2 10/16] ARM: XEN: Move xen_early_init()  before efi_init() Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-19 09:40 +0100
          Re: [Xen-devel] [PATCH v2 10/16] ARM: XEN: Move xen_early_init()  before efi_init() Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-19 13:20 +0100
    [PATCH v2 05/16] xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:10 +0100
    [PATCH v2 04/16] arm/xen: Use xen_xlate_map_ballooned_pages to setup grant table Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:10 +0100
      Re: [PATCH v2 04/16] arm/xen: Use xen_xlate_map_ballooned_pages to  setup grant table Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-15 18:30 +0100
    [PATCH v2 09/16] arm/xen: Get event-channel irq through HVM_PARAM when booting with ACPI Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-15 08:10 +0100
      Re: [Xen-devel] [PATCH v2 09/16] arm/xen: Get event-channel irq  through HVM_PARAM when booting with ACPI Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-01-18 15:40 +0100
        Re: [Xen-devel] [PATCH v2 09/16] arm/xen: Get event-channel irq through  HVM_PARAM when booting with ACPI Shannon Zhao <zhaoshenglong@huawei.com> - 2016-01-19 09:30 +0100

Page 1 of 2  [1] 2  Next page →


#1309909 — [PATCH v2 00/16] Add ACPI support for Xen Dom0 on ARM64

FromShannon Zhao <zhaoshenglong@huawei.com>
Date2016-01-15 08:00 +0100
Subject[PATCH v2 00/16] Add ACPI support for Xen Dom0 on ARM64
Message-ID<qR6wx-2kF-3@gated-at.bofh.it>
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. The corresponding Xen patches can
be fetched from [2].

Thanks,
Shannon

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


Changes since v1:
* Rebase on linux mainline and wallclock patch from Stefano
* Refactor AMBA and platform device MMIO map to one file
* Use EFI_PARAVIRT to check if it supports XEN EFI
* Refactor Xen EFI codes
* Address other comments

Shannon Zhao (16):
  Xen: ACPI: Hide UART used by Xen
  xen/grant-table: Move xlated_setup_gnttab_pages to common place
  Xen: xlate: Use page_to_xen_pfn instead of page_to_pfn
  arm/xen: Use xen_xlate_map_ballooned_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
  xen/hvm/params: Add a new delivery 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()
  ARM64: ACPI: Check if it runs on Xen to enable or disable ACPI
  ARM: Xen: Document UEFI support on Xen ARM virtual platforms
  ARM: XEN: Set EFI_PARAVIRT if Xen supports EFI
  Xen: EFI: Parse DT parameters for Xen specific UEFI
  XEN: EFI: Move x86 specific codes to architecture directory
  ARM64: XEN: Initialize Xen specific UEFI runtime services

 Documentation/devicetree/bindings/arm/xen.txt |  42 ++++++
 arch/arm/xen/enlighten.c                      | 121 +++++++++++++----
 arch/arm64/kernel/acpi.c                      |  12 +-
 arch/arm64/kernel/efi.c                       |   5 +
 arch/arm64/kernel/setup.c                     |   2 +-
 arch/arm64/xen/Makefile                       |   1 +
 arch/arm64/xen/efi.c                          |  36 +++++
 arch/x86/xen/efi.c                            | 112 ++++++++++++++++
 arch/x86/xen/grant-table.c                    |  57 +-------
 drivers/acpi/bus.c                            |  30 +++++
 drivers/firmware/efi/efi.c                    |  45 ++++++-
 drivers/xen/Kconfig                           |   2 +-
 drivers/xen/Makefile                          |   1 +
 drivers/xen/arm-device.c                      | 183 ++++++++++++++++++++++++++
 drivers/xen/efi.c                             | 174 +++++-------------------
 drivers/xen/xlate_mmu.c                       |  61 +++++++++
 include/xen/interface/hvm/params.h            |   5 +
 include/xen/interface/memory.h                |   1 +
 include/xen/xen-ops.h                         |  33 +++--
 19 files changed, 689 insertions(+), 234 deletions(-)
 create mode 100644 arch/arm64/xen/efi.c
 create mode 100644 drivers/xen/arm-device.c

-- 
2.0.4

[toc] | [next] | [standalone]


#1309910 — [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen

FromShannon Zhao <zhaoshenglong@huawei.com>
Date2016-01-15 08:00 +0100
Subject[PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen
Message-ID<qR6wz-2kF-37@gated-at.bofh.it>
In reply to#1309909
From: Shannon Zhao <shannon.zhao@linaro.org>

ACPI 6.0 introduces a new table STAO to list the devices which are used
by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
UART is used by Xen. So here it hides UART from Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
CC: "Rafael J. Wysocki" <rjw@rjwysocki.net> (supporter:ACPI)
CC: Len Brown <lenb@kernel.org> (supporter:ACPI)
CC: linux-acpi@vger.kernel.org (open list:ACPI)
---
 drivers/acpi/bus.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a212cef..d7a559f 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 spcr_uart_addr;
 
 #ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -93,6 +94,17 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
 {
 	acpi_status status;
 
+	if (spcr_uart_addr != 0xffffffffffffffff) {
+		u64 addr;
+
+		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
+					       &addr);
+		if (ACPI_SUCCESS(status) && (addr == spcr_uart_addr)) {
+			*sta = 0;
+			return AE_OK;
+		}
+	}
+
 	status = acpi_evaluate_integer(handle, "_STA", NULL, sta);
 	if (ACPI_SUCCESS(status))
 		return AE_OK;
@@ -1069,6 +1081,8 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
 static int __init acpi_init(void)
 {
 	int result;
+	acpi_status status;
+	struct acpi_table_stao *stao_ptr;
 
 	if (acpi_disabled) {
 		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
@@ -1081,6 +1095,22 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	/* If there is STAO table, check whether it needs to ignore the UART
+	 * device in SPCR table.
+	 */
+	spcr_uart_addr = 0xffffffffffffffff;
+	status = acpi_get_table(ACPI_SIG_STAO, 0,
+				(struct acpi_table_header **)&stao_ptr);
+	if (ACPI_SUCCESS(status)) {
+		if (stao_ptr->ignore_uart) {
+			struct acpi_table_spcr *spcr_ptr;
+
+			acpi_get_table(ACPI_SIG_SPCR, 0,
+				       (struct acpi_table_header **)&spcr_ptr);
+			spcr_uart_addr = spcr_ptr->serial_port.address;
+		}
+	}
+
 	init_acpi_device_notify();
 	result = acpi_bus_init();
 	if (result) {
-- 
2.0.4

[toc] | [prev] | [next] | [standalone]


#1310233 — Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-15 16:50 +0100
SubjectRe: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen
Message-ID<qReNt-89q-29@gated-at.bofh.it>
In reply to#1309910
On Fri, 15 Jan 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> ACPI 6.0 introduces a new table STAO to list the devices which are used
> by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> UART is used by Xen. So here it hides UART from Dom0.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> CC: "Rafael J. Wysocki" <rjw@rjwysocki.net> (supporter:ACPI)
> CC: Len Brown <lenb@kernel.org> (supporter:ACPI)
> CC: linux-acpi@vger.kernel.org (open list:ACPI)
> ---
>  drivers/acpi/bus.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cef..d7a559f 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 spcr_uart_addr;
>  
>  #ifdef CONFIG_X86
>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> @@ -93,6 +94,17 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
>  {
>  	acpi_status status;
>  
> +	if (spcr_uart_addr != 0xffffffffffffffff) {

Please #define something like EFI_INVALID_TABLE_ADDR and use it here
instead of 0xffffffffffffffff for readability. Also spcr_uart_addr could
be initialized where it is defined.

Maybe it could be better to move this check into a separate function and
only call acpi_set_device_status if spcr_uart_addr != addr?


> +		u64 addr;
> +
> +		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
> +					       &addr);
> +		if (ACPI_SUCCESS(status) && (addr == spcr_uart_addr)) {
> +			*sta = 0;
> +			return AE_OK;
> +		}
> +	}
> +
>  	status = acpi_evaluate_integer(handle, "_STA", NULL, sta);
>  	if (ACPI_SUCCESS(status))
>  		return AE_OK;
> @@ -1069,6 +1081,8 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
>  static int __init acpi_init(void)
>  {
>  	int result;
> +	acpi_status status;
> +	struct acpi_table_stao *stao_ptr;
>  
>  	if (acpi_disabled) {
>  		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
> @@ -1081,6 +1095,22 @@ static int __init acpi_init(void)
>  		acpi_kobj = NULL;
>  	}
>  
> +	/* If there is STAO table, check whether it needs to ignore the UART
> +	 * device in SPCR table.
> +	 */
> +	spcr_uart_addr = 0xffffffffffffffff;
> +	status = acpi_get_table(ACPI_SIG_STAO, 0,
> +				(struct acpi_table_header **)&stao_ptr);
> +	if (ACPI_SUCCESS(status)) {
> +		if (stao_ptr->ignore_uart) {

if (ACPI_SUCCESS(status) && stao_ptr->ignore_uart) {


> +			struct acpi_table_spcr *spcr_ptr;
> +
> +			acpi_get_table(ACPI_SIG_SPCR, 0,
> +				       (struct acpi_table_header **)&spcr_ptr);

Maybe we should check that acpi_get_table succeeded, even though it
should at this point.


> +			spcr_uart_addr = spcr_ptr->serial_port.address;
> +		}
> +	}
> +
>  	init_acpi_device_notify();
>  	result = acpi_bus_init();
>  	if (result) {

[toc] | [prev] | [next] | [standalone]


#1311462 — Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-18 11:40 +0100
SubjectRe: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen
Message-ID<qSfo5-7P5-1@gated-at.bofh.it>
In reply to#1309910
On Fri, Jan 15, 2016 at 02:55:14PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> ACPI 6.0 introduces a new table STAO to list the devices which are used
> by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> UART is used by Xen. So here it hides UART from Dom0.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> CC: "Rafael J. Wysocki" <rjw@rjwysocki.net> (supporter:ACPI)
> CC: Len Brown <lenb@kernel.org> (supporter:ACPI)
> CC: linux-acpi@vger.kernel.org (open list:ACPI)
> ---
>  drivers/acpi/bus.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index a212cef..d7a559f 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 spcr_uart_addr;
>  
>  #ifdef CONFIG_X86
>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> @@ -93,6 +94,17 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
>  {
>  	acpi_status status;
>  
> +	if (spcr_uart_addr != 0xffffffffffffffff) {

The SPCR spec says that the Base Address fields being zero means that
console redirection is disabled (though I'm not clear on whether or not
that requires the whole acpi_generic_address to be zero).

Can we not use that here?

Mark.

> +		u64 addr;
> +
> +		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
> +					       &addr);
> +		if (ACPI_SUCCESS(status) && (addr == spcr_uart_addr)) {
> +			*sta = 0;
> +			return AE_OK;
> +		}
> +	}
> +
>  	status = acpi_evaluate_integer(handle, "_STA", NULL, sta);
>  	if (ACPI_SUCCESS(status))
>  		return AE_OK;
> @@ -1069,6 +1081,8 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
>  static int __init acpi_init(void)
>  {
>  	int result;
> +	acpi_status status;
> +	struct acpi_table_stao *stao_ptr;
>  
>  	if (acpi_disabled) {
>  		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
> @@ -1081,6 +1095,22 @@ static int __init acpi_init(void)
>  		acpi_kobj = NULL;
>  	}
>  
> +	/* If there is STAO table, check whether it needs to ignore the UART
> +	 * device in SPCR table.
> +	 */
> +	spcr_uart_addr = 0xffffffffffffffff;
> +	status = acpi_get_table(ACPI_SIG_STAO, 0,
> +				(struct acpi_table_header **)&stao_ptr);
> +	if (ACPI_SUCCESS(status)) {
> +		if (stao_ptr->ignore_uart) {
> +			struct acpi_table_spcr *spcr_ptr;
> +
> +			acpi_get_table(ACPI_SIG_SPCR, 0,
> +				       (struct acpi_table_header **)&spcr_ptr);
> +			spcr_uart_addr = spcr_ptr->serial_port.address;
> +		}
> +	}
> +
>  	init_acpi_device_notify();
>  	result = acpi_bus_init();
>  	if (result) {
> -- 
> 2.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]


#1311493 — Re: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-18 12:40 +0100
SubjectRe: [PATCH v2 01/16] Xen: ACPI: Hide UART used by Xen
Message-ID<qSgka-8sc-15@gated-at.bofh.it>
In reply to#1311462
On Mon, 18 Jan 2016, Mark Rutland wrote:
> On Fri, Jan 15, 2016 at 02:55:14PM +0800, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > UART is used by Xen. So here it hides UART from Dom0.
> > 
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> > CC: "Rafael J. Wysocki" <rjw@rjwysocki.net> (supporter:ACPI)
> > CC: Len Brown <lenb@kernel.org> (supporter:ACPI)
> > CC: linux-acpi@vger.kernel.org (open list:ACPI)
> > ---
> >  drivers/acpi/bus.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index a212cef..d7a559f 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 spcr_uart_addr;
> >  
> >  #ifdef CONFIG_X86
> >  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> > @@ -93,6 +94,17 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
> >  {
> >  	acpi_status status;
> >  
> > +	if (spcr_uart_addr != 0xffffffffffffffff) {
> 
> The SPCR spec says that the Base Address fields being zero means that
> console redirection is disabled (though I'm not clear on whether or not
> that requires the whole acpi_generic_address to be zero).
> 
> Can we not use that here?

Shannon changed the code from 0 to 0xffffffffffffffff following my
suggestion to his previous series, but I didn't realize that the SPCR
spec says 0 means no console redirection. In that case using 0 is
better.

[toc] | [prev] | [next] | [standalone]


#1309911 — [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromShannon Zhao <zhaoshenglong@huawei.com>
Date2016-01-15 08:00 +0100
Subject[PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qR6wy-2kF-21@gated-at.bofh.it>
In reply to#1309909
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/xen/enlighten.c |  5 +++++
 arch/arm64/xen/Makefile  |  1 +
 arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
 drivers/xen/Kconfig      |  2 +-
 include/xen/xen-ops.h    |  1 +
 5 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/xen/efi.c

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 485e117..84f27ec 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
 	if (xen_initial_domain())
 		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
 
+	if (IS_ENABLED(CONFIG_XEN_EFI)) {
+		if (efi_enabled(EFI_PARAVIRT))
+			xen_efi_runtime_setup();
+	}
+
 	return 0;
 }
 early_initcall(xen_guest_init);
diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
index 74a8d87..62e6fe2 100644
--- a/arch/arm64/xen/Makefile
+++ b/arch/arm64/xen/Makefile
@@ -1,2 +1,3 @@
 xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
 obj-y		:= xen-arm.o hypercall.o
+obj-$(CONFIG_XEN_EFI) += efi.o
diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
new file mode 100644
index 0000000..33046b0
--- /dev/null
+++ b/arch/arm64/xen/efi.c
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2015, Linaro Limited, Shannon Zhao
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/efi.h>
+#include <xen/xen-ops.h>
+
+void __init 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;
+	efi.reset_system             = NULL;
+}
+EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 73708ac..27d216a 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/include/xen/xen-ops.h b/include/xen/xen-ops.h
index c83a338..36ff8e4 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -107,6 +107,7 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
 efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
 					unsigned long count, u64 *max_size,
 					int *reset_type);
+void xen_efi_runtime_setup(void);
 
 #ifdef CONFIG_PREEMPT
 
-- 
2.0.4

[toc] | [prev] | [next] | [standalone]


#1311481 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-18 12:10 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSfR8-8gP-23@gated-at.bofh.it>
In reply to#1309911
On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao 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/xen/enlighten.c |  5 +++++
>  arch/arm64/xen/Makefile  |  1 +
>  arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/xen/Kconfig      |  2 +-
>  include/xen/xen-ops.h    |  1 +
>  5 files changed, 44 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/xen/efi.c
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 485e117..84f27ec 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
>  	if (xen_initial_domain())
>  		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
>  
> +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
> +		if (efi_enabled(EFI_PARAVIRT))
> +			xen_efi_runtime_setup();
> +	}
> +
>  	return 0;
>  }
>  early_initcall(xen_guest_init);
> diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> index 74a8d87..62e6fe2 100644
> --- a/arch/arm64/xen/Makefile
> +++ b/arch/arm64/xen/Makefile
> @@ -1,2 +1,3 @@
>  xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
>  obj-y		:= xen-arm.o hypercall.o
> +obj-$(CONFIG_XEN_EFI) += efi.o
> diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
> new file mode 100644
> index 0000000..33046b0
> --- /dev/null
> +++ b/arch/arm64/xen/efi.c
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/efi.h>
> +#include <xen/xen-ops.h>
> +
> +void __init 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;
> +	efi.reset_system             = NULL;
> +}

How do capsules work in the absence of an EFI system reset?

Are there any other mandatory features that are missing in a
Xen-provided pseudo-EFI?

Mark.

> +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 73708ac..27d216a 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/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index c83a338..36ff8e4 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -107,6 +107,7 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
>  efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
>  					unsigned long count, u64 *max_size,
>  					int *reset_type);
> +void xen_efi_runtime_setup(void);
>  
>  #ifdef CONFIG_PREEMPT
>  
> -- 
> 2.0.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]


#1311717 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-18 18:50 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSm6e-3ZZ-29@gated-at.bofh.it>
In reply to#1311481
On Mon, 18 Jan 2016, Mark Rutland wrote:
> On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao 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/xen/enlighten.c |  5 +++++
> >  arch/arm64/xen/Makefile  |  1 +
> >  arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
> >  drivers/xen/Kconfig      |  2 +-
> >  include/xen/xen-ops.h    |  1 +
> >  5 files changed, 44 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm64/xen/efi.c
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index 485e117..84f27ec 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
> >  	if (xen_initial_domain())
> >  		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
> >  
> > +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
> > +		if (efi_enabled(EFI_PARAVIRT))
> > +			xen_efi_runtime_setup();
> > +	}
> > +
> >  	return 0;
> >  }
> >  early_initcall(xen_guest_init);
> > diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> > index 74a8d87..62e6fe2 100644
> > --- a/arch/arm64/xen/Makefile
> > +++ b/arch/arm64/xen/Makefile
> > @@ -1,2 +1,3 @@
> >  xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
> >  obj-y		:= xen-arm.o hypercall.o
> > +obj-$(CONFIG_XEN_EFI) += efi.o
> > diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
> > new file mode 100644
> > index 0000000..33046b0
> > --- /dev/null
> > +++ b/arch/arm64/xen/efi.c
> > @@ -0,0 +1,36 @@
> > +/*
> > + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/efi.h>
> > +#include <xen/xen-ops.h>
> > +
> > +void __init 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;
> > +	efi.reset_system             = NULL;
> > +}
> 
> How do capsules work in the absence of an EFI system reset?

Actually I don't think that capsules are available in Xen on ARM64 yet,
see "TODO - disabled until implemented on ARM" in
xen/common/efi/runtime.c.

FYI system reset is available, but it is provided via a different
mechanism (HYPERVISOR_sched_op(xen_restart...)


> Are there any other mandatory features that are missing in a
> Xen-provided pseudo-EFI?

[toc] | [prev] | [next] | [standalone]


#1311736 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-18 19:30 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSmIV-4vc-3@gated-at.bofh.it>
In reply to#1311717
On Mon, Jan 18, 2016 at 05:45:24PM +0000, Stefano Stabellini wrote:
> On Mon, 18 Jan 2016, Mark Rutland wrote:
> > On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao wrote:
> > > +void __init 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;
> > > +	efi.reset_system             = NULL;
> > > +}
> > 
> > How do capsules work in the absence of an EFI system reset?
> 
> Actually I don't think that capsules are available in Xen on ARM64 yet,
> see "TODO - disabled until implemented on ARM" in
> xen/common/efi/runtime.c.
> 
> FYI system reset is available, but it is provided via a different
> mechanism (HYPERVISOR_sched_op(xen_restart...)

Will that trigger Xen to do the right thing to trigger capsule updates
when implemented in Xen? Or do we need a xen_efi_reset_system?

Does that override PSCI?

In machine_restart we try efi_reboot first specifically to allow for
capsule updates. Similarly drivers/firmware/efi/reboot.c registers
efi_power_off late in order to override anything else, though that's
best-effort at present.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]


#1312170 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-19 13:10 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSDgK-7DY-17@gated-at.bofh.it>
In reply to#1311736
On Mon, 18 Jan 2016, Mark Rutland wrote:
> On Mon, Jan 18, 2016 at 05:45:24PM +0000, Stefano Stabellini wrote:
> > On Mon, 18 Jan 2016, Mark Rutland wrote:
> > > On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao wrote:
> > > > +void __init 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;
> > > > +	efi.reset_system             = NULL;
> > > > +}
> > > 
> > > How do capsules work in the absence of an EFI system reset?
> > 
> > Actually I don't think that capsules are available in Xen on ARM64 yet,
> > see "TODO - disabled until implemented on ARM" in
> > xen/common/efi/runtime.c.
> > 
> > FYI system reset is available, but it is provided via a different
> > mechanism (HYPERVISOR_sched_op(xen_restart...)
> 
> Will that trigger Xen to do the right thing to trigger capsule updates
> when implemented in Xen? Or do we need a xen_efi_reset_system?

On ARM, to reboot the hardware, Xen calls the native PSCI system_reset
method. On x86, Xen calls efi_reset_system on EFI systems, and has
several fall backs if that doesn't work as expected (see
xen/arch/x86/shutdown.c:machine_restart).

But on a second look it doesn't look like that the capsule hypercalls
are implemented correctly even on x86 (there is an "XXX fall through for
now" comment in the code). I guess they are not available on Xen at all
unfortunately.


> Does that override PSCI?

It does not, HYPERVISOR_sched_op(xen_restart,) is in addition to it. It
ends up calling the same function within Xen as PSCI system_reset.


> In machine_restart we try efi_reboot first specifically to allow for
> capsule updates. Similarly drivers/firmware/efi/reboot.c registers
> efi_power_off late in order to override anything else, though that's
> best-effort at present.

That's very interesting. I think that Xen on ARM should follow what
Linux does and what Xen already does on x86 and try efi_reset_system
first on efi systems.

[toc] | [prev] | [next] | [standalone]


#1312192 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromMark Rutland <mark.rutland@arm.com>
Date2016-01-19 14:10 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSEcO-8kf-7@gated-at.bofh.it>
In reply to#1312170
On Tue, Jan 19, 2016 at 12:03:59PM +0000, Stefano Stabellini wrote:
> On Mon, 18 Jan 2016, Mark Rutland wrote:
> > On Mon, Jan 18, 2016 at 05:45:24PM +0000, Stefano Stabellini wrote:
> > > On Mon, 18 Jan 2016, Mark Rutland wrote:
> > > > On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao wrote:
> > > > > +void __init 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;
> > > > > +	efi.reset_system             = NULL;
> > > > > +}
> > > > 
> > > > How do capsules work in the absence of an EFI system reset?
> > > 
> > > Actually I don't think that capsules are available in Xen on ARM64 yet,
> > > see "TODO - disabled until implemented on ARM" in
> > > xen/common/efi/runtime.c.
> > > 
> > > FYI system reset is available, but it is provided via a different
> > > mechanism (HYPERVISOR_sched_op(xen_restart...)
> > 
> > Will that trigger Xen to do the right thing to trigger capsule updates
> > when implemented in Xen? Or do we need a xen_efi_reset_system?
> 
> On ARM, to reboot the hardware, Xen calls the native PSCI system_reset
> method. On x86, Xen calls efi_reset_system on EFI systems, and has
> several fall backs if that doesn't work as expected (see
> xen/arch/x86/shutdown.c:machine_restart).
> 
> But on a second look it doesn't look like that the capsule hypercalls
> are implemented correctly even on x86 (there is an "XXX fall through for
> now" comment in the code). I guess they are not available on Xen at all
> unfortunately.

That is incredibly unfortunate. It effectively renders the firmware
non-updateable when using Xen.

> > Does that override PSCI?
> 
> It does not, HYPERVISOR_sched_op(xen_restart,) is in addition to it. It
> ends up calling the same function within Xen as PSCI system_reset.

I meant within Dom0.

Presumably Dom0 calls HYPERVISOR_sched_op(xen_restart,), and doesn't
ever call PSCI SYSTEM_RESET?

> > In machine_restart we try efi_reboot first specifically to allow for
> > capsule updates. Similarly drivers/firmware/efi/reboot.c registers
> > efi_power_off late in order to override anything else, though that's
> > best-effort at present.
> 
> That's very interesting. I think that Xen on ARM should follow what
> Linux does and what Xen already does on x86 and try efi_reset_system
> first on efi systems.

I would agree.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]


#1312225 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromShannon Zhao <shannon.zhao@linaro.org>
Date2016-01-19 14:40 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSEFP-4S-1@gated-at.bofh.it>
In reply to#1312192

On 2016/1/19 21:03, Mark Rutland wrote:
> On Tue, Jan 19, 2016 at 12:03:59PM +0000, Stefano Stabellini wrote:
>> On Mon, 18 Jan 2016, Mark Rutland wrote:
>>> On Mon, Jan 18, 2016 at 05:45:24PM +0000, Stefano Stabellini wrote:
>>>> On Mon, 18 Jan 2016, Mark Rutland wrote:
>>>>> On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao wrote:
>>>>>> +void __init 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;
>>>>>> +	efi.reset_system             = NULL;
>>>>>> +}
>>>>>
>>>>> How do capsules work in the absence of an EFI system reset?
>>>>
>>>> Actually I don't think that capsules are available in Xen on ARM64 yet,
>>>> see "TODO - disabled until implemented on ARM" in
>>>> xen/common/efi/runtime.c.
>>>>
>>>> FYI system reset is available, but it is provided via a different
>>>> mechanism (HYPERVISOR_sched_op(xen_restart...)
>>>
>>> Will that trigger Xen to do the right thing to trigger capsule updates
>>> when implemented in Xen? Or do we need a xen_efi_reset_system?
>>
>> On ARM, to reboot the hardware, Xen calls the native PSCI system_reset
>> method. On x86, Xen calls efi_reset_system on EFI systems, and has
>> several fall backs if that doesn't work as expected (see
>> xen/arch/x86/shutdown.c:machine_restart).
>>
>> But on a second look it doesn't look like that the capsule hypercalls
>> are implemented correctly even on x86 (there is an "XXX fall through for
>> now" comment in the code). I guess they are not available on Xen at all
>> unfortunately.
>
> That is incredibly unfortunate. It effectively renders the firmware
> non-updateable when using Xen.
>
>>> Does that override PSCI?
>>
>> It does not, HYPERVISOR_sched_op(xen_restart,) is in addition to it. It
>> ends up calling the same function within Xen as PSCI system_reset.
>
> I meant within Dom0.
>
> Presumably Dom0 calls HYPERVISOR_sched_op(xen_restart,), and doesn't
> ever call PSCI SYSTEM_RESET?
>
I think executing reset in Dom0 will reset not only Dom0 but also the 
Xen hypervisor, right?

>>> In machine_restart we try efi_reboot first specifically to allow for
>>> capsule updates. Similarly drivers/firmware/efi/reboot.c registers
>>> efi_power_off late in order to override anything else, though that's
>>> best-effort at present.
>>
>> That's very interesting. I think that Xen on ARM should follow what
>> Linux does and what Xen already does on x86 and try efi_reset_system
>> first on efi systems.
>
> I would agree.
>
> Thanks,
> Mark.
>

-- 
Shannon

[toc] | [prev] | [next] | [standalone]


#1312279 — Re: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-19 15:30 +0100
SubjectRe: [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSFsd-Eb-5@gated-at.bofh.it>
In reply to#1312225
On Tue, 19 Jan 2016, Shannon Zhao wrote:
> On 2016/1/19 21:03, Mark Rutland wrote:
> > On Tue, Jan 19, 2016 at 12:03:59PM +0000, Stefano Stabellini wrote:
> > > On Mon, 18 Jan 2016, Mark Rutland wrote:
> > > > On Mon, Jan 18, 2016 at 05:45:24PM +0000, Stefano Stabellini wrote:
> > > > > On Mon, 18 Jan 2016, Mark Rutland wrote:
> > > > > > On Fri, Jan 15, 2016 at 02:55:29PM +0800, Shannon Zhao wrote:
> > > > > > > +void __init 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;
> > > > > > > +	efi.reset_system             = NULL;
> > > > > > > +}
> > > > > > 
> > > > > > How do capsules work in the absence of an EFI system reset?
> > > > > 
> > > > > Actually I don't think that capsules are available in Xen on ARM64
> > > > > yet,
> > > > > see "TODO - disabled until implemented on ARM" in
> > > > > xen/common/efi/runtime.c.
> > > > > 
> > > > > FYI system reset is available, but it is provided via a different
> > > > > mechanism (HYPERVISOR_sched_op(xen_restart...)
> > > > 
> > > > Will that trigger Xen to do the right thing to trigger capsule updates
> > > > when implemented in Xen? Or do we need a xen_efi_reset_system?
> > > 
> > > On ARM, to reboot the hardware, Xen calls the native PSCI system_reset
> > > method. On x86, Xen calls efi_reset_system on EFI systems, and has
> > > several fall backs if that doesn't work as expected (see
> > > xen/arch/x86/shutdown.c:machine_restart).
> > > 
> > > But on a second look it doesn't look like that the capsule hypercalls
> > > are implemented correctly even on x86 (there is an "XXX fall through for
> > > now" comment in the code). I guess they are not available on Xen at all
> > > unfortunately.
> > 
> > That is incredibly unfortunate. It effectively renders the firmware
> > non-updateable when using Xen.
> > 
> > > > Does that override PSCI?
> > > 
> > > It does not, HYPERVISOR_sched_op(xen_restart,) is in addition to it. It
> > > ends up calling the same function within Xen as PSCI system_reset.
> > 
> > I meant within Dom0.
> > 
> > Presumably Dom0 calls HYPERVISOR_sched_op(xen_restart,), and doesn't
> > ever call PSCI SYSTEM_RESET?
> > 
> I think executing reset in Dom0 will reset not only Dom0 but also the Xen
> hypervisor, right?

Dom0 and DomUs call an HYPERVISOR_sched_op for machine reboot and
shutdown (by setting pm_power_off and arm_pm_restart), but a virtual
PSCI interface is also available and can be used. In the case of DomUs
the virtul machine is rebooted or shut down, in the case of Dom0, the
physical machine is rebooted or shut down.

The native PSCI methods are never exposed to Dom0 (or any DomUs).

[toc] | [prev] | [next] | [standalone]


#1311682 — Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-18 18:10 +0100
SubjectRe: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSltv-3KT-11@gated-at.bofh.it>
In reply to#1309911
On Fri, 15 Jan 2016, Shannon Zhao 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>

Thanks Shannon, much much better!  Just a couple of questions.


>  arch/arm/xen/enlighten.c |  5 +++++
>  arch/arm64/xen/Makefile  |  1 +
>  arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/xen/Kconfig      |  2 +-
>  include/xen/xen-ops.h    |  1 +
>  5 files changed, 44 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/xen/efi.c
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 485e117..84f27ec 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
>  	if (xen_initial_domain())
>  		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
>  
> +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
> +		if (efi_enabled(EFI_PARAVIRT))
> +			xen_efi_runtime_setup();
> +	}
> +
>  	return 0;
>  }
>  early_initcall(xen_guest_init);
> diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> index 74a8d87..62e6fe2 100644
> --- a/arch/arm64/xen/Makefile
> +++ b/arch/arm64/xen/Makefile
> @@ -1,2 +1,3 @@
>  xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
>  obj-y		:= xen-arm.o hypercall.o
> +obj-$(CONFIG_XEN_EFI) += efi.o
> diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
> new file mode 100644
> index 0000000..33046b0
> --- /dev/null
> +++ b/arch/arm64/xen/efi.c
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/efi.h>
> +#include <xen/xen-ops.h>
> +
> +void __init 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;
> +	efi.reset_system             = NULL;
> +}
> +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);

This looks very similar to struct efi efi_xen previously in
drivers/xen/efi.c.  Maybe it makes sense to leave struct efi efi_xen in
drivers/xen/efi.c, export it in include/xen/xen-ops.h, then here just:

  efi = efi_xen;

Would that improve code readability?


Correct me if I am wrong, but on ARM64 (differently from x86) it is not
necessary to set efi.systab because it is not used, right? If so, it
would be best to add a comment here to remember.


> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 73708ac..27d216a 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/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index c83a338..36ff8e4 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -107,6 +107,7 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
>  efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
>  					unsigned long count, u64 *max_size,
>  					int *reset_type);
> +void xen_efi_runtime_setup(void);

xen_efi_runtime_setup is not defined on x86, but this header is not arch
specific.

[toc] | [prev] | [next] | [standalone]


#1312200 — Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromShannon Zhao <shannon.zhao@linaro.org>
Date2016-01-19 14:20 +0100
SubjectRe: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSEmu-8o1-5@gated-at.bofh.it>
In reply to#1311682

On 2016/1/19 1:03, Stefano Stabellini wrote:
> On Fri, 15 Jan 2016, Shannon Zhao 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>
>
> Thanks Shannon, much much better!  Just a couple of questions.
>
>
>>   arch/arm/xen/enlighten.c |  5 +++++
>>   arch/arm64/xen/Makefile  |  1 +
>>   arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
>>   drivers/xen/Kconfig      |  2 +-
>>   include/xen/xen-ops.h    |  1 +
>>   5 files changed, 44 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm64/xen/efi.c
>>
>> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> index 485e117..84f27ec 100644
>> --- a/arch/arm/xen/enlighten.c
>> +++ b/arch/arm/xen/enlighten.c
>> @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
>>   	if (xen_initial_domain())
>>   		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
>>
>> +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
>> +		if (efi_enabled(EFI_PARAVIRT))
>> +			xen_efi_runtime_setup();
>> +	}
>> +
>>   	return 0;
>>   }
>>   early_initcall(xen_guest_init);
>> diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
>> index 74a8d87..62e6fe2 100644
>> --- a/arch/arm64/xen/Makefile
>> +++ b/arch/arm64/xen/Makefile
>> @@ -1,2 +1,3 @@
>>   xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
>>   obj-y		:= xen-arm.o hypercall.o
>> +obj-$(CONFIG_XEN_EFI) += efi.o
>> diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
>> new file mode 100644
>> index 0000000..33046b0
>> --- /dev/null
>> +++ b/arch/arm64/xen/efi.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/efi.h>
>> +#include <xen/xen-ops.h>
>> +
>> +void __init 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;
>> +	efi.reset_system             = NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
>
> This looks very similar to struct efi efi_xen previously in
> drivers/xen/efi.c.  Maybe it makes sense to leave struct efi efi_xen in
> drivers/xen/efi.c, export it in include/xen/xen-ops.h, then here just:
>
>    efi = efi_xen;
>
> Would that improve code readability?
>
Ok.
>
> Correct me if I am wrong, but on ARM64 (differently from x86) it is not
> necessary to set efi.systab because it is not used, right? If so, it
> would be best to add a comment here to remember.
>
Not set efi.systab here because it gets the system table through fdt and 
set efi.systab there. See uefi_init() in arch/arm64/kernel.efi.c

	efi.systab = early_memremap(efi_system_table,
				    sizeof(efi_system_table_t));
>
>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> index 73708ac..27d216a 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/include/xen/xen-ops.h b/include/xen/xen-ops.h
>> index c83a338..36ff8e4 100644
>> --- a/include/xen/xen-ops.h
>> +++ b/include/xen/xen-ops.h
>> @@ -107,6 +107,7 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
>>   efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
>>   					unsigned long count, u64 *max_size,
>>   					int *reset_type);
>> +void xen_efi_runtime_setup(void);
>
> xen_efi_runtime_setup is not defined on x86, but this header is not arch
> specific.
>

-- 
Shannon

[toc] | [prev] | [next] | [standalone]


#1312292 — Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-19 15:30 +0100
SubjectRe: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qSFsf-Eb-41@gated-at.bofh.it>
In reply to#1312200
On Tue, 19 Jan 2016, Shannon Zhao wrote:
> On 2016/1/19 1:03, Stefano Stabellini wrote:
> > On Fri, 15 Jan 2016, Shannon Zhao 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>
> > 
> > Thanks Shannon, much much better!  Just a couple of questions.
> > 
> > 
> > >   arch/arm/xen/enlighten.c |  5 +++++
> > >   arch/arm64/xen/Makefile  |  1 +
> > >   arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
> > >   drivers/xen/Kconfig      |  2 +-
> > >   include/xen/xen-ops.h    |  1 +
> > >   5 files changed, 44 insertions(+), 1 deletion(-)
> > >   create mode 100644 arch/arm64/xen/efi.c
> > > 
> > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > > index 485e117..84f27ec 100644
> > > --- a/arch/arm/xen/enlighten.c
> > > +++ b/arch/arm/xen/enlighten.c
> > > @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
> > >   	if (xen_initial_domain())
> > >   		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
> > > 
> > > +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
> > > +		if (efi_enabled(EFI_PARAVIRT))
> > > +			xen_efi_runtime_setup();
> > > +	}
> > > +
> > >   	return 0;
> > >   }
> > >   early_initcall(xen_guest_init);
> > > diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> > > index 74a8d87..62e6fe2 100644
> > > --- a/arch/arm64/xen/Makefile
> > > +++ b/arch/arm64/xen/Makefile
> > > @@ -1,2 +1,3 @@
> > >   xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o
> > > mm.o)
> > >   obj-y		:= xen-arm.o hypercall.o
> > > +obj-$(CONFIG_XEN_EFI) += efi.o
> > > diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
> > > new file mode 100644
> > > index 0000000..33046b0
> > > --- /dev/null
> > > +++ b/arch/arm64/xen/efi.c
> > > @@ -0,0 +1,36 @@
> > > +/*
> > > + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include <linux/efi.h>
> > > +#include <xen/xen-ops.h>
> > > +
> > > +void __init 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;
> > > +	efi.reset_system             = NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
> > 
> > This looks very similar to struct efi efi_xen previously in
> > drivers/xen/efi.c.  Maybe it makes sense to leave struct efi efi_xen in
> > drivers/xen/efi.c, export it in include/xen/xen-ops.h, then here just:
> > 
> >    efi = efi_xen;
> > 
> > Would that improve code readability?
> > 
> Ok.
> > 
> > Correct me if I am wrong, but on ARM64 (differently from x86) it is not
> > necessary to set efi.systab because it is not used, right? If so, it
> > would be best to add a comment here to remember.
> > 
> Not set efi.systab here because it gets the system table through fdt and set
> efi.systab there. See uefi_init() in arch/arm64/kernel.efi.c
> 
> 	efi.systab = early_memremap(efi_system_table,
> 				    sizeof(efi_system_table_t));

I see now. Then it might be still good to add a comment about that.


> > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > > index 73708ac..27d216a 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/include/xen/xen-ops.h b/include/xen/xen-ops.h
> > > index c83a338..36ff8e4 100644
> > > --- a/include/xen/xen-ops.h
> > > +++ b/include/xen/xen-ops.h
> > > @@ -107,6 +107,7 @@ efi_status_t
> > > xen_efi_update_capsule(efi_capsule_header_t **capsules,
> > >   efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
> > >   					unsigned long count, u64 *max_size,
> > >   					int *reset_type);
> > > +void xen_efi_runtime_setup(void);
> > 
> > xen_efi_runtime_setup is not defined on x86, but this header is not arch
> > specific.
> > 
> 
> -- 
> Shannon
> 

[toc] | [prev] | [next] | [standalone]


#1314737 — Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromShannon Zhao <zhaoshenglong@huawei.com>
Date2016-01-22 05:00 +0100
SubjectRe: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qTB3c-6EO-5@gated-at.bofh.it>
In reply to#1311682

On 2016/1/19 1:03, Stefano Stabellini wrote:
> On Fri, 15 Jan 2016, Shannon Zhao 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>
> Thanks Shannon, much much better!  Just a couple of questions.
> 
> 
>> >  arch/arm/xen/enlighten.c |  5 +++++
>> >  arch/arm64/xen/Makefile  |  1 +
>> >  arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
>> >  drivers/xen/Kconfig      |  2 +-
>> >  include/xen/xen-ops.h    |  1 +
>> >  5 files changed, 44 insertions(+), 1 deletion(-)
>> >  create mode 100644 arch/arm64/xen/efi.c
>> > 
>> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> > index 485e117..84f27ec 100644
>> > --- a/arch/arm/xen/enlighten.c
>> > +++ b/arch/arm/xen/enlighten.c
>> > @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
>> >  	if (xen_initial_domain())
>> >  		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
>> >  
>> > +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
>> > +		if (efi_enabled(EFI_PARAVIRT))
>> > +			xen_efi_runtime_setup();
>> > +	}
>> > +
>> >  	return 0;
>> >  }
>> >  early_initcall(xen_guest_init);
>> > diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
>> > index 74a8d87..62e6fe2 100644
>> > --- a/arch/arm64/xen/Makefile
>> > +++ b/arch/arm64/xen/Makefile
>> > @@ -1,2 +1,3 @@
>> >  xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
>> >  obj-y		:= xen-arm.o hypercall.o
>> > +obj-$(CONFIG_XEN_EFI) += efi.o
>> > diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
>> > new file mode 100644
>> > index 0000000..33046b0
>> > --- /dev/null
>> > +++ b/arch/arm64/xen/efi.c
>> > @@ -0,0 +1,36 @@
>> > +/*
>> > + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation; either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
>> > + */
>> > +
>> > +#include <linux/efi.h>
>> > +#include <xen/xen-ops.h>
>> > +
>> > +void __init 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;
>> > +	efi.reset_system             = NULL;
>> > +}
>> > +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
> This looks very similar to struct efi efi_xen previously in
> drivers/xen/efi.c.  Maybe it makes sense to leave struct efi efi_xen in
> drivers/xen/efi.c, export it in include/xen/xen-ops.h, then here just:
> 
>   efi = efi_xen;
> 
> Would that improve code readability?

Rethink about this. It's a little different on ARM since we call
xen_efi_runtime_setup after parsing the FDT and setting some members of
efi already, e.g. efi.systab, efi.acpi20. So it necessary to have a
different way to initialize the struct efi.

Thanks,
-- 
Shannon

[toc] | [prev] | [next] | [standalone]


#1314901 — Re: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-22 12:10 +0100
SubjectRe: [Xen-devel] [PATCH v2 16/16] ARM64: XEN: Initialize Xen specific UEFI runtime services
Message-ID<qTHLl-3dT-49@gated-at.bofh.it>
In reply to#1314737
On Fri, 22 Jan 2016, Shannon Zhao wrote:
> On 2016/1/19 1:03, Stefano Stabellini wrote:
> > On Fri, 15 Jan 2016, Shannon Zhao 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>
> > Thanks Shannon, much much better!  Just a couple of questions.
> > 
> > 
> >> >  arch/arm/xen/enlighten.c |  5 +++++
> >> >  arch/arm64/xen/Makefile  |  1 +
> >> >  arch/arm64/xen/efi.c     | 36 ++++++++++++++++++++++++++++++++++++
> >> >  drivers/xen/Kconfig      |  2 +-
> >> >  include/xen/xen-ops.h    |  1 +
> >> >  5 files changed, 44 insertions(+), 1 deletion(-)
> >> >  create mode 100644 arch/arm64/xen/efi.c
> >> > 
> >> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> >> > index 485e117..84f27ec 100644
> >> > --- a/arch/arm/xen/enlighten.c
> >> > +++ b/arch/arm/xen/enlighten.c
> >> > @@ -414,6 +414,11 @@ static int __init xen_guest_init(void)
> >> >  	if (xen_initial_domain())
> >> >  		pvclock_gtod_register_notifier(&xen_pvclock_gtod_notifier);
> >> >  
> >> > +	if (IS_ENABLED(CONFIG_XEN_EFI)) {
> >> > +		if (efi_enabled(EFI_PARAVIRT))
> >> > +			xen_efi_runtime_setup();
> >> > +	}
> >> > +
> >> >  	return 0;
> >> >  }
> >> >  early_initcall(xen_guest_init);
> >> > diff --git a/arch/arm64/xen/Makefile b/arch/arm64/xen/Makefile
> >> > index 74a8d87..62e6fe2 100644
> >> > --- a/arch/arm64/xen/Makefile
> >> > +++ b/arch/arm64/xen/Makefile
> >> > @@ -1,2 +1,3 @@
> >> >  xen-arm-y	+= $(addprefix ../../arm/xen/, enlighten.o grant-table.o p2m.o mm.o)
> >> >  obj-y		:= xen-arm.o hypercall.o
> >> > +obj-$(CONFIG_XEN_EFI) += efi.o
> >> > diff --git a/arch/arm64/xen/efi.c b/arch/arm64/xen/efi.c
> >> > new file mode 100644
> >> > index 0000000..33046b0
> >> > --- /dev/null
> >> > +++ b/arch/arm64/xen/efi.c
> >> > @@ -0,0 +1,36 @@
> >> > +/*
> >> > + * Copyright (c) 2015, Linaro Limited, Shannon Zhao
> >> > + *
> >> > + * This program is free software; you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation; either version 2 of the License, or
> >> > + * (at your option) any later version.
> >> > + *
> >> > + * This program is distributed in the hope that 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, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +#include <linux/efi.h>
> >> > +#include <xen/xen-ops.h>
> >> > +
> >> > +void __init 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;
> >> > +	efi.reset_system             = NULL;
> >> > +}
> >> > +EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
> > This looks very similar to struct efi efi_xen previously in
> > drivers/xen/efi.c.  Maybe it makes sense to leave struct efi efi_xen in
> > drivers/xen/efi.c, export it in include/xen/xen-ops.h, then here just:
> > 
> >   efi = efi_xen;
> > 
> > Would that improve code readability?
> 
> Rethink about this. It's a little different on ARM since we call
> xen_efi_runtime_setup after parsing the FDT and setting some members of
> efi already, e.g. efi.systab, efi.acpi20. So it necessary to have a
> different way to initialize the struct efi.

OK, fair enough.

[toc] | [prev] | [next] | [standalone]


#1309912 — [PATCH v2 10/16] ARM: XEN: Move xen_early_init() before efi_init()

FromShannon Zhao <zhaoshenglong@huawei.com>
Date2016-01-15 08:00 +0100
Subject[PATCH v2 10/16] ARM: XEN: Move xen_early_init() before efi_init()
Message-ID<qR6wz-2kF-27@gated-at.bofh.it>
In reply to#1309909
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  | 56 ++++++++++++++++++++++++++++++++++-------------
 arch/arm64/kernel/setup.c |  2 +-
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 4e78c49..5d0fe68 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>
@@ -52,8 +53,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,
@@ -222,6 +221,33 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static __initdata struct {
+	const char *compat;
+	const char *prefix;
+	const char *version;
+	bool found;
+} xen_node = {"xen,xen", "xen,xen-", NULL, false};
+
+static int __init fdt_find_xen_node(unsigned long node, const char *uname,
+				    int depth, void *data)
+{
+	const void *s = NULL;
+	int len;
+
+	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
+		return 0;
+
+	if (of_flat_dt_is_compatible(node, xen_node.compat))
+		xen_node.found = true;
+
+	s = of_get_flat_dt_prop(node, "compatible", &len);
+	if (strlen(xen_node.prefix) + 3  < len &&
+	    !strncmp(xen_node.prefix, s, strlen(xen_node.prefix)))
+		xen_node.version = s + strlen(xen_node.prefix);
+
+	return 0;
+}
+
 /*
  * see Documentation/devicetree/bindings/arm/xen.txt for the
  * documentation of the Xen Device Tree format.
@@ -229,26 +255,18 @@ 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-";
-
-	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
-	if (!xen_node) {
+	of_scan_flat_dt(fdt_find_xen_node, NULL);
+	if (!xen_node.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 (xen_node.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", xen_node.version);
 
 	xen_domain_type = XEN_HVM_DOMAIN;
 
@@ -296,6 +314,14 @@ static int __init xen_guest_init(void)
 		}
 #endif
 	} 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);
 	}
 
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 8119479..a4a2878 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -313,6 +313,7 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	local_async_enable();
 
+	xen_early_init();
 	efi_init();
 	arm64_memblock_init();
 
@@ -334,7 +335,6 @@ void __init setup_arch(char **cmdline_p)
 	} else {
 		psci_acpi_init();
 	}
-	xen_early_init();
 
 	cpu_read_bootcpu_ops();
 	smp_init_cpus();
-- 
2.0.4

[toc] | [prev] | [next] | [standalone]


#1311603 — Re: [Xen-devel] [PATCH v2 10/16] ARM: XEN: Move xen_early_init() before efi_init()

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2016-01-18 16:10 +0100
SubjectRe: [Xen-devel] [PATCH v2 10/16] ARM: XEN: Move xen_early_init() before efi_init()
Message-ID<qSjBo-2nK-25@gated-at.bofh.it>
In reply to#1309912
On Fri, 15 Jan 2016, Shannon Zhao 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  | 56 ++++++++++++++++++++++++++++++++++-------------
>  arch/arm64/kernel/setup.c |  2 +-
>  2 files changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 4e78c49..5d0fe68 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>
> @@ -52,8 +53,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,
> @@ -222,6 +221,33 @@ static irqreturn_t xen_arm_callback(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +static __initdata struct {
> +	const char *compat;
> +	const char *prefix;
> +	const char *version;
> +	bool found;
> +} xen_node = {"xen,xen", "xen,xen-", NULL, false};
> +
> +static int __init fdt_find_xen_node(unsigned long node, const char *uname,
> +				    int depth, void *data)
> +{
> +	const void *s = NULL;
> +	int len;
> +
> +	if (depth != 1 || strcmp(uname, "hypervisor") != 0)
> +		return 0;
> +
> +	if (of_flat_dt_is_compatible(node, xen_node.compat))
> +		xen_node.found = true;
> +
> +	s = of_get_flat_dt_prop(node, "compatible", &len);
> +	if (strlen(xen_node.prefix) + 3  < len &&
> +	    !strncmp(xen_node.prefix, s, strlen(xen_node.prefix)))
> +		xen_node.version = s + strlen(xen_node.prefix);
> +
> +	return 0;
> +}
> +
>  /*
>   * see Documentation/devicetree/bindings/arm/xen.txt for the
>   * documentation of the Xen Device Tree format.
> @@ -229,26 +255,18 @@ 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-";
> -
> -	xen_node = of_find_compatible_node(NULL, NULL, "xen,xen");
> -	if (!xen_node) {
> +	of_scan_flat_dt(fdt_find_xen_node, NULL);
> +	if (!xen_node.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 (xen_node.version == NULL) {
>  		pr_debug("Xen version not found\n");
>  		return;
>  	}

This is going to access data within the fdt, after it has been
unflattened (by the unflatten_device_tree call). Is that allowed?

If that's not allowed, then you could copy the version string to
xen_node.version, rather than just copying the pointer. Otherwise you
could move the version check entirely within fdt_find_xen_node and
remove the version field from xen_node.


> -	pr_info("Xen %s support found\n", version);
> +	pr_info("Xen %s support found\n", xen_node.version);
>  
>  	xen_domain_type = XEN_HVM_DOMAIN;
>  
> @@ -296,6 +314,14 @@ static int __init xen_guest_init(void)
>  		}
>  #endif
>  	} 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;
> +		}

It makes sense to move this into a DT specific init function called from
xen_guest_init. Also, given that at this point we know that a Xen node
should be present, it makes sense to return EFAULT on error, instead of
ENODEV. The error message could be changed to something like: "Xen
support was detected before, but it has disappeared" and should be
pr_err.


>  		xen_events_irq = irq_of_parse_and_map(xen_node, 0);
>  	}
>  
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 8119479..a4a2878 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -313,6 +313,7 @@ void __init setup_arch(char **cmdline_p)
>  	 */
>  	local_async_enable();
>  
> +	xen_early_init();
>  	efi_init();
>  	arm64_memblock_init();
>  
> @@ -334,7 +335,6 @@ void __init setup_arch(char **cmdline_p)
>  	} else {
>  		psci_acpi_init();
>  	}
> -	xen_early_init();
>  
>  	cpu_read_bootcpu_ops();
>  	smp_init_cpus();

[toc] | [prev] | [next] | [standalone]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web