[PATCH] export efi.flags to sysfs

[PATCH] export efi.flags to sysfs

[Dave Young]

For efi=old_map and any old_map quirks like SGI UV in current
tree kexec/kdump will fail because it depends on the new 1:1 mapping.

Thus export the mapping method to sysfs so kexec tools can switch
to original way to boot.

Since we have efi.flags for all efi facilities so let's just export the
efi.flags itself, it maybe useful for other arches and use cases.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 drivers/firmware/efi/efi.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/drivers/firmware/efi/efi.c
===================================================================
--- linux-2.6.orig/drivers/firmware/efi/efi.c
+++ linux-2.6/drivers/firmware/efi/efi.c
@@ -86,16 +86,19 @@ static ssize_t name##_show(struct kobjec
 EFI_ATTR_SHOW(fw_vendor);
 EFI_ATTR_SHOW(runtime);
 EFI_ATTR_SHOW(config_table);
+EFI_ATTR_SHOW(flags);
 
 static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor);
 static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime);
 static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table);
+static struct kobj_attribute efi_attr_flags = __ATTR_RO(flags);
 
 static struct attribute *efi_subsys_attrs[] = {
 	&efi_attr_systab.attr,
 	&efi_attr_fw_vendor.attr,
 	&efi_attr_runtime.attr,
 	&efi_attr_config_table.attr,
+	&efi_attr_flags.attr,
 	NULL,
 };
 

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/26/14 at 04:39pm, Dave Young wrote:
> 
> For efi=old_map and any old_map quirks like SGI UV in current
> tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> 
> Thus export the mapping method to sysfs so kexec tools can switch
> to original way to boot.
> 
> Since we have efi.flags for all efi facilities so let's just export the
> efi.flags itself, it maybe useful for other arches and use cases.

Rethink about this issue, export flags will expose the efi facility
macros to userspace, Matt, what's your opinion? It might be better to export
a file 'old_map' only which is '0|1' 

> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  drivers/firmware/efi/efi.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux-2.6/drivers/firmware/efi/efi.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/efi/efi.c
> +++ linux-2.6/drivers/firmware/efi/efi.c
> @@ -86,16 +86,19 @@ static ssize_t name##_show(struct kobjec
>  EFI_ATTR_SHOW(fw_vendor);
>  EFI_ATTR_SHOW(runtime);
>  EFI_ATTR_SHOW(config_table);
> +EFI_ATTR_SHOW(flags);
>  
>  static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor);
>  static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime);
>  static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table);
> +static struct kobj_attribute efi_attr_flags = __ATTR_RO(flags);
>  
>  static struct attribute *efi_subsys_attrs[] = {
>  	&efi_attr_systab.attr,
>  	&efi_attr_fw_vendor.attr,
>  	&efi_attr_runtime.attr,
>  	&efi_attr_config_table.attr,
> +	&efi_attr_flags.attr,
>  	NULL,
>  };
>  

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> 
> For efi=old_map and any old_map quirks like SGI UV in current
> tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> 
> Thus export the mapping method to sysfs so kexec tools can switch
> to original way to boot.
> 
> Since we have efi.flags for all efi facilities so let's just export the
> efi.flags itself, it maybe useful for other arches and use cases.
> 

Does it require any documentation in Documentation/ABI/..

Vivek

> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  drivers/firmware/efi/efi.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> Index: linux-2.6/drivers/firmware/efi/efi.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/efi/efi.c
> +++ linux-2.6/drivers/firmware/efi/efi.c
> @@ -86,16 +86,19 @@ static ssize_t name##_show(struct kobjec
>  EFI_ATTR_SHOW(fw_vendor);
>  EFI_ATTR_SHOW(runtime);
>  EFI_ATTR_SHOW(config_table);
> +EFI_ATTR_SHOW(flags);
>  
>  static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor);
>  static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime);
>  static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table);
> +static struct kobj_attribute efi_attr_flags = __ATTR_RO(flags);
>  
>  static struct attribute *efi_subsys_attrs[] = {
>  	&efi_attr_systab.attr,
>  	&efi_attr_fw_vendor.attr,
>  	&efi_attr_runtime.attr,
>  	&efi_attr_config_table.attr,
> +	&efi_attr_flags.attr,
>  	NULL,
>  };
>  

