* [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements
@ 2019-04-12 10:19 Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 1/6] Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters Marcin Wojtas
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
Hi,
After a longer time, I was able to put my hands on finalizing the upstream for
Armada7k8k platforms. In the meantime, when I polish the PCIE support,
please check 6 various small improvements / fixes for the support.
The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/misc-upstream-r20190412
I'm looking forward to your comments or remarks.
Best regards,
Marcin
Grzegorz Jaszczyk (1):
Marvell/Armada80x0Db: Configure the CP1, comphy-2 to work with SFI 10G
Marcin Wojtas (4):
Marvell/Armada7k8k: AcpiTables: Enable edge trigger of PMU interrupt
Marvell/Armada7k8k: Implement custom DtLoaderLib
Marvell/Armada7k8k: Switch to software-based watchdog
Marvell/Armada7k8k: ArmadaSoCDescLib: Add more I2C controllers
Mark Kettenis (1):
Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 4 +-
Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc | 4 +-
Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc | 4 +-
Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 2 +-
Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 8 +-
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h | 7 +-
Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 5 +-
Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 5 +
Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc | 10 +-
12 files changed, 209 insertions(+), 15 deletions(-)
create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 1/6] Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 2/6] Marvell/Armada7k8k: AcpiTables: Enable edge trigger of PMU interrupt Marcin Wojtas
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao, Mark Kettenis
From: Mark Kettenis <kettenis@jive.eu>
Adjust bus timing parameters to make reading and updating the RTC
reliable.
This patch aligns the bus configuration to the one used by Linux.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h | 7 ++++++-
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 5 +++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
index 922f959..ee0c303 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.h
@@ -41,10 +41,15 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define RTC_IRQ_2_CONFIG_REG 0x8
#define RTC_IRQ_ALARM_EN 0x1
#define RTC_ALARM_2_REG 0x14
+#define RTC_BRIDGE_TIMING_CTRL0_REG_OFFS 0x80
#define RTC_BRIDGE_TIMING_CTRL1_REG_OFFS 0x84
#define RTC_IRQ_STATUS_REG 0x90
#define RTC_IRQ_ALARM_MASK 0x1
+#define RTC_WRITE_PERIOD_DELAY_MASK 0xFFFF
+#define RTC_WRITE_PERIOD_DELAY_DEFAULT 0x3FF
+#define RTC_WRITE_SETUP_DELAY_MASK (0xFFFF << 16)
+#define RTC_WRITE_SETUP_DELAY_DEFAULT (0x29 << 16)
#define RTC_READ_OUTPUT_DELAY_MASK 0xFFFF
-#define RTC_READ_OUTPUT_DELAY_DEFAULT 0x1F
+#define RTC_READ_OUTPUT_DELAY_DEFAULT 0x3F
#endif /* __RTCLIB_H__ */
diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
index 087bd9a..7de5ed7 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
@@ -250,6 +250,11 @@ LibRtcInitialize (
/* Update RTC-MBUS bridge timing parameters */
MmioAndThenOr32 (
+ mArmadaRtcBase + RTC_BRIDGE_TIMING_CTRL0_REG_OFFS,
+ ~(RTC_WRITE_SETUP_DELAY_MASK | RTC_WRITE_PERIOD_DELAY_MASK),
+ (RTC_WRITE_SETUP_DELAY_DEFAULT | RTC_WRITE_PERIOD_DELAY_DEFAULT)
+ );
+ MmioAndThenOr32 (
mArmadaRtcBase + RTC_BRIDGE_TIMING_CTRL1_REG_OFFS,
~RTC_READ_OUTPUT_DELAY_MASK,
RTC_READ_OUTPUT_DELAY_DEFAULT
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 2/6] Marvell/Armada7k8k: AcpiTables: Enable edge trigger of PMU interrupt
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 1/6] Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib Marcin Wojtas
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
Extend MADT GICC table flags with PERFORMANCE_INTERRUPT_MODEL,
which is responsible for configuring interrupt type in the OS.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc b/Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc
index 3dae5d3..7ab927d 100644
--- a/Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc
+++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Madt.aslc
@@ -35,6 +35,8 @@
#define PMU_INTERRUPT_CPU2 132
#define PMU_INTERRUPT_CPU3 133
+#define PMU_INTERRUPT_FLAG EFI_ACPI_6_0_GIC_ENABLED | EFI_ACPI_6_0_PERFORMANCE_INTERRUPT_MODEL
+
#pragma pack(push, 1)
typedef struct {
EFI_ACPI_6_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header;
@@ -57,7 +59,7 @@ ACPI_6_0_MADT_STRUCTURE Madt = {
EFI_ACPI_6_0_GICC_STRUCTURE_INIT(0, // GicId
0x000, // AcpiCpuUid
0x000, // Mpidr
- EFI_ACPI_6_0_GIC_ENABLED, // Flags
+ PMU_INTERRUPT_FLAG, // Flags
PMU_INTERRUPT_CPU0, // PmuIrq
GICC_BASE, // GicBase
GICV_BASE, // GicVBase
@@ -69,7 +71,7 @@ ACPI_6_0_MADT_STRUCTURE Madt = {
EFI_ACPI_6_0_GICC_STRUCTURE_INIT(1, // GicId
0x001, // AcpiCpuUid
0x001, // Mpidr
- EFI_ACPI_6_0_GIC_ENABLED, // Flags
+ PMU_INTERRUPT_FLAG, // Flags
PMU_INTERRUPT_CPU1, // PmuIrq
GICC_BASE, // GicBase
GICV_BASE, // GicVBase
@@ -81,7 +83,7 @@ ACPI_6_0_MADT_STRUCTURE Madt = {
EFI_ACPI_6_0_GICC_STRUCTURE_INIT(2, // GicId
0x100, // AcpiCpuUid
0x100, // Mpidr
- EFI_ACPI_6_0_GIC_ENABLED, // Flags
+ PMU_INTERRUPT_FLAG, // Flags
PMU_INTERRUPT_CPU2, // PmuIrq
GICC_BASE, // GicBase
GICV_BASE, // GicVBase
@@ -93,7 +95,7 @@ ACPI_6_0_MADT_STRUCTURE Madt = {
EFI_ACPI_6_0_GICC_STRUCTURE_INIT(3, // GicId
0x101, // AcpiCpuUid
0x101, // Mpidr
- EFI_ACPI_6_0_GIC_ENABLED, // Flags
+ PMU_INTERRUPT_FLAG, // Flags
PMU_INTERRUPT_CPU3, // PmuIrq
GICC_BASE, // GicBase
GICV_BASE, // GicVBase
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 1/6] Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 2/6] Marvell/Armada7k8k: AcpiTables: Enable edge trigger of PMU interrupt Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
2019-04-12 21:14 ` Ard Biesheuvel
2019-04-12 10:19 ` [edk2-platforms: PATCH 4/6] Marvell/Armada7k8k: Switch to software-based watchdog Marcin Wojtas
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
In order to provide special handling for DT environment,
implement custom version of DtLoaderLib, which registers
ExitBootServices event upon generic 'DtAcpiPref' variable.
The routine is responsible for disabling the PMU workaround,
that is valid only for UEFI and OS + APCI.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
4 files changed, 176 insertions(+), 1 deletion(-)
create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 8291582..5ed742f 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -200,7 +200,7 @@
PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
- DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
+ DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
[LibraryClasses.common.UEFI_APPLICATION]
PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
new file mode 100644
index 0000000..ec3f3a5
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
@@ -0,0 +1,43 @@
+/** @file
+*
+* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+* Copyright (c) 2018, Marvell International Ltd. and its affiliates
+*
+* This program and the accompanying materials
+* are licensed and made available under the terms and conditions of the BSD License
+* which accompanies this distribution. The full text of the license may be found at
+* http://opensource.org/licenses/bsd-license.php
+*
+* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
+ FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
+
+[Sources]
+ DxeDtPlatformDtbLoaderLib.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+ ArmSmcLib
+ BaseLib
+ DebugLib
+ DxeServicesLib
+ MemoryAllocationLib
+ UefiRuntimeServicesTableLib
+
+[Guids]
+ gDtPlatformDefaultDtbFileGuid
+ gDtPlatformFormSetGuid
diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
index 0c90f11..e5c89d9 100644
--- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
+++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
@@ -20,5 +20,7 @@
#define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
#define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
#define MV_SMC_ID_DRAM_SIZE 0x82000010
+#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
+#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
#endif //__MV_SMC_H__
diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
new file mode 100644
index 0000000..b189f47
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
@@ -0,0 +1,130 @@
+/** @file
+*
+* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+* Copyright (c) 2018, Marvell International Ltd. and its affiliates
+*
+* This program and the accompanying materials
+* are licensed and made available under the terms and conditions of the BSD License
+* which accompanies this distribution. The full text of the license may be found at
+* http://opensource.org/licenses/bsd-license.php
+*
+* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#include <PiDxe.h>
+
+#include <IndustryStandard/MvSmc.h>
+
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
+
+#define DT_ACPI_SELECT_DT 0x0
+#define DT_ACPI_SELECT_ACPI 0x1
+
+typedef struct {
+ UINT8 Pref;
+ UINT8 Reserved[3];
+} DT_ACPI_VARSTORE_DATA;
+
+/**
+ Disable extra EL3 handling of the PMU interrupts for DT world.
+
+ @param[in] Event Event structure
+ @param[in] Context Notification context
+
+**/
+STATIC
+VOID
+Armada7k8kExitBootEvent (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ ARM_SMC_ARGS SmcRegs = {0};
+
+ SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
+ ArmCallSmc (&SmcRegs);
+
+ return;
+}
+
+/**
+ Return a pool allocated copy of the DTB image that is appropriate for
+ booting the current platform via DT.
+
+ @param[out] Dtb Pointer to the DTB copy
+ @param[out] DtbSize Size of the DTB copy
+
+ @retval EFI_SUCCESS Operation completed successfully
+ @retval EFI_NOT_FOUND No suitable DTB image could be located
+ @retval EFI_OUT_OF_RESOURCES No pool memory available
+
+**/
+EFI_STATUS
+EFIAPI
+DtPlatformLoadDtb (
+ OUT VOID **Dtb,
+ OUT UINTN *DtbSize
+ )
+{
+ DT_ACPI_VARSTORE_DATA DtAcpiPref;
+ ARM_SMC_ARGS SmcRegs = {0};
+ EFI_STATUS Status;
+ VOID *OrigDtb;
+ VOID *CopyDtb;
+ UINTN OrigDtbSize;
+ UINTN BufferSize;
+
+ Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
+ EFI_SECTION_RAW,
+ 0,
+ &OrigDtb,
+ &OrigDtbSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_NOT_FOUND;
+ }
+
+ CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
+ if (CopyDtb == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ *Dtb = CopyDtb;
+ *DtbSize = OrigDtbSize;
+
+ /* Enable EL3 handler for regardless HW description */
+ SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
+ ArmCallSmc (&SmcRegs);
+
+ /*
+ * Get the current DT/ACPI preference from the DtAcpiPref variable.
+ * Register ExitBootServices event only in case the DT will be used.
+ */
+ BufferSize = sizeof (DtAcpiPref);
+ Status = gRT->GetVariable (L"DtAcpiPref",
+ &gDtPlatformFormSetGuid,
+ NULL,
+ &BufferSize,
+ &DtAcpiPref);
+ if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
+ Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
+ TPL_NOTIFY,
+ Armada7k8kExitBootEvent,
+ NULL,
+ &mArmada7k8kExitBootServicesEvent);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
+ }
+ }
+
+ return EFI_SUCCESS;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 4/6] Marvell/Armada7k8k: Switch to software-based watchdog
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
` (2 preceding siblings ...)
2019-04-12 10:19 ` [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 5/6] Marvell/Armada7k8k: ArmadaSoCDescLib: Add more I2C controllers Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 6/6] Marvell/Armada80x0Db: Configure the CP1, comphy-2 to work with SFI 10G Marcin Wojtas
5 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
Due to the hardware issue, it is better to
use generic MdeModulePkg version of the watchdog
instead of the ArmPkg driver. It prevents unwanted
resetting of the platform when spending "too long"
inside the default boot manager.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
Silicon/Marvell/Armada7k8k/Armada7k8k.fdf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 5ed742f..e8b6199 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -454,7 +454,7 @@
ArmPkg/Drivers/CpuDxe/CpuDxe.inf
ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
ArmPkg/Drivers/TimerDxe/TimerDxe.inf
- ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+ MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
# Platform Initialization
Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.inf
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
index e143517..d739020 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.fdf
@@ -114,7 +114,7 @@ FvNameGuid = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
- INF ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
+ INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 5/6] Marvell/Armada7k8k: ArmadaSoCDescLib: Add more I2C controllers
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
` (3 preceding siblings ...)
2019-04-12 10:19 ` [edk2-platforms: PATCH 4/6] Marvell/Armada7k8k: Switch to software-based watchdog Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 6/6] Marvell/Armada80x0Db: Configure the CP1, comphy-2 to work with SFI 10G Marcin Wojtas
5 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
Hitherto SoC library of Armada7k8k was missing AP and CP-MSS
I2C controllers. Fix that and update Armada70x0Db and
Armada80x0Db I2C description accordingly.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc | 4 ++--
Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc | 2 +-
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h | 8 ++++++--
Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c | 5 ++++-
4 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index e8cd177..01532b4 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -94,9 +94,9 @@
# I2C
gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x60, 0x21 }
gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x0, 0x0 }
- gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x1 }
+ gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x0, 0x1, 0x1 }
gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57 }
- gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0 }
+ gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x1, 0x1 }
gMarvellTokenSpaceGuid.PcdI2cClockFrequency|250000000
gMarvellTokenSpaceGuid.PcdI2cBaudRate|100000
gMarvellTokenSpaceGuid.PcdI2cBusCount|2
diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
index 8e8c2ba..a28e330 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
@@ -103,7 +103,7 @@
# I2C
gMarvellTokenSpaceGuid.PcdI2cSlaveAddresses|{ 0x50, 0x57, 0x50, 0x57, 0x21, 0x25 }
gMarvellTokenSpaceGuid.PcdI2cSlaveBuses|{ 0x0, 0x0, 0x1, 0x1, 0x0, 0x0 }
- gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x1, 0x0, 0x1 }
+ gMarvellTokenSpaceGuid.PcdI2cControllersEnabled|{ 0x0, 0x1, 0x0, 0x0, 0x1 }
gMarvellTokenSpaceGuid.PcdEepromI2cAddresses|{ 0x50, 0x57, 0x50, 0x57 }
gMarvellTokenSpaceGuid.PcdEepromI2cBuses|{ 0x0, 0x0, 0x1, 0x1 }
gMarvellTokenSpaceGuid.PcdI2cClockFrequency|250000000
diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
index c2d7933..8bbc5b0 100644
--- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
+++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
@@ -56,8 +56,12 @@
//
// Platform description of I2C controllers
//
-#define MV_SOC_I2C_PER_CP_COUNT 2
-#define MV_SOC_I2C_BASE(I2c) (0x701000 + ((I2c) * 0x100))
+#define MV_SOC_I2C_PER_AP_COUNT 1
+#define MV_SOC_I2C_AP_BASE (MV_SOC_AP806_BASE + 0x511000)
+#define MV_SOC_I2C_PER_CP_COUNT 3
+#define MV_SOC_I2C_BASE(I2c) ((I2c < 2) ? \
+ (0x701000 + (I2c) * 0x100) : \
+ 0x211000)
//
// Platform description of ICU (Interrupt Consolidation Unit) controllers
diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
index 584f445..355be64 100644
--- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
@@ -157,7 +157,7 @@ ArmadaSoCDescI2cGet (
CpCount = FixedPcdGet8 (PcdMaxCpCount);
- *DescCount = CpCount * MV_SOC_I2C_PER_CP_COUNT;
+ *DescCount = CpCount * MV_SOC_I2C_PER_CP_COUNT + MV_SOC_I2C_PER_AP_COUNT;
Desc = AllocateZeroPool (*DescCount * sizeof (MV_SOC_I2C_DESC));
if (Desc == NULL) {
DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
@@ -166,6 +166,9 @@ ArmadaSoCDescI2cGet (
*I2cDesc = Desc;
+ Desc->I2cBaseAddress = MV_SOC_I2C_AP_BASE;
+ Desc++;
+
for (CpIndex = 0; CpIndex < CpCount; CpIndex++) {
for (Index = 0; Index < MV_SOC_I2C_PER_CP_COUNT; Index++) {
Desc->I2cBaseAddress = MV_SOC_CP_BASE (CpIndex) + MV_SOC_I2C_BASE (Index);
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms: PATCH 6/6] Marvell/Armada80x0Db: Configure the CP1, comphy-2 to work with SFI 10G
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
` (4 preceding siblings ...)
2019-04-12 10:19 ` [edk2-platforms: PATCH 5/6] Marvell/Armada7k8k: ArmadaSoCDescLib: Add more I2C controllers Marcin Wojtas
@ 2019-04-12 10:19 ` Marcin Wojtas
5 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-12 10:19 UTC (permalink / raw)
To: devel
Cc: leif.lindholm, ard.biesheuvel, nadavh, mw, jsd, jaz, kostap,
jeremy.linton, Jici.Gao
From: Grzegorz Jaszczyk <jaz@semihalf.com>
By mistake port0 of the second PP2 controller was configured
to 5G speed, instead of 10G. Fix it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
index a28e330..c6510bb 100644
--- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
+++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
@@ -137,7 +137,7 @@
# 4: PCIE1 5 Gbps
# 5: PCIE2 5 Gbps
gMarvellTokenSpaceGuid.PcdChip1ComPhyTypes|{ $(CP_PCIE0), $(CP_SATA2), $(CP_SFI), $(CP_SATA3), $(CP_PCIE1), $(CP_PCIE2) }
- gMarvellTokenSpaceGuid.PcdChip1ComPhySpeeds|{ $(CP_5G), $(CP_5G), $(CP_5_15625G), $(CP_5G), $(CP_5G), $(CP_5G) }
+ gMarvellTokenSpaceGuid.PcdChip1ComPhySpeeds|{ $(CP_5G), $(CP_5G), $(CP_10_3125G), $(CP_5G), $(CP_5G), $(CP_5G) }
#UtmiPhy
gMarvellTokenSpaceGuid.PcdUtmiControllersEnabled|{ 0x1, 0x1, 0x1, 0x0 }
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-12 10:19 ` [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib Marcin Wojtas
@ 2019-04-12 21:14 ` Ard Biesheuvel
2019-04-15 9:52 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2019-04-12 21:14 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel-groups-io, Leif Lindholm, Ard Biesheuvel, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
>
> In order to provide special handling for DT environment,
> implement custom version of DtLoaderLib, which registers
> ExitBootServices event upon generic 'DtAcpiPref' variable.
> The routine is responsible for disabling the PMU workaround,
> that is valid only for UEFI and OS + APCI.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> 4 files changed, 176 insertions(+), 1 deletion(-)
> create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 8291582..5ed742f 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -200,7 +200,7 @@
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
>
> [LibraryClasses.common.UEFI_APPLICATION]
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> new file mode 100644
> index 0000000..ec3f3a5
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> @@ -0,0 +1,43 @@
> +/** @file
> +*
> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> +*
Please fix the date here as well as below
> +* This program and the accompanying materials
> +* are licensed and made available under the terms and conditions of the BSD License
> +* which accompanies this distribution. The full text of the license may be found at
> +* http://opensource.org/licenses/bsd-license.php
> +*
> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
> + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> +
> +[Sources]
> + DxeDtPlatformDtbLoaderLib.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdePkg/MdePkg.dec
> + Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> + ArmSmcLib
> + BaseLib
> + DebugLib
> + DxeServicesLib
> + MemoryAllocationLib
> + UefiRuntimeServicesTableLib
> +
Are you really using all of these?
> +[Guids]
> + gDtPlatformDefaultDtbFileGuid
> + gDtPlatformFormSetGuid
and these?
> diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> index 0c90f11..e5c89d9 100644
> --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> @@ -20,5 +20,7 @@
> #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> #define MV_SMC_ID_DRAM_SIZE 0x82000010
> +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
>
> #endif //__MV_SMC_H__
> diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> new file mode 100644
> index 0000000..b189f47
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> @@ -0,0 +1,130 @@
> +/** @file
> +*
> +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> +*
> +* This program and the accompanying materials
> +* are licensed and made available under the terms and conditions of the BSD License
> +* which accompanies this distribution. The full text of the license may be found at
> +* http://opensource.org/licenses/bsd-license.php
> +*
> +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#include <PiDxe.h>
> +
> +#include <IndustryStandard/MvSmc.h>
> +
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
Please make sure that all libraries you include are listed in the .inf
> +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> +
> +#define DT_ACPI_SELECT_DT 0x0
> +#define DT_ACPI_SELECT_ACPI 0x1
> +
> +typedef struct {
> + UINT8 Pref;
> + UINT8 Reserved[3];
> +} DT_ACPI_VARSTORE_DATA;
> +
> +/**
> + Disable extra EL3 handling of the PMU interrupts for DT world.
> +
> + @param[in] Event Event structure
> + @param[in] Context Notification context
> +
> +**/
> +STATIC
> +VOID
> +Armada7k8kExitBootEvent (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + ARM_SMC_ARGS SmcRegs = {0};
> +
> + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> + ArmCallSmc (&SmcRegs);
> +
> + return;
> +}
> +
> +/**
> + Return a pool allocated copy of the DTB image that is appropriate for
> + booting the current platform via DT.
> +
> + @param[out] Dtb Pointer to the DTB copy
> + @param[out] DtbSize Size of the DTB copy
> +
> + @retval EFI_SUCCESS Operation completed successfully
> + @retval EFI_NOT_FOUND No suitable DTB image could be located
> + @retval EFI_OUT_OF_RESOURCES No pool memory available
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +DtPlatformLoadDtb (
> + OUT VOID **Dtb,
> + OUT UINTN *DtbSize
> + )
> +{
> + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> + ARM_SMC_ARGS SmcRegs = {0};
> + EFI_STATUS Status;
> + VOID *OrigDtb;
> + VOID *CopyDtb;
> + UINTN OrigDtbSize;
> + UINTN BufferSize;
> +
> + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> + EFI_SECTION_RAW,
> + 0,
> + &OrigDtb,
> + &OrigDtbSize);
> + if (EFI_ERROR (Status)) {
> + return EFI_NOT_FOUND;
> + }
> +
> + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> + if (CopyDtb == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + *Dtb = CopyDtb;
> + *DtbSize = OrigDtbSize;
> +
> + /* Enable EL3 handler for regardless HW description */
> + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> + ArmCallSmc (&SmcRegs);
> +
> + /*
> + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> + * Register ExitBootServices event only in case the DT will be used.
> + */
> + BufferSize = sizeof (DtAcpiPref);
> + Status = gRT->GetVariable (L"DtAcpiPref",
> + &gDtPlatformFormSetGuid,
> + NULL,
> + &BufferSize,
> + &DtAcpiPref);
> + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> + TPL_NOTIFY,
> + Armada7k8kExitBootEvent,
> + NULL,
> + &mArmada7k8kExitBootServicesEvent);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
> --
> 2.7.4
>
Actually, looking at what you are trying to do, can you use the
gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
directly? That will be installed by the DT platform driver when
choosing ACPI mode.
It would need to be another driver, but it is more appropriate in any
case to wire this up in some platform DXE rather than the generic DTB
loader.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-12 21:14 ` Ard Biesheuvel
@ 2019-04-15 9:52 ` Marcin Wojtas
2019-04-15 19:01 ` Ard Biesheuvel
0 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-15 9:52 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
Hi Ard,
pt., 12 kwi 2019 o 23:15 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
> >
> > In order to provide special handling for DT environment,
> > implement custom version of DtLoaderLib, which registers
> > ExitBootServices event upon generic 'DtAcpiPref' variable.
> > The routine is responsible for disabling the PMU workaround,
> > that is valid only for UEFI and OS + APCI.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> > Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> > 4 files changed, 176 insertions(+), 1 deletion(-)
> > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> >
> > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > index 8291582..5ed742f 100644
> > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > @@ -200,7 +200,7 @@
> > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> > - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> >
> > [LibraryClasses.common.UEFI_APPLICATION]
> > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > new file mode 100644
> > index 0000000..ec3f3a5
> > --- /dev/null
> > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > @@ -0,0 +1,43 @@
> > +/** @file
> > +*
> > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > +*
>
> Please fix the date here as well as below
>
> > +* This program and the accompanying materials
> > +* are licensed and made available under the terms and conditions of the BSD License
> > +* which accompanies this distribution. The full text of the license may be found at
> > +* http://opensource.org/licenses/bsd-license.php
> > +*
> > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +[Defines]
> > + INF_VERSION = 0x0001001A
> > + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> > + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> > + MODULE_TYPE = DXE_DRIVER
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> > +
> > +[Sources]
> > + DxeDtPlatformDtbLoaderLib.c
> > +
> > +[Packages]
> > + ArmPkg/ArmPkg.dec
> > + EmbeddedPkg/EmbeddedPkg.dec
> > + MdePkg/MdePkg.dec
> > + Silicon/Marvell/Marvell.dec
> > +
> > +[LibraryClasses]
> > + ArmSmcLib
> > + BaseLib
> > + DebugLib
> > + DxeServicesLib
> > + MemoryAllocationLib
> > + UefiRuntimeServicesTableLib
> > +
>
> Are you really using all of these?
>
> > +[Guids]
> > + gDtPlatformDefaultDtbFileGuid
> > + gDtPlatformFormSetGuid
>
> and these?
>
> > diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > index 0c90f11..e5c89d9 100644
> > --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > @@ -20,5 +20,7 @@
> > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> > #define MV_SMC_ID_DRAM_SIZE 0x82000010
> > +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> > +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
> >
> > #endif //__MV_SMC_H__
> > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > new file mode 100644
> > index 0000000..b189f47
> > --- /dev/null
> > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > @@ -0,0 +1,130 @@
> > +/** @file
> > +*
> > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > +*
> > +* This program and the accompanying materials
> > +* are licensed and made available under the terms and conditions of the BSD License
> > +* which accompanies this distribution. The full text of the license may be found at
> > +* http://opensource.org/licenses/bsd-license.php
> > +*
> > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +*
> > +**/
> > +
> > +#include <PiDxe.h>
> > +
> > +#include <IndustryStandard/MvSmc.h>
> > +
> > +#include <Library/ArmSmcLib.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/DxeServicesLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiRuntimeServicesTableLib.h>
> > +
>
> Please make sure that all libraries you include are listed in the .inf
>
> > +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> > +
> > +#define DT_ACPI_SELECT_DT 0x0
> > +#define DT_ACPI_SELECT_ACPI 0x1
> > +
> > +typedef struct {
> > + UINT8 Pref;
> > + UINT8 Reserved[3];
> > +} DT_ACPI_VARSTORE_DATA;
> > +
> > +/**
> > + Disable extra EL3 handling of the PMU interrupts for DT world.
> > +
> > + @param[in] Event Event structure
> > + @param[in] Context Notification context
> > +
> > +**/
> > +STATIC
> > +VOID
> > +Armada7k8kExitBootEvent (
> > + IN EFI_EVENT Event,
> > + IN VOID *Context
> > + )
> > +{
> > + ARM_SMC_ARGS SmcRegs = {0};
> > +
> > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> > + ArmCallSmc (&SmcRegs);
> > +
> > + return;
> > +}
> > +
> > +/**
> > + Return a pool allocated copy of the DTB image that is appropriate for
> > + booting the current platform via DT.
> > +
> > + @param[out] Dtb Pointer to the DTB copy
> > + @param[out] DtbSize Size of the DTB copy
> > +
> > + @retval EFI_SUCCESS Operation completed successfully
> > + @retval EFI_NOT_FOUND No suitable DTB image could be located
> > + @retval EFI_OUT_OF_RESOURCES No pool memory available
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +DtPlatformLoadDtb (
> > + OUT VOID **Dtb,
> > + OUT UINTN *DtbSize
> > + )
> > +{
> > + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> > + ARM_SMC_ARGS SmcRegs = {0};
> > + EFI_STATUS Status;
> > + VOID *OrigDtb;
> > + VOID *CopyDtb;
> > + UINTN OrigDtbSize;
> > + UINTN BufferSize;
> > +
> > + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> > + EFI_SECTION_RAW,
> > + 0,
> > + &OrigDtb,
> > + &OrigDtbSize);
> > + if (EFI_ERROR (Status)) {
> > + return EFI_NOT_FOUND;
> > + }
> > +
> > + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> > + if (CopyDtb == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + *Dtb = CopyDtb;
> > + *DtbSize = OrigDtbSize;
> > +
> > + /* Enable EL3 handler for regardless HW description */
> > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> > + ArmCallSmc (&SmcRegs);
> > +
> > + /*
> > + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> > + * Register ExitBootServices event only in case the DT will be used.
> > + */
> > + BufferSize = sizeof (DtAcpiPref);
> > + Status = gRT->GetVariable (L"DtAcpiPref",
> > + &gDtPlatformFormSetGuid,
> > + NULL,
> > + &BufferSize,
> > + &DtAcpiPref);
> > + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> > + TPL_NOTIFY,
> > + Armada7k8kExitBootEvent,
> > + NULL,
> > + &mArmada7k8kExitBootServicesEvent);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> > + }
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > --
> > 2.7.4
> >
>
> Actually, looking at what you are trying to do, can you use the
> gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
> directly? That will be installed by the DT platform driver when
> choosing ACPI mode.
>
> It would need to be another driver, but it is more appropriate in any
> case to wire this up in some platform DXE rather than the generic DTB
> loader.
Ok, I'll move it to DXE. If possible, I'd prefer to use Armada's
PlatformInitDxe - would it be accepatble for you to check
gEdkiiPlatformHasAcpiGuid in ExitBootEvent?
Best regards,
Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-15 9:52 ` Marcin Wojtas
@ 2019-04-15 19:01 ` Ard Biesheuvel
2019-04-16 4:54 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2019-04-15 19:01 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
On Mon, 15 Apr 2019 at 02:52, Marcin Wojtas <mw@semihalf.com> wrote:
>
> Hi Ard,
>
>
> pt., 12 kwi 2019 o 23:15 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> >
> > On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
> > >
> > > In order to provide special handling for DT environment,
> > > implement custom version of DtLoaderLib, which registers
> > > ExitBootServices event upon generic 'DtAcpiPref' variable.
> > > The routine is responsible for disabling the PMU workaround,
> > > that is valid only for UEFI and OS + APCI.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > ---
> > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> > > Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> > > 4 files changed, 176 insertions(+), 1 deletion(-)
> > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > >
> > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > index 8291582..5ed742f 100644
> > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > @@ -200,7 +200,7 @@
> > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > > NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> > > - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > > + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > >
> > > [LibraryClasses.common.UEFI_APPLICATION]
> > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > new file mode 100644
> > > index 0000000..ec3f3a5
> > > --- /dev/null
> > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > @@ -0,0 +1,43 @@
> > > +/** @file
> > > +*
> > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > +*
> >
> > Please fix the date here as well as below
> >
> > > +* This program and the accompanying materials
> > > +* are licensed and made available under the terms and conditions of the BSD License
> > > +* which accompanies this distribution. The full text of the license may be found at
> > > +* http://opensource.org/licenses/bsd-license.php
> > > +*
> > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > +*
> > > +**/
> > > +
> > > +[Defines]
> > > + INF_VERSION = 0x0001001A
> > > + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> > > + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> > > + MODULE_TYPE = DXE_DRIVER
> > > + VERSION_STRING = 1.0
> > > + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> > > +
> > > +[Sources]
> > > + DxeDtPlatformDtbLoaderLib.c
> > > +
> > > +[Packages]
> > > + ArmPkg/ArmPkg.dec
> > > + EmbeddedPkg/EmbeddedPkg.dec
> > > + MdePkg/MdePkg.dec
> > > + Silicon/Marvell/Marvell.dec
> > > +
> > > +[LibraryClasses]
> > > + ArmSmcLib
> > > + BaseLib
> > > + DebugLib
> > > + DxeServicesLib
> > > + MemoryAllocationLib
> > > + UefiRuntimeServicesTableLib
> > > +
> >
> > Are you really using all of these?
> >
> > > +[Guids]
> > > + gDtPlatformDefaultDtbFileGuid
> > > + gDtPlatformFormSetGuid
> >
> > and these?
> >
> > > diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > index 0c90f11..e5c89d9 100644
> > > --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > @@ -20,5 +20,7 @@
> > > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> > > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> > > #define MV_SMC_ID_DRAM_SIZE 0x82000010
> > > +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> > > +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
> > >
> > > #endif //__MV_SMC_H__
> > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > new file mode 100644
> > > index 0000000..b189f47
> > > --- /dev/null
> > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > @@ -0,0 +1,130 @@
> > > +/** @file
> > > +*
> > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > +*
> > > +* This program and the accompanying materials
> > > +* are licensed and made available under the terms and conditions of the BSD License
> > > +* which accompanies this distribution. The full text of the license may be found at
> > > +* http://opensource.org/licenses/bsd-license.php
> > > +*
> > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > +*
> > > +**/
> > > +
> > > +#include <PiDxe.h>
> > > +
> > > +#include <IndustryStandard/MvSmc.h>
> > > +
> > > +#include <Library/ArmSmcLib.h>
> > > +#include <Library/BaseLib.h>
> > > +#include <Library/DebugLib.h>
> > > +#include <Library/DxeServicesLib.h>
> > > +#include <Library/MemoryAllocationLib.h>
> > > +#include <Library/UefiBootServicesTableLib.h>
> > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > +
> >
> > Please make sure that all libraries you include are listed in the .inf
> >
> > > +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> > > +
> > > +#define DT_ACPI_SELECT_DT 0x0
> > > +#define DT_ACPI_SELECT_ACPI 0x1
> > > +
> > > +typedef struct {
> > > + UINT8 Pref;
> > > + UINT8 Reserved[3];
> > > +} DT_ACPI_VARSTORE_DATA;
> > > +
> > > +/**
> > > + Disable extra EL3 handling of the PMU interrupts for DT world.
> > > +
> > > + @param[in] Event Event structure
> > > + @param[in] Context Notification context
> > > +
> > > +**/
> > > +STATIC
> > > +VOID
> > > +Armada7k8kExitBootEvent (
> > > + IN EFI_EVENT Event,
> > > + IN VOID *Context
> > > + )
> > > +{
> > > + ARM_SMC_ARGS SmcRegs = {0};
> > > +
> > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> > > + ArmCallSmc (&SmcRegs);
> > > +
> > > + return;
> > > +}
> > > +
> > > +/**
> > > + Return a pool allocated copy of the DTB image that is appropriate for
> > > + booting the current platform via DT.
> > > +
> > > + @param[out] Dtb Pointer to the DTB copy
> > > + @param[out] DtbSize Size of the DTB copy
> > > +
> > > + @retval EFI_SUCCESS Operation completed successfully
> > > + @retval EFI_NOT_FOUND No suitable DTB image could be located
> > > + @retval EFI_OUT_OF_RESOURCES No pool memory available
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +DtPlatformLoadDtb (
> > > + OUT VOID **Dtb,
> > > + OUT UINTN *DtbSize
> > > + )
> > > +{
> > > + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> > > + ARM_SMC_ARGS SmcRegs = {0};
> > > + EFI_STATUS Status;
> > > + VOID *OrigDtb;
> > > + VOID *CopyDtb;
> > > + UINTN OrigDtbSize;
> > > + UINTN BufferSize;
> > > +
> > > + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> > > + EFI_SECTION_RAW,
> > > + 0,
> > > + &OrigDtb,
> > > + &OrigDtbSize);
> > > + if (EFI_ERROR (Status)) {
> > > + return EFI_NOT_FOUND;
> > > + }
> > > +
> > > + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> > > + if (CopyDtb == NULL) {
> > > + return EFI_OUT_OF_RESOURCES;
> > > + }
> > > +
> > > + *Dtb = CopyDtb;
> > > + *DtbSize = OrigDtbSize;
> > > +
> > > + /* Enable EL3 handler for regardless HW description */
> > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> > > + ArmCallSmc (&SmcRegs);
> > > +
> > > + /*
> > > + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> > > + * Register ExitBootServices event only in case the DT will be used.
> > > + */
> > > + BufferSize = sizeof (DtAcpiPref);
> > > + Status = gRT->GetVariable (L"DtAcpiPref",
> > > + &gDtPlatformFormSetGuid,
> > > + NULL,
> > > + &BufferSize,
> > > + &DtAcpiPref);
> > > + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> > > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> > > + TPL_NOTIFY,
> > > + Armada7k8kExitBootEvent,
> > > + NULL,
> > > + &mArmada7k8kExitBootServicesEvent);
> > > + if (EFI_ERROR (Status)) {
> > > + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> > > + }
> > > + }
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > > --
> > > 2.7.4
> > >
> >
> > Actually, looking at what you are trying to do, can you use the
> > gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
> > directly? That will be installed by the DT platform driver when
> > choosing ACPI mode.
> >
> > It would need to be another driver, but it is more appropriate in any
> > case to wire this up in some platform DXE rather than the generic DTB
> > loader.
>
> Ok, I'll move it to DXE. If possible, I'd prefer to use Armada's
> PlatformInitDxe - would it be accepatble for you to check
> gEdkiiPlatformHasAcpiGuid in ExitBootEvent?
>
Yes, but using OnReadyToBoot is more idiomatic for things like this.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-15 19:01 ` Ard Biesheuvel
@ 2019-04-16 4:54 ` Marcin Wojtas
2019-04-16 5:01 ` Ard Biesheuvel
0 siblings, 1 reply; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-16 4:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
Hi Ard,
pon., 15 kwi 2019 o 21:01 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On Mon, 15 Apr 2019 at 02:52, Marcin Wojtas <mw@semihalf.com> wrote:
> >
> > Hi Ard,
> >
> >
> > pt., 12 kwi 2019 o 23:15 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> > >
> > > On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
> > > >
> > > > In order to provide special handling for DT environment,
> > > > implement custom version of DtLoaderLib, which registers
> > > > ExitBootServices event upon generic 'DtAcpiPref' variable.
> > > > The routine is responsible for disabling the PMU workaround,
> > > > that is valid only for UEFI and OS + APCI.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > > ---
> > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> > > > Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> > > > 4 files changed, 176 insertions(+), 1 deletion(-)
> > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > >
> > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > index 8291582..5ed742f 100644
> > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > @@ -200,7 +200,7 @@
> > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > > > NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> > > > - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > > > + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > >
> > > > [LibraryClasses.common.UEFI_APPLICATION]
> > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > new file mode 100644
> > > > index 0000000..ec3f3a5
> > > > --- /dev/null
> > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > @@ -0,0 +1,43 @@
> > > > +/** @file
> > > > +*
> > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > +*
> > >
> > > Please fix the date here as well as below
> > >
> > > > +* This program and the accompanying materials
> > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > +* http://opensource.org/licenses/bsd-license.php
> > > > +*
> > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > +*
> > > > +**/
> > > > +
> > > > +[Defines]
> > > > + INF_VERSION = 0x0001001A
> > > > + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> > > > + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> > > > + MODULE_TYPE = DXE_DRIVER
> > > > + VERSION_STRING = 1.0
> > > > + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> > > > +
> > > > +[Sources]
> > > > + DxeDtPlatformDtbLoaderLib.c
> > > > +
> > > > +[Packages]
> > > > + ArmPkg/ArmPkg.dec
> > > > + EmbeddedPkg/EmbeddedPkg.dec
> > > > + MdePkg/MdePkg.dec
> > > > + Silicon/Marvell/Marvell.dec
> > > > +
> > > > +[LibraryClasses]
> > > > + ArmSmcLib
> > > > + BaseLib
> > > > + DebugLib
> > > > + DxeServicesLib
> > > > + MemoryAllocationLib
> > > > + UefiRuntimeServicesTableLib
> > > > +
> > >
> > > Are you really using all of these?
> > >
> > > > +[Guids]
> > > > + gDtPlatformDefaultDtbFileGuid
> > > > + gDtPlatformFormSetGuid
> > >
> > > and these?
> > >
> > > > diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > index 0c90f11..e5c89d9 100644
> > > > --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > @@ -20,5 +20,7 @@
> > > > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> > > > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> > > > #define MV_SMC_ID_DRAM_SIZE 0x82000010
> > > > +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> > > > +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
> > > >
> > > > #endif //__MV_SMC_H__
> > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > new file mode 100644
> > > > index 0000000..b189f47
> > > > --- /dev/null
> > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > @@ -0,0 +1,130 @@
> > > > +/** @file
> > > > +*
> > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > +*
> > > > +* This program and the accompanying materials
> > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > +* http://opensource.org/licenses/bsd-license.php
> > > > +*
> > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > +*
> > > > +**/
> > > > +
> > > > +#include <PiDxe.h>
> > > > +
> > > > +#include <IndustryStandard/MvSmc.h>
> > > > +
> > > > +#include <Library/ArmSmcLib.h>
> > > > +#include <Library/BaseLib.h>
> > > > +#include <Library/DebugLib.h>
> > > > +#include <Library/DxeServicesLib.h>
> > > > +#include <Library/MemoryAllocationLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > > +
> > >
> > > Please make sure that all libraries you include are listed in the .inf
> > >
> > > > +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> > > > +
> > > > +#define DT_ACPI_SELECT_DT 0x0
> > > > +#define DT_ACPI_SELECT_ACPI 0x1
> > > > +
> > > > +typedef struct {
> > > > + UINT8 Pref;
> > > > + UINT8 Reserved[3];
> > > > +} DT_ACPI_VARSTORE_DATA;
> > > > +
> > > > +/**
> > > > + Disable extra EL3 handling of the PMU interrupts for DT world.
> > > > +
> > > > + @param[in] Event Event structure
> > > > + @param[in] Context Notification context
> > > > +
> > > > +**/
> > > > +STATIC
> > > > +VOID
> > > > +Armada7k8kExitBootEvent (
> > > > + IN EFI_EVENT Event,
> > > > + IN VOID *Context
> > > > + )
> > > > +{
> > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > +
> > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> > > > + ArmCallSmc (&SmcRegs);
> > > > +
> > > > + return;
> > > > +}
> > > > +
> > > > +/**
> > > > + Return a pool allocated copy of the DTB image that is appropriate for
> > > > + booting the current platform via DT.
> > > > +
> > > > + @param[out] Dtb Pointer to the DTB copy
> > > > + @param[out] DtbSize Size of the DTB copy
> > > > +
> > > > + @retval EFI_SUCCESS Operation completed successfully
> > > > + @retval EFI_NOT_FOUND No suitable DTB image could be located
> > > > + @retval EFI_OUT_OF_RESOURCES No pool memory available
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +DtPlatformLoadDtb (
> > > > + OUT VOID **Dtb,
> > > > + OUT UINTN *DtbSize
> > > > + )
> > > > +{
> > > > + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > + EFI_STATUS Status;
> > > > + VOID *OrigDtb;
> > > > + VOID *CopyDtb;
> > > > + UINTN OrigDtbSize;
> > > > + UINTN BufferSize;
> > > > +
> > > > + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> > > > + EFI_SECTION_RAW,
> > > > + 0,
> > > > + &OrigDtb,
> > > > + &OrigDtbSize);
> > > > + if (EFI_ERROR (Status)) {
> > > > + return EFI_NOT_FOUND;
> > > > + }
> > > > +
> > > > + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> > > > + if (CopyDtb == NULL) {
> > > > + return EFI_OUT_OF_RESOURCES;
> > > > + }
> > > > +
> > > > + *Dtb = CopyDtb;
> > > > + *DtbSize = OrigDtbSize;
> > > > +
> > > > + /* Enable EL3 handler for regardless HW description */
> > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> > > > + ArmCallSmc (&SmcRegs);
> > > > +
> > > > + /*
> > > > + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> > > > + * Register ExitBootServices event only in case the DT will be used.
> > > > + */
> > > > + BufferSize = sizeof (DtAcpiPref);
> > > > + Status = gRT->GetVariable (L"DtAcpiPref",
> > > > + &gDtPlatformFormSetGuid,
> > > > + NULL,
> > > > + &BufferSize,
> > > > + &DtAcpiPref);
> > > > + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> > > > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> > > > + TPL_NOTIFY,
> > > > + Armada7k8kExitBootEvent,
> > > > + NULL,
> > > > + &mArmada7k8kExitBootServicesEvent);
> > > > + if (EFI_ERROR (Status)) {
> > > > + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> > > > + }
> > > > + }
> > > > +
> > > > + return EFI_SUCCESS;
> > > > +}
> > > > --
> > > > 2.7.4
> > > >
> > >
> > > Actually, looking at what you are trying to do, can you use the
> > > gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
> > > directly? That will be installed by the DT platform driver when
> > > choosing ACPI mode.
> > >
> > > It would need to be another driver, but it is more appropriate in any
> > > case to wire this up in some platform DXE rather than the generic DTB
> > > loader.
> >
> > Ok, I'll move it to DXE. If possible, I'd prefer to use Armada's
> > PlatformInitDxe - would it be accepatble for you to check
> > gEdkiiPlatformHasAcpiGuid in ExitBootEvent?
> >
>
> Yes, but using OnReadyToBoot is more idiomatic for things like this.
I just checked it - unfortunately it is triggered to early. I need the
PMU irq handling as long as I'm in UEFI (or boot with ACPI). With the
gEfiEventReadyToBootGuid the event hits before I enter the shell, so
I'm moving to DXE, but stick with ExitBoot event solution.
Best regards,
Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-16 4:54 ` Marcin Wojtas
@ 2019-04-16 5:01 ` Ard Biesheuvel
2019-04-16 5:49 ` Marcin Wojtas
0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2019-04-16 5:01 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
On Mon, 15 Apr 2019 at 21:54, Marcin Wojtas <mw@semihalf.com> wrote:
>
> Hi Ard,
>
> pon., 15 kwi 2019 o 21:01 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> >
> > On Mon, 15 Apr 2019 at 02:52, Marcin Wojtas <mw@semihalf.com> wrote:
> > >
> > > Hi Ard,
> > >
> > >
> > > pt., 12 kwi 2019 o 23:15 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> > > >
> > > > On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
> > > > >
> > > > > In order to provide special handling for DT environment,
> > > > > implement custom version of DtLoaderLib, which registers
> > > > > ExitBootServices event upon generic 'DtAcpiPref' variable.
> > > > > The routine is responsible for disabling the PMU workaround,
> > > > > that is valid only for UEFI and OS + APCI.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > > > ---
> > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> > > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> > > > > Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> > > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> > > > > 4 files changed, 176 insertions(+), 1 deletion(-)
> > > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > >
> > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > index 8291582..5ed742f 100644
> > > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > @@ -200,7 +200,7 @@
> > > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > > > > NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> > > > > - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > > > > + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > >
> > > > > [LibraryClasses.common.UEFI_APPLICATION]
> > > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > new file mode 100644
> > > > > index 0000000..ec3f3a5
> > > > > --- /dev/null
> > > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > @@ -0,0 +1,43 @@
> > > > > +/** @file
> > > > > +*
> > > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > > +*
> > > >
> > > > Please fix the date here as well as below
> > > >
> > > > > +* This program and the accompanying materials
> > > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > > +* http://opensource.org/licenses/bsd-license.php
> > > > > +*
> > > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > > +*
> > > > > +**/
> > > > > +
> > > > > +[Defines]
> > > > > + INF_VERSION = 0x0001001A
> > > > > + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> > > > > + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> > > > > + MODULE_TYPE = DXE_DRIVER
> > > > > + VERSION_STRING = 1.0
> > > > > + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> > > > > +
> > > > > +[Sources]
> > > > > + DxeDtPlatformDtbLoaderLib.c
> > > > > +
> > > > > +[Packages]
> > > > > + ArmPkg/ArmPkg.dec
> > > > > + EmbeddedPkg/EmbeddedPkg.dec
> > > > > + MdePkg/MdePkg.dec
> > > > > + Silicon/Marvell/Marvell.dec
> > > > > +
> > > > > +[LibraryClasses]
> > > > > + ArmSmcLib
> > > > > + BaseLib
> > > > > + DebugLib
> > > > > + DxeServicesLib
> > > > > + MemoryAllocationLib
> > > > > + UefiRuntimeServicesTableLib
> > > > > +
> > > >
> > > > Are you really using all of these?
> > > >
> > > > > +[Guids]
> > > > > + gDtPlatformDefaultDtbFileGuid
> > > > > + gDtPlatformFormSetGuid
> > > >
> > > > and these?
> > > >
> > > > > diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > index 0c90f11..e5c89d9 100644
> > > > > --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > @@ -20,5 +20,7 @@
> > > > > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> > > > > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> > > > > #define MV_SMC_ID_DRAM_SIZE 0x82000010
> > > > > +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> > > > > +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
> > > > >
> > > > > #endif //__MV_SMC_H__
> > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > > new file mode 100644
> > > > > index 0000000..b189f47
> > > > > --- /dev/null
> > > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > > @@ -0,0 +1,130 @@
> > > > > +/** @file
> > > > > +*
> > > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > > +*
> > > > > +* This program and the accompanying materials
> > > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > > +* http://opensource.org/licenses/bsd-license.php
> > > > > +*
> > > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > > +*
> > > > > +**/
> > > > > +
> > > > > +#include <PiDxe.h>
> > > > > +
> > > > > +#include <IndustryStandard/MvSmc.h>
> > > > > +
> > > > > +#include <Library/ArmSmcLib.h>
> > > > > +#include <Library/BaseLib.h>
> > > > > +#include <Library/DebugLib.h>
> > > > > +#include <Library/DxeServicesLib.h>
> > > > > +#include <Library/MemoryAllocationLib.h>
> > > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > > > +
> > > >
> > > > Please make sure that all libraries you include are listed in the .inf
> > > >
> > > > > +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> > > > > +
> > > > > +#define DT_ACPI_SELECT_DT 0x0
> > > > > +#define DT_ACPI_SELECT_ACPI 0x1
> > > > > +
> > > > > +typedef struct {
> > > > > + UINT8 Pref;
> > > > > + UINT8 Reserved[3];
> > > > > +} DT_ACPI_VARSTORE_DATA;
> > > > > +
> > > > > +/**
> > > > > + Disable extra EL3 handling of the PMU interrupts for DT world.
> > > > > +
> > > > > + @param[in] Event Event structure
> > > > > + @param[in] Context Notification context
> > > > > +
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +Armada7k8kExitBootEvent (
> > > > > + IN EFI_EVENT Event,
> > > > > + IN VOID *Context
> > > > > + )
> > > > > +{
> > > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > > +
> > > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> > > > > + ArmCallSmc (&SmcRegs);
> > > > > +
> > > > > + return;
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + Return a pool allocated copy of the DTB image that is appropriate for
> > > > > + booting the current platform via DT.
> > > > > +
> > > > > + @param[out] Dtb Pointer to the DTB copy
> > > > > + @param[out] DtbSize Size of the DTB copy
> > > > > +
> > > > > + @retval EFI_SUCCESS Operation completed successfully
> > > > > + @retval EFI_NOT_FOUND No suitable DTB image could be located
> > > > > + @retval EFI_OUT_OF_RESOURCES No pool memory available
> > > > > +
> > > > > +**/
> > > > > +EFI_STATUS
> > > > > +EFIAPI
> > > > > +DtPlatformLoadDtb (
> > > > > + OUT VOID **Dtb,
> > > > > + OUT UINTN *DtbSize
> > > > > + )
> > > > > +{
> > > > > + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> > > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > > + EFI_STATUS Status;
> > > > > + VOID *OrigDtb;
> > > > > + VOID *CopyDtb;
> > > > > + UINTN OrigDtbSize;
> > > > > + UINTN BufferSize;
> > > > > +
> > > > > + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> > > > > + EFI_SECTION_RAW,
> > > > > + 0,
> > > > > + &OrigDtb,
> > > > > + &OrigDtbSize);
> > > > > + if (EFI_ERROR (Status)) {
> > > > > + return EFI_NOT_FOUND;
> > > > > + }
> > > > > +
> > > > > + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> > > > > + if (CopyDtb == NULL) {
> > > > > + return EFI_OUT_OF_RESOURCES;
> > > > > + }
> > > > > +
> > > > > + *Dtb = CopyDtb;
> > > > > + *DtbSize = OrigDtbSize;
> > > > > +
> > > > > + /* Enable EL3 handler for regardless HW description */
> > > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> > > > > + ArmCallSmc (&SmcRegs);
> > > > > +
> > > > > + /*
> > > > > + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> > > > > + * Register ExitBootServices event only in case the DT will be used.
> > > > > + */
> > > > > + BufferSize = sizeof (DtAcpiPref);
> > > > > + Status = gRT->GetVariable (L"DtAcpiPref",
> > > > > + &gDtPlatformFormSetGuid,
> > > > > + NULL,
> > > > > + &BufferSize,
> > > > > + &DtAcpiPref);
> > > > > + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> > > > > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> > > > > + TPL_NOTIFY,
> > > > > + Armada7k8kExitBootEvent,
> > > > > + NULL,
> > > > > + &mArmada7k8kExitBootServicesEvent);
> > > > > + if (EFI_ERROR (Status)) {
> > > > > + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + return EFI_SUCCESS;
> > > > > +}
> > > > > --
> > > > > 2.7.4
> > > > >
> > > >
> > > > Actually, looking at what you are trying to do, can you use the
> > > > gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
> > > > directly? That will be installed by the DT platform driver when
> > > > choosing ACPI mode.
> > > >
> > > > It would need to be another driver, but it is more appropriate in any
> > > > case to wire this up in some platform DXE rather than the generic DTB
> > > > loader.
> > >
> > > Ok, I'll move it to DXE. If possible, I'd prefer to use Armada's
> > > PlatformInitDxe - would it be accepatble for you to check
> > > gEdkiiPlatformHasAcpiGuid in ExitBootEvent?
> > >
> >
> > Yes, but using OnReadyToBoot is more idiomatic for things like this.
>
> I just checked it - unfortunately it is triggered to early. I need the
> PMU irq handling as long as I'm in UEFI (or boot with ACPI). With the
> gEfiEventReadyToBootGuid the event hits before I enter the shell, so
> I'm moving to DXE, but stick with ExitBoot event solution.
>
Perhaps you could check for the protocol at ready to boot, and only
install the exit boot handler when booting in ACPI mode?
It seems a bit convoluted perhaps, but it is better to do as little as
possible in exit boot services.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib
2019-04-16 5:01 ` Ard Biesheuvel
@ 2019-04-16 5:49 ` Marcin Wojtas
0 siblings, 0 replies; 13+ messages in thread
From: Marcin Wojtas @ 2019-04-16 5:49 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-groups-io, Leif Lindholm, Nadav Haklai,
Jan Dąbroś, Grzegorz Jaszczyk, Kostya Porotchkin,
Jeremy Linton, Jici Gao
wt., 16 kwi 2019 o 07:01 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On Mon, 15 Apr 2019 at 21:54, Marcin Wojtas <mw@semihalf.com> wrote:
> >
> > Hi Ard,
> >
> > pon., 15 kwi 2019 o 21:01 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> > >
> > > On Mon, 15 Apr 2019 at 02:52, Marcin Wojtas <mw@semihalf.com> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > >
> > > > pt., 12 kwi 2019 o 23:15 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
> > > > >
> > > > > On Fri, 12 Apr 2019 at 03:20, Marcin Wojtas <mw@semihalf.com> wrote:
> > > > > >
> > > > > > In order to provide special handling for DT environment,
> > > > > > implement custom version of DtLoaderLib, which registers
> > > > > > ExitBootServices event upon generic 'DtAcpiPref' variable.
> > > > > > The routine is responsible for disabling the PMU workaround,
> > > > > > that is valid only for UEFI and OS + APCI.
> > > > > >
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > > > > > ---
> > > > > > Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc | 2 +-
> > > > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf | 43 +++++++
> > > > > > Silicon/Marvell/Include/IndustryStandard/MvSmc.h | 2 +
> > > > > > Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c | 130 ++++++++++++++++++++
> > > > > > 4 files changed, 176 insertions(+), 1 deletion(-)
> > > > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > > create mode 100644 Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > > >
> > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > > index 8291582..5ed742f 100644
> > > > > > --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > > +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> > > > > > @@ -200,7 +200,7 @@
> > > > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > > > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> > > > > > NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
> > > > > > - DtPlatformDtbLoaderLib|EmbeddedPkg/Library/DxeDtPlatformDtbLoaderLibDefault/DxeDtPlatformDtbLoaderLibDefault.inf
> > > > > > + DtPlatformDtbLoaderLib|Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > >
> > > > > > [LibraryClasses.common.UEFI_APPLICATION]
> > > > > > PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > > new file mode 100644
> > > > > > index 0000000..ec3f3a5
> > > > > > --- /dev/null
> > > > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.inf
> > > > > > @@ -0,0 +1,43 @@
> > > > > > +/** @file
> > > > > > +*
> > > > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > > > +*
> > > > >
> > > > > Please fix the date here as well as below
> > > > >
> > > > > > +* This program and the accompanying materials
> > > > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > > > +* http://opensource.org/licenses/bsd-license.php
> > > > > > +*
> > > > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > > > +*
> > > > > > +**/
> > > > > > +
> > > > > > +[Defines]
> > > > > > + INF_VERSION = 0x0001001A
> > > > > > + BASE_NAME = DxeDtPlatformDtbLoaderLibDefault
> > > > > > + FILE_GUID = dde55569-ad3f-421d-b94b-3be66bb916ce
> > > > > > + MODULE_TYPE = DXE_DRIVER
> > > > > > + VERSION_STRING = 1.0
> > > > > > + LIBRARY_CLASS = DtPlatformDtbLoaderLib|DXE_DRIVER
> > > > > > +
> > > > > > +[Sources]
> > > > > > + DxeDtPlatformDtbLoaderLib.c
> > > > > > +
> > > > > > +[Packages]
> > > > > > + ArmPkg/ArmPkg.dec
> > > > > > + EmbeddedPkg/EmbeddedPkg.dec
> > > > > > + MdePkg/MdePkg.dec
> > > > > > + Silicon/Marvell/Marvell.dec
> > > > > > +
> > > > > > +[LibraryClasses]
> > > > > > + ArmSmcLib
> > > > > > + BaseLib
> > > > > > + DebugLib
> > > > > > + DxeServicesLib
> > > > > > + MemoryAllocationLib
> > > > > > + UefiRuntimeServicesTableLib
> > > > > > +
> > > > >
> > > > > Are you really using all of these?
> > > > >
> > > > > > +[Guids]
> > > > > > + gDtPlatformDefaultDtbFileGuid
> > > > > > + gDtPlatformFormSetGuid
> > > > >
> > > > > and these?
> > > > >
> > > > > > diff --git a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > > index 0c90f11..e5c89d9 100644
> > > > > > --- a/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > > +++ b/Silicon/Marvell/Include/IndustryStandard/MvSmc.h
> > > > > > @@ -20,5 +20,7 @@
> > > > > > #define MV_SMC_ID_COMPHY_POWER_OFF 0x82000002
> > > > > > #define MV_SMC_ID_COMPHY_PLL_LOCK 0x82000003
> > > > > > #define MV_SMC_ID_DRAM_SIZE 0x82000010
> > > > > > +#define MV_SMC_ID_PMU_IRQ_ENABLE 0x82000012
> > > > > > +#define MV_SMC_ID_PMU_IRQ_DISABLE 0x82000013
> > > > > >
> > > > > > #endif //__MV_SMC_H__
> > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > > > new file mode 100644
> > > > > > index 0000000..b189f47
> > > > > > --- /dev/null
> > > > > > +++ b/Silicon/Marvell/Armada7k8k/Library/DxeDtPlatformDtbLoaderLib/DxeDtPlatformDtbLoaderLib.c
> > > > > > @@ -0,0 +1,130 @@
> > > > > > +/** @file
> > > > > > +*
> > > > > > +* Copyright (c) 2017, Linaro, Ltd. All rights reserved.
> > > > > > +* Copyright (c) 2018, Marvell International Ltd. and its affiliates
> > > > > > +*
> > > > > > +* This program and the accompanying materials
> > > > > > +* are licensed and made available under the terms and conditions of the BSD License
> > > > > > +* which accompanies this distribution. The full text of the license may be found at
> > > > > > +* http://opensource.org/licenses/bsd-license.php
> > > > > > +*
> > > > > > +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > > > +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > > > > > +*
> > > > > > +**/
> > > > > > +
> > > > > > +#include <PiDxe.h>
> > > > > > +
> > > > > > +#include <IndustryStandard/MvSmc.h>
> > > > > > +
> > > > > > +#include <Library/ArmSmcLib.h>
> > > > > > +#include <Library/BaseLib.h>
> > > > > > +#include <Library/DebugLib.h>
> > > > > > +#include <Library/DxeServicesLib.h>
> > > > > > +#include <Library/MemoryAllocationLib.h>
> > > > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > > > +#include <Library/UefiRuntimeServicesTableLib.h>
> > > > > > +
> > > > >
> > > > > Please make sure that all libraries you include are listed in the .inf
> > > > >
> > > > > > +STATIC EFI_EVENT mArmada7k8kExitBootServicesEvent;
> > > > > > +
> > > > > > +#define DT_ACPI_SELECT_DT 0x0
> > > > > > +#define DT_ACPI_SELECT_ACPI 0x1
> > > > > > +
> > > > > > +typedef struct {
> > > > > > + UINT8 Pref;
> > > > > > + UINT8 Reserved[3];
> > > > > > +} DT_ACPI_VARSTORE_DATA;
> > > > > > +
> > > > > > +/**
> > > > > > + Disable extra EL3 handling of the PMU interrupts for DT world.
> > > > > > +
> > > > > > + @param[in] Event Event structure
> > > > > > + @param[in] Context Notification context
> > > > > > +
> > > > > > +**/
> > > > > > +STATIC
> > > > > > +VOID
> > > > > > +Armada7k8kExitBootEvent (
> > > > > > + IN EFI_EVENT Event,
> > > > > > + IN VOID *Context
> > > > > > + )
> > > > > > +{
> > > > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > > > +
> > > > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_DISABLE;
> > > > > > + ArmCallSmc (&SmcRegs);
> > > > > > +
> > > > > > + return;
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + Return a pool allocated copy of the DTB image that is appropriate for
> > > > > > + booting the current platform via DT.
> > > > > > +
> > > > > > + @param[out] Dtb Pointer to the DTB copy
> > > > > > + @param[out] DtbSize Size of the DTB copy
> > > > > > +
> > > > > > + @retval EFI_SUCCESS Operation completed successfully
> > > > > > + @retval EFI_NOT_FOUND No suitable DTB image could be located
> > > > > > + @retval EFI_OUT_OF_RESOURCES No pool memory available
> > > > > > +
> > > > > > +**/
> > > > > > +EFI_STATUS
> > > > > > +EFIAPI
> > > > > > +DtPlatformLoadDtb (
> > > > > > + OUT VOID **Dtb,
> > > > > > + OUT UINTN *DtbSize
> > > > > > + )
> > > > > > +{
> > > > > > + DT_ACPI_VARSTORE_DATA DtAcpiPref;
> > > > > > + ARM_SMC_ARGS SmcRegs = {0};
> > > > > > + EFI_STATUS Status;
> > > > > > + VOID *OrigDtb;
> > > > > > + VOID *CopyDtb;
> > > > > > + UINTN OrigDtbSize;
> > > > > > + UINTN BufferSize;
> > > > > > +
> > > > > > + Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> > > > > > + EFI_SECTION_RAW,
> > > > > > + 0,
> > > > > > + &OrigDtb,
> > > > > > + &OrigDtbSize);
> > > > > > + if (EFI_ERROR (Status)) {
> > > > > > + return EFI_NOT_FOUND;
> > > > > > + }
> > > > > > +
> > > > > > + CopyDtb = AllocateCopyPool (OrigDtbSize, OrigDtb);
> > > > > > + if (CopyDtb == NULL) {
> > > > > > + return EFI_OUT_OF_RESOURCES;
> > > > > > + }
> > > > > > +
> > > > > > + *Dtb = CopyDtb;
> > > > > > + *DtbSize = OrigDtbSize;
> > > > > > +
> > > > > > + /* Enable EL3 handler for regardless HW description */
> > > > > > + SmcRegs.Arg0 = MV_SMC_ID_PMU_IRQ_ENABLE;
> > > > > > + ArmCallSmc (&SmcRegs);
> > > > > > +
> > > > > > + /*
> > > > > > + * Get the current DT/ACPI preference from the DtAcpiPref variable.
> > > > > > + * Register ExitBootServices event only in case the DT will be used.
> > > > > > + */
> > > > > > + BufferSize = sizeof (DtAcpiPref);
> > > > > > + Status = gRT->GetVariable (L"DtAcpiPref",
> > > > > > + &gDtPlatformFormSetGuid,
> > > > > > + NULL,
> > > > > > + &BufferSize,
> > > > > > + &DtAcpiPref);
> > > > > > + if (EFI_ERROR (Status) || DtAcpiPref.Pref == DT_ACPI_SELECT_DT) {
> > > > > > + Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES,
> > > > > > + TPL_NOTIFY,
> > > > > > + Armada7k8kExitBootEvent,
> > > > > > + NULL,
> > > > > > + &mArmada7k8kExitBootServicesEvent);
> > > > > > + if (EFI_ERROR (Status)) {
> > > > > > + DEBUG ((DEBUG_ERROR, "%a: Fail to register EBS event\n", __FUNCTION__));
> > > > > > + }
> > > > > > + }
> > > > > > +
> > > > > > + return EFI_SUCCESS;
> > > > > > +}
> > > > > > --
> > > > > > 2.7.4
> > > > > >
> > > > >
> > > > > Actually, looking at what you are trying to do, can you use the
> > > > > gEdkiiPlatformHasAcpiGuid protocol instead of looking at the variable
> > > > > directly? That will be installed by the DT platform driver when
> > > > > choosing ACPI mode.
> > > > >
> > > > > It would need to be another driver, but it is more appropriate in any
> > > > > case to wire this up in some platform DXE rather than the generic DTB
> > > > > loader.
> > > >
> > > > Ok, I'll move it to DXE. If possible, I'd prefer to use Armada's
> > > > PlatformInitDxe - would it be accepatble for you to check
> > > > gEdkiiPlatformHasAcpiGuid in ExitBootEvent?
> > > >
> > >
> > > Yes, but using OnReadyToBoot is more idiomatic for things like this.
> >
> > I just checked it - unfortunately it is triggered to early. I need the
> > PMU irq handling as long as I'm in UEFI (or boot with ACPI). With the
> > gEfiEventReadyToBootGuid the event hits before I enter the shell, so
> > I'm moving to DXE, but stick with ExitBoot event solution.
> >
>
> Perhaps you could check for the protocol at ready to boot, and only
> install the exit boot handler when booting in ACPI mode?
>
> It seems a bit convoluted perhaps, but it is better to do as little as
> possible in exit boot services.
A lot of combination just to avoid using custom DtLoaderLib (simple,
only smc in EBS) or checking the status of gEdkiiPlatformHasAcpiGuid
in the EBS :)
I thought about forcing PlatInitDxe to load after DtPlatformDxe, but
that ends with the chicken and egg problem (variables vs setting pin
control). I could also add extra drive (PlatDeinitDxe) only for that
purpose, but I do not like this idea either.
So, I'll implement chained events.
Thanks,
Marcin
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-04-16 5:49 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-12 10:19 [edk2-platforms: PATCH 0/6] Armada7k8k misc improvements Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 1/6] Marvell/Armada7k8k: RealTimeClockLib: Update bus parameters Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 2/6] Marvell/Armada7k8k: AcpiTables: Enable edge trigger of PMU interrupt Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 3/6] Marvell/Armada7k8k: Implement custom DtLoaderLib Marcin Wojtas
2019-04-12 21:14 ` Ard Biesheuvel
2019-04-15 9:52 ` Marcin Wojtas
2019-04-15 19:01 ` Ard Biesheuvel
2019-04-16 4:54 ` Marcin Wojtas
2019-04-16 5:01 ` Ard Biesheuvel
2019-04-16 5:49 ` Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 4/6] Marvell/Armada7k8k: Switch to software-based watchdog Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 5/6] Marvell/Armada7k8k: ArmadaSoCDescLib: Add more I2C controllers Marcin Wojtas
2019-04-12 10:19 ` [edk2-platforms: PATCH 6/6] Marvell/Armada80x0Db: Configure the CP1, comphy-2 to work with SFI 10G Marcin Wojtas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox