[RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Ard Biesheuvel]

As noted by Ian, many DMI based quirks for x86 ACPI machines disable
features that can also be disabled via the kernel command line. Similarly,
a quirk is under discussion for a arm64 ACPI machine [0] that explodes
when enabling support for hardware error reporting via the ACPI HEST table.

When support for DMI tables was introduced to arm64 and ARM, the agreement
was that they are informational only, i.e., provided to userland to describe
the platform, not for keying off of to enable quirks in the kernel.

There are a couple of reasons for this:
- Unlike the x86 architecture, where virtually all platforms are PC variants,
  and the presence of ACPI and DMI tables may be assumed, the arm64 architecture
  is much more heterogeneous, and none of UEFI, ACPI or DMI or actually mandated
  or especially common across arm64 platforms; using DMI only makes sense for
  working around a limited subset of platform issues that have to do with
  firmware running on platforms that bother to implement it in the first place.
- DMI is not initialized as early as it is on x86, and doing so is not trivial.
  This means that even on ACPI/DMI machines, some issues may require quirks that
  are enabled in a different way, or we need to refactor the DMI support so that
  we can enable it as early as x86 does.
- Using DMI tables fundamentally means quirking *after* the fact, which makes it
  a moving target. Some quirks may require the DMI match table to be updated if
  someone happens to change a string in the DMI tables when shipping a mostly
  identical platform.

So instead, let's provide these platforms with the facilities required to enable
such quirks at the platform level. Especially for UEFI systems, if upgrading
firmware is a reasonable prerequisite for being able to upgrade to the latest
kernel, having to run a script that sets a UEFI variable (either via the Linux
command line of from the UEFI shell) should not be an unreasonable requirement
either, and so we can solve part of this issue by configuring extra command line
arguments persistenly from the firmware environment. This solves all the above
issues in an unintrusive manner, given that the kernel command line is already
made available extremely early in the boot, and the fact that it already permits
a wide range of configuration options and overrides to be set, including the
'hest_disable=1' option that works around the issue addressed by [0].

Note that this deviates from the current common practice on x86. I am aware
that the distros want everything to be the same in this regard, but I don't
think the x86 maintainers will refer to the DMI quirks infrastructure as a
shining example of functionality that should be ported across architectures.

ACPI on arm64 is intended to support a narrowly defined class of server systems,
as opposed to the wide range of Windows-logo hardware that arch/x86 aims to
support, and a lot of time and effort has been going into validation against
Linux itself. This means the likelihood that we will need a quirks
infrastructure as elaborate as the x86 one is low, and so it would be my 
preference to defer introducing it until the moment where we have exhausted
all other options.

[0] https://marc.info/?l=linux-acpi&m=153018042727125&w=2

Cc: Mark Salter <msalter@redhat.com>
Cc: Geoff Levand <geoff@infradead.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Riku Voipio <riku.voipio@linaro.org>
Cc: James Morse <james.morse@arm.com>
Cc: Ian Campbell <ijc@debian.org>

Ard Biesheuvel (2):
  efi/libstub: refactor load option command line processing for reuse
  efi/libstub: taken contents of LinuxExtraArgs UEFI variable into
    account

 .../firmware/efi/libstub/efi-stub-helper.c    | 94 +++++++++++++++----
 include/linux/efi.h                           |  1 +
 2 files changed, 77 insertions(+), 18 deletions(-)

-- 
2.17.1

[RFC PATCH 1/2] efi/libstub: refactor load option command line processing for reuse

[Ard Biesheuvel]

As a preparatory step towards adding support for extra kernel command
line arguments passed via a UEFI variable, do a slight refactor of the
existing code so we can extend it more cleanly in a subsequent patch.
No change is functionality is intended.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 43 ++++++++++++--------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 50a9cab5a834..97a2423782af 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -726,6 +726,22 @@ static int efi_utf8_bytes(u16 c)
 	return 1 + (c >= 0x80) + (c >= 0x800);
 }
 