Re: [PATCH] export efi.flags to sysfs

[Fleming, Matt]

On 27 May 2014 04:00, Dave Young <dyoung@redhat.com> wrote:
> On 05/26/14 at 04:39pm, Dave Young wrote:
>>
>> For efi=old_map and any old_map quirks like SGI UV in current
>> tree kexec/kdump will fail because it depends on the new 1:1 mapping.
>>
>> Thus export the mapping method to sysfs so kexec tools can switch
>> to original way to boot.
>>
>> Since we have efi.flags for all efi facilities so let's just export the
>> efi.flags itself, it maybe useful for other arches and use cases.
>
> Rethink about this issue, export flags will expose the efi facility
> macros to userspace, Matt, what's your opinion? It might be better to export
> a file 'old_map' only which is '0|1'

Exporting efi.flags is a non-starter. Those flags are part of an
internal interface and I'm not prepared to turn them into a userspace
ABI that we can never, ever change without a massive amount of pain.

I've only vaguely been following along with the other thread, so please
summarise everything again in your patch. Particularly, I need answers
to the following questions,

 - Are you trying to fix a kexec/kdump regression?
 - Does SGI UV work with kexec + UEFI at all?

The 1:1 mapping was required to make kexec + EFI work in the first
instance. If a machine implements the EFI 1:1 mapping, kexec should
work. If it doesn't implement the 1:1 mapping, then it's probably not
going to work, right?

The crux of the question: are you trying to fix a regression?

If not, then we just need to get SGI UV working with the EFI 1:1
mapping. No?

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/27/14 at 02:36pm, Fleming, Matt wrote:
> On 27 May 2014 04:00, Dave Young <dyoung@redhat.com> wrote:
> > On 05/26/14 at 04:39pm, Dave Young wrote:
> >>
> >> For efi=old_map and any old_map quirks like SGI UV in current
> >> tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> >>
> >> Thus export the mapping method to sysfs so kexec tools can switch
> >> to original way to boot.
> >>
> >> Since we have efi.flags for all efi facilities so let's just export the
> >> efi.flags itself, it maybe useful for other arches and use cases.
> >
> > Rethink about this issue, export flags will expose the efi facility
> > macros to userspace, Matt, what's your opinion? It might be better to export
> > a file 'old_map' only which is '0|1'
> 
> Exporting efi.flags is a non-starter. Those flags are part of an
> internal interface and I'm not prepared to turn them into a userspace
> ABI that we can never, ever change without a massive amount of pain.

Agree that it's not good to move them to external ones.

> 
> I've only vaguely been following along with the other thread, so please
> summarise everything again in your patch. Particularly, I need answers
> to the following questions,
> 
>  - Are you trying to fix a kexec/kdump regression?

Somehow it is a regression.
Before the 1:1 mapping kexec/kdump works with 'noefi'
plus acpi_rsdp= kernel cmdline. kexec-tools does not fill efi_info in boot_params
so kexec kernel will simply boot like 'noefi'.

Now we have 1:1 mapping, kexec-tools will boot with efi enabled but SGI UV is
still using old maping thus it become a problem. 

So kexec-tools need to know whether it's old_map or nor so it can switch to the
right way in case efi boot.

>  - Does SGI UV work with kexec + UEFI at all?

It works previously without enabling efi in boot_params. 

> 
> The 1:1 mapping was required to make kexec + EFI work in the first
> instance. If a machine implements the EFI 1:1 mapping, kexec should
> work. If it doesn't implement the 1:1 mapping, then it's probably not
> going to work, right?
> 
> The crux of the question: are you trying to fix a regression?
> 
> If not, then we just need to get SGI UV working with the EFI 1:1
> mapping. No?

Ditto as before explanation...

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/27/14 at 09:34am, Vivek Goyal wrote:
> On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > 
> > For efi=old_map and any old_map quirks like SGI UV in current
> > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > 
> > Thus export the mapping method to sysfs so kexec tools can switch
> > to original way to boot.
> > 
> > Since we have efi.flags for all efi facilities so let's just export the
> > efi.flags itself, it maybe useful for other arches and use cases.
> > 
> 
> Does it require any documentation in Documentation/ABI/..

Yes, it's necessary. Will do in next version.

I'm still discussing with Matt, exporting efi.flags seems not a good way
because they are more internal interfaces. 

Probably I should export only a file 'old_map' instead.

> 
> Vivek
> 
> > Signed-off-by: Dave Young <dyoung@redhat.com>
> > ---
> >  drivers/firmware/efi/efi.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > Index: linux-2.6/drivers/firmware/efi/efi.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/firmware/efi/efi.c
> > +++ linux-2.6/drivers/firmware/efi/efi.c
> > @@ -86,16 +86,19 @@ static ssize_t name##_show(struct kobjec
> >  EFI_ATTR_SHOW(fw_vendor);
> >  EFI_ATTR_SHOW(runtime);
> >  EFI_ATTR_SHOW(config_table);
> > +EFI_ATTR_SHOW(flags);
> >  
> >  static struct kobj_attribute efi_attr_fw_vendor = __ATTR_RO(fw_vendor);
> >  static struct kobj_attribute efi_attr_runtime = __ATTR_RO(runtime);
> >  static struct kobj_attribute efi_attr_config_table = __ATTR_RO(config_table);
> > +static struct kobj_attribute efi_attr_flags = __ATTR_RO(flags);
> >  
> >  static struct attribute *efi_subsys_attrs[] = {
> >  	&efi_attr_systab.attr,
> >  	&efi_attr_fw_vendor.attr,
> >  	&efi_attr_runtime.attr,
> >  	&efi_attr_config_table.attr,
> > +	&efi_attr_flags.attr,
> >  	NULL,
> >  };
> >  

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

Add Alex to cc

> > The 1:1 mapping was required to make kexec + EFI work in the first
> > instance. If a machine implements the EFI 1:1 mapping, kexec should
> > work. If it doesn't implement the 1:1 mapping, then it's probably not
> > going to work, right?
> > 
> > The crux of the question: are you trying to fix a regression?
> > 
> > If not, then we just need to get SGI UV working with the EFI 1:1
> > mapping. No?

Alex is working on a SGI UV fix for 1:1 mapping.
Question to Alex, if userspace can get the firmware version there might
be another option that kexec-tools checking the firmware version and
switch to old noefi way automaticlly. Is it doable?

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Wed, May 28, 2014 at 10:13:59AM +0800, Dave Young wrote:
> On 05/27/14 at 09:34am, Vivek Goyal wrote:
> > On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > > 
> > > For efi=old_map and any old_map quirks like SGI UV in current
> > > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > > 
> > > Thus export the mapping method to sysfs so kexec tools can switch
> > > to original way to boot.
> > > 
> > > Since we have efi.flags for all efi facilities so let's just export the
> > > efi.flags itself, it maybe useful for other arches and use cases.
> > > 
> > 
> > Does it require any documentation in Documentation/ABI/..
> 
> Yes, it's necessary. Will do in next version.
> 
> I'm still discussing with Matt, exporting efi.flags seems not a good way
> because they are more internal interfaces. 
> 
> Probably I should export only a file 'old_map' instead.

How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
we look at it and figure out if it is 1:1 or not.

Thanks
Vivek

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Wed, May 28, 2014 at 10:09:35AM +0800, Dave Young wrote:

[..]
> > I've only vaguely been following along with the other thread, so please
> > summarise everything again in your patch. Particularly, I need answers
> > to the following questions,
> > 
> >  - Are you trying to fix a kexec/kdump regression?
> 
> Somehow it is a regression.

Well, it is a *regression*. Previously kdump would work with
SGI UV machines as it used "noefi". Now kexec by default thinks that
efi is enabled and 1:1 mappings are in place and that does not work
with SGI UV machines.

So just providing --noefi is not enough. Somebody needs to automatically
figure out that this SGI machine does not contain 1:1 mapping and hence
need to fall back to old mode. --noefi is good for override.

Thanks
Vivek

Re: [PATCH] export efi.flags to sysfs

[Alex Thorlton]

On Wed, May 28, 2014 at 05:47:12PM +0800, Dave Young wrote:
> Add Alex to cc
> 
> > > The 1:1 mapping was required to make kexec + EFI work in the first
> > > instance. If a machine implements the EFI 1:1 mapping, kexec should
> > > work. If it doesn't implement the 1:1 mapping, then it's probably not
> > > going to work, right?
> > > 
> > > The crux of the question: are you trying to fix a regression?
> > > 
> > > If not, then we just need to get SGI UV working with the EFI 1:1
> > > mapping. No?
> 
> Alex is working on a SGI UV fix for 1:1 mapping.
> Question to Alex, if userspace can get the firmware version there might
> be another option that kexec-tools checking the firmware version and
> switch to old noefi way automaticlly. Is it doable?

Userspace can get the firmware version easily, as long as our hwperf
module is installed (as it should be on any supported SGI machine).  The
firmware version can be found in /proc/sgi_uv/bios_version on any
machine where hwperf is loaded.

I don't know if that's really an ideal method for what you're doing, but
that's the only way I know of to get the firmware version in userspace.

A side note, though:  We're going to have to figure out some way to
determine whether or not to apply the old_map quirk on during boot
anyway, so if it's easiest for you to just determine how the original
kernel was booted and key off of that, then I wouldn't worry about
finding a different method to get the firmware version in userspace,
unless there are other reasons for you to do it that way.

- Alex

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Wed, May 28, 2014 at 10:51:04AM -0500, Alex Thorlton wrote:

[..]
> A side note, though:  We're going to have to figure out some way to
> determine whether or not to apply the old_map quirk on during boot
> anyway, so if it's easiest for you to just determine how the original
> kernel was booted and key off of that, then I wouldn't worry about
> finding a different method to get the firmware version in userspace,
> unless there are other reasons for you to do it that way.

Agreed. Let kernel and firmware figure it out when to apply quirk and
user space should not worry about firmware version.

I liked the other approach better where user space just needs to know
if 1:1 mapping are in effect or not and take decision based on that.

So once SGI firmware gets fixed for 1:1 mapping, and kernel knows it,
then user space will not have to be changed at all.

Thanks
Vivek

Re: [PATCH] export efi.flags to sysfs

[Alex Thorlton]

On Wed, May 28, 2014 at 03:04:00PM -0400, Vivek Goyal wrote:
> On Wed, May 28, 2014 at 10:51:04AM -0500, Alex Thorlton wrote:
> 
> [..]
> > A side note, though:  We're going to have to figure out some way to
> > determine whether or not to apply the old_map quirk on during boot
> > anyway, so if it's easiest for you to just determine how the original
> > kernel was booted and key off of that, then I wouldn't worry about
> > finding a different method to get the firmware version in userspace,
> > unless there are other reasons for you to do it that way.
> 
> Agreed. Let kernel and firmware figure it out when to apply quirk and
> user space should not worry about firmware version.
> 
> I liked the other approach better where user space just needs to know
> if 1:1 mapping are in effect or not and take decision based on that.
> 
> So once SGI firmware gets fixed for 1:1 mapping, and kernel knows it,
> then user space will not have to be changed at all.

Sounds good to me.  Working on a potential kernel fix as we speak.

- Alex

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/28/14 at 08:40am, Vivek Goyal wrote:
> On Wed, May 28, 2014 at 10:13:59AM +0800, Dave Young wrote:
> > On 05/27/14 at 09:34am, Vivek Goyal wrote:
> > > On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > > > 
> > > > For efi=old_map and any old_map quirks like SGI UV in current
> > > > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > > > 
> > > > Thus export the mapping method to sysfs so kexec tools can switch
> > > > to original way to boot.
> > > > 
> > > > Since we have efi.flags for all efi facilities so let's just export the
> > > > efi.flags itself, it maybe useful for other arches and use cases.
> > > > 
> > > 
> > > Does it require any documentation in Documentation/ABI/..
> > 
> > Yes, it's necessary. Will do in next version.
> > 
> > I'm still discussing with Matt, exporting efi.flags seems not a good way
> > because they are more internal interfaces. 
> > 
> > Probably I should export only a file 'old_map' instead.
> 
> How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
> we look at it and figure out if it is 1:1 or not.

There's phys_addr and virt_addr, (virt_addr - phys_addr) will always be
-64G for 1:1 map, ioremapped addresses space is different.

So this is a good idea, we can do this way to avoid more ABI.

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

> > Question to Alex, if userspace can get the firmware version there might
> > be another option that kexec-tools checking the firmware version and
> > switch to old noefi way automaticlly. Is it doable?
> 
> Userspace can get the firmware version easily, as long as our hwperf
> module is installed (as it should be on any supported SGI machine).  The
> firmware version can be found in /proc/sgi_uv/bios_version on any
> machine where hwperf is loaded.
> 
> I don't know if that's really an ideal method for what you're doing, but
> that's the only way I know of to get the firmware version in userspace.
> 
> A side note, though:  We're going to have to figure out some way to
> determine whether or not to apply the old_map quirk on during boot
> anyway, so if it's easiest for you to just determine how the original
> kernel was booted and key off of that, then I wouldn't worry about
> finding a different method to get the firmware version in userspace,
> unless there are other reasons for you to do it that way.

I'm dropping the idea to check firmware version in user space, your kernel
fix to automaticlly select mapping method based on firmware version is still
needed.

I'm tending to use the idea from Vivek in kexec-tools to check the virt addr
in /sys/firmware/efi/runtime-map/*/virt_addr

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/29/14 at 10:08am, Dave Young wrote:
> On 05/28/14 at 08:40am, Vivek Goyal wrote:
> > On Wed, May 28, 2014 at 10:13:59AM +0800, Dave Young wrote:
> > > On 05/27/14 at 09:34am, Vivek Goyal wrote:
> > > > On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > > > > 
> > > > > For efi=old_map and any old_map quirks like SGI UV in current
> > > > > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > > > > 
> > > > > Thus export the mapping method to sysfs so kexec tools can switch
> > > > > to original way to boot.
> > > > > 
> > > > > Since we have efi.flags for all efi facilities so let's just export the
> > > > > efi.flags itself, it maybe useful for other arches and use cases.
> > > > > 
> > > > 
> > > > Does it require any documentation in Documentation/ABI/..
> > > 
> > > Yes, it's necessary. Will do in next version.
> > > 
> > > I'm still discussing with Matt, exporting efi.flags seems not a good way
> > > because they are more internal interfaces. 
> > > 
> > > Probably I should export only a file 'old_map' instead.
> > 
> > How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
> > we look at it and figure out if it is 1:1 or not.
> 
> There's phys_addr and virt_addr, (virt_addr - phys_addr) will always be
> -64G for 1:1 map, ioremapped addresses space is different.
> 
> So this is a good idea, we can do this way to avoid more ABI.

Thinking more about it. There's another another way. ioremapped addresses can not
be garanteed to be same in 2nd kernel, but for same kernel it's usually are same
so in 2nd kernel just try ioremap the regions again hopefully it will work.
At lease it works for me before Boris's patchset. So user can do some test if it
works then just use it as is. If kexec/kdump test failed he can switch to use
--noefi in userspace. Like below patch:


Subject: [PATCH 2/2] x86: kexec efi boot use ioremap in case efi=old_map

In case efi=old_map or quirked system which use old ioremap kexec should
hopefully work for same kernel but it is not garanteed to be true.

It will help if one has to use old_map and he want kexec support as well.
At the same time he has succesfully tested the kexec boot. If it does not
work then user can switch to use kexec --noefi in userspace instead.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/platform/efi/efi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 3781dd3..d2eaa6f 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -1030,7 +1030,11 @@ static void __init kexec_enter_virtual_mode(void)
 	*/
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
-		efi_map_region_fixed(md); /* FIXME: add error handling */
+		/* For ioremapped ranges it will hopefully work with same kernel. */
+		if (efi_enabled(EFI_OLD_MEMMAP))
+			old_map_region(md);
+		else
+			efi_map_region_fixed(md); /* FIXME: add error handling */
 		get_systab_virt_addr(md);
 	}
 

Re: [PATCH] export efi.flags to sysfs

[Fleming, Matt]

On 28 May 2014 15:51, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, May 28, 2014 at 10:09:35AM +0800, Dave Young wrote:
>
> [..]
>> > I've only vaguely been following along with the other thread, so please
>> > summarise everything again in your patch. Particularly, I need answers
>> > to the following questions,
>> >
>> >  - Are you trying to fix a kexec/kdump regression?
>>
>> Somehow it is a regression.
>
> Well, it is a *regression*. Previously kdump would work with
> SGI UV machines as it used "noefi". Now kexec by default thinks that
> efi is enabled and 1:1 mappings are in place and that does not work
> with SGI UV machines.

So, if someone boots an SGI UV machine with the "noefi" kernel command
line parameter, the kernel still creates
/sys/firmware/efi/runtime-map/ ? Why does kexec/kdump think a kernel
booted with "noefi" supports EFI?

Or are we talking about a different "noefi" parameter?

Re: [PATCH] export efi.flags to sysfs

[Fleming, Matt]

On 28 May 2014 20:04, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, May 28, 2014 at 10:51:04AM -0500, Alex Thorlton wrote:
>
> [..]
>> A side note, though:  We're going to have to figure out some way to
>> determine whether or not to apply the old_map quirk on during boot
>> anyway, so if it's easiest for you to just determine how the original
>> kernel was booted and key off of that, then I wouldn't worry about
>> finding a different method to get the firmware version in userspace,
>> unless there are other reasons for you to do it that way.
>
> Agreed. Let kernel and firmware figure it out when to apply quirk and
> user space should not worry about firmware version.
>
> I liked the other approach better where user space just needs to know
> if 1:1 mapping are in effect or not and take decision based on that.

The problem with going down this route is that if we ever change the
way we map the EFI regions in the kernel corresponding changes will be
required for kexec-tools. That's not ideal.

Kexec-tools should not be interpreting /sys/firmware/efi/runtime-map/
any further than building a memory map, i.e. it has to understand how
to build a memory region descriptor but it shouldn't care what
*values* are in them.

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Thu, May 29, 2014 at 10:08:37AM +0800, Dave Young wrote:
> On 05/28/14 at 08:40am, Vivek Goyal wrote:
> > On Wed, May 28, 2014 at 10:13:59AM +0800, Dave Young wrote:
> > > On 05/27/14 at 09:34am, Vivek Goyal wrote:
> > > > On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > > > > 
> > > > > For efi=old_map and any old_map quirks like SGI UV in current
> > > > > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > > > > 
> > > > > Thus export the mapping method to sysfs so kexec tools can switch
> > > > > to original way to boot.
> > > > > 
> > > > > Since we have efi.flags for all efi facilities so let's just export the
> > > > > efi.flags itself, it maybe useful for other arches and use cases.
> > > > > 
> > > > 
> > > > Does it require any documentation in Documentation/ABI/..
> > > 
> > > Yes, it's necessary. Will do in next version.
> > > 
> > > I'm still discussing with Matt, exporting efi.flags seems not a good way
> > > because they are more internal interfaces. 
> > > 
> > > Probably I should export only a file 'old_map' instead.
> > 
> > How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
> > we look at it and figure out if it is 1:1 or not.
> 
> There's phys_addr and virt_addr, (virt_addr - phys_addr) will always be
> -64G for 1:1 map, ioremapped addresses space is different.

I am curious that what's the meaning of 1:1 mapping here? So far I thought
that means virt and physical addresses are same but that does not seem
to be the case. So what does it mean?

Thanks
Vivek

Re: [PATCH] export efi.flags to sysfs

[Borislav Petkov]

On Thu, May 29, 2014 at 08:45:20AM -0400, Vivek Goyal wrote:
> I am curious that what's the meaning of 1:1 mapping here? So far I
> thought that means virt and physical addresses are same but that does
> not seem to be the case. So what does it mean?

1:1 mapping in the EFI's case (and maybe in any other case I'm aware of)
is taking the physical addresses of the EFI regions and mapping them at
the exact same addresses in the EFI page table.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] export efi.flags to sysfs

[Vivek Goyal]

On Thu, May 29, 2014 at 12:53:19PM +0100, Fleming, Matt wrote:
> On 28 May 2014 15:51, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Wed, May 28, 2014 at 10:09:35AM +0800, Dave Young wrote:
> >
> > [..]
> >> > I've only vaguely been following along with the other thread, so please
> >> > summarise everything again in your patch. Particularly, I need answers
> >> > to the following questions,
> >> >
> >> >  - Are you trying to fix a kexec/kdump regression?
> >>
> >> Somehow it is a regression.
> >
> > Well, it is a *regression*. Previously kdump would work with
> > SGI UV machines as it used "noefi". Now kexec by default thinks that
> > efi is enabled and 1:1 mappings are in place and that does not work
> > with SGI UV machines.
> 
> So, if someone boots an SGI UV machine with the "noefi" kernel command
> line parameter, the kernel still creates
> /sys/firmware/efi/runtime-map/ ? Why does kexec/kdump think a kernel
> booted with "noefi" supports EFI?
> 
> Or are we talking about a different "noefi" parameter?

Only second kernel boots with "noefi" and this parameter is appened by
kexec-tools to second kernel command line. So first kernel will still
boot *without noefi* and kexec-tools wil think that this system support
booting second kernel with UEFI enabled.

I don't know if we export /sys/firmware/efi/runtime-map/ in case of
non 1:1 mapping or not. Dave and Boris will know better.

Thanks
Vivek

Re: [PATCH] export efi.flags to sysfs

[Fleming, Matt]

On 29 May 2014 13:59, Vivek Goyal <vgoyal@redhat.com> wrote:
>
> Only second kernel boots with "noefi" and this parameter is appened by
> kexec-tools to second kernel command line. So first kernel will still
> boot *without noefi* and kexec-tools wil think that this system support
> booting second kernel with UEFI enabled.
>
> I don't know if we export /sys/firmware/efi/runtime-map/ in case of
> non 1:1 mapping or not. Dave and Boris will know better.

Looking at the code the answer is: yes. I think that's the bug right there.

By exporting entries in /sys/firmware/efi/runtime-map we're saying
"These mappings persist across kexec, you can trust that they won't
change", for SGI UV (which currently uses the quirk) or if you boot
with efi=old_map on the command line, that's simply not true.

So, it would seem to me that we're missing a "if
(efi_enabled(OLD_MAP))" in drivers/firmware/efi/runtime-map.c.
Borislav, Dave? What do you guys think?

This is all assuming that the root of the regression is that
kexec-tools sees entries in /sys/firmware/efi/runtime-map/ and says
"Yay! EFI boot!".

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/29/14 at 02:10pm, Fleming, Matt wrote:
> On 29 May 2014 13:59, Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > Only second kernel boots with "noefi" and this parameter is appened by
> > kexec-tools to second kernel command line. So first kernel will still
> > boot *without noefi* and kexec-tools wil think that this system support
> > booting second kernel with UEFI enabled.
> >
> > I don't know if we export /sys/firmware/efi/runtime-map/ in case of
> > non 1:1 mapping or not. Dave and Boris will know better.
> 
> Looking at the code the answer is: yes. I think that's the bug right there.
> 
> By exporting entries in /sys/firmware/efi/runtime-map we're saying
> "These mappings persist across kexec, you can trust that they won't
> change", for SGI UV (which currently uses the quirk) or if you boot
> with efi=old_map on the command line, that's simply not true.
> 
> So, it would seem to me that we're missing a "if
> (efi_enabled(OLD_MAP))" in drivers/firmware/efi/runtime-map.c.
> Borislav, Dave? What do you guys think?

