* [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements
@ 2019-10-11 11:07 Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions Pete Batard
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
Changes from v2:
- Consistent use of curly brackets for if statements
- Remove unused variables that may produce an issue with git bisect
- Use gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor to populate vendor
Note that 5/5, which has been R-b, is unchanged.
Pete Batard (5):
Platform/RPi3/RpiFirmwareDxe: Add more query functions
Platform/RPi3/RpiFirmwareDxe: Improve serial number population
Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries
Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD
Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 198 ++++++++++++--------
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf | 2 +
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 167 ++++++++++++++++-
Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58 ++++--
Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
5 files changed, 333 insertions(+), 96 deletions(-)
--
2.21.0.windows.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
@ 2019-10-11 11:07 ` Pete Batard
2019-10-14 9:55 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population Pete Batard
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
This patch introduces the capability to also query the Model Name/
Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
protocol. This is aims at making the driver more suitable to cater
for platforms other than the Raspberry Pi 3 as well as simplifying
the population of entries in PlatformSmbiosDxe.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 158 +++++++++++++++++++-
Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58 +++++--
2 files changed, 199 insertions(+), 17 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 9b5ee1946279..25d7fa3974c0 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -461,7 +461,7 @@ typedef struct {
RPI_FW_TAG_HEAD TagHead;
RPI_FW_MODEL_REVISION_TAG TagBody;
UINT32 EndTag;
-} RPI_FW_GET_MODEL_REVISION_CMD;
+} RPI_FW_GET_REVISION_CMD;
#pragma pack()
STATIC
@@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
OUT UINT32 *Revision
)
{
- RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
+ RPI_FW_GET_REVISION_CMD *Cmd;
EFI_STATUS Status;
UINT32 Result;
@@ -506,6 +506,156 @@ RpiFirmwareGetModelRevision (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareGetFirmwareRevision (
+ OUT UINT32 *Revision
+ )
+{
+ RPI_FW_GET_REVISION_CMD *Cmd;
+ EFI_STATUS Status;
+ UINT32 Result;
+
+ if (!AcquireSpinLockOrFail (&mMailboxLock)) {
+ DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
+ return EFI_DEVICE_ERROR;
+ }
+
+ Cmd = mDmaBuffer;
+ ZeroMem (Cmd, sizeof (*Cmd));
+
+ Cmd->BufferHead.BufferSize = sizeof (*Cmd);
+ Cmd->BufferHead.Response = 0;
+ Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
+ Cmd->TagHead.TagSize = sizeof (Cmd->TagBody);
+ Cmd->TagHead.TagValueSize = 0;
+ Cmd->EndTag = 0;
+
+ Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);
+
+ ReleaseSpinLock (&mMailboxLock);
+
+ if (EFI_ERROR (Status) ||
+ Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+ DEBUG ((DEBUG_ERROR,
+ "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
+ __FUNCTION__, Status, Cmd->BufferHead.Response));
+ return EFI_DEVICE_ERROR;
+ }
+
+ *Revision = Cmd->TagBody.Revision;
+ return EFI_SUCCESS;
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetModelName (
+ IN INTN ModelId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ModelId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
+ ModelId = (Revision >> 4) & 0xFF;
+ }
+
+ switch (ModelId) {
+ // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+ return "Raspberry Pi Model A";
+ case 0x01:
+ return "Raspberry Pi Model B";
+ case 0x02:
+ return "Raspberry Pi Model A+";
+ case 0x03:
+ return "Raspberry Pi Model B+";
+ case 0x04:
+ return "Raspberry Pi 2 Model B";
+ case 0x06:
+ return "Raspberry Pi Compute Module 1";
+ case 0x08:
+ return "Raspberry Pi 3 Model B";
+ case 0x09:
+ return "Raspberry Pi Zero";
+ case 0x0A:
+ return "Raspberry Pi Compute Module 3";
+ case 0x0C:
+ return "Raspberry Pi Zero W";
+ case 0x0D:
+ return "Raspberry Pi 3 Model B+";
+ case 0x0E:
+ return "Raspberry Pi 3 Model A+";
+ case 0x11:
+ return "Raspberry Pi 4 Model B";
+ default:
+ return "Unknown Raspberry Pi Model";
+ }
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetManufacturerName (
+ IN INTN ManufacturerId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ManufacturerId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
+ ManufacturerId = (Revision >> 16) & 0x0F;
+ }
+
+ switch (ManufacturerId) {
+ // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+ return "Sony UK";
+ case 0x01:
+ return "Egoman";
+ case 0x02:
+ case 0x04:
+ return "Embest";
+ case 0x03:
+ return "Sony Japan";
+ case 0x05:
+ return "Stadium";
+ default:
+ return "Unknown Manufacturer";
+ }
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetCpuName (
+ IN INTN CpuId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative CpuId is passed, detect it.
+ if ((CpuId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
+ CpuId = (Revision >> 12) & 0x0F;
+ }
+
+ switch (CpuId) {
+ // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+ return "BCM2835 (ARM11)";
+ case 0x01:
+ return "BCM2836 (ARM Cortex-A7)";
+ case 0x02:
+ return "BCM2837 (ARM Cortex-A53)";
+ case 0x03:
+ return "BCM2711 (ARM Cortex-A72)";
+ default:
+ return "Unknown CPU Model";
+ }
+}
+
#pragma pack()
typedef struct {
UINT32 Width;
@@ -1009,6 +1159,10 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
RpiFirmwareGetSerial,
RpiFirmwareGetModel,
RpiFirmwareGetModelRevision,
+ RpiFirmwareGetModelName,
+ RpiFirmwareGetFirmwareRevision,
+ RpiFirmwareGetManufacturerName,
+ RpiFirmwareGetCpuName,
RpiFirmwareGetArmMemory
};
diff --git a/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h b/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
index ec70f28efe1a..e49d6e6132d9 100644
--- a/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
+++ b/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
@@ -96,6 +96,30 @@ EFI_STATUS
UINT32 *Revision
);
+typedef
+CHAR8*
+(EFIAPI *GET_MODEL_NAME) (
+ INTN ModelId
+ );
+
+typedef
+EFI_STATUS
+(EFIAPI *GET_FIRMWARE_REVISION) (
+ UINT32 *Revision
+ );
+
+typedef
+CHAR8*
+(EFIAPI *GET_MANUFACTURER_NAME) (
+ INTN ManufacturerId
+ );
+
+typedef
+CHAR8*
+(EFIAPI *GET_CPU_NAME) (
+ INTN CpuId
+ );
+
typedef
EFI_STATUS
(EFIAPI *GET_ARM_MEM) (
@@ -104,21 +128,25 @@ EFI_STATUS
);
typedef struct {
- SET_POWER_STATE SetPowerState;
- GET_MAC_ADDRESS GetMacAddress;
- GET_COMMAND_LINE GetCommandLine;
- GET_CLOCK_RATE GetClockRate;
- GET_CLOCK_RATE GetMaxClockRate;
- GET_CLOCK_RATE GetMinClockRate;
- SET_CLOCK_RATE SetClockRate;
- GET_FB GetFB;
- FREE_FB FreeFB;
- GET_FB_SIZE GetFBSize;
- SET_LED SetLed;
- GET_SERIAL GetSerial;
- GET_MODEL GetModel;
- GET_MODEL_REVISION GetModelRevision;
- GET_ARM_MEM GetArmMem;
+ SET_POWER_STATE SetPowerState;
+ GET_MAC_ADDRESS GetMacAddress;
+ GET_COMMAND_LINE GetCommandLine;
+ GET_CLOCK_RATE GetClockRate;
+ GET_CLOCK_RATE GetMaxClockRate;
+ GET_CLOCK_RATE GetMinClockRate;
+ SET_CLOCK_RATE SetClockRate;
+ GET_FB GetFB;
+ FREE_FB FreeFB;
+ GET_FB_SIZE GetFBSize;
+ SET_LED SetLed;
+ GET_SERIAL GetSerial;
+ GET_MODEL GetModel;
+ GET_MODEL_REVISION GetModelRevision;
+ GET_MODEL_NAME GetModelName;
+ GET_FIRMWARE_REVISION GetFirmwareRevision;
+ GET_MANUFACTURER_NAME GetManufacturerName;
+ GET_CPU_NAME GetCpuName;
+ GET_ARM_MEM GetArmMem;
} RASPBERRY_PI_FIRMWARE_PROTOCOL;
extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [edk2-platforms][PATCH v3 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions Pete Batard
@ 2019-10-11 11:07 ` Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 3/5] Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries Pete Batard
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
Improve RpiFirmwareGetSerial() to derive a serial number from the
MAC address, in case the platform returns 0 for the serial number.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 25d7fa3974c0..5a9d4c3f1787 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -393,7 +393,14 @@ RpiFirmwareGetSerial (
}
*Serial = Cmd->TagBody.Serial;
- return EFI_SUCCESS;
+ // Some platforms return 0 for serial. For those, try to use the MAC address.
+ if (*Serial == 0) {
+ Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
+ // Convert to a more user-friendly value
+ *Serial = SwapBytes64 (*Serial << 16);
+ }
+
+ return Status;
}
#pragma pack()
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [edk2-platforms][PATCH v3 3/5] Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population Pete Batard
@ 2019-10-11 11:07 ` Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 4/5] Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD Pete Batard
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
This patch cleans up the population SMBIOS entries by removing elements
that we don't have data for, as well as properly filling the ones for
which we do, through the newly added queries from RpiFirmwareDxe.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 135 ++++++++++----------
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf | 2 +
Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
3 files changed, 69 insertions(+), 72 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index bc35175279f2..9150dcfd8c5b 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -35,6 +35,7 @@
#include <Library/UefiDriverEntryPoint.h>
#include <Library/UefiLib.h>
#include <Library/BaseLib.h>
+#include <Library/PcdLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/UefiBootServicesTableLib.h>
@@ -104,16 +105,19 @@ SMBIOS_TABLE_TYPE0 mBIOSInfoType0 = {
// VirtualMachineSupported :1;
// ExtensionByte2Reserved :3;
},
- 0xFF, // SystemBiosMajorRelease
- 0xFF, // SystemBiosMinorRelease
- 0xFF, // EmbeddedControllerFirmwareMajorRelease
- 0xFF, // EmbeddedControllerFirmwareMinorRelease
+ 0, // SystemBiosMajorRelease
+ 0, // SystemBiosMinorRelease
+ 0, // EmbeddedControllerFirmwareMajorRelease
+ 0, // EmbeddedControllerFirmwareMinorRelease
};
+CHAR8 mBiosVendor[128] = "EDK2";
+CHAR8 mBiosVersion[128] = "EDK2-DEV";
+
CHAR8 *mBIOSInfoType0Strings[] = {
- "https://github.com/andreiw/RaspberryPiPkg", // Vendor String
- "Raspberry Pi 64-bit UEFI (" __DATE__ " " __TIME__ ")", // BiosVersion String
- __DATE__,
+ mBiosVendor, // Vendor
+ mBiosVersion, // Version
+ __DATE__ " " __TIME__, // Release Date
NULL
};
@@ -132,42 +136,19 @@ SMBIOS_TABLE_TYPE1 mSysInfoType1 = {
6, // Family String
};
-#define PROD_BASE 8
-#define PROD_KNOWN 13
-#define PROD_UNKNOWN 11
-CHAR8 *ProductNames[] = {
- /* 8 */ "3",
- /* 9 */ "Zero",
- /* 10 */ "CM3",
- /* 11 */ "???",
- /* 12 */ "Zero W",
- /* 13 */ "3B+"
-};
-
-#define MANU_UNKNOWN 0
-#define MANU_KNOWN 4
-#define MANU_BASE 1
-CHAR8 *ManufNames[] = {
- "???",
- /* 0 */ "Sony",
- /* 1 */ "Egoman",
- /* 2 */ "Embest",
- /* 3 */ "Sony Japan",
- /* 4 */ "Embest"
-};
-
-CHAR8 mSysInfoManufName[sizeof ("Sony Japan")];
-CHAR8 mSysInfoProductName[sizeof ("64-bit Raspberry Pi XXXXXX (rev. xxxxxxxx)")];
+CHAR8 mSysInfoManufName[128];
+CHAR8 mSysInfoProductName[128];
+CHAR8 mSysInfoVersionName[128];
CHAR8 mSysInfoSerial[sizeof (UINT64) * 2 + 1];
CHAR8 mSysInfoSKU[sizeof (UINT64) * 2 + 1];
CHAR8 *mSysInfoType1Strings[] = {
mSysInfoManufName,
mSysInfoProductName,
- mSysInfoProductName,
+ mSysInfoVersionName,
mSysInfoSerial,
mSysInfoSKU,
- "edk2",
+ "Raspberry Pi",
NULL
};
@@ -180,7 +161,7 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
2, // ProductName String
3, // Version String
4, // SerialNumber String
- 5, // AssetTag String
+ 0, // AssetTag String
{ // FeatureFlag
1, // Motherboard :1;
0, // RequiresDaughterCard :1;
@@ -189,7 +170,7 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
0, // HotSwappable :1;
0, // Reserved :3;
},
- 6, // LocationInChassis String
+ 0, // LocationInChassis String
0, // ChassisHandle;
BaseBoardTypeMotherBoard, // BoardType;
0, // NumberOfContainedObjectHandles;
@@ -198,10 +179,8 @@ SMBIOS_TABLE_TYPE2 mBoardInfoType2 = {
CHAR8 *mBoardInfoType2Strings[] = {
mSysInfoManufName,
mSysInfoProductName,
- mSysInfoProductName,
+ mSysInfoVersionName,
mSysInfoSerial,
- "None",
- mSysInfoSKU,
NULL
};
@@ -214,7 +193,7 @@ SMBIOS_TABLE_TYPE3 mEnclosureInfoType3 = {
MiscChassisEmbeddedPc, // Type;
2, // Version String
3, // SerialNumber String
- 4, // AssetTag String
+ 0, // AssetTag String
ChassisStateSafe, // BootupState;
ChassisStateSafe, // PowerSupplyState;
ChassisStateSafe, // ThermalState;
@@ -230,7 +209,6 @@ CHAR8 *mEnclosureInfoType3Strings[] = {
mSysInfoManufName,
mSysInfoProductName,
mSysInfoSerial,
- "None",
NULL
};
@@ -306,9 +284,9 @@ SMBIOS_TABLE_TYPE4 mProcessorInfoType4 = {
0, // L1CacheHandle;
0, // L2CacheHandle;
0, // L3CacheHandle;
- 4, // SerialNumber;
- 5, // AssetTag;
- 6, // PartNumber;
+ 0, // SerialNumber;
+ 0, // AssetTag;
+ 0, // PartNumber;
4, // CoreCount;
4, // EnabledCoreCount;
4, // ThreadCount;
@@ -316,13 +294,12 @@ SMBIOS_TABLE_TYPE4 mProcessorInfoType4 = {
ProcessorFamilyARM, // ARM Processor Family;
};
+CHAR8 mCpuName[128] = "Unknown ARM CPU";
+
CHAR8 *mProcessorInfoType4Strings[] = {
"Socket",
- "ARM",
- "BCM2837 ARMv8",
- "1.0",
- "1.0",
- "1.0",
+ "Broadcom",
+ mCpuName,
NULL
};
@@ -618,6 +595,29 @@ BIOSInfoUpdateSmbiosType0 (
VOID
)
{
+ UINT32 FirmwareRevision = 0;
+ EFI_STATUS Status = EFI_SUCCESS;
+
+ // Populate the Firmware major and minor.
+ Status = mFwProtocol->GetFirmwareRevision (&FirmwareRevision);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to get firmware revision: %r\n", Status));
+ } else {
+ // This expects Broadcom / The Raspberry Pi Foundation to switch to
+ // less nonsensical VideoCore firmware revisions in the future...
+ mBIOSInfoType0.EmbeddedControllerFirmwareMajorRelease =
+ (UINT8)((FirmwareRevision >> 16) & 0xFF);
+ mBIOSInfoType0.EmbeddedControllerFirmwareMinorRelease =
+ (UINT8)(FirmwareRevision & 0xFF);
+ }
+
+ // mBiosVendor and mBiosVersion, which are referenced in mBIOSInfoType0Strings,
+ // are left unchanged if the following calls fail.
+ UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVendor),
+ mBiosVendor, sizeof (mBiosVendor));
+ UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
+ mBiosVersion, sizeof (mBiosVersion));
+
LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mBIOSInfoType0, mBIOSInfoType0Strings, NULL);
}
@@ -664,34 +664,25 @@ SysInfoUpdateSmbiosType1 (
UINT32 BoardRevision = 0;
EFI_STATUS Status = EFI_SUCCESS;
UINT64 BoardSerial = 0;
- UINTN Prod = PROD_UNKNOWN;
- UINTN Manu = MANU_UNKNOWN;
+ INTN Prod = -1;
+ INTN Manu = -1;
Status = mFwProtocol->GetModelRevision (&BoardRevision);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Failed to get board model: %r\n", Status));
} else {
Prod = (BoardRevision >> 4) & 0xFF;
+ Manu = (BoardRevision >> 16) & 0x0F;
}
- if (Prod > PROD_KNOWN) {
- Prod = PROD_UNKNOWN;
- }
- Prod -= PROD_BASE;
- AsciiSPrint (mSysInfoProductName, sizeof (mSysInfoProductName),
- "64-bit Raspberry Pi %a (rev. %x)", ProductNames[Prod], BoardRevision);
-
- Manu = (BoardRevision >> 16) & 0xF;
- if (Manu > MANU_KNOWN) {
- Manu = MANU_UNKNOWN;
- } else {
- Manu += MANU_BASE;
- }
- AsciiSPrint (mSysInfoManufName, sizeof (mSysInfoManufName), "%a", ManufNames[Manu]);
+ AsciiStrCpyS (mSysInfoProductName, sizeof (mSysInfoProductName),
+ mFwProtocol->GetModelName (Prod));
+ AsciiStrCpyS (mSysInfoManufName, sizeof (mSysInfoManufName),
+ mFwProtocol->GetManufacturerName (Manu));
+ AsciiSPrint (mSysInfoVersionName, sizeof (mSysInfoVersionName),
+ "%X", BoardRevision);
- I64ToHexString (mSysInfoSKU,
- sizeof (mSysInfoSKU),
- BoardRevision);
+ I64ToHexString (mSysInfoSKU, sizeof (mSysInfoSKU), BoardRevision);
Status = mFwProtocol->GetSerial (&BoardSerial);
if (EFI_ERROR (Status)) {
@@ -702,9 +693,11 @@ SysInfoUpdateSmbiosType1 (
DEBUG ((DEBUG_ERROR, "Board Serial Number: %a\n", mSysInfoSerial));
- mSysInfoType1.Uuid.Data1 = *(UINT32*)"RPi3";
+ mSysInfoType1.Uuid.Data1 = BoardRevision;
mSysInfoType1.Uuid.Data2 = 0x0;
mSysInfoType1.Uuid.Data3 = 0x0;
+ // Swap endianness, so that the serial is more user-friendly as a UUID
+ BoardSerial = SwapBytes64 (BoardSerial);
CopyMem (mSysInfoType1.Uuid.Data4, &BoardSerial, sizeof (BoardSerial));
LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mSysInfoType1, mSysInfoType1Strings, NULL);
@@ -763,6 +756,8 @@ ProcessorInfoUpdateSmbiosType4 (
DEBUG ((DEBUG_INFO, "Current CPU speed: %uHz\n", Rate));
}
+ AsciiStrCpyS (mCpuName, sizeof (mCpuName), mFwProtocol->GetCpuName (-1));
+
LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mProcessorInfoType4, mProcessorInfoType4Strings, NULL);
}
diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
index f7c74f7f5456..485450625b54 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
+++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf
@@ -48,3 +48,5 @@ [Depex]
[Pcd]
gArmTokenSpaceGuid.PcdSystemMemorySize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index b37a02e97da7..bc424a06bb45 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -309,7 +309,7 @@ [PcdsFixedAtBuild.common]
gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0xc0000000
- gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"edk2-1.0"
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"EDK2-DEV"
!if $(SECURE_BOOT_ENABLE) == TRUE
# override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
@@ -383,7 +383,7 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
- gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"Raspberry Pi 3 64-bit UEFI"
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
[PcdsDynamicHii.common.DEFAULT]
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [edk2-platforms][PATCH v3 4/5] Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
` (2 preceding siblings ...)
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 3/5] Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries Pete Batard
@ 2019-10-11 11:07 ` Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision Pete Batard
2019-10-15 20:00 ` [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Leif Lindholm
5 siblings, 0 replies; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
String parsing code is added to BIOSInfoUpdateSmbiosType0() so that
any numeric "x.y" value being passed in PcdFirmwareVersionString is
now used to populate the BIOS major and minor.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 45 ++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index 9150dcfd8c5b..b5dcff897a59 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -41,6 +41,8 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/PrintLib.h>
+#define SMB_IS_DIGIT(c) (((c) >= '0') && ((c) <= '9'))
+
STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
/***********************************************************************
@@ -597,6 +599,9 @@ BIOSInfoUpdateSmbiosType0 (
{
UINT32 FirmwareRevision = 0;
EFI_STATUS Status = EFI_SUCCESS;
+ INTN i;
+ INTN State = 0;
+ INTN Value[2];
// Populate the Firmware major and minor.
Status = mFwProtocol->GetFirmwareRevision (&FirmwareRevision);
@@ -618,6 +623,46 @@ BIOSInfoUpdateSmbiosType0 (
UnicodeStrToAsciiStrS ((CHAR16*)PcdGetPtr (PcdFirmwareVersionString),
mBiosVersion, sizeof (mBiosVersion));
+ // Look for a "x.y" numeric string anywhere in mBiosVersion and
+ // try to parse it to populate the BIOS major and minor.
+ for (i = 0; (i < AsciiStrLen (mBiosVersion)) && (State < 4); i++) {
+ switch (State) {
+ case 0:
+ if (!SMB_IS_DIGIT (mBiosVersion[i]))
+ break;
+ Value[0] = Value[1] = 0;
+ State++;
+ // Fall through
+ case 1:
+ case 3:
+ if (SMB_IS_DIGIT (mBiosVersion[i])) {
+ Value[State / 2] = (Value[State / 2] * 10) + (mBiosVersion[i] - '0');
+ if (Value[State / 2] > 255) {
+ while (SMB_IS_DIGIT (mBiosVersion[i + 1]))
+ i++;
+ // Reset our state (we may have something like "Firmware X83737.1 v1.23")
+ State = 0;
+ }
+ } else {
+ State++;
+ }
+ if (State != 2)
+ break;
+ // Fall through
+ case 2:
+ if ((mBiosVersion[i] == '.') && (SMB_IS_DIGIT (mBiosVersion[i + 1]))) {
+ State++;
+ } else {
+ State = 0;
+ }
+ break;
+ }
+ }
+ if ((State == 3) || (State == 4)) {
+ mBIOSInfoType0.SystemBiosMajorRelease = (UINT8)Value[0];
+ mBIOSInfoType0.SystemBiosMinorRelease = (UINT8)Value[1];
+ }
+
LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mBIOSInfoType0, mBIOSInfoType0Strings, NULL);
}
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
` (3 preceding siblings ...)
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 4/5] Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD Pete Batard
@ 2019-10-11 11:07 ` Pete Batard
2019-10-14 10:01 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-15 20:00 ` [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Leif Lindholm
5 siblings, 1 reply; 14+ messages in thread
From: Pete Batard @ 2019-10-11 11:07 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif.lindholm
The board revision is the proper channel to use to detect the amount of
RAM available as bits [20-22] report the effective RAM size for the board
starting with 256 MB (000b) and doubling in size for each value.
Signed-off-by: Pete Batard <pete@akeo.ie>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
index b5dcff897a59..5abc82b8d363 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
@@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
)
{
EFI_STATUS Status;
- UINT32 Base;
- UINT32 Size;
+ UINT32 BoardRevision = 0;
- Status = mFwProtocol->GetArmMem (&Base, &Size);
+ // Note: Type 19 addresses are expressed in KB, not bytes
+ mMemArrMapInfoType19.StartingAddress = 0;
+ // The minimum RAM size used on any Raspberry Pi model is 256 MB
+ mMemArrMapInfoType19.EndingAddress = 256 * 1024;
+ Status = mFwProtocol->GetModelRevision (&BoardRevision);
if (Status != EFI_SUCCESS) {
- DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n", Status));
+ DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size - defaulting to 256 MB: %r\n", Status));
} else {
- mMemArrMapInfoType19.StartingAddress = Base / 1024;
- mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
+ // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ // Bits [20-22] indicate the amount of memory starting with 256MB (000b)
+ // and doubling in size for each value (001b = 512 MB, 010b = 1GB, etc.)
+ mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) & 0x07;
}
+ mMemArrMapInfoType19.EndingAddress -= 1;
LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19, mMemArrMapInfoType19Strings, NULL);
}
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions Pete Batard
@ 2019-10-14 9:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-14 9:55 UTC (permalink / raw)
To: devel, pete; +Cc: ard.biesheuvel, leif.lindholm
On 10/11/19 1:07 PM, Pete Batard wrote:
> This patch introduces the capability to also query the Model Name/
> Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
> protocol. This is aims at making the driver more suitable to cater
> for platforms other than the Raspberry Pi 3 as well as simplifying
> the population of entries in PlatformSmbiosDxe.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
> Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 158 +++++++++++++++++++-
> Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58 +++++--
> 2 files changed, 199 insertions(+), 17 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index 9b5ee1946279..25d7fa3974c0 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -461,7 +461,7 @@ typedef struct {
> RPI_FW_TAG_HEAD TagHead;
> RPI_FW_MODEL_REVISION_TAG TagBody;
> UINT32 EndTag;
> -} RPI_FW_GET_MODEL_REVISION_CMD;
> +} RPI_FW_GET_REVISION_CMD;
> #pragma pack()
>
> STATIC
> @@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
> OUT UINT32 *Revision
> )
> {
> - RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
> + RPI_FW_GET_REVISION_CMD *Cmd;
> EFI_STATUS Status;
> UINT32 Result;
>
> @@ -506,6 +506,156 @@ RpiFirmwareGetModelRevision (
> return EFI_SUCCESS;
> }
>
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RpiFirmwareGetFirmwareRevision (
> + OUT UINT32 *Revision
> + )
> +{
> + RPI_FW_GET_REVISION_CMD *Cmd;
> + EFI_STATUS Status;
> + UINT32 Result;
> +
> + if (!AcquireSpinLockOrFail (&mMailboxLock)) {
> + DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
> + return EFI_DEVICE_ERROR;
> + }
> +
> + Cmd = mDmaBuffer;
> + ZeroMem (Cmd, sizeof (*Cmd));
> +
> + Cmd->BufferHead.BufferSize = sizeof (*Cmd);
> + Cmd->BufferHead.Response = 0;
> + Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
> + Cmd->TagHead.TagSize = sizeof (Cmd->TagBody);
> + Cmd->TagHead.TagValueSize = 0;
> + Cmd->EndTag = 0;
> +
> + Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);
> +
> + ReleaseSpinLock (&mMailboxLock);
> +
> + if (EFI_ERROR (Status) ||
> + Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
> + DEBUG ((DEBUG_ERROR,
> + "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
> + __FUNCTION__, Status, Cmd->BufferHead.Response));
> + return EFI_DEVICE_ERROR;
> + }
> +
> + *Revision = Cmd->TagBody.Revision;
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +CHAR8*
> +EFIAPI
> +RpiFirmwareGetModelName (
> + IN INTN ModelId
> + )
> +{
> + UINT32 Revision;
> +
> + // If a negative ModelId is passed, detect it.
> + if ((ModelId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
> + ModelId = (Revision >> 4) & 0xFF;
> + }
> +
> + switch (ModelId) {
> + // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + case 0x00:
> + return "Raspberry Pi Model A";
> + case 0x01:
> + return "Raspberry Pi Model B";
> + case 0x02:
> + return "Raspberry Pi Model A+";
> + case 0x03:
> + return "Raspberry Pi Model B+";
> + case 0x04:
> + return "Raspberry Pi 2 Model B";
> + case 0x06:
> + return "Raspberry Pi Compute Module 1";
> + case 0x08:
> + return "Raspberry Pi 3 Model B";
> + case 0x09:
> + return "Raspberry Pi Zero";
> + case 0x0A:
> + return "Raspberry Pi Compute Module 3";
> + case 0x0C:
> + return "Raspberry Pi Zero W";
> + case 0x0D:
> + return "Raspberry Pi 3 Model B+";
> + case 0x0E:
> + return "Raspberry Pi 3 Model A+";
> + case 0x11:
> + return "Raspberry Pi 4 Model B";
> + default:
> + return "Unknown Raspberry Pi Model";
> + }
> +}
> +
> +STATIC
> +CHAR8*
> +EFIAPI
> +RpiFirmwareGetManufacturerName (
> + IN INTN ManufacturerId
> + )
> +{
> + UINT32 Revision;
> +
> + // If a negative ModelId is passed, detect it.
> + if ((ManufacturerId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
> + ManufacturerId = (Revision >> 16) & 0x0F;
> + }
> +
> + switch (ManufacturerId) {
> + // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + case 0x00:
> + return "Sony UK";
> + case 0x01:
> + return "Egoman";
> + case 0x02:
> + case 0x04:
> + return "Embest";
> + case 0x03:
> + return "Sony Japan";
> + case 0x05:
> + return "Stadium";
> + default:
> + return "Unknown Manufacturer";
> + }
> +}
> +
> +STATIC
> +CHAR8*
> +EFIAPI
> +RpiFirmwareGetCpuName (
> + IN INTN CpuId
> + )
> +{
> + UINT32 Revision;
> +
> + // If a negative CpuId is passed, detect it.
> + if ((CpuId < 0) && (RpiFirmwareGetModelRevision (&Revision) == EFI_SUCCESS)) {
> + CpuId = (Revision >> 12) & 0x0F;
> + }
> +
> + switch (CpuId) {
> + // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + case 0x00:
> + return "BCM2835 (ARM11)";
> + case 0x01:
> + return "BCM2836 (ARM Cortex-A7)";
> + case 0x02:
> + return "BCM2837 (ARM Cortex-A53)";
> + case 0x03:
> + return "BCM2711 (ARM Cortex-A72)";
> + default:
> + return "Unknown CPU Model";
> + }
> +}
> +
> #pragma pack()
> typedef struct {
> UINT32 Width;
> @@ -1009,6 +1159,10 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
> RpiFirmwareGetSerial,
> RpiFirmwareGetModel,
> RpiFirmwareGetModelRevision,
> + RpiFirmwareGetModelName,
> + RpiFirmwareGetFirmwareRevision,
> + RpiFirmwareGetManufacturerName,
> + RpiFirmwareGetCpuName,
> RpiFirmwareGetArmMemory
> };
>
> diff --git a/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h b/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
> index ec70f28efe1a..e49d6e6132d9 100644
> --- a/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
> +++ b/Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h
> @@ -96,6 +96,30 @@ EFI_STATUS
> UINT32 *Revision
> );
>
> +typedef
> +CHAR8*
> +(EFIAPI *GET_MODEL_NAME) (
> + INTN ModelId
> + );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *GET_FIRMWARE_REVISION) (
> + UINT32 *Revision
> + );
> +
> +typedef
> +CHAR8*
> +(EFIAPI *GET_MANUFACTURER_NAME) (
> + INTN ManufacturerId
> + );
> +
> +typedef
> +CHAR8*
> +(EFIAPI *GET_CPU_NAME) (
> + INTN CpuId
> + );
> +
> typedef
> EFI_STATUS
> (EFIAPI *GET_ARM_MEM) (
> @@ -104,21 +128,25 @@ EFI_STATUS
> );
>
> typedef struct {
> - SET_POWER_STATE SetPowerState;
> - GET_MAC_ADDRESS GetMacAddress;
> - GET_COMMAND_LINE GetCommandLine;
> - GET_CLOCK_RATE GetClockRate;
> - GET_CLOCK_RATE GetMaxClockRate;
> - GET_CLOCK_RATE GetMinClockRate;
> - SET_CLOCK_RATE SetClockRate;
> - GET_FB GetFB;
> - FREE_FB FreeFB;
> - GET_FB_SIZE GetFBSize;
> - SET_LED SetLed;
> - GET_SERIAL GetSerial;
> - GET_MODEL GetModel;
> - GET_MODEL_REVISION GetModelRevision;
> - GET_ARM_MEM GetArmMem;
> + SET_POWER_STATE SetPowerState;
> + GET_MAC_ADDRESS GetMacAddress;
> + GET_COMMAND_LINE GetCommandLine;
> + GET_CLOCK_RATE GetClockRate;
> + GET_CLOCK_RATE GetMaxClockRate;
> + GET_CLOCK_RATE GetMinClockRate;
> + SET_CLOCK_RATE SetClockRate;
> + GET_FB GetFB;
> + FREE_FB FreeFB;
> + GET_FB_SIZE GetFBSize;
> + SET_LED SetLed;
> + GET_SERIAL GetSerial;
> + GET_MODEL GetModel;
> + GET_MODEL_REVISION GetModelRevision;
> + GET_MODEL_NAME GetModelName;
> + GET_FIRMWARE_REVISION GetFirmwareRevision;
> + GET_MANUFACTURER_NAME GetManufacturerName;
> + GET_CPU_NAME GetCpuName;
> + GET_ARM_MEM GetArmMem;
> } RASPBERRY_PI_FIRMWARE_PROTOCOL;
>
> extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision Pete Batard
@ 2019-10-14 10:01 ` Philippe Mathieu-Daudé
2019-10-14 11:44 ` Pete Batard
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-14 10:01 UTC (permalink / raw)
To: devel, pete; +Cc: ard.biesheuvel, leif.lindholm
Hi Pete,
On 10/11/19 1:07 PM, Pete Batard wrote:
> The board revision is the proper channel to use to detect the amount of
> RAM available as bits [20-22] report the effective RAM size for the board
> starting with 256 MB (000b) and doubling in size for each value.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> index b5dcff897a59..5abc82b8d363 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
> )
> {
> EFI_STATUS Status;
> - UINT32 Base;
> - UINT32 Size;
> + UINT32 BoardRevision = 0;
>
> - Status = mFwProtocol->GetArmMem (&Base, &Size);
> + // Note: Type 19 addresses are expressed in KB, not bytes
> + mMemArrMapInfoType19.StartingAddress = 0;
Now you assume the ARM base RAM address is always 0, why?
> + // The minimum RAM size used on any Raspberry Pi model is 256 MB
> + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
> + Status = mFwProtocol->GetModelRevision (&BoardRevision);
> if (Status != EFI_SUCCESS) {
> - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n", Status));
> + DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size - defaulting to 256 MB: %r\n", Status));
> } else {
> - mMemArrMapInfoType19.StartingAddress = Base / 1024;
> - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
> + // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + // Bits [20-22] indicate the amount of memory starting with 256MB (000b)
> + // and doubling in size for each value (001b = 512 MB, 010b = 1GB, etc.)
> + mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) & 0x07;
> }
> + mMemArrMapInfoType19.EndingAddress -= 1;
>
> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19, mMemArrMapInfoType19Strings, NULL);
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-14 10:01 ` [edk2-devel] " Philippe Mathieu-Daudé
@ 2019-10-14 11:44 ` Pete Batard
2019-10-14 11:53 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Pete Batard @ 2019-10-14 11:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, devel; +Cc: ard.biesheuvel, leif.lindholm
Hi Philippe,
On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
> Hi Pete,
>
> On 10/11/19 1:07 PM, Pete Batard wrote:
>> The board revision is the proper channel to use to detect the amount of
>> RAM available as bits [20-22] report the effective RAM size for the board
>> starting with 256 MB (000b) and doubling in size for each value.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>
>> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c |
>> 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git
>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> index b5dcff897a59..5abc82b8d363 100644
>> ---
>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> +++
>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>> @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT32 Base;
>> - UINT32 Size;
>> + UINT32 BoardRevision = 0;
>> - Status = mFwProtocol->GetArmMem (&Base, &Size);
>> + // Note: Type 19 addresses are expressed in KB, not bytes
>> + mMemArrMapInfoType19.StartingAddress = 0;
>
> Now you assume the ARM base RAM address is always 0, why?
Because, in the case that is of interest to us here (Broadcom SoCs used
for the various Raspberry Pi platforms), this is what the documentation
says.
If you look at something like
https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
(which, as per the Pi Foundation, also applies to the later models) and
especially the memory layout graphic that you see early in the document,
you can find that the ARM base RAM address is indeed set to 0 always.
At this stage, we have no reason to think that we are going to contend
with a model where the RAM base address isn't 0.
Regards,
/Pete
>> + // The minimum RAM size used on any Raspberry Pi model is 256 MB
>> + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
>> + Status = mFwProtocol->GetModelRevision (&BoardRevision);
>> if (Status != EFI_SUCCESS) {
>> - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n",
>> Status));
>> + DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size -
>> defaulting to 256 MB: %r\n", Status));
>> } else {
>> - mMemArrMapInfoType19.StartingAddress = Base / 1024;
>> - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
>> + //
>> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
>>
>> + // Bits [20-22] indicate the amount of memory starting with 256MB
>> (000b)
>> + // and doubling in size for each value (001b = 512 MB, 010b =
>> 1GB, etc.)
>> + mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) & 0x07;
>> }
>> + mMemArrMapInfoType19.EndingAddress -= 1;
>> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
>> mMemArrMapInfoType19Strings, NULL);
>> }
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-14 11:44 ` Pete Batard
@ 2019-10-14 11:53 ` Philippe Mathieu-Daudé
2019-10-14 12:03 ` Pete Batard
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-14 11:53 UTC (permalink / raw)
To: devel, pete; +Cc: ard.biesheuvel, leif.lindholm
On 10/14/19 1:44 PM, Pete Batard wrote:
> Hi Philippe,
>
> On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
>> Hi Pete,
>>
>> On 10/11/19 1:07 PM, Pete Batard wrote:
>>> The board revision is the proper channel to use to detect the amount of
>>> RAM available as bits [20-22] report the effective RAM size for the
>>> board
>>> starting with 256 MB (000b) and doubling in size for each value.
>>>
>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>> ---
>>> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>> | 18 ++++++++++++------
>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git
>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>
>>> index b5dcff897a59..5abc82b8d363 100644
>>> ---
>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>
>>> +++
>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>
>>> @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
>>> )
>>> {
>>> EFI_STATUS Status;
>>> - UINT32 Base;
>>> - UINT32 Size;
>>> + UINT32 BoardRevision = 0;
>>> - Status = mFwProtocol->GetArmMem (&Base, &Size);
>>> + // Note: Type 19 addresses are expressed in KB, not bytes
>>> + mMemArrMapInfoType19.StartingAddress = 0;
>>
>> Now you assume the ARM base RAM address is always 0, why?
>
> Because, in the case that is of interest to us here (Broadcom SoCs used
> for the various Raspberry Pi platforms), this is what the documentation
> says.
>
> If you look at something like
> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> (which, as per the Pi Foundation, also applies to the later models) and
> especially the memory layout graphic that you see early in the document,
> you can find that the ARM base RAM address is indeed set to 0 always.
>
> At this stage, we have no reason to think that we are going to contend
> with a model where the RAM base address isn't 0.
OK. I have no idea what are Broadcom plans, I read somewhere they were
going to use tricks to allow more than 4GB of RAM on the Raspberry Pi 4,
so I was wondering, since they provide a firmware API call to get the
RAM base address.
Maybe you can add a comment that we expect all following boards to use
RAM base at 0 (simply replying to this email, and eventually Leif would
amend it previous to push).
Otherwise your patch looks OK.
Regards,
Phil.
>>> + // The minimum RAM size used on any Raspberry Pi model is 256 MB
>>> + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
>>> + Status = mFwProtocol->GetModelRevision (&BoardRevision);
>>> if (Status != EFI_SUCCESS) {
>>> - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n",
>>> Status));
>>> + DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size -
>>> defaulting to 256 MB: %r\n", Status));
>>> } else {
>>> - mMemArrMapInfoType19.StartingAddress = Base / 1024;
>>> - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
>>> + //
>>> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
>>>
>>> + // Bits [20-22] indicate the amount of memory starting with
>>> 256MB (000b)
>>> + // and doubling in size for each value (001b = 512 MB, 010b =
>>> 1GB, etc.)
>>> + mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) &
>>> 0x07;
>>> }
>>> + mMemArrMapInfoType19.EndingAddress -= 1;
>>> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
>>> mMemArrMapInfoType19Strings, NULL);
>>> }
>>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-14 11:53 ` Philippe Mathieu-Daudé
@ 2019-10-14 12:03 ` Pete Batard
2019-10-14 12:08 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Pete Batard @ 2019-10-14 12:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, devel; +Cc: ard.biesheuvel, leif.lindholm
On 2019.10.14 12:53, Philippe Mathieu-Daudé wrote:
> On 10/14/19 1:44 PM, Pete Batard wrote:
>> Hi Philippe,
>>
>> On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
>>> Hi Pete,
>>>
>>> On 10/11/19 1:07 PM, Pete Batard wrote:
>>>> The board revision is the proper channel to use to detect the amount of
>>>> RAM available as bits [20-22] report the effective RAM size for the
>>>> board
>>>> starting with 256 MB (000b) and doubling in size for each value.
>>>>
>>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>> ---
>>>> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>> | 18 ++++++++++++------
>>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git
>>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>
>>>> index b5dcff897a59..5abc82b8d363 100644
>>>> ---
>>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>
>>>> +++
>>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>
>>>> @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
>>>> )
>>>> {
>>>> EFI_STATUS Status;
>>>> - UINT32 Base;
>>>> - UINT32 Size;
>>>> + UINT32 BoardRevision = 0;
>>>> - Status = mFwProtocol->GetArmMem (&Base, &Size);
>>>> + // Note: Type 19 addresses are expressed in KB, not bytes
Comment to add:
// The memory layout used in all known Pi SoC's starts at 0
>>>> + mMemArrMapInfoType19.StartingAddress = 0;
>>>
>>> Now you assume the ARM base RAM address is always 0, why?
>>
>> Because, in the case that is of interest to us here (Broadcom SoCs
>> used for the various Raspberry Pi platforms), this is what the
>> documentation says.
>>
>> If you look at something like
>> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>> (which, as per the Pi Foundation, also applies to the later models)
>> and especially the memory layout graphic that you see early in the
>> document, you can find that the ARM base RAM address is indeed set to
>> 0 always.
>>
>> At this stage, we have no reason to think that we are going to contend
>> with a model where the RAM base address isn't 0.
>
> OK. I have no idea what are Broadcom plans, I read somewhere they were
> going to use tricks to allow more than 4GB of RAM on the Raspberry Pi 4,
> so I was wondering, since they provide a firmware API call to get the
> RAM base address.
Well the thing is, this patch comes straight from work that is being
carried out to add support for the Pi 4 to edk2-platforms. So we pretty
much already validated that 0 is what needs to be used for the Pi 4 as
well...
> Maybe you can add a comment that we expect all following boards to use
> RAM base at 0 (simply replying to this email, and eventually Leif would
> amend it previous to push).
Sure. Comment added above.
Regards,
/Pete
>
> Otherwise your patch looks OK.
>
> Regards,
>
> Phil.
>
>>>> + // The minimum RAM size used on any Raspberry Pi model is 256 MB
>>>> + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
>>>> + Status = mFwProtocol->GetModelRevision (&BoardRevision);
>>>> if (Status != EFI_SUCCESS) {
>>>> - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n",
>>>> Status));
>>>> + DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size -
>>>> defaulting to 256 MB: %r\n", Status));
>>>> } else {
>>>> - mMemArrMapInfoType19.StartingAddress = Base / 1024;
>>>> - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
>>>> + //
>>>> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
>>>>
>>>> + // Bits [20-22] indicate the amount of memory starting with
>>>> 256MB (000b)
>>>> + // and doubling in size for each value (001b = 512 MB, 010b =
>>>> 1GB, etc.)
>>>> + mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) &
>>>> 0x07;
>>>> }
>>>> + mMemArrMapInfoType19.EndingAddress -= 1;
>>>> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
>>>> mMemArrMapInfoType19Strings, NULL);
>>>> }
>>>>
>>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-14 12:03 ` Pete Batard
@ 2019-10-14 12:08 ` Philippe Mathieu-Daudé
2019-10-14 12:17 ` Leif Lindholm
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-14 12:08 UTC (permalink / raw)
To: Pete Batard, devel; +Cc: ard.biesheuvel, leif.lindholm
On 10/14/19 2:03 PM, Pete Batard wrote:
> On 2019.10.14 12:53, Philippe Mathieu-Daudé wrote:
>> On 10/14/19 1:44 PM, Pete Batard wrote:
>>> Hi Philippe,
>>>
>>> On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
>>>> Hi Pete,
>>>>
>>>> On 10/11/19 1:07 PM, Pete Batard wrote:
>>>>> The board revision is the proper channel to use to detect the
>>>>> amount of
>>>>> RAM available as bits [20-22] report the effective RAM size for the
>>>>> board
>>>>> starting with 256 MB (000b) and doubling in size for each value.
>>>>>
>>>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>>> ---
>>>>> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>> | 18 ++++++++++++------
>>>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>>
>>>>> index b5dcff897a59..5abc82b8d363 100644
>>>>> ---
>>>>> a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>>
>>>>> +++
>>>>> b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
>>>>>
>>>>> @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
>>>>> )
>>>>> {
>>>>> EFI_STATUS Status;
>>>>> - UINT32 Base;
>>>>> - UINT32 Size;
>>>>> + UINT32 BoardRevision = 0;
>>>>> - Status = mFwProtocol->GetArmMem (&Base, &Size);
>>>>> + // Note: Type 19 addresses are expressed in KB, not bytes
>
> Comment to add:
>
> // The memory layout used in all known Pi SoC's starts at 0
Thanks, Leif do you mind amending this comment?
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>>>> + mMemArrMapInfoType19.StartingAddress = 0;
>>>>
>>>> Now you assume the ARM base RAM address is always 0, why?
>>>
>>> Because, in the case that is of interest to us here (Broadcom SoCs
>>> used for the various Raspberry Pi platforms), this is what the
>>> documentation says.
>>>
>>> If you look at something like
>>> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>>> (which, as per the Pi Foundation, also applies to the later models)
>>> and especially the memory layout graphic that you see early in the
>>> document, you can find that the ARM base RAM address is indeed set to
>>> 0 always.
>>>
>>> At this stage, we have no reason to think that we are going to
>>> contend with a model where the RAM base address isn't 0.
>>
>> OK. I have no idea what are Broadcom plans, I read somewhere they were
>> going to use tricks to allow more than 4GB of RAM on the Raspberry Pi
>> 4, so I was wondering, since they provide a firmware API call to get
>> the RAM base address.
>
> Well the thing is, this patch comes straight from work that is being
> carried out to add support for the Pi 4 to edk2-platforms. So we pretty
> much already validated that 0 is what needs to be used for the Pi 4 as
> well...
OK, good news :)
>> Maybe you can add a comment that we expect all following boards to use
>> RAM base at 0 (simply replying to this email, and eventually Leif
>> would amend it previous to push).
>
> Sure. Comment added above.
Thanks!
> Regards,
>
> /Pete
>
>>
>> Otherwise your patch looks OK.
>>
>> Regards,
>>
>> Phil.
>>
>>>>> + // The minimum RAM size used on any Raspberry Pi model is 256 MB
>>>>> + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
>>>>> + Status = mFwProtocol->GetModelRevision (&BoardRevision);
>>>>> if (Status != EFI_SUCCESS) {
>>>>> - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory size: %r\n",
>>>>> Status));
>>>>> + DEBUG ((DEBUG_WARNING, "Couldn't get the board memory size -
>>>>> defaulting to 256 MB: %r\n", Status));
>>>>> } else {
>>>>> - mMemArrMapInfoType19.StartingAddress = Base / 1024;
>>>>> - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
>>>>> + //
>>>>> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
>>>>>
>>>>> + // Bits [20-22] indicate the amount of memory starting with
>>>>> 256MB (000b)
>>>>> + // and doubling in size for each value (001b = 512 MB, 010b =
>>>>> 1GB, etc.)
>>>>> + mMemArrMapInfoType19.EndingAddress <<= (BoardRevision >> 20) &
>>>>> 0x07;
>>>>> }
>>>>> + mMemArrMapInfoType19.EndingAddress -= 1;
>>>>> LogSmbiosData ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
>>>>> mMemArrMapInfoType19Strings, NULL);
>>>>> }
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
2019-10-14 12:08 ` Philippe Mathieu-Daudé
@ 2019-10-14 12:17 ` Leif Lindholm
0 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2019-10-14 12:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Pete Batard, devel, ard.biesheuvel
On Mon, Oct 14, 2019 at 02:08:10PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/14/19 2:03 PM, Pete Batard wrote:
> > On 2019.10.14 12:53, Philippe Mathieu-Daudé wrote:
> > > On 10/14/19 1:44 PM, Pete Batard wrote:
> > > > Hi Philippe,
> > > >
> > > > On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
> > > > > Hi Pete,
> > > > >
> > > > > On 10/11/19 1:07 PM, Pete Batard wrote:
> > > > > > The board revision is the proper channel to use to
> > > > > > detect the amount of
> > > > > > RAM available as bits [20-22] report the effective RAM
> > > > > > size for the board
> > > > > > starting with 256 MB (000b) and doubling in size for each value.
> > > > > >
> > > > > > Signed-off-by: Pete Batard <pete@akeo.ie>
> > > > > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > > > > > ---
> > > > > > Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > > | 18 ++++++++++++------
> > > > > > 1 file changed, 12 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > >
> > > > > > index b5dcff897a59..5abc82b8d363 100644
> > > > > > --- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > >
> > > > > > +++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > >
> > > > > > @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
> > > > > > )
> > > > > > {
> > > > > > EFI_STATUS Status;
> > > > > > - UINT32 Base;
> > > > > > - UINT32 Size;
> > > > > > + UINT32 BoardRevision = 0;
> > > > > > - Status = mFwProtocol->GetArmMem (&Base, &Size);
> > > > > > + // Note: Type 19 addresses are expressed in KB, not bytes
> >
> > Comment to add:
> >
> > // The memory layout used in all known Pi SoC's starts at 0
>
> Thanks, Leif do you mind amending this comment?
No issue with that.
Have the rest of today off, but wil get back to this set tomorrow
morning.
/
Leif
> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>
> > > > > > + mMemArrMapInfoType19.StartingAddress = 0;
> > > > >
> > > > > Now you assume the ARM base RAM address is always 0, why?
> > > >
> > > > Because, in the case that is of interest to us here (Broadcom
> > > > SoCs used for the various Raspberry Pi platforms), this is what
> > > > the documentation says.
> > > >
> > > > If you look at something like https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> > > > (which, as per the Pi Foundation, also applies to the later
> > > > models) and especially the memory layout graphic that you see
> > > > early in the document, you can find that the ARM base RAM
> > > > address is indeed set to 0 always.
> > > >
> > > > At this stage, we have no reason to think that we are going to
> > > > contend with a model where the RAM base address isn't 0.
> > >
> > > OK. I have no idea what are Broadcom plans, I read somewhere they
> > > were going to use tricks to allow more than 4GB of RAM on the
> > > Raspberry Pi 4, so I was wondering, since they provide a firmware
> > > API call to get the RAM base address.
> >
> > Well the thing is, this patch comes straight from work that is being
> > carried out to add support for the Pi 4 to edk2-platforms. So we pretty
> > much already validated that 0 is what needs to be used for the Pi 4 as
> > well...
>
> OK, good news :)
>
> > > Maybe you can add a comment that we expect all following boards to
> > > use RAM base at 0 (simply replying to this email, and eventually
> > > Leif would amend it previous to push).
> >
> > Sure. Comment added above.
>
> Thanks!
>
> > Regards,
> >
> > /Pete
> >
> > >
> > > Otherwise your patch looks OK.
> > >
> > > Regards,
> > >
> > > Phil.
> > >
> > > > > > + // The minimum RAM size used on any Raspberry Pi model is 256 MB
> > > > > > + mMemArrMapInfoType19.EndingAddress = 256 * 1024;
> > > > > > + Status = mFwProtocol->GetModelRevision (&BoardRevision);
> > > > > > if (Status != EFI_SUCCESS) {
> > > > > > - DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory
> > > > > > size: %r\n", Status));
> > > > > > + DEBUG ((DEBUG_WARNING, "Couldn't get the board
> > > > > > memory size - defaulting to 256 MB: %r\n", Status));
> > > > > > } else {
> > > > > > - mMemArrMapInfoType19.StartingAddress = Base / 1024;
> > > > > > - mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
> > > > > > + // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> > > > > >
> > > > > > + // Bits [20-22] indicate the amount of memory
> > > > > > starting with 256MB (000b)
> > > > > > + // and doubling in size for each value (001b = 512
> > > > > > MB, 010b = 1GB, etc.)
> > > > > > + mMemArrMapInfoType19.EndingAddress <<=
> > > > > > (BoardRevision >> 20) & 0x07;
> > > > > > }
> > > > > > + mMemArrMapInfoType19.EndingAddress -= 1;
> > > > > > LogSmbiosData
> > > > > > ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
> > > > > > mMemArrMapInfoType19Strings, NULL);
> > > > > > }
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
` (4 preceding siblings ...)
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision Pete Batard
@ 2019-10-15 20:00 ` Leif Lindholm
5 siblings, 0 replies; 14+ messages in thread
From: Leif Lindholm @ 2019-10-15 20:00 UTC (permalink / raw)
To: Pete Batard; +Cc: devel, ard.biesheuvel
On Fri, Oct 11, 2019 at 12:07:41PM +0100, Pete Batard wrote:
> Changes from v2:
> - Consistent use of curly brackets for if statements
> - Remove unused variables that may produce an issue with git bisect
> - Use gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor to populate vendor
>
> Note that 5/5, which has been R-b, is unchanged.
For the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 48a95de81ede..2a4e369d588e.
Thanks!
/
Leif
> Pete Batard (5):
> Platform/RPi3/RpiFirmwareDxe: Add more query functions
> Platform/RPi3/RpiFirmwareDxe: Improve serial number population
> Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries
> Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD
> Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision
>
> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c | 198 ++++++++++++--------
> Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.inf | 2 +
> Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 167 ++++++++++++++++-
> Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58 ++++--
> Platform/RaspberryPi/RPi3/RPi3.dsc | 4 +-
> 5 files changed, 333 insertions(+), 96 deletions(-)
>
> --
> 2.21.0.windows.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-15 20:00 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-11 11:07 [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions Pete Batard
2019-10-14 9:55 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 3/5] Platform/RPi3/PlatformSmbiosDxe: Improve population of SMBIOS entries Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 4/5] Platform/RPi3/PlatformSmbiosDxe: Populate BIOS major/minor from PCD Pete Batard
2019-10-11 11:07 ` [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision Pete Batard
2019-10-14 10:01 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-14 11:44 ` Pete Batard
2019-10-14 11:53 ` Philippe Mathieu-Daudé
2019-10-14 12:03 ` Pete Batard
2019-10-14 12:08 ` Philippe Mathieu-Daudé
2019-10-14 12:17 ` Leif Lindholm
2019-10-15 20:00 ` [edk2-platforms][PATCH v3 0/5] Various SMBIOS improvements Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox