[PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

In general, we should not present two separate (and inevitably different)
hardware descriptions to the OS, in the form of ACPI tables and a device
tree blob. For this reason, we recently added the logic to ArmVirtQemu to
only expose the ACPI 2.0 entry point if no DT binary is being passed, and
vice versa.

However, this is arguably a regression for those who rely on both
descriptions being available, even if the use cases in question are
uncommon.

So allow a secret handshake with the UEFI Shell, to set a variable that
will result in both descriptions being exposed on the next boot, if
executing in the default 'ACPI-only' mode.

  setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
 ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index efe83a383d55..ae5473a3f3f3 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -34,6 +34,8 @@ [Guids.common]
   gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
   gEarlyPL011BaseAddressGuid       = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
 
+  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
+
 [Protocols]
   gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
 
@@ -58,3 +60,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # EFI_VT_100_GUID.
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
+
+[PcdsDynamic]
+  #
+  # Whether to force the DT to be exposed to the OS, even in the presence of
+  # ACPI tables.
+  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|0x0|UINT8|0x00000003
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 4075b92aa2cb..bbb517e40242 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
+[PcdsDynamicHii]
+  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|L"ForceDt"|gArmVirtVariableGuid|0x0|0|NV,BS
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
index 8932dacabec5..4c53825d54ad 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
@@ -58,7 +58,12 @@ PlatformHasAcpiDt (
       goto Failed;
     }
 
-    return Status;
+    if (!PcdGet8 (PcdAcpiDtForceDt)) {
+      return EFI_SUCCESS;
+    }
+    DEBUG ((DEBUG_WARN,
+      "%a: ForceDt is set: installing both ACPI and DT tables\n",
+      __FUNCTION__));
   }
 
   //
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
index 4466bead57c2..5bc9ea43c05b 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
@@ -25,6 +25,7 @@ [Sources]
   PlatformHasAcpiDtDxe.c
 
 [Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
@@ -32,6 +33,7 @@ [Packages]
 [LibraryClasses]
   BaseLib
   DebugLib
+  PcdLib
   QemuFwCfgLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
@@ -40,5 +42,8 @@ [Guids]
   gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
   gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
 
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt
+
 [Depex]
   TRUE
-- 
2.9.3

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 17:19, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably different)
> hardware descriptions to the OS, in the form of ACPI tables and a device
> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
> vice versa.
> 
> However, this is arguably a regression for those who rely on both
> descriptions being available, even if the use cases in question are
> uncommon.
> 
> So allow a secret handshake with the UEFI Shell, to set a variable that
> will result in both descriptions being exposed on the next boot, if
> executing in the default 'ACPI-only' mode.
> 
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>  4 files changed, 22 insertions(+), 1 deletion(-)

Nacked-by: Laszlo Ersek <lersek@redhat.com>

This will cause everyone *at all* to set the secret handshake, and we
will be back to square one, where everyone just exposes ACPI and DT at
the same time, and delegate the decision to the guest kernel.

And then vendors will continue to ignore ACPI testing, and they will
continue shipping crap in their ACPI tables.

We might as well rip out the recent patches that implement the mechanism
and the policy for the mutual exclusion.

As Leif proved so eloquently (in the pub) in Budapest during Connect, no
OS needs both descriptions at the same time. Virt users can make up
their minds about what to expose. We (RH virt) had been worriedly
planning to make the same proposal to Leif, you, et al, and then we were
happy to see the violent agreement that ensued.

Sorry for getting political, but the kernel's unwavering preference for
DT over ACPI is political, and the recent edk2 patches only exist to
rectify that, from the firmware side. Users don't lose DT. What they do
lose is the (harmful) freedom of not choosing one over the other. That
freedom has had a terrible effect on the quality of ACPI tables shipped
with *allegedly* SBBR-compliant hardware.

Feel free to diverge from this in downstream distributions, but this
loophole has no place in upstream edk2.

NACK

Thanks
Laszlo

> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index efe83a383d55..ae5473a3f3f3 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -34,6 +34,8 @@ [Guids.common]
>    gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>    gEarlyPL011BaseAddressGuid       = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>  
> +  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
> +
>  [Protocols]
>    gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>  
> @@ -58,3 +60,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # EFI_VT_100_GUID.
>    #
>    gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
> +
> +[PcdsDynamic]
> +  #
> +  # Whether to force the DT to be exposed to the OS, even in the presence of
> +  # ACPI tables.
> +  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|0x0|UINT8|0x00000003
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 4075b92aa2cb..bbb517e40242 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>  
> +[PcdsDynamicHii]
> +  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|L"ForceDt"|gArmVirtVariableGuid|0x0|0|NV,BS
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> index 8932dacabec5..4c53825d54ad 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> @@ -58,7 +58,12 @@ PlatformHasAcpiDt (
>        goto Failed;
>      }
>  
> -    return Status;
> +    if (!PcdGet8 (PcdAcpiDtForceDt)) {
> +      return EFI_SUCCESS;
> +    }
> +    DEBUG ((DEBUG_WARN,
> +      "%a: ForceDt is set: installing both ACPI and DT tables\n",
> +      __FUNCTION__));
>    }
>  
>    //
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> index 4466bead57c2..5bc9ea43c05b 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> @@ -25,6 +25,7 @@ [Sources]
>    PlatformHasAcpiDtDxe.c
>  
>  [Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>    OvmfPkg/OvmfPkg.dec
> @@ -32,6 +33,7 @@ [Packages]
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> +  PcdLib
>    QemuFwCfgLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> @@ -40,5 +42,8 @@ [Guids]
>    gEdkiiPlatformHasAcpiGuid       ## SOMETIMES_PRODUCES ## PROTOCOL
>    gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>  
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt
> +
>  [Depex]
>    TRUE
> 

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/29/17 17:19, Ard Biesheuvel wrote:
>> In general, we should not present two separate (and inevitably different)
>> hardware descriptions to the OS, in the form of ACPI tables and a device
>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>> vice versa.
>>
>> However, this is arguably a regression for those who rely on both
>> descriptions being available, even if the use cases in question are
>> uncommon.
>>
>> So allow a secret handshake with the UEFI Shell, to set a variable that
>> will result in both descriptions being exposed on the next boot, if
>> executing in the default 'ACPI-only' mode.
>>
>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>  4 files changed, 22 insertions(+), 1 deletion(-)
>
> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>
> This will cause everyone *at all* to set the secret handshake, and we
> will be back to square one, where everyone just exposes ACPI and DT at
> the same time, and delegate the decision to the guest kernel.
>
> And then vendors will continue to ignore ACPI testing, and they will
> continue shipping crap in their ACPI tables.
>
> We might as well rip out the recent patches that implement the mechanism
> and the policy for the mutual exclusion.
>
> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
> OS needs both descriptions at the same time. Virt users can make up
> their minds about what to expose. We (RH virt) had been worriedly
> planning to make the same proposal to Leif, you, et al, and then we were
> happy to see the violent agreement that ensued.
>
> Sorry for getting political, but the kernel's unwavering preference for
> DT over ACPI is political, and the recent edk2 patches only exist to
> rectify that, from the firmware side. Users don't lose DT. What they do
> lose is the (harmful) freedom of not choosing one over the other. That
> freedom has had a terrible effect on the quality of ACPI tables shipped
> with *allegedly* SBBR-compliant hardware.
>
> Feel free to diverge from this in downstream distributions, but this
> loophole has no place in upstream edk2.
>
> NACK
>

OK, fair enough. How do you propose to handle this regression then?

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 18:02, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>> In general, we should not present two separate (and inevitably different)
>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>> vice versa.
>>>
>>> However, this is arguably a regression for those who rely on both
>>> descriptions being available, even if the use cases in question are
>>> uncommon.
>>>
>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>> will result in both descriptions being exposed on the next boot, if
>>> executing in the default 'ACPI-only' mode.
>>>
>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>
>> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>>
>> This will cause everyone *at all* to set the secret handshake, and we
>> will be back to square one, where everyone just exposes ACPI and DT at
>> the same time, and delegate the decision to the guest kernel.
>>
>> And then vendors will continue to ignore ACPI testing, and they will
>> continue shipping crap in their ACPI tables.
>>
>> We might as well rip out the recent patches that implement the mechanism
>> and the policy for the mutual exclusion.
>>
>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>> OS needs both descriptions at the same time. Virt users can make up
>> their minds about what to expose. We (RH virt) had been worriedly
>> planning to make the same proposal to Leif, you, et al, and then we were
>> happy to see the violent agreement that ensued.
>>
>> Sorry for getting political, but the kernel's unwavering preference for
>> DT over ACPI is political, and the recent edk2 patches only exist to
>> rectify that, from the firmware side. Users don't lose DT. What they do
>> lose is the (harmful) freedom of not choosing one over the other. That
>> freedom has had a terrible effect on the quality of ACPI tables shipped
>> with *allegedly* SBBR-compliant hardware.
>>
>> Feel free to diverge from this in downstream distributions, but this
>> loophole has no place in upstream edk2.
>>
>> NACK
>>
> 
> OK, fair enough. How do you propose to handle this regression then?
> 

I don't.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/29/17 18:02, Ard Biesheuvel wrote:
>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>> In general, we should not present two separate (and inevitably different)
>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>> vice versa.
>>>>
>>>> However, this is arguably a regression for those who rely on both
>>>> descriptions being available, even if the use cases in question are
>>>> uncommon.
>>>>
>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>> will result in both descriptions being exposed on the next boot, if
>>>> executing in the default 'ACPI-only' mode.
>>>>
>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> ---
>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>> This will cause everyone *at all* to set the secret handshake, and we
>>> will be back to square one, where everyone just exposes ACPI and DT at
>>> the same time, and delegate the decision to the guest kernel.
>>>
>>> And then vendors will continue to ignore ACPI testing, and they will
>>> continue shipping crap in their ACPI tables.
>>>
>>> We might as well rip out the recent patches that implement the mechanism
>>> and the policy for the mutual exclusion.
>>>
>>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>>> OS needs both descriptions at the same time. Virt users can make up
>>> their minds about what to expose. We (RH virt) had been worriedly
>>> planning to make the same proposal to Leif, you, et al, and then we were
>>> happy to see the violent agreement that ensued.
>>>
>>> Sorry for getting political, but the kernel's unwavering preference for
>>> DT over ACPI is political, and the recent edk2 patches only exist to
>>> rectify that, from the firmware side. Users don't lose DT. What they do
>>> lose is the (harmful) freedom of not choosing one over the other. That
>>> freedom has had a terrible effect on the quality of ACPI tables shipped
>>> with *allegedly* SBBR-compliant hardware.
>>>
>>> Feel free to diverge from this in downstream distributions, but this
>>> loophole has no place in upstream edk2.
>>>
>>> NACK
>>>
>>
>> OK, fair enough. How do you propose to handle this regression then?
>>
>
> I don't.

I think I am entitled to a more constructive attitude from you. I
don't care about politics, but I do care about not breaking people's
workflows. So if you insist on getting on your high horse and throw
capitalized NACKs at me, you could at least *pretend* to care about
other users than *your* downstream, and come up with some alternative.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 18:07, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>> In general, we should not present two separate (and inevitably different)
>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>> vice versa.
>>>>>
>>>>> However, this is arguably a regression for those who rely on both
>>>>> descriptions being available, even if the use cases in question are
>>>>> uncommon.
>>>>>
>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>> will result in both descriptions being exposed on the next boot, if
>>>>> executing in the default 'ACPI-only' mode.
>>>>>
>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>
>>>> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>>>>
>>>> This will cause everyone *at all* to set the secret handshake, and we
>>>> will be back to square one, where everyone just exposes ACPI and DT at
>>>> the same time, and delegate the decision to the guest kernel.
>>>>
>>>> And then vendors will continue to ignore ACPI testing, and they will
>>>> continue shipping crap in their ACPI tables.
>>>>
>>>> We might as well rip out the recent patches that implement the mechanism
>>>> and the policy for the mutual exclusion.
>>>>
>>>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>>>> OS needs both descriptions at the same time. Virt users can make up
>>>> their minds about what to expose. We (RH virt) had been worriedly
>>>> planning to make the same proposal to Leif, you, et al, and then we were
>>>> happy to see the violent agreement that ensued.
>>>>
>>>> Sorry for getting political, but the kernel's unwavering preference for
>>>> DT over ACPI is political, and the recent edk2 patches only exist to
>>>> rectify that, from the firmware side. Users don't lose DT. What they do
>>>> lose is the (harmful) freedom of not choosing one over the other. That
>>>> freedom has had a terrible effect on the quality of ACPI tables shipped
>>>> with *allegedly* SBBR-compliant hardware.
>>>>
>>>> Feel free to diverge from this in downstream distributions, but this
>>>> loophole has no place in upstream edk2.
>>>>
>>>> NACK
>>>>
>>>
>>> OK, fair enough. How do you propose to handle this regression then?
>>>
>>
>> I don't.
> 
> I think I am entitled to a more constructive attitude from you. I
> don't care about politics, but I do care about not breaking people's
> workflows. So if you insist on getting on your high horse and throw
> capitalized NACKs at me, you could at least *pretend* to care about
> other users than *your* downstream, and come up with some alternative.

Ard, I'm not on my high horse. I thought we were in agreement about
this. It was obvious (to me at least) that this policy would be
considered a regression by those who wanted both DT and ACPI in the
guest. From my side, breaking that was all intentional.

I'm not deliberately screwing with users (just because they have "niche"
needs), and normally I would fully agree with you. The problem is that
by providing an automated, upstream (centralized) opt-out from the
mutual exclusion, the ecosystem will be allowed to suffer from the same
nonchalance towards ACPI that has been the case until now. It's clear
that your well-meaning change will be immediately exploited as the
new-old default.

