[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 09/23] efi: create efi_enabled()
From: |
Jan Beulich |
Subject: |
Re: [PATCH v2 09/23] efi: create efi_enabled() |
Date: |
Thu, 20 Aug 2015 09:18:17 -0600 |
>>> On 20.07.15 at 16:29, <address@hidden> wrote:
> --- a/xen/arch/x86/efi/stub.c
> +++ b/xen/arch/x86/efi/stub.c
> @@ -4,9 +4,14 @@
> #include <xen/lib.h>
> #include <asm/page.h>
>
> -#ifndef efi_enabled
> -const bool_t efi_enabled = 0;
> -#endif
> +struct efi __read_mostly efi = {
> + .flags = 0, /* Initialized later. */
> + .acpi = EFI_INVALID_TABLE_ADDR,
> + .acpi20 = EFI_INVALID_TABLE_ADDR,
> + .mps = EFI_INVALID_TABLE_ADDR,
> + .smbios = EFI_INVALID_TABLE_ADDR,
> + .smbios3 = EFI_INVALID_TABLE_ADDR
> +};
How is this change related to the subject of the patch?
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -191,8 +191,6 @@ SECTIONS
> .pad : {
> . = ALIGN(MB(16));
> } :text
> -#else
> - efi = .;
> #endif
Same here.
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -717,6 +717,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> char *option_str;
> bool_t use_cfg_file;
>
> +#ifndef CONFIG_ARM /* Disabled until runtime services implemented. */
> + set_bit(EFI_PLATFORM, &efi.flags);
> +#endif
Just for this to work? I don't see the need for all the pointers in
the stub case - why can't this be a separate variable? We don't
need to follow Linux with where the flags actually live...
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -10,14 +10,10 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16);
>
> #ifndef COMPAT
>
> -#ifdef CONFIG_ARM /* Disabled until runtime services implemented */
> -const bool_t efi_enabled = 0;
> -#else
> +#ifndef CONFIG_ARM
> # include <asm/i387.h>
> # include <asm/xstate.h>
> # include <public/platform.h>
> -
> -const bool_t efi_enabled = 1;
> #endif
Afaict the proper replacement (at least at this point in the series) for
this would be to statically initialize the flag set in this xen.efi case.
> @@ -40,6 +42,16 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *);
> int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *);
> int efi_compat_runtime_call(struct compat_pf_efi_runtime_call *);
>
> +/*
> + * Test whether the above EFI_* bits are enabled.
> + *
> + * Stolen from Linux Kernel.
I don't think this second part of the comment makes a lot of sense
for a minor piece of functionality like this.
Jan
- Re: [PATCH v2 09/23] efi: create efi_enabled(),
Jan Beulich <=