| Message ID | 1452840929-19612-17-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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?
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.
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.
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.
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. >
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. >
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).
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 >
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,
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.
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