Yes, I missed the old_map case when I did the patchset.
Only exporting runtime map in case 1:1 map looks good. Thus kexec-tools can
easily just check the existance of the sysfs files.

Will send out a patch soon.

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Dave Young]

On 05/29/14 at 08:45am, Vivek Goyal wrote:
> On Thu, May 29, 2014 at 10:08:37AM +0800, Dave Young wrote:
> > On 05/28/14 at 08:40am, Vivek Goyal wrote:
> > > On Wed, May 28, 2014 at 10:13:59AM +0800, Dave Young wrote:
> > > > On 05/27/14 at 09:34am, Vivek Goyal wrote:
> > > > > On Mon, May 26, 2014 at 04:39:35PM +0800, Dave Young wrote:
> > > > > > 
> > > > > > For efi=old_map and any old_map quirks like SGI UV in current
> > > > > > tree kexec/kdump will fail because it depends on the new 1:1 mapping.
> > > > > > 
> > > > > > Thus export the mapping method to sysfs so kexec tools can switch
> > > > > > to original way to boot.
> > > > > > 
> > > > > > Since we have efi.flags for all efi facilities so let's just export the
> > > > > > efi.flags itself, it maybe useful for other arches and use cases.
> > > > > > 
> > > > > 
> > > > > Does it require any documentation in Documentation/ABI/..
> > > > 
> > > > Yes, it's necessary. Will do in next version.
> > > > 
> > > > I'm still discussing with Matt, exporting efi.flags seems not a good way
> > > > because they are more internal interfaces. 
> > > > 
> > > > Probably I should export only a file 'old_map' instead.
> > > 
> > > How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
> > > we look at it and figure out if it is 1:1 or not.
> > 
> > There's phys_addr and virt_addr, (virt_addr - phys_addr) will always be
> > -64G for 1:1 map, ioremapped addresses space is different.

Correct myself it's top to down (-4G - -64G) instead of down to top. 

> 
> I am curious that what's the meaning of 1:1 mapping here? So far I thought
> that means virt and physical addresses are same but that does not seem
> to be the case. So what does it mean?

while doing the mapping, we will iterate the memory ranges (md[])

Like below without considering alignment:
Virt addr   (down) <------------------------------> (top)
md0 (size0)                                 <-----> 
                                             (size0)
md1 (size1)                        <------->
                                    (size1)
...

Boris can correct me if I'm wrong.

Thanks
Dave

Re: [PATCH] export efi.flags to sysfs

[Borislav Petkov]

On Fri, May 30, 2014 at 10:24:38AM +0800, Dave Young wrote:
> > > > How does /sys/firmware/efi/runtime-map/* look like with old mapping? Can't
> > > > we look at it and figure out if it is 1:1 or not.
> > > 
> > > There's phys_addr and virt_addr, (virt_addr - phys_addr) will always be
> > > -64G for 1:1 map, ioremapped addresses space is different.
> 
> Correct myself it's top to down (-4G - -64G) instead of down to top.

This is the new mapping.

AFAICR, the question is about the old mapping. Which was "normal"
ioremap into the kernel page table. The new mapping is into another page
table.

> > I am curious that what's the meaning of 1:1 mapping here? So far I thought
> > that means virt and physical addresses are same but that does not seem
> > to be the case. So what does it mean?
> 
> while doing the mapping, we will iterate the memory ranges (md[])
> 
> Like below without considering alignment:
> Virt addr   (down) <------------------------------> (top)
> md0 (size0)                                 <-----> 
>                                              (size0)
> md1 (size1)                        <------->
>                                     (size1)
> ...
> 
> Boris can correct me if I'm wrong.

Yes, this is the new mapping, top = -2G.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--