Platform/Qualcomm/NordBoardPkg: add Nord Generic board#1012
Conversation
| (FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet64 (PcdSystemMemorySize)) | ||
| - (FixedPcdGet64 (PcdTrace32DdrBase) + FixedPcdGet64 (PcdTrace32DdrSize)) | ||
| ); | ||
| // |
There was a problem hiding this comment.
Make this T32 block conditional ?
There was a problem hiding this comment.
I suppose you mean here to make T32 support only available in edk2 DEBUG builds, right?
@vishalo do you have any comments on this with respect to Glymur?
There was a problem hiding this comment.
Commit: 50d3bd4 now builds t32 under debug builds only.
There was a problem hiding this comment.
This allows us to load symbols on t32 simulator from crashdumps. We have plenty of cases where we have to debug RELEASE builds. Let's keep this.
There was a problem hiding this comment.
A platform can still override this in it's own .dsc but we can't enable JTAG in the release builds by default. JTAG is also disabled by default in default secure boot policy.
|
Update: rather than IQ10 EVK specific board package, renamed it to |
trsoni
left a comment
There was a problem hiding this comment.
so we don't want to add Assisted-by AI tag?
I don't believe the contribution guidelines for edk2 requires it at this point. |
Sorry but it looks odd that Qualcomm is not declaring that anywhere in PR. It is not about policy of the project but more transparency I think could help. |
Updated PR description to mention it. |
We also have upstream projects that do ask explicitly to not add the assisted tag (expecting the submitter to completely own the work, even if done with help from AI), that is why our internal guidelines (check the guidelines from OSDO) suggests following the upstream project policies. |
|
Commit df14b8f has been added to introduce |
Thank you. I have also filed the github issue #1014 . I don't see much traction on the past github issues though in this project, I can send it to mailing list if that is most desired to initiate the discussion. |
QcomPlatformPkg.dec only declared the UFS register-window PCDs used by QcomUfsHcDxe. Declare them in QualcommPlatformPkg as PcdUfsHcMmioBase/Size (UINT64), point QcomUfsHcDxe and RB3Gen2 at them, drop the unused QcomPlatformPkg dependency from ArmPlatformLib, and delete the package. No functional change. Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
|
While refactoring changes further found another duplicated package, removed that as part of this 0541364. |
|
@vishalo mind a review? |
Thank you @b49020 and @ricardosalveti . Hi @ricardosalveti, some of these patches don't align with our internal discussions. Let's reconvene and work through them together to make sure we're aligned before moving forward. |
Sure, but what is actually conflicting here? Would be nice if you could still help reviewing the PR anyway. |
vishalo
left a comment
There was a problem hiding this comment.
Can we keep this around, till we have plan on where things go?
| # instance for RELEASE builds. | ||
| # | ||
| !if $(TARGET) == DEBUG | ||
| PeCoffExtraActionLib|QualcommPlatformPkg/Library/DebugPecoffExtraActionTrace32Lib/DebugPecoffExtraActionTrace32Lib.inf |
There was a problem hiding this comment.
We have plenty of cases where we have to debug RELEASE builds.
There was a problem hiding this comment.
Then keep it platform specific where needed.
| (FixedPcdGet64 (PcdFdBaseAddress) + FixedPcdGet64 (PcdSystemMemorySize)) | ||
| - (FixedPcdGet64 (PcdTrace32DdrBase) + FixedPcdGet64 (PcdTrace32DdrSize)) | ||
| ); | ||
| // |
There was a problem hiding this comment.
This allows us to load symbols on t32 simulator from crashdumps. We have plenty of cases where we have to debug RELEASE builds. Let's keep this.
| mMemRegions[mNumMemRegions].CacheAttributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; | ||
| mNumMemRegions++; | ||
| /* IMEM Cookies (only when the platform configured the region) */ | ||
| if ((FixedPcdGet64 (PcdIMemCookiesBase) != 0) && (FixedPcdGet64 (PcdIMemCookiesSize) != 0)) { |
There was a problem hiding this comment.
If we need to do this, it is better to use FeaturePcd, and not rely on address PCD. How about remove it once DT support is enabled ?
There was a problem hiding this comment.
Switched to use FeaturePcd for all the optional memory region mapping. And we shouldn't enforce boot DT for every platform, Pcds are the de-facto standard ways of edk2 configuration so we should support that option too.
| @retval Other Pointer to the validated table. | ||
| **/ | ||
| STATIC | ||
| USABLE_RAM_PARTITION_TABLE * |
There was a problem hiding this comment.
Reworked for SmemLib to abstract out the RamPartitionTable.
As I see it is full-fledged SMEM driver with a lot of redundant functionality not needed for the Nord board support. So, there isn't a reason to derail Nord board support just on the SMEM PR. For sure, the SMEM driver can be extended later to add functionality as and when required.
| VERSION_STRING = 1.0 | ||
|
|
||
| [Binaries] | ||
| BIN|nord-board.dtb |
There was a problem hiding this comment.
This is not ideal - we will have to recompile UEFI if dtb requires update. Downstream has partition-based solution, we can enable that.
There was a problem hiding this comment.
This can be supported until we have the partition-based implementation available and merged here, shouldn't be a problem to merge this as is and later improve.
There was a problem hiding this comment.
Happy to work with you to get those pieces enabled ..
There was a problem hiding this comment.
No please we don't want to enforce partition-based implementation for every use-case. It is already implemented in non-standard way to use EFI signatures on top of U-Boot FIT blob for authentication.
We really have to review it carefully to have a partition based solution upstream aligning with Arm SystemReady practices.
Gate the optional memory regions ArmPlatformLib maps (SMEM, boot DT, IMEM cookies and the Trace32 DDR debug buffer) on per-region boolean FeaturePcds instead of inferring "present" from a non-zero base/size, which conflated a valid base of 0 with "disabled" and could not be overridden per board. Each flag defaults to FALSE so a board maps only what it provides. Also add PcdUfsHcEnable and map the UFS register window under it, which NonDiscoverablePciDeviceDxe does not map itself. Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
9870020 to
a62fce2
Compare
Add a functional SmemLib that reads SMEM items by item id so RamPartitionTableLib can obtain the usable RAM partition table; previously only SmemLibNull existed. The library is a generic SMEM item accessor with no knowledge of any item's payload, so the usable RAM partition table's magic validation and fallback scan live in the consumer, RamPartitionTableLib. Move the shared RamPartition.h from QualcommPlatformPkg into QualcommSiliconPkg (consumers already depend on it) and bound the fallback scan with PcdSmemSize. Co-developed-by: Tyler Baker <tyler.baker@oss.qualcomm.com> Signed-off-by: Tyler Baker <tyler.baker@oss.qualcomm.com> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
RB3Gen2's OemMiscLib is board-agnostic (it reads platform PCDs). Move it into the shared QualcommPlatformPkg so other Qualcomm boards can reuse it, and point RB3Gen2 at it. Also migrate the SMBIOS OEM PCDs it consumes from KodiakPkg to QualcommPlatformPkg so the shared library no longer depends on a SoC package. No functional change. Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Move the reusable MinPlatform pieces out of GlymurOpenBoardPkg into a new Platform/Qualcomm/QualcommMinPlatformPkg so multiple Qualcomm boards can share them, and migrate GlymurOpenBoardPkg onto it: - the Stage1-4 DSC skeleton, and - the flash-map include, which now derives the FV/NV offsets and base addresses from the per-board sizes each board FDF sets. Board-specific content (DSC overrides and the flash sizes/NV bases) stays in the board. No functional change: the firmware volume is identical. Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
…ilds The Trace32 PeCoffExtraActionLib emits JTAG hooks only useful with a Trace32 debugger. Gate it behind $(TARGET) == DEBUG and use BasePeCoffExtraActionLibNull for RELEASE. Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add the Qualcomm Nord Generic board port, built on the MinPlatform staged-boot model and the QualcommPlatformPkg/QualcommSiliconPkg foundation. It runs as the TF-A BL33 payload and boots to the OS; the name is generic so it can host further Nord-family boards. The staged-boot Stage1-4 skeleton comes from the shared QualcommMinPlatformPkg; Nord ships its .dsc/.fdf/.dec, board-local flash map, BoardInitLib and DeviceTree (USER_PROVIDED_DTB), and reuses the shared OemMiscLib. ArmPlatformLib sizes DRAM from the SMEM RAM partition table, so no board-specific memory drivers are needed. Co-developed-by: Tyler Baker <tyler.baker@oss.qualcomm.com> Signed-off-by: Tyler Baker <tyler.baker@oss.qualcomm.com> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add the Qualcomm Nord Generic board port, built on the MinPlatform staged-boot model (PcdBootStage, Core{Common,Pei,Dxe}Lib, Stage1-4 includes, BoardInitLib, PlatformInitPei/ReportFvPei) and the QualcommPlatformPkg / QualcommSiliconPkg foundation. It runs as the TF-A BL33 payload (SEC first in a multi-FV FD at the BL33 base) and boots to the OS.
The package is named generically (NordBoardPkg) so it can host further
Qualcomm Nord-family boards.
Included:
The memory map needs no board-specific drivers: ArmPlatformLib describes system DRAM (including the upper banks) from the SMEM RAM partition table during PEI, and the OS reserved-memory carveouts are carried in the
device tree.
Developed in an AI assisted workflow.