Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Initial support for LS1028A. Booting into OCRAM app. #306

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Initial support for LS1028A. Booting into OCRAM app. #306

wants to merge 4 commits into from

Conversation

billphipps
Copy link
Contributor

Still getting through some problems. Here's what's working:

  1. Booting without TPM into OCRAM-based app. Used NO_XIP=1 to make wolfBoot copy into the load address
  2. Booting with TPM sealing is working, except I haven't gotten the initial key into the TPM yet, so it fails to unseal and verify the signature. TPM access is working fine.
  3. I updated spi_drv_nxp to gate on platform. There are some shenanigans that should be cleaner up as we stabilize the API.

Here's some hacks that I wasn't sure about:

  1. Updated stack size usage test in options.mk because it checks for TPM first (limits to 6k) but with SP-math==0, we really need 7.5k. This needs a systemic update to handle max() of the options.
  2. Ignored DTS stuff for now. Didn't seem to cause an issue.
  3. With TPM enabled, there's a duplication of some functions with wolf crypt/misc.c. I added a guard on WOLFBOOT_TPM.
  4. Skipped getting the DDR working for reals as we are targeting a simple OCRAM app for now.
  5. Updated the wolfTPM submodule to the latest and greatest. This got sealing working.
  6. I snuck the wolfBoot and app into the SPI NOR flash after the RCW and before the uBoot. Simply updated the start address in the RCW to point to wolf boot instead of uBoot. Can easily swap it back to get into uBoot and linux. Likely need this to preload the TPM key...

Here's the list of things that are broke

  1. Measured boot now fails to compile. Likely a simple typo somewhere, but I didn't dig into it too far.
  2. Test app is still likely still running at EL3. This must be pushed down.
  3. MMU tables are likely not correct for anything past 512MB. Need to use the ones from CW
  4. Documentation is still out of date. Prepping slides tomorrow (today) to add pictures of what we are doing and what we should/could do instead.
  5. Still need to clean up the initial startup sequence and get it closer to the fully functions CW version.

Overall, pretty darn close and likely suitable for the customer demo. Glide path seems clear on the technical issues.

dgarske
dgarske requested changes May 11, 2023
View reviewed changes
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just cleanups, but otherwise okay to merge.

docs/Targets.md
@@ -13,6 +13,7 @@ This README describes configuration of supported targets.
* [NXP P1021 PPC](#nxp-p1021-ppc)
* [NXP T2080 PPC](#nxp-t2080-ppc)
* [Qemu x86-64 UEFI](#qemu-x86-64-uefi)
* [NXP LS1028A](#nxp-ls1028a)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move up one line to sort with other NXP targets.

hal/nxp_ls1028a.c
@@ -0,0 +1,1370 @@
/* ls1028a.c
*
* Copyright (C) 2021 wolfSSL Inc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2023

hal/nxp_ls1028a.c
#define XSPI_MCR_LEARNEN_MASK (0x1 << 15) /* XSPI Learn Mode Enable */

/* XSPI Init */
/*#define XSPI_MCR0_CFG 0xFFFF80C0*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup? Remove this or comment on why MCR0 has possible variation

hal/nxp_ls1028a.c

void erratum_err050568()
{
/* Use IP bus only if systembus PLL is 300MHz (Dont use 300MHz) */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do a replace of tab with 4 spaces please.

hal/nxp_ls1028a.c
}

/* Application on Serial NOR Flash device 18.6.3 */
void xspi_init()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

void xspi_init(void)

hal/nxp_ls1028a.c
}

/* Called from boot_aarch64_start.S */
void hal_ddr_init() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function brace always on new line. Thanks

options.mk
@@ -65,7 +65,8 @@ ifeq ($(SIGN),ECC256)
STACK_USAGE=4096
else
ifeq ($(WOLFTPM),1)
STACK_USAGE=6680
## Was 6680. Increasing to handle SPMATH=0
STACK_USAGE=7600
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay to increase or gate on the SPMATH the two options.

src/boot_aarch64_start.S
@@ -99,3 +107,255 @@ gicv2_init_secure:
str w0, [x1, #4] /* GICC_PMR */
1:
ret


.global invalidate_ivac
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is shared with a few other targets. Make sure any changes can be tolerated. zynqmp and raspi.

@billphipps billphipps requested a review from jpbland1 May 11, 2023 16:18
jpbland1
jpbland1 reviewed May 11, 2023
View reviewed changes
config/examples/nxp-ls1028a.config Outdated
DUALBANK_SWAP?=0
PKA?=0

WOLFTPM?=0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TPM and WOLFBOOT_TPM_KEYSTORE are disabled, why define the NV indicies if it's disabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a config that I was using without TPM support. Note compilation fails if ONLY WOLFBOOT_TPM is enabled without MEASURED_BOOT nor TPM_KEYSTORE, which was a suprise

jpbland1
jpbland1 requested changes May 18, 2023
View reviewed changes
Copy link
Contributor

@jpbland1 jpbland1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDE/Renesas/e2studio/RA6M4/app_RA/src/SEGGER_RTT/myprint.c empty file

jpbland1
jpbland1 reviewed May 18, 2023
View reviewed changes
include/wolfboot/wolfboot.h Outdated
@@ -195,7 +195,11 @@ extern "C" {

/* KeyStore API */
int keystore_num_pubkeys(void);
#if defined(WOLFBOOT_RENESAS_SCEPROTECT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't do this, just cast your buffer to uint32_t* in one place if you need it in that form

@dgarske dgarske mentioned this pull request Aug 10, 2023
Closed
@dgarske dgarske assigned dgarske and unassigned billphipps Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpbland1 jpbland1 jpbland1 requested changes

@dgarske dgarske dgarske requested changes

@danielinux danielinux Awaiting requested review from danielinux

Requested changes must be addressed to merge this pull request.

Assignees

@dgarske dgarske

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@billphipps @dgarske @jpbland1