+/*
+ * Get an upper bound for the number of UTF-8 bytes corresponding
+ * to a \n or \0 terminated UTF-16 string.
+ */
+static int count_utf8_bytes(const u16 *s, int max_chars, int *num_chars)
+{
+	int ret = 0;
+
+	while (*s != '\0' && *s != '\n' && --max_chars >= 0) {
+		ret += efi_utf8_bytes(*s++);
+		++*num_chars;
+	}
+
+	return ret;
+}
+
 /*
  * Convert an UTF-16 string, not necessarily null terminated, to UTF-8.
  */
@@ -779,44 +795,35 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
 			  efi_loaded_image_t *image,
 			  int *cmd_line_len)
 {
-	const u16 *s2;
-	u8 *s1 = NULL;
+	u8 *s1;
 	unsigned long cmdline_addr = 0;
 	int load_options_chars = image->load_options_size / 2; /* UTF-16 */
 	const u16 *options = image->load_options;
-	int options_bytes = 0;  /* UTF-8 bytes */
+	int cmd_line_bytes = 0;  /* UTF-8 bytes */
 	int options_chars = 0;  /* UTF-16 chars */
 	efi_status_t status;
 	u16 zero = 0;
 
-	if (options) {
-		s2 = options;
-		while (*s2 && *s2 != '\n'
-		       && options_chars < load_options_chars) {
-			options_bytes += efi_utf8_bytes(*s2++);
-			options_chars++;
-		}
-	}
+	if (options)
+		cmd_line_bytes = count_utf8_bytes(options, load_options_chars,
+						  &options_chars);
 
 	if (!options_chars) {
 		/* No command line options, so return empty string*/
 		options = &zero;
 	}
 
-	options_bytes++;	/* NUL termination */
+	cmd_line_bytes++;	/* NUL termination */
 
-	status = efi_high_alloc(sys_table_arg, options_bytes, 0,
+	status = efi_high_alloc(sys_table_arg, cmd_line_bytes, 0,
 				&cmdline_addr, MAX_CMDLINE_ADDRESS);
 	if (status != EFI_SUCCESS)
 		return NULL;
 
-	s1 = (u8 *)cmdline_addr;
-	s2 = (const u16 *)options;
-
-	s1 = efi_utf16_to_utf8(s1, s2, options_chars);
+	s1 = efi_utf16_to_utf8((u8 *)cmdline_addr, options, options_chars);
 	*s1 = '\0';
 
-	*cmd_line_len = options_bytes;
+	*cmd_line_len = cmd_line_bytes;
 	return (char *)cmdline_addr;
 }
 
-- 
2.17.1

[RFC PATCH 2/2] efi/libstub: taken contents of LinuxExtraArgs UEFI variable into account

[Ard Biesheuvel]

In order to allow UEFI platforms to persistently configure Linux kernel
options without relying on the contents of the EFI System Partition (ESP)
or other block devices, implement support for passing extra kernel
command line arguments via the LinuxExtraArgs UEFI variable.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/efi-stub-helper.c | 51 ++++++++++++++++++++
 include/linux/efi.h                            |  1 +
 2 files changed, 52 insertions(+)

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 97a2423782af..49a3b03b9f1f 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -35,6 +35,9 @@ static unsigned long __chunk_size = EFI_READ_CHUNK_SIZE;
 static int __section(.data) __nokaslr;
 static int __section(.data) __quiet;
 
+static const efi_guid_t linux_args_guid = LINUX_EFI_EXTRA_ARGS_GUID;
+static const efi_char16_t linux_args_name[] = L"LinuxExtraArgs";
+
 int __pure nokaslr(void)
 {
 	return __nokaslr;
@@ -786,6 +789,33 @@ static u8 *efi_utf16_to_utf8(u8 *dst, const u16 *src, int n)
 #define MAX_CMDLINE_ADDRESS	ULONG_MAX
 #endif
 
+static u16 *get_extra_args(efi_system_table_t *sys_table_arg,
+			   unsigned long *extra_args_size)
+{
+	u16 *extra_args;
+	efi_status_t status;
+
+	*extra_args_size = 0;
+	status = efi_call_runtime(get_variable, (efi_char16_t *)linux_args_name,
+				  (efi_guid_t *)&linux_args_guid, NULL,
+				  extra_args_size, NULL);
+	if (status != EFI_BUFFER_TOO_SMALL)
+		return NULL;
+
+	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+				*extra_args_size, (void **)&extra_args);
+	if (status != EFI_SUCCESS)
+		return NULL;
+
+	status = efi_call_runtime(get_variable, (efi_char16_t *)linux_args_name,
+				  (efi_guid_t *)&linux_args_guid, NULL,
+				  extra_args_size, extra_args);
+	if (status != EFI_SUCCESS)
+		return NULL;
+
+	return extra_args;
+}
+
 /*
  * Convert the unicode UEFI command line to ASCII to pass to kernel.
  * Size of memory allocated return in *cmd_line_len.
@@ -799,9 +829,12 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
 	unsigned long cmdline_addr = 0;
 	int load_options_chars = image->load_options_size / 2; /* UTF-16 */
 	const u16 *options = image->load_options;
+	u16 *extra_args;
 	int cmd_line_bytes = 0;  /* UTF-8 bytes */
 	int options_chars = 0;  /* UTF-16 chars */
+	int extra_args_chars = 0;  /* UTF-16 chars */
 	efi_status_t status;
+	unsigned long extra_args_size;
 	u16 zero = 0;
 
 	if (options)
@@ -813,6 +846,19 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
 		options = &zero;
 	}
 
+	extra_args = get_extra_args(sys_table_arg, &extra_args_size);
+	if (extra_args) {
+		cmd_line_bytes += 1 + count_utf8_bytes(extra_args,
+						       extra_args_size / 2,
+						       &extra_args_chars);
+
+		pr_efi(sys_table_arg,
+		       "Appending contents of 'LinuxExtraArgs' UEFI variable to kernel command line.\n");
+	} else if (extra_args_size > 0) {
+		pr_efi_err(sys_table_arg,
+			   "Failed to read 'LinuxExtraArgs' UEFI variable\n");
+	}
+
 	cmd_line_bytes++;	/* NUL termination */
 
 	status = efi_high_alloc(sys_table_arg, cmd_line_bytes, 0,
@@ -821,6 +867,11 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
 		return NULL;
 
 	s1 = efi_utf16_to_utf8((u8 *)cmdline_addr, options, options_chars);
+	if (extra_args) {
+		*s1++ = ' ';
+		s1 = efi_utf16_to_utf8(s1, extra_args, extra_args_chars);
+		efi_call_early(free_pool, extra_args);
+	}
 	*s1 = '\0';
 
 	*cmd_line_len = cmd_line_bytes;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 56add823f190..c0902384fa13 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -672,6 +672,7 @@ void efi_native_runtime_setup(void);
 #define LINUX_EFI_LOADER_ENTRY_GUID		EFI_GUID(0x4a67b082, 0x0a4c, 0x41cf,  0xb6, 0xc7, 0x44, 0x0b, 0x29, 0xbb, 0x8c, 0x4f)
 #define LINUX_EFI_RANDOM_SEED_TABLE_GUID	EFI_GUID(0x1ce1e5bc, 0x7ceb, 0x42f2,  0x81, 0xe5, 0x8a, 0xad, 0xf1, 0x80, 0xf5, 0x7b)
 #define LINUX_EFI_TPM_EVENT_LOG_GUID		EFI_GUID(0xb7799cb0, 0xeca2, 0x4943,  0x96, 0x67, 0x1f, 0xae, 0x07, 0xb7, 0x47, 0xfa)
+#define LINUX_EFI_EXTRA_ARGS_GUID		EFI_GUID(0x7cae4e6a, 0x08d7, 0x4079,  0x8e, 0xcd, 0x8c, 0x2e, 0xf4, 0x72, 0x30, 0x40)
 
 typedef struct {
 	efi_guid_t guid;
-- 
2.17.1

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Will Deacon]

Hi Ard,

On Wed, Jul 04, 2018 at 05:49:17PM +0200, Ard Biesheuvel wrote:
> As noted by Ian, many DMI based quirks for x86 ACPI machines disable
> features that can also be disabled via the kernel command line. Similarly,
> a quirk is under discussion for a arm64 ACPI machine [0] that explodes
> when enabling support for hardware error reporting via the ACPI HEST table.
> 
> When support for DMI tables was introduced to arm64 and ARM, the agreement
> was that they are informational only, i.e., provided to userland to describe
> the platform, not for keying off of to enable quirks in the kernel.
> 
> There are a couple of reasons for this:
> - Unlike the x86 architecture, where virtually all platforms are PC variants,
>   and the presence of ACPI and DMI tables may be assumed, the arm64 architecture
>   is much more heterogeneous, and none of UEFI, ACPI or DMI or actually mandated
>   or especially common across arm64 platforms; using DMI only makes sense for
>   working around a limited subset of platform issues that have to do with
>   firmware running on platforms that bother to implement it in the first place.
> - DMI is not initialized as early as it is on x86, and doing so is not trivial.
>   This means that even on ACPI/DMI machines, some issues may require quirks that
>   are enabled in a different way, or we need to refactor the DMI support so that
>   we can enable it as early as x86 does.
> - Using DMI tables fundamentally means quirking *after* the fact, which makes it
>   a moving target. Some quirks may require the DMI match table to be updated if
>   someone happens to change a string in the DMI tables when shipping a mostly
>   identical platform.
> 
> So instead, let's provide these platforms with the facilities required to enable
> such quirks at the platform level. Especially for UEFI systems, if upgrading
> firmware is a reasonable prerequisite for being able to upgrade to the latest
> kernel, having to run a script that sets a UEFI variable (either via the Linux
> command line of from the UEFI shell) should not be an unreasonable requirement
> either, and so we can solve part of this issue by configuring extra command line
> arguments persistenly from the firmware environment. This solves all the above
> issues in an unintrusive manner, given that the kernel command line is already
> made available extremely early in the boot, and the fact that it already permits
> a wide range of configuration options and overrides to be set, including the
> 'hest_disable=1' option that works around the issue addressed by [0].

I'm torn on this one. Whilst I strongly agree that keying off DMI tables
to detect firmware quirks is a bad idea on arm64, silently extending the
kernel command-line also has its downsides. The command-line provides ways
to override kernel defaults, so if a user has forced a feature on/off,
then I think this should take precedence over quirks and we should taint
instead, rather than silently override the option.

I'd be interested in other opinions on this.

Will

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Ard Biesheuvel]

> On 12 Jul 2018, at 19:01, Will Deacon <will.deacon@arm.com> wrote:
> 
> Hi Ard,
> 
>> On Wed, Jul 04, 2018 at 05:49:17PM +0200, Ard Biesheuvel wrote:
>> As noted by Ian, many DMI based quirks for x86 ACPI machines disable
>> features that can also be disabled via the kernel command line. Similarly,
>> a quirk is under discussion for a arm64 ACPI machine [0] that explodes
>> when enabling support for hardware error reporting via the ACPI HEST table.
>> 
>> When support for DMI tables was introduced to arm64 and ARM, the agreement
>> was that they are informational only, i.e., provided to userland to describe
>> the platform, not for keying off of to enable quirks in the kernel.
>> 
>> There are a couple of reasons for this:
>> - Unlike the x86 architecture, where virtually all platforms are PC variants,
>>  and the presence of ACPI and DMI tables may be assumed, the arm64 architecture
>>  is much more heterogeneous, and none of UEFI, ACPI or DMI or actually mandated
>>  or especially common across arm64 platforms; using DMI only makes sense for
>>  working around a limited subset of platform issues that have to do with
>>  firmware running on platforms that bother to implement it in the first place.
>> - DMI is not initialized as early as it is on x86, and doing so is not trivial.
>>  This means that even on ACPI/DMI machines, some issues may require quirks that
>>  are enabled in a different way, or we need to refactor the DMI support so that
>>  we can enable it as early as x86 does.
>> - Using DMI tables fundamentally means quirking *after* the fact, which makes it
>>  a moving target. Some quirks may require the DMI match table to be updated if
>>  someone happens to change a string in the DMI tables when shipping a mostly
>>  identical platform.
>> 
>> So instead, let's provide these platforms with the facilities required to enable
>> such quirks at the platform level. Especially for UEFI systems, if upgrading
>> firmware is a reasonable prerequisite for being able to upgrade to the latest
>> kernel, having to run a script that sets a UEFI variable (either via the Linux
>> command line of from the UEFI shell) should not be an unreasonable requirement
>> either, and so we can solve part of this issue by configuring extra command line
>> arguments persistenly from the firmware environment. This solves all the above
>> issues in an unintrusive manner, given that the kernel command line is already
>> made available extremely early in the boot, and the fact that it already permits
>> a wide range of configuration options and overrides to be set, including the
>> 'hest_disable=1' option that works around the issue addressed by [0].
> 
> I'm torn on this one. Whilst I strongly agree that keying off DMI tables
> to detect firmware quirks is a bad idea on arm64, silently extending the
> kernel command-line also has its downsides. The command-line provides ways
> to override kernel defaults, so if a user has forced a feature on/off,
> then I think this should take precedence over quirks and we should taint
> instead, rather than silently override the option.
> 

Isn’t that just a matter of putting the contents of LinuxExtraArgs first on the kernel command line?

> I'd be interested in other opinions on this.

+1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Ian Campbell]

On Thu, 2018-07-12 at 18:01 +0100, Will Deacon wrote:
On Wed, Jul 04, 2018 at 05:49:17PM +0200, Ard Biesheuvel wrote:
> >                                    Especially for UEFI systems, if upgrading
> > firmware is a reasonable prerequisite for being able to upgrade to the latest

Is it?

There's zero chance that there will be new firmware for the system in
question, but even if there were I'm not sure it should be considered a
n at all reasonable thing to ask in the general case (I know that if
x86 jumped off a cliff ARM wouldn't follow, but it certainly isn't
considered reasonable in the common case in that world).

> I'm torn on this one. Whilst I strongly agree that keying off DMI tables
> to detect firmware quirks is a bad idea on arm64, silently extending the
> kernel command-line also has its downsides. The command-line provides ways
> to override kernel defaults, so if a user has forced a feature on/off,
> then I think this should take precedence over quirks and we should taint
> instead, rather than silently override the option.
> 
> I'd be interested in other opinions on this.

FWIW it seems to me that if we aren't going to have these systems Just
work with newer kernels and instead require users to perform some sort
of reconfiguration (which seems like a great shame to me, but I'll let
that horse lie[0]) then it seems like they can just as easily add this
command line parameter via existing mechanisms (/etc/default/grub etc)
without needing to invent some new way of doing so (one which is more
obscure and perhaps harder to configure).

Likewise distros, they can chose to either require their users to make
those changes (see above), or they can choose not to support those
systems any more or to not enable the config options which expose the
issue.

Ian.

[0] OK, I will just mention that I don't believe it needs to be a DMI
quirk -- if there were some other way to detect and make things just
work then that would be great too. Now I'm done with the horse.

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Geoff Levand]

Hi Ard,

On 07/04/2018 08:49 AM, Ard Biesheuvel wrote:
>   efi/libstub: taken contents of LinuxExtraArgs UEFI variable into
>     account

To me this seems an overly complicated interface to the kernel, and
still doesn't in itself solve the problem at hand -- make the
generic distro kernel with APEI support run on m400 systems.

As for me, I'd prefer something like my original patch.  It fixes
that problem, it is simple and self contained, and is very clear
in what it does.  Also, there is a limited life.  When the time
comes an announcement is made to the mailing lists 'Linux-XYZ will
no longer support the m400 Moonshot'.  Then, when the Linux-XYZ-rc1
merge window opens a patch goes in that removes the
acpi_fixup_m400_quirks() routine.

-Geoff

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Ard Biesheuvel]

On 13 July 2018 at 00:22, Geoff Levand <geoff@infradead.org> wrote:
> Hi Ard,
>
> On 07/04/2018 08:49 AM, Ard Biesheuvel wrote:
>>   efi/libstub: taken contents of LinuxExtraArgs UEFI variable into
>>     account
>
> To me this seems an overly complicated interface to the kernel, and
> still doesn't in itself solve the problem at hand -- make the
> generic distro kernel with APEI support run on m400 systems.
>
> As for me, I'd prefer something like my original patch.  It fixes
> that problem, it is simple and self contained, and is very clear
> in what it does.  Also, there is a limited life.  When the time
> comes an announcement is made to the mailing lists 'Linux-XYZ will
> no longer support the m400 Moonshot'.  Then, when the Linux-XYZ-rc1
> merge window opens a patch goes in that removes the
> acpi_fixup_m400_quirks() routine.
>

Actually, that is a very good point. One of the issues I have with
these quirks is that the burden is on the maintainers to keep them
around forever, unless they can prove that it is no longer needed.

This is not my call to make, but I would be much less averse to this
being merged if we could agree upfront on an expiration time of, say,
2 years (or more?), after which it will be removed (unless anyone
makes a very good case for why it needs to be retained). This should
be mentioned in the kernel log as well when the quirk is triggered.

I still think we should not be using DMI, though.

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Will Deacon]

On Fri, Jul 13, 2018 at 08:15:26AM +0200, Ard Biesheuvel wrote:
> On 13 July 2018 at 00:22, Geoff Levand <geoff@infradead.org> wrote:
> > Hi Ard,
> >
> > On 07/04/2018 08:49 AM, Ard Biesheuvel wrote:
> >>   efi/libstub: taken contents of LinuxExtraArgs UEFI variable into
> >>     account
> >
> > To me this seems an overly complicated interface to the kernel, and
> > still doesn't in itself solve the problem at hand -- make the
> > generic distro kernel with APEI support run on m400 systems.
> >
> > As for me, I'd prefer something like my original patch.  It fixes
> > that problem, it is simple and self contained, and is very clear
> > in what it does.  Also, there is a limited life.  When the time
> > comes an announcement is made to the mailing lists 'Linux-XYZ will
> > no longer support the m400 Moonshot'.  Then, when the Linux-XYZ-rc1
> > merge window opens a patch goes in that removes the
> > acpi_fixup_m400_quirks() routine.
> >
> 
> Actually, that is a very good point. One of the issues I have with
> these quirks is that the burden is on the maintainers to keep them
> around forever, unless they can prove that it is no longer needed.
> 
> This is not my call to make, but I would be much less averse to this
> being merged if we could agree upfront on an expiration time of, say,
> 2 years (or more?), after which it will be removed (unless anyone
> makes a very good case for why it needs to be retained). This should
> be mentioned in the kernel log as well when the quirk is triggered.

The problem with that is it all falls apart when somebody who wasn't
involved in the initial discussion crops up after we remove the quirk to
complain that their machine broke. In such a situation, we don't have a
leg to stand on in my opinion.

Will

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Geoff Levand]

Hi Will,

On 07/13/2018 02:56 AM, Will Deacon wrote:
> On Fri, Jul 13, 2018 at 08:15:26AM +0200, Ard Biesheuvel wrote:
> 
>> This is not my call to make, but I would be much less averse to this
>> being merged if we could agree upfront on an expiration time of, say,
>> 2 years (or more?), after which it will be removed (unless anyone
>> makes a very good case for why it needs to be retained). This should
>> be mentioned in the kernel log as well when the quirk is triggered.
> 
> The problem with that is it all falls apart when somebody who wasn't
> involved in the initial discussion crops up after we remove the quirk to
> complain that their machine broke. In such a situation, we don't have a
> leg to stand on in my opinion.

That's the purpose of the announcement in advance.  It should be
several release cycles before hand to give plenty of time for feedback
and consensus.

I think it is generally accepted that if a kernel feature has no known
users and no dedicated maintainer then that code should be removed.
The powerpc celleb platform was in a similar situation.  Not so many
available, no real dedicated mainliner, etc. Support was removed [1],
and I don't know of any problems that came of it.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bf4981a00636347ddcef3fc008e4dd979380a851

-Geoff

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Geoff Levand]

Hi Will,

This m400 HEST problem is blocking me from enabling APEI support in
Debian [1], so I'd like to come to a conclusion on it.

In summary from [2], Riku and Graeme mention they still have some m400
systems in use.  Mark did some investigation and found the cause to be
generated by the m400's firmware, appearing after the EFI stub's call
to ExitBootServices.

A few solutions to the problem were posted [2], [3] & [4], but none
seemed to get overwhelming support.

Do we want to have an arm64 specific in-kernel fix for this, or should
it be left to the distro maintainers and end users to decide if and
how they want to handle it?

If an in-kernel fix is OK, what would be an acceptable solution that
could be merged?

Thanks for your input on this.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=900581 (linux: Enable Buster kernel features for newer ARM64 servers)
[2] https://patchwork.codeaurora.org/patch/547277/ (arm64/acpi: Add fixup for HPE m400 quirks)
[3] https://www.spinics.net/lists/arm-kernel/msg661799.html (disable_hest quirk on HP m400 with bad UEFI firmwware)
[4] https://www.mail-archive.com/linux-efi@vger.kernel.org/msg10212.html (efi: add contents of LinuxExtraArgs EFI var to command line)

-Geoff

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[James Morse]

Hi Geoff,

On 31/07/18 16:54, Geoff Levand wrote:
> This m400 HEST problem is blocking me from enabling APEI support in
> Debian [1], so I'd like to come to a conclusion on it.
> 
> In summary from [2], Riku and Graeme mention they still have some m400
> systems in use.  Mark did some investigation and found the cause to be
> generated by the m400's firmware, appearing after the EFI stub's call
> to ExitBootServices.

I did manage to borrow of one of these boxes to discover one additional piece of
information: this firmware bug doesn't exist in the latest released firmware.

The box was running the most recent firmware, and quotes its System ROM version as:
| U02 v1.01 (10/02/2015)

This thing doesn't have a HEST at all.


Why is the the latest? A current Moonshot component pack[0]'s release notes
quote m400 as removed, and say 'MCP 2017.06.0' was the latest with support. I
can't find the Release Notes for '2017.07.0', but '2017.04.0' is here[1], and it
contains:
| ProLiant_m400_Server_ROM_U02_2015_10_02.HPb


It looks like we've been talking about a bug in firmware that was never
released. Can anyone shed any light on where "U02 v1.10 (08/19/2016)" comes from?


> Do we want to have an arm64 specific in-kernel fix for this, or should
> it be left to the distro maintainers and end users to decide if and
> how they want to handle it?

Leave it to the distro maintainers to decide. Any user capable of installing
unofficial firmware should be able to add a string to the kernel command line.


Thanks,

James

[0]
https://support.hpe.com/hpsc/swd/public/detail?sp4ts.oid=5409784&swItemId=MTX_7ad3cb148b4a444f96470839d8&swEnvOid=4184#tab4
[1] https://support.hpe.com/hpsc/doc/public/display?docId=c05335771

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Graeme Gregory]

