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(-)
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.