Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, Mar 27, 2015 at 02:04:22PM +, Jan Beulich wrote: > >>> On 27.03.15 at 14:53, wrote: > > On 27/03/15 13:43, Jan Beulich wrote: > > On 27.03.15 at 14:32, wrote: > >>> On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: > >>> On 30.01.15 at 18:54, wrote: > > We need more fine grained knowledge about EFI environment and check > > for EFI platform and EFI loader separately to properly support > > multiboot2 protocol. > ... because of ... (i.e. I can't see from the description what the > separation is good for). Looking at the comments you placed > aside the variables doesn't help me either. > > > In general Xen loaded by this protocol uses > > memory mappings and loaded modules in simliar way to Xen loaded > > by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform > > and efi_loader. > And if I'm guessing things right, then introducing efi_loader but > leaving efi_enabled alone (only converting where needed) would > >>> efi_enabled is not fortunate name for new usage. Currently it means > >>> that Xen binary have or does not have EFI support build in. However, > >>> if we build in multiboot2 protocol into xen.gz then it means that > >>> it can ran on legacy BIOS or EFI platform. So, I think that we > >>> should rename efi_enabled to efi_platform because it will mean > >>> that Xen runs on EFI platform or not. > >> I disagree here. > >> > >>> efi_loader is used to differentiate between EFI native loader > >>> and multiboot2 protocol. > >> And I agree here. > > > > I suppose "built with efi support" is known because of CONFIG_EFI or > > not, and doesn't need a variable. > > > > However, "booted legacy" vs "booted EFI" does need distinguishing at > > runtime, as either is possible. > > Right, but that's what efi_enabled is supposed to express after > the change; there's no need to express "built with EFI support". > It just so happens that right now, without all these changes, > built-with-EFI-support == runs-on-EFI. Then how about 'efi_booted' as the variable name. -- Len Sorensen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
>>> On 27.03.15 at 14:53, wrote: > On 27/03/15 13:43, Jan Beulich wrote: > On 27.03.15 at 14:32, wrote: >>> On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: >>> On 30.01.15 at 18:54, wrote: > We need more fine grained knowledge about EFI environment and check > for EFI platform and EFI loader separately to properly support > multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. > In general Xen loaded by this protocol uses > memory mappings and loaded modules in simliar way to Xen loaded > by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform > and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would >>> efi_enabled is not fortunate name for new usage. Currently it means >>> that Xen binary have or does not have EFI support build in. However, >>> if we build in multiboot2 protocol into xen.gz then it means that >>> it can ran on legacy BIOS or EFI platform. So, I think that we >>> should rename efi_enabled to efi_platform because it will mean >>> that Xen runs on EFI platform or not. >> I disagree here. >> >>> efi_loader is used to differentiate between EFI native loader >>> and multiboot2 protocol. >> And I agree here. > > I suppose "built with efi support" is known because of CONFIG_EFI or > not, and doesn't need a variable. > > However, "booted legacy" vs "booted EFI" does need distinguishing at > runtime, as either is possible. Right, but that's what efi_enabled is supposed to express after the change; there's no need to express "built with EFI support". It just so happens that right now, without all these changes, built-with-EFI-support == runs-on-EFI. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose "built with efi support" is known because of CONFIG_EFI or not, and doesn't need a variable. However, "booted legacy" vs "booted EFI" does need distinguishing at runtime, as either is possible. ~Andrew ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
>>> On 27.03.15 at 14:32, wrote: > On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: >> >>> On 30.01.15 at 18:54, wrote: >> > We need more fine grained knowledge about EFI environment and check >> > for EFI platform and EFI loader separately to properly support >> > multiboot2 protocol. >> >> ... because of ... (i.e. I can't see from the description what the >> separation is good for). Looking at the comments you placed >> aside the variables doesn't help me either. >> >> > In general Xen loaded by this protocol uses >> > memory mappings and loaded modules in simliar way to Xen loaded >> > by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform >> > and efi_loader. >> >> And if I'm guessing things right, then introducing efi_loader but >> leaving efi_enabled alone (only converting where needed) would > > efi_enabled is not fortunate name for new usage. Currently it means > that Xen binary have or does not have EFI support build in. However, > if we build in multiboot2 protocol into xen.gz then it means that > it can ran on legacy BIOS or EFI platform. So, I think that we > should rename efi_enabled to efi_platform because it will mean > that Xen runs on EFI platform or not. I disagree here. > efi_loader is used to differentiate between EFI native loader > and multiboot2 protocol. And I agree here. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: > >>> On 30.01.15 at 18:54, wrote: > > We need more fine grained knowledge about EFI environment and check > > for EFI platform and EFI loader separately to properly support > > multiboot2 protocol. > > ... because of ... (i.e. I can't see from the description what the > separation is good for). Looking at the comments you placed > aside the variables doesn't help me either. > > > In general Xen loaded by this protocol uses > > memory mappings and loaded modules in simliar way to Xen loaded > > by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform > > and efi_loader. > > And if I'm guessing things right, then introducing efi_loader but > leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. > seem the right approach. [...] > > --- a/xen/include/xen/efi.h > > +++ b/xen/include/xen/efi.h > > @@ -5,7 +5,11 @@ > > #include > > #endif > > > > -extern const bool_t efi_enabled; > > +/* If true then Xen runs on EFI platform. */ > > +extern bool_t efi_platform; > > + > > +/* If true then Xen was loaded by native EFI loader as PE application. */ > > +extern bool_t efi_loader; > > I don't see why you're losing the constness. Both of them could be set during runtime. Please look above for more details. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
>>> On 03.03.15 at 00:40, wrote: > Reviewing the #ifndef CONFIG_ARM in EFI code, and the efi_enabled > usage elsewhere, > the remaining EFI tasks on ARM look like: > * Support for SetVirtualAddressMap > * Runtime service support - looks like just time function used by x86 > in get_cmos_time() * Provide runtime service access for Dom0 Jan > Not strictly EFI, but related: > * Lookup of ACPI tables > * Lookup of SMBIOS tables > > Roy ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Mon, Mar 2, 2015 at 10:43 AM, Roy Franz wrote:
> On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
> wrote:
>> On Fri, 30 Jan 2015, Daniel Kiper wrote:
>>> We need more fine grained knowledge about EFI environment and check
>>> for EFI platform and EFI loader separately to properly support
>>> multiboot2 protocol. In general Xen loaded by this protocol uses
>>> memory mappings and loaded modules in simliar way to Xen loaded
>>> by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
>>> and efi_loader.
>>>
>>> Signed-off-by: Daniel Kiper
>>> ---
>>> xen/arch/x86/dmi_scan.c|4 ++--
>>> xen/arch/x86/domain_page.c |2 +-
>>> xen/arch/x86/efi/stub.c|5 +++--
>>> xen/arch/x86/mpparse.c |4 ++--
>>> xen/arch/x86/setup.c |8
>>> xen/arch/x86/time.c|2 +-
>>> xen/common/efi/boot.c |5 +
>>> xen/common/efi/runtime.c |5 +++--
>>> xen/drivers/acpi/osl.c |2 +-
>>> xen/include/xen/efi.h |6 +-
>>> 10 files changed, 27 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
>>> index 500133a..63b976c 100644
>>> --- a/xen/arch/x86/dmi_scan.c
>>> +++ b/xen/arch/x86/dmi_scan.c
>>> @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
>>> struct dmi_eps eps;
>>> char __iomem *p, *q;
>>>
>>> - if (efi_enabled) {
>>> + if (efi_platform) {
>>> if (!efi_dmi_size)
>>> return -1;
>>> *base = efi_dmi_address;
>>> @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)
>>>
>>> void __init dmi_scan_machine(void)
>>> {
>>> - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
>>> + if ((!efi_platform ? dmi_iterate(dmi_decode) :
>>> dmi_efi_iterate(dmi_decode)) == 0)
>>> dmi_check_system(dmi_blacklist);
>>> else
>>> diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
>>> index 158a164..5d4564c 100644
>>> --- a/xen/arch/x86/domain_page.c
>>> +++ b/xen/arch/x86/domain_page.c
>>> @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
>>> sync_local_execstate();
>>> /* We must now be running on the idle page table. */
>>> ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
>>> - (efi_enabled && cr3 == efi_rs_page_table()));
>>> + (efi_platform && cr3 == efi_rs_page_table()));
>>> }
>>>
>>> return v;
>>> diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
>>> index b8f49f8..5060e6f 100644
>>> --- a/xen/arch/x86/efi/stub.c
>>> +++ b/xen/arch/x86/efi/stub.c
>>> @@ -3,8 +3,9 @@
>>> #include
>>> #include
>>>
>>> -#ifndef efi_enabled
>>> -const bool_t efi_enabled = 0;
>>> +#ifndef efi_platform
>>> +bool_t efi_platform = 0;
>>> +bool_t efi_loader = 0;
>>> #endif
>>>
>>> void __init efi_init_memory(void) { }
>>> diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
>>> index a38e016..c4e3041 100644
>>> --- a/xen/arch/x86/mpparse.c
>>> +++ b/xen/arch/x86/mpparse.c
>>> @@ -540,7 +540,7 @@ static inline void __init
>>> construct_default_ISA_mptable(int mpc_default_type)
>>>
>>> static __init void efi_unmap_mpf(void)
>>> {
>>> - if (efi_enabled)
>>> + if (efi_platform)
>>> __set_fixmap(FIX_EFI_MPF, 0, 0);
>>> }
>>>
>>> @@ -698,7 +698,7 @@ void __init find_smp_config (void)
>>> {
>>> unsigned int address;
>>>
>>> - if (efi_enabled) {
>>> + if (efi_platform) {
>>> efi_check_config();
>>> return;
>>> }
>>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>>> index c27c49c..711fdb0 100644
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
>>> struct boot_video_info *bvi = &bootsym(boot_vid_info);
>>>
>>> /* The EFI loader fills vga_console_info directly. */
>>> -if ( efi_enabled )
>>> +if ( efi_platform )
>>> return;
>>>
>>> if ( (bvi->orig_video_isVGA == 1) && (bvi->orig_video_mode == 3) )
>>> @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>> if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
>>> panic("Misaligned CPU0 stack.");
>>>
>>> -if ( efi_enabled )
>>> +if ( efi_loader )
>>> {
>>> set_pdx_range(xen_phys_start >> PAGE_SHIFT,
>>>(xen_phys_start + BOOTSTRAP_MAP_BASE) >> PAGE_SHIFT);
>>> @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>> * we can relocate the dom0 kernel and other multiboot modules. Also,
>>> on
>>> * x86/64, we relocate Xen to higher memory.
>>> */
>>> -for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
>>> +for ( i = 0; !efi_loader && i < mbi->mods_count; i++ )
>>> {
>>> if ( mod[i].mod_start & (PAGE_SIZE - 1) )
>>> panic("Bootlo
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Mon, Mar 2, 2015 at 9:21 AM, Stefano Stabellini
wrote:
> On Fri, 30 Jan 2015, Daniel Kiper wrote:
>> We need more fine grained knowledge about EFI environment and check
>> for EFI platform and EFI loader separately to properly support
>> multiboot2 protocol. In general Xen loaded by this protocol uses
>> memory mappings and loaded modules in simliar way to Xen loaded
>> by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
>> and efi_loader.
>>
>> Signed-off-by: Daniel Kiper
>> ---
>> xen/arch/x86/dmi_scan.c|4 ++--
>> xen/arch/x86/domain_page.c |2 +-
>> xen/arch/x86/efi/stub.c|5 +++--
>> xen/arch/x86/mpparse.c |4 ++--
>> xen/arch/x86/setup.c |8
>> xen/arch/x86/time.c|2 +-
>> xen/common/efi/boot.c |5 +
>> xen/common/efi/runtime.c |5 +++--
>> xen/drivers/acpi/osl.c |2 +-
>> xen/include/xen/efi.h |6 +-
>> 10 files changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
>> index 500133a..63b976c 100644
>> --- a/xen/arch/x86/dmi_scan.c
>> +++ b/xen/arch/x86/dmi_scan.c
>> @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
>> struct dmi_eps eps;
>> char __iomem *p, *q;
>>
>> - if (efi_enabled) {
>> + if (efi_platform) {
>> if (!efi_dmi_size)
>> return -1;
>> *base = efi_dmi_address;
>> @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)
>>
>> void __init dmi_scan_machine(void)
>> {
>> - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
>> + if ((!efi_platform ? dmi_iterate(dmi_decode) :
>> dmi_efi_iterate(dmi_decode)) == 0)
>> dmi_check_system(dmi_blacklist);
>> else
>> diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
>> index 158a164..5d4564c 100644
>> --- a/xen/arch/x86/domain_page.c
>> +++ b/xen/arch/x86/domain_page.c
>> @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
>> sync_local_execstate();
>> /* We must now be running on the idle page table. */
>> ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
>> - (efi_enabled && cr3 == efi_rs_page_table()));
>> + (efi_platform && cr3 == efi_rs_page_table()));
>> }
>>
>> return v;
>> diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
>> index b8f49f8..5060e6f 100644
>> --- a/xen/arch/x86/efi/stub.c
>> +++ b/xen/arch/x86/efi/stub.c
>> @@ -3,8 +3,9 @@
>> #include
>> #include
>>
>> -#ifndef efi_enabled
>> -const bool_t efi_enabled = 0;
>> +#ifndef efi_platform
>> +bool_t efi_platform = 0;
>> +bool_t efi_loader = 0;
>> #endif
>>
>> void __init efi_init_memory(void) { }
>> diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
>> index a38e016..c4e3041 100644
>> --- a/xen/arch/x86/mpparse.c
>> +++ b/xen/arch/x86/mpparse.c
>> @@ -540,7 +540,7 @@ static inline void __init
>> construct_default_ISA_mptable(int mpc_default_type)
>>
>> static __init void efi_unmap_mpf(void)
>> {
>> - if (efi_enabled)
>> + if (efi_platform)
>> __set_fixmap(FIX_EFI_MPF, 0, 0);
>> }
>>
>> @@ -698,7 +698,7 @@ void __init find_smp_config (void)
>> {
>> unsigned int address;
>>
>> - if (efi_enabled) {
>> + if (efi_platform) {
>> efi_check_config();
>> return;
>> }
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index c27c49c..711fdb0 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
>> struct boot_video_info *bvi = &bootsym(boot_vid_info);
>>
>> /* The EFI loader fills vga_console_info directly. */
>> -if ( efi_enabled )
>> +if ( efi_platform )
>> return;
>>
>> if ( (bvi->orig_video_isVGA == 1) && (bvi->orig_video_mode == 3) )
>> @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>> if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
>> panic("Misaligned CPU0 stack.");
>>
>> -if ( efi_enabled )
>> +if ( efi_loader )
>> {
>> set_pdx_range(xen_phys_start >> PAGE_SHIFT,
>>(xen_phys_start + BOOTSTRAP_MAP_BASE) >> PAGE_SHIFT);
>> @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>> * we can relocate the dom0 kernel and other multiboot modules. Also, on
>> * x86/64, we relocate Xen to higher memory.
>> */
>> -for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
>> +for ( i = 0; !efi_loader && i < mbi->mods_count; i++ )
>> {
>> if ( mod[i].mod_start & (PAGE_SIZE - 1) )
>> panic("Bootloader didn't honor module alignment request.");
>> @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>
>> if ( !xen_phys_start )
>> panic("Not enough
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, 30 Jan 2015, Daniel Kiper wrote:
> We need more fine grained knowledge about EFI environment and check
> for EFI platform and EFI loader separately to properly support
> multiboot2 protocol. In general Xen loaded by this protocol uses
> memory mappings and loaded modules in simliar way to Xen loaded
> by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
> and efi_loader.
>
> Signed-off-by: Daniel Kiper
> ---
> xen/arch/x86/dmi_scan.c|4 ++--
> xen/arch/x86/domain_page.c |2 +-
> xen/arch/x86/efi/stub.c|5 +++--
> xen/arch/x86/mpparse.c |4 ++--
> xen/arch/x86/setup.c |8
> xen/arch/x86/time.c|2 +-
> xen/common/efi/boot.c |5 +
> xen/common/efi/runtime.c |5 +++--
> xen/drivers/acpi/osl.c |2 +-
> xen/include/xen/efi.h |6 +-
> 10 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/x86/dmi_scan.c b/xen/arch/x86/dmi_scan.c
> index 500133a..63b976c 100644
> --- a/xen/arch/x86/dmi_scan.c
> +++ b/xen/arch/x86/dmi_scan.c
> @@ -150,7 +150,7 @@ int __init dmi_get_table(u32 *base, u32 *len)
> struct dmi_eps eps;
> char __iomem *p, *q;
>
> - if (efi_enabled) {
> + if (efi_platform) {
> if (!efi_dmi_size)
> return -1;
> *base = efi_dmi_address;
> @@ -516,7 +516,7 @@ static void __init dmi_decode(struct dmi_header *dm)
>
> void __init dmi_scan_machine(void)
> {
> - if ((!efi_enabled ? dmi_iterate(dmi_decode) :
> + if ((!efi_platform ? dmi_iterate(dmi_decode) :
> dmi_efi_iterate(dmi_decode)) == 0)
> dmi_check_system(dmi_blacklist);
> else
> diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c
> index 158a164..5d4564c 100644
> --- a/xen/arch/x86/domain_page.c
> +++ b/xen/arch/x86/domain_page.c
> @@ -45,7 +45,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
> sync_local_execstate();
> /* We must now be running on the idle page table. */
> ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
> - (efi_enabled && cr3 == efi_rs_page_table()));
> + (efi_platform && cr3 == efi_rs_page_table()));
> }
>
> return v;
> diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
> index b8f49f8..5060e6f 100644
> --- a/xen/arch/x86/efi/stub.c
> +++ b/xen/arch/x86/efi/stub.c
> @@ -3,8 +3,9 @@
> #include
> #include
>
> -#ifndef efi_enabled
> -const bool_t efi_enabled = 0;
> +#ifndef efi_platform
> +bool_t efi_platform = 0;
> +bool_t efi_loader = 0;
> #endif
>
> void __init efi_init_memory(void) { }
> diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
> index a38e016..c4e3041 100644
> --- a/xen/arch/x86/mpparse.c
> +++ b/xen/arch/x86/mpparse.c
> @@ -540,7 +540,7 @@ static inline void __init
> construct_default_ISA_mptable(int mpc_default_type)
>
> static __init void efi_unmap_mpf(void)
> {
> - if (efi_enabled)
> + if (efi_platform)
> __set_fixmap(FIX_EFI_MPF, 0, 0);
> }
>
> @@ -698,7 +698,7 @@ void __init find_smp_config (void)
> {
> unsigned int address;
>
> - if (efi_enabled) {
> + if (efi_platform) {
> efi_check_config();
> return;
> }
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index c27c49c..711fdb0 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
> struct boot_video_info *bvi = &bootsym(boot_vid_info);
>
> /* The EFI loader fills vga_console_info directly. */
> -if ( efi_enabled )
> +if ( efi_platform )
> return;
>
> if ( (bvi->orig_video_isVGA == 1) && (bvi->orig_video_mode == 3) )
> @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
> panic("Misaligned CPU0 stack.");
>
> -if ( efi_enabled )
> +if ( efi_loader )
> {
> set_pdx_range(xen_phys_start >> PAGE_SHIFT,
>(xen_phys_start + BOOTSTRAP_MAP_BASE) >> PAGE_SHIFT);
> @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> * we can relocate the dom0 kernel and other multiboot modules. Also, on
> * x86/64, we relocate Xen to higher memory.
> */
> -for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
> +for ( i = 0; !efi_loader && i < mbi->mods_count; i++ )
> {
> if ( mod[i].mod_start & (PAGE_SIZE - 1) )
> panic("Bootloader didn't honor module alignment request.");
> @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>
> if ( !xen_phys_start )
> panic("Not enough memory to relocate Xen.");
> -reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper :
> __pa(&_start),
> +reserve_e820_ram(&boot_e820, efi_loader ? mbi
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
>>> On 30.01.15 at 18:54, wrote:
> We need more fine grained knowledge about EFI environment and check
> for EFI platform and EFI loader separately to properly support
> multiboot2 protocol.
... because of ... (i.e. I can't see from the description what the
separation is good for). Looking at the comments you placed
aside the variables doesn't help me either.
> In general Xen loaded by this protocol uses
> memory mappings and loaded modules in simliar way to Xen loaded
> by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform
> and efi_loader.
And if I'm guessing things right, then introducing efi_loader but
leaving efi_enabled alone (only converting where needed) would
seem the right approach.
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -431,7 +431,7 @@ static void __init parse_video_info(void)
> struct boot_video_info *bvi = &bootsym(boot_vid_info);
>
> /* The EFI loader fills vga_console_info directly. */
> -if ( efi_enabled )
> +if ( efi_platform )
This looks wrong considering the comment.
> @@ -663,7 +663,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
> panic("Misaligned CPU0 stack.");
>
> -if ( efi_enabled )
> +if ( efi_loader )
> {
> set_pdx_range(xen_phys_start >> PAGE_SHIFT,
>(xen_phys_start + BOOTSTRAP_MAP_BASE) >> PAGE_SHIFT);
> @@ -774,7 +774,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> * we can relocate the dom0 kernel and other multiboot modules. Also, on
> * x86/64, we relocate Xen to higher memory.
> */
> -for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
> +for ( i = 0; !efi_loader && i < mbi->mods_count; i++ )
> {
> if ( mod[i].mod_start & (PAGE_SIZE - 1) )
> panic("Bootloader didn't honor module alignment request.");
> @@ -962,7 +962,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>
> if ( !xen_phys_start )
> panic("Not enough memory to relocate Xen.");
> -reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper :
> __pa(&_start),
> +reserve_e820_ram(&boot_e820, efi_loader ? mbi->mem_upper : __pa(&_start),
For all of these it's really hard to tell whether they're right without
knowing which parts of the current EFI loading code you intend to
re-use. Perhaps switching these would better be done along with
adding the re-using code (or splitting out the not re-used parts)?
> --- a/xen/include/xen/efi.h
> +++ b/xen/include/xen/efi.h
> @@ -5,7 +5,11 @@
> #include
> #endif
>
> -extern const bool_t efi_enabled;
> +/* If true then Xen runs on EFI platform. */
> +extern bool_t efi_platform;
> +
> +/* If true then Xen was loaded by native EFI loader as PE application. */
> +extern bool_t efi_loader;
I don't see why you're losing the constness.
Jan
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