On Wed, Aug 01, 2018 at 11:07:17AM +0100, James Morse wrote:
> Hi Geoff,
> 
> On 31/07/18 16:54, Geoff Levand wrote:
> > This m400 HEST problem is blocking me from enabling APEI support in
> > Debian [1], so I'd like to come to a conclusion on it.
> > 
> > In summary from [2], Riku and Graeme mention they still have some m400
> > systems in use.  Mark did some investigation and found the cause to be
> > generated by the m400's firmware, appearing after the EFI stub's call
> > to ExitBootServices.
> 
> I did manage to borrow of one of these boxes to discover one additional piece of
> information: this firmware bug doesn't exist in the latest released firmware.
> 
> The box was running the most recent firmware, and quotes its System ROM version as:
> | U02 v1.01 (10/02/2015)
> 
> This thing doesn't have a HEST at all.
> 
> 
> Why is the the latest? A current Moonshot component pack[0]'s release notes
> quote m400 as removed, and say 'MCP 2017.06.0' was the latest with support. I
> can't find the Release Notes for '2017.07.0', but '2017.04.0' is here[1], and it
> contains:
> | ProLiant_m400_Server_ROM_U02_2015_10_02.HPb
> 
> 
> It looks like we've been talking about a bug in firmware that was never
> released. Can anyone shed any light on where "U02 v1.10 (08/19/2016)" comes from?
> 
> 
> > Do we want to have an arm64 specific in-kernel fix for this, or should
> > it be left to the distro maintainers and end users to decide if and
> > how they want to handle it?
> 
> Leave it to the distro maintainers to decide. Any user capable of installing
> unofficial firmware should be able to add a string to the kernel command line.
> 
> 
I think most people are running the firmware provided from HPe support
but was never put on release site. NDA prevents us talking about it
though!

Graeme

> Thanks,
> 
> James
> 
> [0]
> https://support.hpe.com/hpsc/swd/public/detail?sp4ts.oid=5409784&swItemId=MTX_7ad3cb148b4a444f96470839d8&swEnvOid=4184#tab4
> [1] https://support.hpe.com/hpsc/doc/public/display?docId=c05335771

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Geoff Levand]

Hi All,

On 08/01/2018 03:07 AM, James Morse wrote:
> I did manage to borrow of one of these boxes to discover one additional piece of
> information: this firmware bug doesn't exist in the latest released firmware.

I went through and verified the info James provided.  The last Moonshot
Component Pack to support the m400 was 2017.06.0.  Unfortunately, the
2017.06.0 Release Notes are not available to the public, and so we
cannot check the System ROM version released.  The previous Moonshot
Component Pack, 017.04.0, from two months earlier, included System ROM
v1.01 (10/02/2015).  It seems reasonable that since there had been no
System ROM update for two years, the same System ROM v1.01 was included
in the 2017.06.0 release. 

For anyone interested, a summary of m400/Moonshot info:

Moonshot Component Pack History
	https://support.hpe.com/hpsc/swd/public/detail?swItemId=MTX_2ec80e32b5d44f328d1cf9c9c7#tab-history

Moonshot Component Pack 2017.08.0 (Release Notes)
	https://support.hpe.com/hpsc/doc/public/display?docId=emr_na-a00018563en_us
	Supersedes: MCP 2017.06.0
	Components for the cartridges listed below have been removed from this
	MCP. MCP 2017.06.0 was the last MCP to contain components for the
	following cartridges: HP ProLiant m400 Server Cartridge

Moonshot Component Pack 2017.06.0 (Announcement)
	(No public Release Notes.)
	* RECOMMENDED * Moonshot Component Pack 2017.06.0
		https://support.hpe.com/hpsc/swd/public/detail?swItemId=MTX_2ec80e32b5d44f328d1cf9c9c7

Moonshot Component Pack 2017.04.0 (Release Notes)
	https://support.hpe.com/hpsc/doc/public/display?docId=c05335771
	ProLiant_m400_Server_ROM_U02_2015_10_02.HPb - System ROM for ProLiant m400 ServerCartridge 10-02-2015
	(BIOS Version: U02 v1.01 (10/02/2015))

Supported Operating Systems for Edgeline, Moonshot, and IoT GatewaySystems
	https://support.hpe.com/hpsc/doc/public/display?sp4ts.oid=null&docLocale=en_US&docId=emr_na-a00037370en_us
	ProLiant m400 Server Cartridge: Ubuntu LTS 14.04

-Geoff

Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line

[Ian Campbell]

On Fri, 2018-07-13 at 08:15 +0200, Ard Biesheuvel wrote:
> I still think we should not be using DMI, though.

Would a quirk be acceptable if keyed on something other than DMI?

Are there some other options, perhaps some property of the ACPI tables
proper? Something which can be queried from UEFI perhaps? (I'm really
grasping at straws, but maybe someone has a smart idea).

Ian.