| Message ID | 85047448dc1d2d3c725b6b78d5ef2a89fc81b83b.1519659254.git.jtoppins@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jonathan, On 27 February 2018 at 06:05, Jonathan Toppins <jtoppins@redhat.com> wrote: > This patch allows a user to configure ACPI to be preferred over > device-tree. > This comes up once a year or so, and the consensus has been so far that it is not up to the kernel to reason about whether DT or ACPI should be preferred if both are supplied, Instead, it is up to the firmware to ensure that only one of those is provided, and we added a generic driver to EDK2 that implements this. > Currently for ACPI to be used a user either has to set acpi=on on the > kernel command line or make sure any device tree passed to the kernel > is empty. If the dtb passed to the kernel is non-empty then device-tree > will be chosen as the boot method of choice even if it is not correct. Your firmware is terminally broken if it provides an incorrect DT, and we should not be fixing it in the kernel. > To prevent this situation where a system is only intended to be booted > via ACPI a user can set this kernel configuration so it ignores > device-tree settings unless ACPI table checks fail. > 'only intended to be booted via ACPI' is a property of the system, not of the OS. If you need this functionality for development, you can append 'acpi=on' to the kernel command line via kconfig. > Signed-off-by: Jonathan Toppins <jtoppins@redhat.com> > --- > > arch/arm64/Kconfig | 13 +++++++++++++ > arch/arm64/kernel/acpi.c | 2 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 7381eeb7ef8e..da8d9ab62825 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1170,6 +1170,19 @@ config ARM64_ACPI_PARKING_PROTOCOL > protocol even if the corresponding data is present in the ACPI > MADT table. > > +config ARM64_PREFER_ACPI > + bool "Prefer usage of ACPI boot tables over device-tree" > + depends on ACPI > + help > + Normally device-tree is preferred over ACPI on arm64 unless > + explicitly preferred via kernel command line, something like: acpi=on > + This configuration changes this default behaviour by pretending > + the user set acpi=on on the command line. This configuration still > + allows the user to turn acpi table parsing off via acpi=off. If > + for some reason the table checks fail the system will still fall > + back to using device-tree unless the user explicitly sets acpi=force > + on the command line. > + > config CMDLINE > string "Default kernel command string" > default "" > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c > index 7b09487ff8fb..315b001c45f1 100644 > --- a/arch/arm64/kernel/acpi.c > +++ b/arch/arm64/kernel/acpi.c > @@ -44,7 +44,7 @@ int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ > EXPORT_SYMBOL(acpi_pci_disabled); > > static bool param_acpi_off __initdata; > -static bool param_acpi_on __initdata; > +static bool param_acpi_on __initdata = IS_ENABLED(CONFIG_ARM64_PREFER_ACPI); > static bool param_acpi_force __initdata; > > static int __init parse_acpi(char *arg) > -- > 2.13.6 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, On Tue, Feb 27, 2018 at 11:35 AM, Jonathan Toppins <jtoppins@redhat.com> wrote: > This patch allows a user to configure ACPI to be preferred over > device-tree. > > Currently for ACPI to be used a user either has to set acpi=on on the > kernel command line or make sure any device tree passed to the kernel > is empty. If the dtb passed to the kernel is non-empty then device-tree > will be chosen as the boot method of choice even if it is not correct. Hmm. Not sure if I correctly understand what you mean here by the term 'incorrect device tree'. Do you mean a corrupted device tree blob (but normally we can catch those cases via the device tree header) or a deliberate (or a broken) firmware attempt to pass an incorrect device tree blob to the OS. If its the later, I think the onus should be on the firmware (u-boot or UEFI or others.. ) to fix the problems. I know we carry a lot of fixes in the kernel for x86 firmware quirks but then we have several broken x86 machines on which kernel has been running since several years now. IMO, if we have a known firmware quirk (to fix the broken DTB being passed from the firmware), we can look at adding it in the early arm64 kernel code (similar to the quirk handling we do in the x86 early kernel code for broken firmware), rather than forcing ACPI as the boot method. > To prevent this situation where a system is only intended to be booted > via ACPI a user can set this kernel configuration so it ignores > device-tree settings unless ACPI table checks fail. Or, if decide to depreciate DTB as the boot method for such boards, can we look at setting 'acpi=force' in the bootagrs always to make sure that ACPI (and no fallback on DTB) is forced as the boot method for such arm64 machines. Regards, Bhupesh > Signed-off-by: Jonathan Toppins <jtoppins@redhat.com> > --- > > arch/arm64/Kconfig | 13 +++++++++++++ > arch/arm64/kernel/acpi.c | 2 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 7381eeb7ef8e..da8d9ab62825 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1170,6 +1170,19 @@ config ARM64_ACPI_PARKING_PROTOCOL > protocol even if the corresponding data is present in the ACPI > MADT table. > > +config ARM64_PREFER_ACPI > + bool "Prefer usage of ACPI boot tables over device-tree" > + depends on ACPI > + help > + Normally device-tree is preferred over ACPI on arm64 unless > + explicitly preferred via kernel command line, something like: acpi=on > + This configuration changes this default behaviour by pretending > + the user set acpi=on on the command line. This configuration still > + allows the user to turn acpi table parsing off via acpi=off. If > + for some reason the table checks fail the system will still fall > + back to using device-tree unless the user explicitly sets acpi=force > + on the command line. > + > config CMDLINE > string "Default kernel command string" > default "" > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c > index 7b09487ff8fb..315b001c45f1 100644 > --- a/arch/arm64/kernel/acpi.c > +++ b/arch/arm64/kernel/acpi.c > @@ -44,7 +44,7 @@ int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ > EXPORT_SYMBOL(acpi_pci_disabled); > > static bool param_acpi_off __initdata; > -static bool param_acpi_on __initdata; > +static bool param_acpi_on __initdata = IS_ENABLED(CONFIG_ARM64_PREFER_ACPI); > static bool param_acpi_force __initdata; > > static int __init parse_acpi(char *arg) > -- > 2.13.6 >
On 02/27/2018 02:22 AM, Ard Biesheuvel wrote: > Hi Jonathan, > > On 27 February 2018 at 06:05, Jonathan Toppins <jtoppins@redhat.com> wrote: >> This patch allows a user to configure ACPI to be preferred over >> device-tree. >> > > This comes up once a year or so, and the consensus has been so far > that it is not up to the kernel to reason about whether DT or ACPI > should be preferred if both are supplied, Instead, it is up to the > firmware to ensure that only one of those is provided, and we added a > generic driver to EDK2 that implements this. I am assuming EDK2 is some sort of firmware code base, not that I have ever heard of it before. This logic compared with the implementation in the kernel seems divergent. The kernel is clearly making a choice about which boot table (DT vs ACPI) to configure with. So to claim it is not the kernel's job to reason about which to use is clearly incorrect. The only thing this patch is doing is making it easier for distributions to configure which preference the kernel has. > >> Currently for ACPI to be used a user either has to set acpi=on on the >> kernel command line or make sure any device tree passed to the kernel >> is empty. If the dtb passed to the kernel is non-empty then device-tree >> will be chosen as the boot method of choice even if it is not correct. > > Your firmware is terminally broken if it provides an incorrect DT, and > we should not be fixing it in the kernel. This logic would seem akin to me asking Bjorn to remove all PCI quirks from the kernel, because the systems that use them are "terminally broken" or "don't follow the standard". This doesn't seem like a valid reason to deny a change that would allow the kernel to boot on more hardware. Further it seems rather rigid to effectively say that we only run on perfect firmware. > >> To prevent this situation where a system is only intended to be booted >> via ACPI a user can set this kernel configuration so it ignores >> device-tree settings unless ACPI table checks fail. >> > > 'only intended to be booted via ACPI' is a property of the system, not > of the OS. If you need this functionality for development, you can > append 'acpi=on' to the kernel command line via kconfig. This is not for development this is for production rate shipping firmware.
On 02/27/2018 07:40 AM, Bhupesh Sharma wrote: > Hi, > > On Tue, Feb 27, 2018 at 11:35 AM, Jonathan Toppins <jtoppins@redhat.com> wrote: >> This patch allows a user to configure ACPI to be preferred over >> device-tree. >> >> Currently for ACPI to be used a user either has to set acpi=on on the >> kernel command line or make sure any device tree passed to the kernel >> is empty. If the dtb passed to the kernel is non-empty then device-tree >> will be chosen as the boot method of choice even if it is not correct. > > Hmm. Not sure if I correctly understand what you mean here by the term > 'incorrect device tree'. Do you mean a corrupted device tree blob (but > normally we can catch those cases via the device tree header) or a > deliberate (or a broken) firmware attempt to pass an incorrect device > tree blob to the OS. I mean a device tree that doesn't list all devices in the SOC. So it is more incomplete than incorrect. It could also be incorrect in that it doesn't list proper timings, memory/pci/etc. > > If its the later, I think the onus should be on the firmware (u-boot > or UEFI or others.. ) to fix the problems. I know we carry a lot of > fixes in the kernel for x86 firmware quirks but then we have several > broken x86 machines on which kernel has been running since several > years now. > > IMO, if we have a known firmware quirk (to fix the broken DTB being > passed from the firmware), we can look at adding it in the early arm64 > kernel code (similar to the quirk handling we do in the x86 early > kernel code for broken firmware), rather than forcing ACPI as the boot > method. > >> To prevent this situation where a system is only intended to be booted >> via ACPI a user can set this kernel configuration so it ignores >> device-tree settings unless ACPI table checks fail. > > Or, if decide to depreciate DTB as the boot method for such boards, > can we look at setting 'acpi=force' in the bootagrs always to make > sure that ACPI (and no fallback on DTB) is forced as the boot method > for such arm64 machines. For arm64 DT is suppose to *not* be the preferred method, yet still DT is preferred if the firmware provides both tables to the kernel. So it seems reasonable to eventually have a default that effectively does acpi=force for arm64. This patch was taking the halfway approach to just have the kernel prefer acpi. I would prefer to just change to kernel bool values instead of compiling in kernel command line args, as command line args are usually left for the end user to play with. -Jon
On 27 February 2018 at 14:28, Jonathan Toppins <jtoppins@redhat.com> wrote: > On 02/27/2018 02:22 AM, Ard Biesheuvel wrote: >> Hi Jonathan, >> >> On 27 February 2018 at 06:05, Jonathan Toppins <jtoppins@redhat.com> wrote: >>> This patch allows a user to configure ACPI to be preferred over >>> device-tree. >>> >> >> This comes up once a year or so, and the consensus has been so far >> that it is not up to the kernel to reason about whether DT or ACPI >> should be preferred if both are supplied, Instead, it is up to the >> firmware to ensure that only one of those is provided, and we added a >> generic driver to EDK2 that implements this. > > I am assuming EDK2 is some sort of firmware code base, not that I have > ever heard of it before. This logic compared with the implementation in > the kernel seems divergent. The kernel is clearly making a choice about > which boot table (DT vs ACPI) to configure with. So to claim it is not > the kernel's job to reason about which to use is clearly incorrect. The arm64 ACPI kernel requires DT support to boot in the first place (due to its dependency on UEFI), and DT support predates ACPI support by a couple of years. So it is far from unreasonable that the default assumption is DT. > The > only thing this patch is doing is making it easier for distributions to > configure which preference the kernel has. > No, it does more than that: - it doubles the size of your validation matrix, because now you will have 'DT kernels' and 'ACPI kernels' - it will remove the little incentive vendors have to get with the program, adhere to the SBBR and ensure that the OS is not left to choose between DT and ACPI. >> >>> Currently for ACPI to be used a user either has to set acpi=on on the >>> kernel command line or make sure any device tree passed to the kernel >>> is empty. If the dtb passed to the kernel is non-empty then device-tree >>> will be chosen as the boot method of choice even if it is not correct. >> >> Your firmware is terminally broken if it provides an incorrect DT, and >> we should not be fixing it in the kernel. > > This logic would seem akin to me asking Bjorn to remove all PCI quirks > from the kernel, because the systems that use them are "terminally > broken" or "don't follow the standard". Do you really think PCI quirks are there to work around software bugs? > This doesn't seem like a valid > reason to deny a change that would allow the kernel to boot on more > hardware. Further it seems rather rigid to effectively say that we only > run on perfect firmware. > Could you please try to respond to what is actually being said rather than put words in my mouth? Nobody is saying we only run on perfect firmware. I am saying it should not be left to the OS to reason about which hardware description it should use. >> >>> To prevent this situation where a system is only intended to be booted >>> via ACPI a user can set this kernel configuration so it ignores >>> device-tree settings unless ACPI table checks fail. >>> >> >> 'only intended to be booted via ACPI' is a property of the system, not >> of the OS. If you need this functionality for development, you can >> append 'acpi=on' to the kernel command line via kconfig. > > This is not for development this is for production rate shipping firmware. ... that is unmaintained and exposes a broken DT alongside ACPI tables?
On 27/02/18 14:28, Jonathan Toppins wrote: >> 'only intended to be booted via ACPI' is a property of the system, not >> of the OS. If you need this functionality for development, you can >> append 'acpi=on' to the kernel command line via kconfig. > > This is not for development this is for production rate shipping firmware. Obvious question: if this is "production" firmware on systems which "do not support DT", what the hell is a non-trivial DT doing in there in the first place? Robin.
On Tue, Feb 27, 2018 at 09:44:09AM -0500, Jonathan Toppins wrote:
> For arm64 DT is suppose to *not* be the preferred method,
This has never been the upstream position.
DT has been the default since day one, and changing this would be a
regression, as has been stated several times in the past.
If *the system vendor* intends that the system only uses ACPI, they
would not provide a DT.
If *you* prefer to use ACPI, feel free to alter your kernel command
line.
Thanks,
Mark.
On Tue, Feb 27, 2018 at 8:14 PM, Jonathan Toppins <jtoppins@redhat.com> wrote: > On 02/27/2018 07:40 AM, Bhupesh Sharma wrote: >> Hi, >> >> On Tue, Feb 27, 2018 at 11:35 AM, Jonathan Toppins <jtoppins@redhat.com> wrote: >>> This patch allows a user to configure ACPI to be preferred over >>> device-tree. >>> >>> Currently for ACPI to be used a user either has to set acpi=on on the >>> kernel command line or make sure any device tree passed to the kernel >>> is empty. If the dtb passed to the kernel is non-empty then device-tree >>> will be chosen as the boot method of choice even if it is not correct. >> >> Hmm. Not sure if I correctly understand what you mean here by the term >> 'incorrect device tree'. Do you mean a corrupted device tree blob (but >> normally we can catch those cases via the device tree header) or a >> deliberate (or a broken) firmware attempt to pass an incorrect device >> tree blob to the OS. > > I mean a device tree that doesn't list all devices in the SOC. So it is > more incomplete than incorrect. It could also be incorrect in that it > doesn't list proper timings, memory/pci/etc. > >> >> If its the later, I think the onus should be on the firmware (u-boot >> or UEFI or others.. ) to fix the problems. I know we carry a lot of >> fixes in the kernel for x86 firmware quirks but then we have several >> broken x86 machines on which kernel has been running since several >> years now. >> >> IMO, if we have a known firmware quirk (to fix the broken DTB being >> passed from the firmware), we can look at adding it in the early arm64 >> kernel code (similar to the quirk handling we do in the x86 early >> kernel code for broken firmware), rather than forcing ACPI as the boot >> method. >> >>> To prevent this situation where a system is only intended to be booted >>> via ACPI a user can set this kernel configuration so it ignores >>> device-tree settings unless ACPI table checks fail. >> >> Or, if decide to depreciate DTB as the boot method for such boards, >> can we look at setting 'acpi=force' in the bootagrs always to make >> sure that ACPI (and no fallback on DTB) is forced as the boot method >> for such arm64 machines. > > For arm64 DT is suppose to *not* be the preferred method, yet still DT > is preferred if the firmware provides both tables to the kernel. That is true mainly for arm64 systems which are compliant to SBSA/SBBR specifications and are mainly targeted for server markets. However several arm64 products in embedded applications are still not SBSA/SBBR compliant (and I have worked on a couple of such implementations earlier) and still use bootloaders like u-boot (and also closed-source implementations) which have no support for ACPI currently and still rely on a DT to pass the system hardware information to the kernel. So far only open source implementation of a ACPI compliant firmware is EDK2/UEFI which supports ACPI as the preferred boot method and I am not sure if all u-boot/in-house firmware implementations are planned to be ported over to EDK2/UEFI for embedded applications. Even if they do, it would be safe to assume that to maintain backward compatibility they will still look to support boot via DT as the default boot method rather than ACPI. Regards, Bhupesh
On Wed, 2018-02-28 at 00:29 +0530, Bhupesh Sharma wrote: > On Tue, Feb 27, 2018 at 8:14 PM, Jonathan Toppins <jtoppins@redhat.com > > wrote: > > On 02/27/2018 07:40 AM, Bhupesh Sharma wrote: > > > > > For arm64 DT is suppose to *not* be the preferred method, yet still > > DT > > is preferred if the firmware provides both tables to the kernel. > However several arm64 products in embedded applications are still not > SBSA/SBBR compliant (and I have worked on a couple of such > implementations earlier) and still use bootloaders like u-boot (and > also closed-source implementations) which have no support for ACPI > currently and still rely on a DT to pass the system hardware > information to the kernel. > So far only open source implementation of a ACPI compliant firmware is > EDK2/UEFI which supports ACPI as the preferred boot method You mean for non-x86? > and I am > not sure if all u-boot/in-house firmware implementations are planned > to be ported over to EDK2/UEFI for embedded applications. Why do you need that? ACPI (if you are talking about ACPI only, w/o EFI) is supported in U-Boot for few x86 SoCs/platforms. Moreover, one of them had never been shipped with ACPI/EFI complaint services in firmware and ACPI layer is purely done in U-Boot.
On 2/28/2018 11:07 AM, Andy Shevchenko wrote: > On Wed, 2018-02-28 at 00:29 +0530, Bhupesh Sharma wrote: >> On Tue, Feb 27, 2018 at 8:14 PM, Jonathan Toppins <jtoppins@redhat.com >>> wrote: >>> On 02/27/2018 07:40 AM, Bhupesh Sharma wrote: >>> For arm64 DT is suppose to *not* be the preferred method, yet still >>> DT >>> is preferred if the firmware provides both tables to the kernel. >> However several arm64 products in embedded applications are still not >> SBSA/SBBR compliant (and I have worked on a couple of such >> implementations earlier) and still use bootloaders like u-boot (and >> also closed-source implementations) which have no support for ACPI >> currently and still rely on a DT to pass the system hardware >> information to the kernel. >> So far only open source implementation of a ACPI compliant firmware is >> EDK2/UEFI which supports ACPI as the preferred boot method > You mean for non-x86? > >> and I am >> not sure if all u-boot/in-house firmware implementations are planned >> to be ported over to EDK2/UEFI for embedded applications. > Why do you need that? ACPI (if you are talking about ACPI only, w/o EFI) > is supported in U-Boot for few x86 SoCs/platforms. Moreover, one of them > had never been shipped with ACPI/EFI complaint services in firmware and > ACPI layer is purely done in U-Boot. > Right, let alone Chromebooks. :-)
On Wed, Feb 28, 2018 at 3:37 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, 2018-02-28 at 00:29 +0530, Bhupesh Sharma wrote: >> On Tue, Feb 27, 2018 at 8:14 PM, Jonathan Toppins <jtoppins@redhat.com >> > wrote: >> > On 02/27/2018 07:40 AM, Bhupesh Sharma wrote: >> > > > >> > For arm64 DT is suppose to *not* be the preferred method, yet still >> > DT >> > is preferred if the firmware provides both tables to the kernel. > >> However several arm64 products in embedded applications are still not >> SBSA/SBBR compliant (and I have worked on a couple of such >> implementations earlier) and still use bootloaders like u-boot (and >> also closed-source implementations) which have no support for ACPI >> currently and still rely on a DT to pass the system hardware >> information to the kernel. > >> So far only open source implementation of a ACPI compliant firmware is >> EDK2/UEFI which supports ACPI as the preferred boot method > > You mean for non-x86? Yes the patch in discussion is for arm64. So my comments above are in context to arm64. >> and I am >> not sure if all u-boot/in-house firmware implementations are planned >> to be ported over to EDK2/UEFI for embedded applications. > > Why do you need that? ACPI (if you are talking about ACPI only, w/o EFI) > is supported in U-Boot for few x86 SoCs/platforms. Moreover, one of them > had never been shipped with ACPI/EFI complaint services in firmware and > ACPI layer is purely done in U-Boot. AFAIK upstream arm64 u-boot doesn't support ACPI boot method for armv8/arm64 yet. Regards, Bhupesh
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 7381eeb7ef8e..da8d9ab62825 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1170,6 +1170,19 @@ config ARM64_ACPI_PARKING_PROTOCOL protocol even if the corresponding data is present in the ACPI MADT table. +config ARM64_PREFER_ACPI + bool "Prefer usage of ACPI boot tables over device-tree" + depends on ACPI + help + Normally device-tree is preferred over ACPI on arm64 unless + explicitly preferred via kernel command line, something like: acpi=on + This configuration changes this default behaviour by pretending + the user set acpi=on on the command line. This configuration still + allows the user to turn acpi table parsing off via acpi=off. If + for some reason the table checks fail the system will still fall + back to using device-tree unless the user explicitly sets acpi=force + on the command line. + config CMDLINE string "Default kernel command string" default "" diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 7b09487ff8fb..315b001c45f1 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -44,7 +44,7 @@ int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */ EXPORT_SYMBOL(acpi_pci_disabled); static bool param_acpi_off __initdata; -static bool param_acpi_on __initdata; +static bool param_acpi_on __initdata = IS_ENABLED(CONFIG_ARM64_PREFER_ACPI); static bool param_acpi_force __initdata; static int __init parse_acpi(char *arg)
This patch allows a user to configure ACPI to be preferred over device-tree. Currently for ACPI to be used a user either has to set acpi=on on the kernel command line or make sure any device tree passed to the kernel is empty. If the dtb passed to the kernel is non-empty then device-tree will be chosen as the boot method of choice even if it is not correct. To prevent this situation where a system is only intended to be booted via ACPI a user can set this kernel configuration so it ignores device-tree settings unless ACPI table checks fail. Signed-off-by: Jonathan Toppins <jtoppins@redhat.com> --- arch/arm64/Kconfig | 13 +++++++++++++ arch/arm64/kernel/acpi.c | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-)