* [PATCH 0/2] xen/efi: Fixes
@ 2017-06-20 20:14 Daniel Kiper
2017-06-20 20:14 ` [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP Daniel Kiper
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
0 siblings, 2 replies; 9+ messages in thread
From: Daniel Kiper @ 2017-06-20 20:14 UTC (permalink / raw)
To: linux-efi, linux-kernel, x86, stable, xen-devel
Cc: boris.ostrovsky, jgross, tglx, mingo, hpa, matt, ard.biesheuvel
Hey,
Two small fixes for Xen dom0 running on x86_64 EFI platforms.
I am CC-ing stable maintainers because similar stuff is needed for various
stable kernels too. Unfortunately, almost every version needs a bit different
set of fixes. So, please treat this email more as head up than real set of
patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me
a line. Then I will prepare set of patches for your kernel (if needed).
Daniel
arch/x86/xen/efi.c | 45 ++++++++++++---------------------------------
drivers/firmware/efi/efi.c | 3 ++-
2 files changed, 14 insertions(+), 34 deletions(-)
Daniel Kiper (2):
efi: Process MEMATTR table only if EFI_MEMMAP
x86/xen/efi: Init only efi struct members used by Xen
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
2017-06-20 20:14 [PATCH 0/2] xen/efi: Fixes Daniel Kiper
@ 2017-06-20 20:14 ` Daniel Kiper
2017-06-21 7:48 ` Ard Biesheuvel
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
1 sibling, 1 reply; 9+ messages in thread
From: Daniel Kiper @ 2017-06-20 20:14 UTC (permalink / raw)
To: linux-efi, linux-kernel, x86, stable, xen-devel
Cc: boris.ostrovsky, jgross, tglx, mingo, hpa, matt, ard.biesheuvel
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more generic and covers more cases.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
drivers/firmware/efi/efi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-20 20:14 [PATCH 0/2] xen/efi: Fixes Daniel Kiper
2017-06-20 20:14 ` [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP Daniel Kiper
@ 2017-06-20 20:14 ` Daniel Kiper
2017-06-21 7:53 ` Ard Biesheuvel
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: Daniel Kiper @ 2017-06-20 20:14 UTC (permalink / raw)
To: linux-efi, linux-kernel, x86, stable, xen-devel
Cc: boris.ostrovsky, jgross, tglx, mingo, hpa, matt, ard.biesheuvel
Current approach, wholesale efi struct initialization from efi_xen, is not
good. Usually if new member is defined then it is properly initialized in
drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
arch/x86/xen/efi.c | 45 ++++++++++++---------------------------------
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..01b9faf 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0, /* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info = EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab = EFI_INVALID_TABLE_ADDR,
- .fw_vendor = EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable = xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags = 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ 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 = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
2017-06-20 20:14 ` [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP Daniel Kiper
@ 2017-06-21 7:48 ` Ard Biesheuvel
0 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2017-06-21 7:48 UTC (permalink / raw)
To: Daniel Kiper
Cc: linux-efi, linux-kernel, x86, stable, xen-devel, Boris Ostrovsky,
Juergen Gross, tglx, mingo, hpa, Matt Fleming
On 20 June 2017 at 22:14, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
>
> In theory we can check EFI_PARAVIRT too, however,
> EFI_MEMMAP looks more generic and covers more cases.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> drivers/firmware/efi/efi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index b372aad..045d6d3 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
> }
> }
>
> - efi_memattr_init();
> + if (efi_enabled(EFI_MEMMAP))
> + efi_memattr_init();
>
> /* Parse the EFI Properties table if it exists */
> if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
@ 2017-06-21 7:53 ` Ard Biesheuvel
2017-06-21 8:10 ` [Xen-devel] " Andrew Cooper
2017-06-21 9:24 ` Ingo Molnar
2 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2017-06-21 7:53 UTC (permalink / raw)
To: Daniel Kiper
Cc: linux-efi, linux-kernel, x86, stable, xen-devel, Boris Ostrovsky,
Juergen Gross, tglx, mingo, hpa, Matt Fleming
On 20 June 2017 at 22:14, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> Current approach, wholesale efi struct initialization from efi_xen, is not
> good. Usually if new member is defined then it is properly initialized in
> drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
> a few times until now. So, let's initialize only efi struct members used by
> Xen to avoid such issues in the future.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/x86/xen/efi.c | 45 ++++++++++++---------------------------------
> 1 file changed, 12 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> index 30bb2e8..01b9faf 100644
> --- a/arch/x86/xen/efi.c
> +++ b/arch/x86/xen/efi.c
> @@ -54,38 +54,6 @@
> .tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
> };
>
> -static const struct efi efi_xen __initconst = {
> - .systab = NULL, /* Initialized later. */
> - .runtime_version = 0, /* Initialized later. */
> - .mps = EFI_INVALID_TABLE_ADDR,
> - .acpi = EFI_INVALID_TABLE_ADDR,
> - .acpi20 = EFI_INVALID_TABLE_ADDR,
> - .smbios = EFI_INVALID_TABLE_ADDR,
> - .smbios3 = EFI_INVALID_TABLE_ADDR,
> - .sal_systab = EFI_INVALID_TABLE_ADDR,
> - .boot_info = EFI_INVALID_TABLE_ADDR,
> - .hcdp = EFI_INVALID_TABLE_ADDR,
> - .uga = EFI_INVALID_TABLE_ADDR,
> - .uv_systab = EFI_INVALID_TABLE_ADDR,
> - .fw_vendor = EFI_INVALID_TABLE_ADDR,
> - .runtime = EFI_INVALID_TABLE_ADDR,
> - .config_table = EFI_INVALID_TABLE_ADDR,
> - .get_time = xen_efi_get_time,
> - .set_time = xen_efi_set_time,
> - .get_wakeup_time = xen_efi_get_wakeup_time,
> - .set_wakeup_time = xen_efi_set_wakeup_time,
> - .get_variable = xen_efi_get_variable,
> - .get_next_variable = xen_efi_get_next_variable,
> - .set_variable = xen_efi_set_variable,
> - .query_variable_info = xen_efi_query_variable_info,
> - .update_capsule = xen_efi_update_capsule,
> - .query_capsule_caps = xen_efi_query_capsule_caps,
> - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> - .reset_system = xen_efi_reset_system,
> - .set_virtual_address_map = NULL, /* Not used under Xen. */
> - .flags = 0 /* Initialized later. */
> -};
> -
> static efi_system_table_t __init *xen_efi_probe(void)
> {
> struct xen_platform_op op = {
> @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
>
> /* Here we know that Xen runs on EFI platform. */
>
> - efi = efi_xen;
> + 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 = xen_efi_reset_system;
>
> efi_systab_xen.tables = info->cfg.addr;
> efi_systab_xen.nr_tables = info->cfg.nent;
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
2017-06-21 7:53 ` Ard Biesheuvel
@ 2017-06-21 8:10 ` Andrew Cooper
2017-06-21 9:29 ` Daniel Kiper
2017-06-21 9:24 ` Ingo Molnar
2 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2017-06-21 8:10 UTC (permalink / raw)
To: Daniel Kiper, linux-efi, linux-kernel, x86, stable, xen-devel
Cc: jgross, ard.biesheuvel, matt, mingo, hpa, boris.ostrovsky, tglx
On 20/06/2017 21:14, Daniel Kiper wrote:
> Current approach, wholesale efi struct initialization from efi_xen, is not
> good. Usually if new member is defined then it is properly initialized in
> drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
> a few times until now. So, let's initialize only efi struct members used by
> Xen to avoid such issues in the future.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> arch/x86/xen/efi.c | 45 ++++++++++++---------------------------------
> 1 file changed, 12 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> index 30bb2e8..01b9faf 100644
> --- a/arch/x86/xen/efi.c
> +++ b/arch/x86/xen/efi.c
> @@ -54,38 +54,6 @@
> .tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
> };
>
> -static const struct efi efi_xen __initconst = {
> - .systab = NULL, /* Initialized later. */
> - .runtime_version = 0, /* Initialized later. */
> - .mps = EFI_INVALID_TABLE_ADDR,
> - .acpi = EFI_INVALID_TABLE_ADDR,
> - .acpi20 = EFI_INVALID_TABLE_ADDR,
> - .smbios = EFI_INVALID_TABLE_ADDR,
> - .smbios3 = EFI_INVALID_TABLE_ADDR,
> - .sal_systab = EFI_INVALID_TABLE_ADDR,
> - .boot_info = EFI_INVALID_TABLE_ADDR,
> - .hcdp = EFI_INVALID_TABLE_ADDR,
> - .uga = EFI_INVALID_TABLE_ADDR,
> - .uv_systab = EFI_INVALID_TABLE_ADDR,
> - .fw_vendor = EFI_INVALID_TABLE_ADDR,
> - .runtime = EFI_INVALID_TABLE_ADDR,
> - .config_table = EFI_INVALID_TABLE_ADDR,
> - .get_time = xen_efi_get_time,
> - .set_time = xen_efi_set_time,
> - .get_wakeup_time = xen_efi_get_wakeup_time,
> - .set_wakeup_time = xen_efi_set_wakeup_time,
> - .get_variable = xen_efi_get_variable,
> - .get_next_variable = xen_efi_get_next_variable,
> - .set_variable = xen_efi_set_variable,
> - .query_variable_info = xen_efi_query_variable_info,
> - .update_capsule = xen_efi_update_capsule,
> - .query_capsule_caps = xen_efi_query_capsule_caps,
> - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> - .reset_system = xen_efi_reset_system,
> - .set_virtual_address_map = NULL, /* Not used under Xen. */
> - .flags = 0 /* Initialized later. */
> -};
> -
> static efi_system_table_t __init *xen_efi_probe(void)
> {
> struct xen_platform_op op = {
> @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
>
> /* Here we know that Xen runs on EFI platform. */
>
> - efi = efi_xen;
> + 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 = xen_efi_reset_system;
This presumably means that the system default values are already present
in efi at the point that we overwrite some Xen specifics?
If so, surely you need to retain the clobbering of set_virtual_address_map ?
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
2017-06-21 7:53 ` Ard Biesheuvel
2017-06-21 8:10 ` [Xen-devel] " Andrew Cooper
@ 2017-06-21 9:24 ` Ingo Molnar
2017-06-21 9:32 ` Daniel Kiper
2 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2017-06-21 9:24 UTC (permalink / raw)
To: Daniel Kiper
Cc: linux-efi, linux-kernel, x86, stable, xen-devel, boris.ostrovsky,
jgross, tglx, mingo, hpa, matt, ard.biesheuvel
* Daniel Kiper <daniel.kiper@oracle.com> wrote:
> -static const struct efi efi_xen __initconst = {
> - .systab = NULL, /* Initialized later. */
> - .runtime_version = 0, /* Initialized later. */
> - .mps = EFI_INVALID_TABLE_ADDR,
> - .acpi = EFI_INVALID_TABLE_ADDR,
> - .acpi20 = EFI_INVALID_TABLE_ADDR,
> - .smbios = EFI_INVALID_TABLE_ADDR,
> - .smbios3 = EFI_INVALID_TABLE_ADDR,
> - .sal_systab = EFI_INVALID_TABLE_ADDR,
> - .boot_info = EFI_INVALID_TABLE_ADDR,
> - .hcdp = EFI_INVALID_TABLE_ADDR,
> - .uga = EFI_INVALID_TABLE_ADDR,
> - .uv_systab = EFI_INVALID_TABLE_ADDR,
> - .fw_vendor = EFI_INVALID_TABLE_ADDR,
> - .runtime = EFI_INVALID_TABLE_ADDR,
> - .config_table = EFI_INVALID_TABLE_ADDR,
> - .get_time = xen_efi_get_time,
> - .set_time = xen_efi_set_time,
> - .get_wakeup_time = xen_efi_get_wakeup_time,
> - .set_wakeup_time = xen_efi_set_wakeup_time,
> - .get_variable = xen_efi_get_variable,
> - .get_next_variable = xen_efi_get_next_variable,
> - .set_variable = xen_efi_set_variable,
> - .query_variable_info = xen_efi_query_variable_info,
> - .update_capsule = xen_efi_update_capsule,
> - .query_capsule_caps = xen_efi_query_capsule_caps,
> - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> - .reset_system = xen_efi_reset_system,
> - .set_virtual_address_map = NULL, /* Not used under Xen. */
> - .flags = 0 /* Initialized later. */
> -};
> -
> static efi_system_table_t __init *xen_efi_probe(void)
> {
> struct xen_platform_op op = {
> @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
>
> /* Here we know that Xen runs on EFI platform. */
>
> - efi = efi_xen;
> + 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 = xen_efi_reset_system;
This is a step back stylistically, as you lost the nice vertical tabulation of the
original initializer ...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-21 8:10 ` [Xen-devel] " Andrew Cooper
@ 2017-06-21 9:29 ` Daniel Kiper
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Kiper @ 2017-06-21 9:29 UTC (permalink / raw)
To: Andrew Cooper
Cc: linux-efi, linux-kernel, x86, stable, xen-devel, jgross,
ard.biesheuvel, matt, mingo, hpa, boris.ostrovsky, tglx
On Wed, Jun 21, 2017 at 09:10:51AM +0100, Andrew Cooper wrote:
> On 20/06/2017 21:14, Daniel Kiper wrote:
> > Current approach, wholesale efi struct initialization from efi_xen, is not
> > good. Usually if new member is defined then it is properly initialized in
> > drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
> > a few times until now. So, let's initialize only efi struct members used by
> > Xen to avoid such issues in the future.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > ---
> > arch/x86/xen/efi.c | 45 ++++++++++++---------------------------------
> > 1 file changed, 12 insertions(+), 33 deletions(-)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index 30bb2e8..01b9faf 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -54,38 +54,6 @@
> > .tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
> > };
> >
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0, /* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info = EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab = EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor = EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable = xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags = 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + 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 = xen_efi_reset_system;
>
> This presumably means that the system default values are already present
> in efi at the point that we overwrite some Xen specifics?
More or less.
> If so, surely you need to retain the clobbering of set_virtual_address_map ?
Nope, by default efi.set_virtual_address_map is NULL (please take a look
at efi struct initialization in drivers/firmware/efi/efi.c). And it is
not touched if efi_enabled(EFI_PARAVIRT).
Daniel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
2017-06-21 9:24 ` Ingo Molnar
@ 2017-06-21 9:32 ` Daniel Kiper
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Kiper @ 2017-06-21 9:32 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-efi, linux-kernel, x86, stable, xen-devel, boris.ostrovsky,
jgross, tglx, mingo, hpa, matt, ard.biesheuvel
On Wed, Jun 21, 2017 at 11:24:06AM +0200, Ingo Molnar wrote:
>
> * Daniel Kiper <daniel.kiper@oracle.com> wrote:
>
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0, /* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info = EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab = EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor = EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable = xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags = 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + 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 = xen_efi_reset_system;
>
> This is a step back stylistically, as you lost the nice vertical tabulation of the
> original initializer ...
If you wish and others do not object I can realign it back.
Daniel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-21 9:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 20:14 [PATCH 0/2] xen/efi: Fixes Daniel Kiper
2017-06-20 20:14 ` [PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP Daniel Kiper
2017-06-21 7:48 ` Ard Biesheuvel
2017-06-20 20:14 ` [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen Daniel Kiper
2017-06-21 7:53 ` Ard Biesheuvel
2017-06-21 8:10 ` [Xen-devel] " Andrew Cooper
2017-06-21 9:29 ` Daniel Kiper
2017-06-21 9:24 ` Ingo Molnar
2017-06-21 9:32 ` Daniel Kiper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).