Do you plan to add the same loophole to the HII-enabled driver that you
recently committed as well?


Also, please don't accuse me of caring only about RH's downstream. The
following blog post wasn't authored by a Red Hat employee:

http://www.secretlab.ca/archives/151

The following document is also not Red Hat exclusive:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html

What you are arguing for is, indirectly, to relicense platform vendors
to continue ignoring ACPI, while (falsely) labeling their systems SBBR
conformant. And then any OS vendor that actually cares about SBBR
conformance (hint: it's not just Red Hat) sees brutal boot splats. That
is not your intent (obviously), but that is the (seen, experienced)
effect of providing both DT and ACPI.

My Nacked-by is not for the specific technical solution that you
proposed. The technical solution is fine. The goal is wrong. Which makes
any technical solution (original or alternative) wrong.

If you want, you can go ahead and commit this patch, adding my Nacked-by
from above. I won't get into a fight with you over this (or anything
else), but I want my conviction -- namely, that this is entirely wrong
-- clearly marked.

On the technical side:

- I think a dynamic boolean PCD would be superior, if that is possible
with HII (= directly nvvar-backed) PCDs -- absence of the variable
should map to FALSE. I'm unsure though if that were as easy to set from
the UEFI shell as a UINT8. So stick with the current data type if you
deem that superior (maybe comment on it in the commit message).

- please include <Library/PcdLib.h> in the C source, to reflect the
[LibraryClasses] update in the INF.

Peace
Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 18:01, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 29/03/17 17:40, Laszlo Ersek wrote:
>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>>>> In general, we should not present two separate (and inevitably different)
>>>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>>>> vice versa.
>>>>>>>
>>>>>>> However, this is arguably a regression for those who rely on both
>>>>>>> descriptions being available, even if the use cases in question are
>>>>>>> uncommon.
>>>>>>>
>>>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>>>> will result in both descriptions being exposed on the next boot, if
>>>>>>> executing in the default 'ACPI-only' mode.
>>>>>>>
>>>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>> ---
>>>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>
>
> [snip the policy argumentation, I only care about the technical aspects]
>
>> On the technical side:
>>
>> - I think a dynamic boolean PCD would be superior, if that is possible
>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>> should map to FALSE. I'm unsure though if that were as easy to set from
>> the UEFI shell as a UINT8. So stick with the current data type if you
>> deem that superior (maybe comment on it in the commit message).
>>
>> - please include <Library/PcdLib.h> in the C source, to reflect the
>> [LibraryClasses] update in the INF.
>
> My personal choice would be *not* to expose both tables at the same
> time, but instead to be able to shift the preference from one side or
> the other, similarly to what a menu on a bare metal system would do.
>

So to clarify, you want something sticky in the firmware settings
rather than having to use the -no-acpi command line argument to QEMU?

> Lets call the variable PreferDT (rather than ForceDT), with the
> following (exhaustive) behaviour :
>
> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==0 and ACPI present    -> ACPI
> - PreferDT==0 and DT present      -> DT
> - PreferDT==1 and ACPI+DT present -> DT
> - PreferDT==1 and ACPI present    -> ACPI
> - PreferDT==1 and DT present      -> DT
>

DT is always available, so this condenses to

> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==1 and ACPI+DT present -> DT

unless -no-acpi is set, which gives us

> - PreferDT==0 and DT present      -> DT
> - PreferDT==1 and DT present      -> DT

> It allows people with existing setups to still have something that works
> with minimal effort (still need to set this variable though).
>

For symmetry, it would make sense to call it ForceNoAcpi then, after
the command line param.

> Could people agree on something like this?
>

