mbox series

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

Message ID 20180704154919.18564-1-ard.biesheuvel@linaro.org
Headers show
Series efi: add contents of LinuxExtraArgs EFI var to command line | expand

Message

Ard Biesheuvel July 4, 2018, 3:49 p.m. UTC
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

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ard Biesheuvel July 12, 2018, 5:39 p.m. UTC | #1
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geoff Levand July 12, 2018, 10:22 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Deacon July 13, 2018, 9:56 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geoff Levand July 13, 2018, 3:59 p.m. UTC | #4
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

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geoff Levand July 31, 2018, 3:54 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Morse Aug. 1, 2018, 10:07 a.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geoff Levand Aug. 6, 2018, 9:23 p.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html