Works for me.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 19:07, Ard Biesheuvel wrote:
> On 29 March 2017 at 18:01, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 29/03/17 17:40, Laszlo Ersek wrote:
>>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>>>>> In general, we should not present two separate (and inevitably different)
>>>>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>>>>> vice versa.
>>>>>>>>
>>>>>>>> However, this is arguably a regression for those who rely on both
>>>>>>>> descriptions being available, even if the use cases in question are
>>>>>>>> uncommon.
>>>>>>>>
>>>>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>>>>> will result in both descriptions being exposed on the next boot, if
>>>>>>>> executing in the default 'ACPI-only' mode.
>>>>>>>>
>>>>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>>>>
>>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>>> ---
>>>>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>>
>>
>> [snip the policy argumentation, I only care about the technical aspects]
>>
>>> On the technical side:
>>>
>>> - I think a dynamic boolean PCD would be superior, if that is possible
>>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>>> should map to FALSE. I'm unsure though if that were as easy to set from
>>> the UEFI shell as a UINT8. So stick with the current data type if you
>>> deem that superior (maybe comment on it in the commit message).
>>>
>>> - please include <Library/PcdLib.h> in the C source, to reflect the
>>> [LibraryClasses] update in the INF.
>>
>> My personal choice would be *not* to expose both tables at the same
>> time, but instead to be able to shift the preference from one side or
>> the other, similarly to what a menu on a bare metal system would do.
>>
> 
> So to clarify, you want something sticky in the firmware settings
> rather than having to use the -no-acpi command line argument to QEMU?

If you *really* want to control it within the guest, on the guest
firmware level, while keeping the exclusive nature, then there are
better options for that.

Namely, a variation of your HII-exposed driver added in:

https://github.com/tianocore/edk2/commit/779cc439e881

In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
driver that exposes the same exclusive toggle as an HII checkbox. The
HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
vs EdkiiPlatformHasDeviceTreeGuid.

Note that I disagree with *combining* a HII toggle with the current
fw_cfg-based knob (that is, the -no-acpi switch). That means there are
multiple masters for the same decision, which invariably leads to confusion.

Therefore any ArmVirtQemu platform build should either offer the fw_cfg
swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
Never both.

In fact I've fully expected Ard to post such an HII driver down the
road, for the (upcoming) "showcase QEMU" virtualization platform (and
QEMU  machine type). The target environment of that platform wouldn't be
the data center (where you really want to control most things from the
host side) -- the goal would be to model physical hardware very closely,
even as far as human interaction goes.

So, let us figure out what we ultimately want (well, I know what I want,
what do you guys want)?

- For the current (data center virtualization oriented case), the
DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
- For the "showcase QEMU" case, another HII driver would be necessary.
DT and ACPI would remain exclusive, but they would be controlled
(visually, interactively) from the guest firmware.
- Both of these control methods should never be enabled in the same
firmware build, at the same time. If Ard writes the HII driver, we can
introduce a build time flag that switches between the two control
methods. In no case would it be possible for DT and ACPI to appear together.

Thanks
Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 18:23, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/29/17 19:07, Ard Biesheuvel wrote:
>> On 29 March 2017 at 18:01, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 29/03/17 17:40, Laszlo Ersek wrote:
[...]
>>>> On the technical side:
>>>>
>>>> - I think a dynamic boolean PCD would be superior, if that is possible
>>>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>>>> should map to FALSE. I'm unsure though if that were as easy to set from
>>>> the UEFI shell as a UINT8. So stick with the current data type if you
>>>> deem that superior (maybe comment on it in the commit message).
>>>>
>>>> - please include <Library/PcdLib.h> in the C source, to reflect the
>>>> [LibraryClasses] update in the INF.
>>>
>>> My personal choice would be *not* to expose both tables at the same
>>> time, but instead to be able to shift the preference from one side or
>>> the other, similarly to what a menu on a bare metal system would do.
>>>
>>
>> So to clarify, you want something sticky in the firmware settings
>> rather than having to use the -no-acpi command line argument to QEMU?
>
> If you *really* want to control it within the guest, on the guest
> firmware level, while keeping the exclusive nature, then there are
> better options for that.
>
> Namely, a variation of your HII-exposed driver added in:
>
> https://github.com/tianocore/edk2/commit/779cc439e881
>
> In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
> driver that exposes the same exclusive toggle as an HII checkbox. The
> HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
> vs EdkiiPlatformHasDeviceTreeGuid.
>
> Note that I disagree with *combining* a HII toggle with the current
> fw_cfg-based knob (that is, the -no-acpi switch). That means there are
> multiple masters for the same decision, which invariably leads to confusion.
>
> Therefore any ArmVirtQemu platform build should either offer the fw_cfg
> swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
> Never both.
>

I don't see why we couldn't have two ways to disable ACPI.

> In fact I've fully expected Ard to post such an HII driver down the
> road, for the (upcoming) "showcase QEMU" virtualization platform (and
> QEMU  machine type). The target environment of that platform wouldn't be
> the data center (where you really want to control most things from the
> host side) -- the goal would be to model physical hardware very closely,
> even as far as human interaction goes.
>
> So, let us figure out what we ultimately want (well, I know what I want,
> what do you guys want)?
>
> - For the current (data center virtualization oriented case), the
> DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
> - For the "showcase QEMU" case, another HII driver would be necessary.
> DT and ACPI would remain exclusive, but they would be controlled
> (visually, interactively) from the guest firmware.
> - Both of these control methods should never be enabled in the same
> firmware build, at the same time. If Ard writes the HII driver, we can
> introduce a build time flag that switches between the two control
> methods. In no case would it be possible for DT and ACPI to appear together.
>

Again, the aim is to recover some of the lost functionality for users
with special requirements. If the alternative implementation will not
be made available as widely, and needs to be built from source, it is
not really an improvement over the current situation.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 19:30, Ard Biesheuvel wrote:
> On 29 March 2017 at 18:23, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 03/29/17 19:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 18:01, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> On 29/03/17 17:40, Laszlo Ersek wrote:
> [...]
>>>>> On the technical side:
>>>>>
>>>>> - I think a dynamic boolean PCD would be superior, if that is possible
>>>>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>>>>> should map to FALSE. I'm unsure though if that were as easy to set from
>>>>> the UEFI shell as a UINT8. So stick with the current data type if you
>>>>> deem that superior (maybe comment on it in the commit message).
>>>>>
>>>>> - please include <Library/PcdLib.h> in the C source, to reflect the
>>>>> [LibraryClasses] update in the INF.
>>>>
>>>> My personal choice would be *not* to expose both tables at the same
>>>> time, but instead to be able to shift the preference from one side or
>>>> the other, similarly to what a menu on a bare metal system would do.
>>>>
>>>
>>> So to clarify, you want something sticky in the firmware settings
>>> rather than having to use the -no-acpi command line argument to QEMU?
>>
>> If you *really* want to control it within the guest, on the guest
>> firmware level, while keeping the exclusive nature, then there are
>> better options for that.
>>
>> Namely, a variation of your HII-exposed driver added in:
>>
>> https://github.com/tianocore/edk2/commit/779cc439e881
>>
>> In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
>> driver that exposes the same exclusive toggle as an HII checkbox. The
>> HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
>> vs EdkiiPlatformHasDeviceTreeGuid.
>>
>> Note that I disagree with *combining* a HII toggle with the current
>> fw_cfg-based knob (that is, the -no-acpi switch). That means there are
>> multiple masters for the same decision, which invariably leads to confusion.
>>
>> Therefore any ArmVirtQemu platform build should either offer the fw_cfg
>> swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
>> Never both.
>>
> 
> I don't see why we couldn't have two ways to disable ACPI.

I argue against it because it will confuse users and will lead to bug
reports we'll have to field. I'm sure you would not like two separate
config files, with two differently called options in them, for the same
daemon process. You would flip one setting, and wonder why the daemon
doesn't care.

> 
>> In fact I've fully expected Ard to post such an HII driver down the
>> road, for the (upcoming) "showcase QEMU" virtualization platform (and
>> QEMU  machine type). The target environment of that platform wouldn't be
>> the data center (where you really want to control most things from the
>> host side) -- the goal would be to model physical hardware very closely,
>> even as far as human interaction goes.
>>
>> So, let us figure out what we ultimately want (well, I know what I want,
>> what do you guys want)?
>>
>> - For the current (data center virtualization oriented case), the
>> DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
>> - For the "showcase QEMU" case, another HII driver would be necessary.
>> DT and ACPI would remain exclusive, but they would be controlled
>> (visually, interactively) from the guest firmware.
>> - Both of these control methods should never be enabled in the same
>> firmware build, at the same time. If Ard writes the HII driver, we can
>> introduce a build time flag that switches between the two control
>> methods. In no case would it be possible for DT and ACPI to appear together.
>>
> 
> Again, the aim is to recover some of the lost functionality for users
> with special requirements. If the alternative implementation will not
> be made available as widely, and needs to be built from source, it is
> not really an improvement over the current situation.
> 

I understand your point.

Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 19:01, Marc Zyngier wrote:
> On 29/03/17 17:40, Laszlo Ersek wrote:
>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>>>> In general, we should not present two separate (and inevitably different)
>>>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>>>> vice versa.
>>>>>>>
>>>>>>> However, this is arguably a regression for those who rely on both
>>>>>>> descriptions being available, even if the use cases in question are
>>>>>>> uncommon.
>>>>>>>
>>>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>>>> will result in both descriptions being exposed on the next boot, if
>>>>>>> executing in the default 'ACPI-only' mode.
>>>>>>>
>>>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>> ---
>>>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>
> 
> [snip the policy argumentation, I only care about the technical aspects]
> 
>> On the technical side:
>>
>> - I think a dynamic boolean PCD would be superior, if that is possible
>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>> should map to FALSE. I'm unsure though if that were as easy to set from
>> the UEFI shell as a UINT8. So stick with the current data type if you
>> deem that superior (maybe comment on it in the commit message).
>>
>> - please include <Library/PcdLib.h> in the C source, to reflect the
>> [LibraryClasses] update in the INF.
> 
> My personal choice would be *not* to expose both tables at the same
> time, but instead to be able to shift the preference from one side or
> the other, similarly to what a menu on a bare metal system would do.

Umm... Are we in violent agreement? This works already.

If you invoke QEMU with the normal command like, like you've always
done, you get ACPI only (right now).

If you pass the "-no-acpi" switch in addition to your normal command
line, you get DT only. This is documented in detail on the following commit:

https://github.com/tianocore/edk2/commit/110316a995ed

If you pass -no-acpi on your QEMU command line, you can perform the
whole guest kernel bisection using purely DT, without ever having to
re-launch QEMU.

> 
> Lets call the variable PreferDT (rather than ForceDT), with the
> following (exhaustive) behaviour :
> 
> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==0 and ACPI present    -> ACPI
> - PreferDT==0 and DT present      -> DT
> - PreferDT==1 and ACPI+DT present -> DT
> - PreferDT==1 and ACPI present    -> ACPI
> - PreferDT==1 and DT present      -> DT
> 
> It allows people with existing setups to still have something that works
> with minimal effort (still need to set this variable though).
> 
> Could people agree on something like this?

It's overly complex. With QEMU (and the current edk2 master state) you
already have a single (host-side) master knob for controlling the ACPI
vs. DT exposure.

Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 19:30, Marc Zyngier wrote:
> On 29/03/17 18:15, Laszlo Ersek wrote:
>> On 03/29/17 19:01, Marc Zyngier wrote:
>>> On 29/03/17 17:40, Laszlo Ersek wrote:
>>>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>>>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>>>>>> In general, we should not present two separate (and inevitably different)
>>>>>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>>>>>> vice versa.
>>>>>>>>>
>>>>>>>>> However, this is arguably a regression for those who rely on both
>>>>>>>>> descriptions being available, even if the use cases in question are
>>>>>>>>> uncommon.
>>>>>>>>>
>>>>>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>>>>>> will result in both descriptions being exposed on the next boot, if
>>>>>>>>> executing in the default 'ACPI-only' mode.
>>>>>>>>>
>>>>>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>>>>>
>>>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>>>> ---
>>>>>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>>>
>>>
>>> [snip the policy argumentation, I only care about the technical aspects]
>>>
>>>> On the technical side:
>>>>
>>>> - I think a dynamic boolean PCD would be superior, if that is possible
>>>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>>>> should map to FALSE. I'm unsure though if that were as easy to set from
>>>> the UEFI shell as a UINT8. So stick with the current data type if you
>>>> deem that superior (maybe comment on it in the commit message).
>>>>
>>>> - please include <Library/PcdLib.h> in the C source, to reflect the
>>>> [LibraryClasses] update in the INF.
>>>
>>> My personal choice would be *not* to expose both tables at the same
>>> time, but instead to be able to shift the preference from one side or
>>> the other, similarly to what a menu on a bare metal system would do.
>>
>> Umm... Are we in violent agreement? This works already.
>>
>> If you invoke QEMU with the normal command like, like you've always
>> done, you get ACPI only (right now).
>>
>> If you pass the "-no-acpi" switch in addition to your normal command
>> line, you get DT only. This is documented in detail on the following commit:
>>
>> https://github.com/tianocore/edk2/commit/110316a995ed
>>
>> If you pass -no-acpi on your QEMU command line, you can perform the
>> whole guest kernel bisection using purely DT, without ever having to
>> re-launch QEMU.
>>
>>>
>>> Lets call the variable PreferDT (rather than ForceDT), with the
>>> following (exhaustive) behaviour :
>>>
>>> - PreferDT==0 and ACPI+DT present -> ACPI
>>> - PreferDT==0 and ACPI present    -> ACPI
>>> - PreferDT==0 and DT present      -> DT
>>> - PreferDT==1 and ACPI+DT present -> DT
>>> - PreferDT==1 and ACPI present    -> ACPI
>>> - PreferDT==1 and DT present      -> DT
>>>
>>> It allows people with existing setups to still have something that works
>>> with minimal effort (still need to set this variable though).
>>>
>>> Could people agree on something like this?
>>
>> It's overly complex. With QEMU (and the current edk2 master state) you
>> already have a single (host-side) master knob for controlling the ACPI
>> vs. DT exposure.
> 
> You're missing the essential bit. I don't want a knob in QEMU. Having to
> mess with QEMU means that I can't have a uniform way of running a VM. I
> want one way of booting a VM, and let the VM pick the description it has
> been configured for.
> 
> On bare metal, I can switch UEFI from a menu entry to pick ACPI or DT.
> And that's good. I want the same freedom in a VM, at the same level.
> There is no technical reason to have a different behaviour.

Correct, you can have the exact same menu entry (which is what I've been
calling the HII checkbox) in virtual firmware, assuming someone writes
the platform DXE driver that implements this kind of policy.

As I wrote, I expected Ard to submit (at some undeterminate time in the
future) such a driver, which would replace the current policy drivers in:

- the Xen build of ArmVirtQemu, which has no access to fw_cfg, and

- the upcoming "showcase QEMU" platform, which would entirely ignore
fw_cfg for this purpose.

Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 17:40, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/29/17 18:07, Ard Biesheuvel wrote:
>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
[..]
>>>>> NACK
>>>>>
>>>>
>>>> OK, fair enough. How do you propose to handle this regression then?
>>>>
>>>
>>> I don't.
>>
>> I think I am entitled to a more constructive attitude from you. I
>> don't care about politics, but I do care about not breaking people's
>> workflows. So if you insist on getting on your high horse and throw
>> capitalized NACKs at me, you could at least *pretend* to care about
>> other users than *your* downstream, and come up with some alternative.
>
> Ard, I'm not on my high horse. I thought we were in agreement about
> this. It was obvious (to me at least) that this policy would be
> considered a regression by those who wanted both DT and ACPI in the
> guest. From my side, breaking that was all intentional.
>

Well, speaking for myself, I did not consider the need of some to have
both descriptions available. I take those needs very seriously.

> I'm not deliberately screwing with users (just because they have "niche"
> needs), and normally I would fully agree with you. The problem is that
> by providing an automated, upstream (centralized) opt-out from the
> mutual exclusion, the ecosystem will be allowed to suffer from the same
> nonchalance towards ACPI that has been the case until now. It's clear
> that your well-meaning change will be immediately exploited as the
> new-old default.
>
> Do you plan to add the same loophole to the HII-enabled driver that you
> recently committed as well?
>

No. That is mostly intended for new users (such as the Marvell
platform), though, which is why I only proposed to add it to TC2
(which is 32-bit so it does not use ACPI to begin with) and FVP (which
serves as a reference implementation for us.) Whether Juno can be
ported as well remains to be seen, but I would prefer to make ACPI and
DT mutually exclusive on that platform as well.

>
> Also, please don't accuse me of caring only about RH's downstream. The
> following blog post wasn't authored by a Red Hat employee:
>
> http://www.secretlab.ca/archives/151
>
> The following document is also not Red Hat exclusive:
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html
>
> What you are arguing for is, indirectly, to relicense platform vendors
> to continue ignoring ACPI, while (falsely) labeling their systems SBBR
> conformant. And then any OS vendor that actually cares about SBBR
> conformance (hint: it's not just Red Hat) sees brutal boot splats. That
> is not your intent (obviously), but that is the (seen, experienced)
> effect of providing both DT and ACPI.
>

Call me naive, but I really don't see how this change is going to
bring about all of that.

I understand that RedHat intends to start complaining noisily if ACPI
and DT are both exposed, and that having this wrong behavior in the
bundled VM firmware is undesirable, but that does not mean the world
is going to end for everybody if there is a manual override.

> My Nacked-by is not for the specific technical solution that you
> proposed. The technical solution is fine. The goal is wrong. Which makes
> any technical solution (original or alternative) wrong.
>
> If you want, you can go ahead and commit this patch, adding my Nacked-by
> from above. I won't get into a fight with you over this (or anything
> else), but I want my conviction -- namely, that this is entirely wrong
> -- clearly marked.
>

I'd rather have a solution that we can agree on.

> On the technical side:
>
> - I think a dynamic boolean PCD would be superior, if that is possible
> with HII (= directly nvvar-backed) PCDs -- absence of the variable
> should map to FALSE. I'm unsure though if that were as easy to set from
> the UEFI shell as a UINT8. So stick with the current data type if you
> deem that superior (maybe comment on it in the commit message).
>
> - please include <Library/PcdLib.h> in the C source, to reflect the
> [LibraryClasses] update in the INF.
>

Much appreciated. I will keep this in mind.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 18:16, Marc Zyngier wrote:
> On 29/03/17 17:03, Laszlo Ersek wrote:
>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>> In general, we should not present two separate (and inevitably different)
>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>> vice versa.
>>>>>
>>>>> However, this is arguably a regression for those who rely on both
>>>>> descriptions being available, even if the use cases in question are
>>>>> uncommon.
>>>>>
>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>> will result in both descriptions being exposed on the next boot, if
>>>>> executing in the default 'ACPI-only' mode.
>>>>>
>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>
>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>
>>>> Nacked-by: Laszlo Ersek <lersek@redhat.com>
>>>>
>>>> This will cause everyone *at all* to set the secret handshake, and we
>>>> will be back to square one, where everyone just exposes ACPI and DT at
>>>> the same time, and delegate the decision to the guest kernel.
>>>>
>>>> And then vendors will continue to ignore ACPI testing, and they will
>>>> continue shipping crap in their ACPI tables.
>>>>
>>>> We might as well rip out the recent patches that implement the mechanism
>>>> and the policy for the mutual exclusion.
>>>>
>>>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>>>> OS needs both descriptions at the same time. Virt users can make up
>>>> their minds about what to expose. We (RH virt) had been worriedly
>>>> planning to make the same proposal to Leif, you, et al, and then we were
>>>> happy to see the violent agreement that ensued.
>>>>
>>>> Sorry for getting political, but the kernel's unwavering preference for
>>>> DT over ACPI is political, and the recent edk2 patches only exist to
>>>> rectify that, from the firmware side. Users don't lose DT. What they do
>>>> lose is the (harmful) freedom of not choosing one over the other. That
>>>> freedom has had a terrible effect on the quality of ACPI tables shipped
>>>> with *allegedly* SBBR-compliant hardware.
>>>>
>>>> Feel free to diverge from this in downstream distributions, but this
>>>> loophole has no place in upstream edk2.
>>>>
>>>> NACK
>>>>
>>>
>>> OK, fair enough. How do you propose to handle this regression then?
>>>
>>
>> I don't.
> 
> Well, we then have an issue here. As a user of QEMU+UEFI, I expect my
> existing VM images to continue to work (mostly because I'm trying to
> track regression in KVM itself). And some are pretty old (circa 4.2). I
> upgrade my "firmware" (which is QEMU+UEFI), and my VM doesn't boot
> anymore, because someone decided that ACPI was better for my soul.

I understand. In my opinion, this can be considered an advanced enough
use case that justifies rebuilding the firmware on your side. The same
way I rebuild both QEMU and (occasionally) the kernel when I try to
track down something related to, or exposed by, the guest firmware.

(
Case in point:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg440255.html

To me it seems like a recent vhost regression in the host kernel, and
I'd already be bisecting the kernel if I had hardware in my home where
the issue reproduces. But, I digress; a team mate is doing that now.
)

> I'm sorry, but I don't think that's the right thing to do. If RH decides
> to mandate ACPI VMs, that's RH's call. You can eviscerate DT support
> from your build of QEMU and UEFI, and I'm fine with it. Imposing this on
> all users with existing setups is just wrong.

What is wrong is the *practical* effect of the freedom that ACPI+DT
together provide. It undermines SBBR conformance in the wild. If people
*can* not care, they *will* not care. And SBBR is not just an RH preference.

(Sorry about the over-use of bold -- I'm not excited, just trying to get
the emphases over the wire.)

> I appreciate you may not care, but I had to say it.

I appreciate that you said it!

Thanks,
Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Ard Biesheuvel]

On 29 March 2017 at 17:09, Jon Masters <jcm@redhat.com> wrote:
> Thanks Laszlo. A quick note from me that regardless of this discussion I will be pushing to ensure the version Red Hat ships makes ACPI the default with it being extremely painful to use DT. It is time the ecosystem got with the program we all agreed upon and there will be no more excuses.
>

This has *nothing* to do with the ecosystem. This has to do with
existing users of ArmVirtQemu (admittedly, a small fraction) that will
be forced to compile their UEFI images from source because we made a
backwards incompatible change.

I am 100% on board with making ACPI and DT mutually exclusive. But I
don't believe for a second that 'everybody just exposes ACPI and DT at
the same time' if this gets merged. That happens because people are
clueless, not because they are deliberately spending time and effort
on producing two hardware descriptions.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 18:17, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:09, Jon Masters <jcm@redhat.com> wrote:
>> Thanks Laszlo. A quick note from me that regardless of this discussion I will be pushing to ensure the version Red Hat ships makes ACPI the default with it being extremely painful to use DT. It is time the ecosystem got with the program we all agreed upon and there will be no more excuses.
>>
> 
> This has *nothing* to do with the ecosystem. This has to do with
> existing users of ArmVirtQemu (admittedly, a small fraction) that will
> be forced to compile their UEFI images from source because we made a
> backwards incompatible change.
> 
> I am 100% on board with making ACPI and DT mutually exclusive. But I
> don't believe for a second that 'everybody just exposes ACPI and DT at
> the same time' if this gets merged.

That's where we disagree, 100%.

> That happens because people are
> clueless, not because they are deliberately spending time and effort
> on producing two hardware descriptions.

If this were true, then the kernel's preference would have been changed
to ACPI aeons ago (assuming both DT and ACPI were present). ACPI is
superior to DT (cue again Grant Likely's blog post), yet kernel people
resist it. That's not cluelessness. If the kernel's DT camp has any
influence on platform vendors (and that's rather more a "given that"
than an "if"), when they find out about this loophole, I expect them to
actively recommend it as a way to perpetuate the status quo.

IMO, with this patch you are eviscerating the work we've been doing in
the last few weeks. Well, politics is nasty.

Laszlo

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Mark Rutland]

Hi,

On Wed, Mar 29, 2017 at 06:55:26PM +0200, Laszlo Ersek wrote:
> On 03/29/17 18:17, Ard Biesheuvel wrote:
> > On 29 March 2017 at 17:09, Jon Masters <jcm@redhat.com> wrote:
> >> Thanks Laszlo. A quick note from me that regardless of this
> >> discussion I will be pushing to ensure the version Red Hat ships
> >> makes ACPI the default with it being extremely painful to use DT.
> >> It is time the ecosystem got with the program we all agreed upon
> >> and there will be no more excuses.
> > 
> > This has *nothing* to do with the ecosystem. This has to do with
> > existing users of ArmVirtQemu (admittedly, a small fraction) that will
> > be forced to compile their UEFI images from source because we made a
> > backwards incompatible change.
> > 
> > I am 100% on board with making ACPI and DT mutually exclusive. But I
> > don't believe for a second that 'everybody just exposes ACPI and DT at
> > the same time' if this gets merged.
> 
> That's where we disagree, 100%.
> 
> > That happens because people are clueless, not because they are
> > deliberately spending time and effort on producing two hardware
> > descriptions.
> 
> If this were true, then the kernel's preference would have been changed
> to ACPI aeons ago (assuming both DT and ACPI were present).

As has been covered elsewhere, unilaterally changing the default breaks
existing systems, regardless of what vendors do from now on.

> ACPI is superior to DT (cue again Grant Likely's blog post), yet
> kernel people resist it. 

In several respects, sure, though let's not be under the illusion that
it is not free of novel technical problems.

The big deal with ACPI is that there are other major OS vendors who will
support ACPI, but not DT. To avoid a fragmented market, commodity
servers must use ACPI.

> That's not cluelessness. If the kernel's DT camp has any
> influence on platform vendors (and that's rather more a "given that"
> than an "if"), when they find out about this loophole, I expect them to
> actively recommend it as a way to perpetuate the status quo.

Who is this "DT camp"?

As a DT binding maintainer, I would be livid were people recommending
using this in generic OS images.

Thanks,
Mark.

Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek]

On 03/29/17 19:44, Mark Rutland wrote:
> Hi,
> 
> On Wed, Mar 29, 2017 at 06:55:26PM +0200, Laszlo Ersek wrote:
>> On 03/29/17 18:17, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:09, Jon Masters <jcm@redhat.com> wrote:
>>>> Thanks Laszlo. A quick note from me that regardless of this
>>>> discussion I will be pushing to ensure the version Red Hat ships
>>>> makes ACPI the default with it being extremely painful to use DT.
>>>> It is time the ecosystem got with the program we all agreed upon
>>>> and there will be no more excuses.
>>>
>>> This has *nothing* to do with the ecosystem. This has to do with
>>> existing users of ArmVirtQemu (admittedly, a small fraction) that will
>>> be forced to compile their UEFI images from source because we made a
>>> backwards incompatible change.
>>>
>>> I am 100% on board with making ACPI and DT mutually exclusive. But I
>>> don't believe for a second that 'everybody just exposes ACPI and DT at
>>> the same time' if this gets merged.
>>
>> That's where we disagree, 100%.
>>
>>> That happens because people are clueless, not because they are
>>> deliberately spending time and effort on producing two hardware
>>> descriptions.
>>
>> If this were true, then the kernel's preference would have been changed
>> to ACPI aeons ago (assuming both DT and ACPI were present).
> 
> As has been covered elsewhere, unilaterally changing the default breaks
> existing systems, regardless of what vendors do from now on.
> 
>> ACPI is superior to DT (cue again Grant Likely's blog post), yet
>> kernel people resist it. 
> 
> In several respects, sure, though let's not be under the illusion that
> it is not free of novel technical problems.
> 
> The big deal with ACPI is that there are other major OS vendors who will
> support ACPI, but not DT. To avoid a fragmented market, commodity
> servers must use ACPI.
> 
>> That's not cluelessness. If the kernel's DT camp has any
>> influence on platform vendors (and that's rather more a "given that"
>> than an "if"), when they find out about this loophole, I expect them to
>> actively recommend it as a way to perpetuate the status quo.
> 
> Who is this "DT camp"?

I assume the people who have thus far prevented the preference
switch-over from DT to ACPI, provided a system exposed both.

I realize (as you say above) that this might render some systems
temporarily unbootable. Yes. But without that pain, no platform vendor
will *ever* be incentivized to get their ACPI tables in order. So the
idea here is to force that pain from the firmware side.

On one hand, it is a regression, yes. On the other hand, if it's not
broken (intentionally), why would anyone ever fix it? (This argument is
based on the fact that the shippers of broken ACPI tables actually label
their machines SBBR-conformant. Nominally, they *want* to support ACPI.)

> As a DT binding maintainer, I would be livid were people recommending
> using this in generic OS images.

The kernel still prefers DT over ACPI if both are present; that's just
the same mindset.

Laszlo