Skip to content

Commit a06c05c

Browse files
sasdfsasdf
committed
[cov,test] Fix hardcoded flash layout constants
The flash layout constants in several tests were hardcoded, assuming a 64K ROM_EXT. This caused tests to fail when linked for coverage, as the coverage-enabled ROM_EXT is 128K. This change replaces the hardcoded constants with calculated values or named constants that account for the size of the ROM_EXT. Change-Id: I29f0796eec59d4fd9c70fdb0b72d3c70f17da9a4 Signed-off-by: Yi-Hsuan Deng <[email protected]>
1 parent 35d271d commit a06c05c

File tree

6 files changed

+200
-37
lines changed

6 files changed

+200
-37
lines changed

‎sw/device/silicon_creator/lib/ownership/BUILD‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,9 @@ cc_library(
256256
":datatypes",
257257
":owner_block",
258258
":ownership",
259+
"//hw/top_earlgrey/ip_autogen/flash_ctrl:flash_ctrl_c_regs",
259260
"//sw/device/silicon_creator/lib:boot_data",
261+
"//sw/device/silicon_creator/lib/base:chip",
260262
"//sw/device/silicon_creator/lib/boot_svc:boot_svc_msg",
261263
"//sw/device/silicon_creator/lib/drivers:flash_ctrl",
262264
"//sw/device/silicon_creator/lib/ownership/keys/fake:includes",
@@ -275,7 +277,9 @@ cc_library(
275277
":datatypes",
276278
":owner_block",
277279
":ownership",
280+
"//hw/top_earlgrey/ip_autogen/flash_ctrl:flash_ctrl_c_regs",
278281
"//sw/device/silicon_creator/lib:boot_data",
282+
"//sw/device/silicon_creator/lib/base:chip",
279283
"//sw/device/silicon_creator/lib/boot_svc:boot_svc_msg",
280284
"//sw/device/silicon_creator/lib/drivers:flash_ctrl",
281285
"//sw/device/silicon_creator/lib/ownership/keys/fake:includes",

‎sw/device/silicon_creator/lib/ownership/test_owner.c‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "sw/device/lib/base/hardened_memory.h"
66
#include "sw/device/lib/base/macros.h"
77
#include "sw/device/lib/base/memory.h"
8+
#include "sw/device/silicon_creator/lib/base/chip.h"
89
#include "sw/device/silicon_creator/lib/boot_svc/boot_svc_msg.h"
910
#include "sw/device/silicon_creator/lib/dbg_print.h"
1011
#include "sw/device/silicon_creator/lib/drivers/flash_ctrl.h"
@@ -26,6 +27,8 @@
2627
#include "sw/device/silicon_creator/lib/ownership/ownership_key.h"
2728
#include "sw/device/silicon_creator/lib/rescue/rescue.h"
2829

30+
#include "flash_ctrl_regs.h"
31+
2932
/*
3033
* This module overrides the weak `sku_creator_owner_init` symbol in
3134
* ownership.c, thus allowing FPGA builds to boot in the `LockedOwner` state
@@ -136,12 +139,16 @@
136139
kBootSvcOwnershipUnlockReqType,
137140
#endif
138141
#ifndef WITH_RESCUE_START
139-
#define WITH_RESCUE_START (32)
142+
#define WITH_RESCUE_START (kRomExtSizeInPages)
140143
#endif
141144
#ifndef WITH_RESCUE_SIZE
142-
#define WITH_RESCUE_SIZE (224)
145+
#define WITH_RESCUE_SIZE (256 - kRomExtSizeInPages)
143146
#endif
144147

148+
enum {
149+
kRomExtSizeInPages = CHIP_ROM_EXT_SIZE_MAX / FLASH_CTRL_PARAM_BYTES_PER_PAGE,
150+
};
151+
145152
rom_error_t sku_creator_owner_init(boot_data_t *bootdata) {
146153
#ifdef TEST_FAULT_NO_OWNER
147154
return kErrorOwnershipNoOwner;

‎sw/device/silicon_creator/rom/e2e/boot_policy_flash_ecc_error/defs.bzl‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ BOOT_POLICY_FLASH_ECC_ERROR_TESTS = [
7070
"name": "a_corrupt_b_valid_{}",
7171
"a": ":flash_ecc_self_corruption_slot_a_{}",
7272
"b": ":uncorrupted_test_slot_b",
73-
# The regex allows either 8 or 1 in the address, which accomodates the test program
74-
# being linked as either a ROM_EXT or application.
75-
"exit_success": "Booted slot=0x200[89]0000; Cause={}",
73+
# The regex allows either 8, 9, a in the address, which accomodates the test program
74+
# being linked as either a ROM_EXT or application normal or coverage builds.
75+
"exit_success": "Booted slot=0x200[89a]0000; Cause={}",
7676
# When running under the ROM_EXT, we want to try SlotA first and make sure that
7777
# we handle the corrupted flash properly.
7878
"primary": "SlotA",
@@ -81,9 +81,9 @@ BOOT_POLICY_FLASH_ECC_ERROR_TESTS = [
8181
"name": "a_valid_b_corrupt_{}",
8282
"a": ":uncorrupted_test_slot_a",
8383
"b": ":flash_ecc_self_corruption_slot_b_{}",
84-
# The regex allows either 0 or 1 in the address, which accomodates the test program
85-
# being linked as either a ROM_EXT or application.
86-
"exit_success": "Booted slot=0x200[01]0000; Cause={}",
84+
# The regex allows either 0, 1, 2 in the address, which accomodates the test program
85+
# being linked as either a ROM_EXT or application in normal or coverage builds.
86+
"exit_success": "Booted slot=0x200[012]0000; Cause={}",
8787
# When running under the ROM_EXT, we want to try SlotB first and make sure that
8888
# we handle the corrupted flash properly.
8989
"primary": "SlotB",

‎sw/device/silicon_creator/rom/e2e/boot_policy_flash_ecc_error/flash_ecc_error_test.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,7 @@ bool test_main(void) {
309309
if (kBootStage == kBootStageOwner) {
310310
// If we're running at the owner stage, we want to compute a location
311311
// inside the owner code, which starts after the ROM_EXT.
312-
// The ROM_EXT is 64K.
313-
addr_of_corruption += 0x10000;
312+
addr_of_corruption += CHIP_ROM_EXT_SIZE_MAX;
314313
}
315314

316315
// Corrupt the ECC of a targeted flash word by performing a double write.

‎sw/host/tests/ownership/flash_permission_test.rs‎

Lines changed: 116 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ use opentitanlib::rescue::serial::RescueSerial;
1818
use opentitanlib::rescue::{EntryMode, Rescue};
1919
use opentitanlib::test_utils::init::InitializeTest;
2020
use opentitanlib::uart::console::UartConsole;
21-
use transfer_lib::HybridPair;
21+
use transfer_lib::{
22+
HybridPair, OWNER_FLASH_FILE_SIZE, OWNER_FLASH_FILE_START, OWNER_FLASH_OWNER_SIZE,
23+
OWNER_FLASH_OWNER_START, OWNER_FLASH_ROM_EXT_SIZE, OWNER_FLASH_ROM_EXT_START,
24+
};
2225

2326
#[derive(Debug, Parser)]
2427
struct Opts {
@@ -247,11 +250,25 @@ fn flash_permission_test(opts: &Opts, transport: &TransportWrapper) -> Result<()
247250
// The ROM_EXT always protects itself in regions 0 and 1.
248251
assert_eq!(
249252
region[0],
250-
FlashRegion("data", 0, 0, 32, romext_region[0], "LK")
253+
FlashRegion(
254+
"data",
255+
0,
256+
OWNER_FLASH_ROM_EXT_START as u32,
257+
OWNER_FLASH_ROM_EXT_SIZE as u32,
258+
romext_region[0],
259+
"LK"
260+
)
251261
);
252262
assert_eq!(
253263
region[1],
254-
FlashRegion("data", 1, 256, 32, romext_region[1], "LK")
264+
FlashRegion(
265+
"data",
266+
1,
267+
256 + OWNER_FLASH_ROM_EXT_START as u32,
268+
OWNER_FLASH_ROM_EXT_SIZE as u32,
269+
romext_region[1],
270+
"LK"
271+
)
255272
);
256273

257274
// Current owner (fake_owner in flash SideA) doesn't have a configuration,
@@ -260,11 +277,25 @@ fn flash_permission_test(opts: &Opts, transport: &TransportWrapper) -> Result<()
260277
// Next owner (dummy_owner in Flash SideB) is the next owner configuration.
261278
assert_eq!(
262279
region[2],
263-
FlashRegion("data", 2, 288, 192, "RD-WR-ER-SC-EC-xx", "UN")
280+
FlashRegion(
281+
"data",
282+
2,
283+
256 + OWNER_FLASH_OWNER_START as u32,
284+
OWNER_FLASH_OWNER_SIZE as u32,
285+
"RD-WR-ER-SC-EC-xx",
286+
"UN"
287+
)
264288
);
265289
assert_eq!(
266290
region[3],
267-
FlashRegion("data", 3, 480, 32, "RD-WR-ER-xx-xx-HE", "UN")
291+
FlashRegion(
292+
"data",
293+
3,
294+
256 + OWNER_FLASH_FILE_START as u32,
295+
OWNER_FLASH_FILE_SIZE as u32,
296+
"RD-WR-ER-xx-xx-HE",
297+
"UN"
298+
)
268299
);
269300

270301
// The remaining flash regions are unused.
@@ -338,29 +369,99 @@ fn flash_permission_test(opts: &Opts, transport: &TransportWrapper) -> Result<()
338369
let app_region = match opts.rescue_slot {
339370
BootSlot::SlotA => [
340371
// Slot A protected, Slot B writable.
341-
FlashRegion("data", 2, 32, 192, "RD-xx-xx-SC-EC-xx", locked),
342-
FlashRegion("data", 3, 224, 32, "RD-WR-ER-xx-xx-HE", locked),
343-
FlashRegion("data", 4, 288, 192, "RD-WR-ER-SC-EC-xx", locked),
344-
FlashRegion("data", 5, 480, 32, "RD-WR-ER-xx-xx-HE", locked),
372+
FlashRegion(
373+
"data",
374+
2,
375+
OWNER_FLASH_OWNER_START as u32,
376+
OWNER_FLASH_OWNER_SIZE as u32,
377+
"RD-xx-xx-SC-EC-xx",
378+
locked,
379+
),
380+
FlashRegion(
381+
"data",
382+
3,
383+
OWNER_FLASH_FILE_START as u32,
384+
OWNER_FLASH_FILE_SIZE as u32,
385+
"RD-WR-ER-xx-xx-HE",
386+
locked,
387+
),
388+
FlashRegion(
389+
"data",
390+
4,
391+
256 + OWNER_FLASH_OWNER_START as u32,
392+
OWNER_FLASH_OWNER_SIZE as u32,
393+
"RD-WR-ER-SC-EC-xx",
394+
locked,
395+
),
396+
FlashRegion(
397+
"data",
398+
5,
399+
256 + OWNER_FLASH_FILE_START as u32,
400+
OWNER_FLASH_FILE_SIZE as u32,
401+
"RD-WR-ER-xx-xx-HE",
402+
locked,
403+
),
345404
],
346405
BootSlot::SlotB => [
347406
// Slot A writable, Slot B protected.
348-
FlashRegion("data", 2, 32, 192, "RD-WR-ER-SC-EC-xx", locked),
349-
FlashRegion("data", 3, 224, 32, "RD-WR-ER-xx-xx-HE", locked),
350-
FlashRegion("data", 4, 288, 192, "RD-xx-xx-SC-EC-xx", locked),
351-
FlashRegion("data", 5, 480, 32, "RD-WR-ER-xx-xx-HE", locked),
407+
FlashRegion(
408+
"data",
409+
2,
410+
OWNER_FLASH_OWNER_START as u32,
411+
OWNER_FLASH_OWNER_SIZE as u32,
412+
"RD-WR-ER-SC-EC-xx",
413+
locked,
414+
),
415+
FlashRegion(
416+
"data",
417+
3,
418+
OWNER_FLASH_FILE_START as u32,
419+
OWNER_FLASH_FILE_SIZE as u32,
420+
"RD-WR-ER-xx-xx-HE",
421+
locked,
422+
),
423+
FlashRegion(
424+
"data",
425+
4,
426+
256 + OWNER_FLASH_OWNER_START as u32,
427+
OWNER_FLASH_OWNER_SIZE as u32,
428+
"RD-xx-xx-SC-EC-xx",
429+
locked,
430+
),
431+
FlashRegion(
432+
"data",
433+
5,
434+
256 + OWNER_FLASH_FILE_START as u32,
435+
OWNER_FLASH_FILE_SIZE as u32,
436+
"RD-WR-ER-xx-xx-HE",
437+
locked,
438+
),
352439
],
353440
_ => return Err(anyhow!("Unknown boot slot {}", data.bl0_slot)),
354441
};
355442

356443
// The ROM_EXT always protects itself in regions 0 and 1.
357444
assert_eq!(
358445
region[0],
359-
FlashRegion("data", 0, 0, 32, romext_region[0], "LK")
446+
FlashRegion(
447+
"data",
448+
0,
449+
OWNER_FLASH_ROM_EXT_START as u32,
450+
OWNER_FLASH_ROM_EXT_SIZE as u32,
451+
romext_region[0],
452+
"LK"
453+
)
360454
);
361455
assert_eq!(
362456
region[1],
363-
FlashRegion("data", 1, 256, 32, romext_region[1], "LK")
457+
FlashRegion(
458+
"data",
459+
1,
460+
256 + OWNER_FLASH_ROM_EXT_START as u32,
461+
OWNER_FLASH_ROM_EXT_SIZE as u32,
462+
romext_region[1],
463+
"LK"
464+
)
364465
);
365466
// Flash Slot A:
366467
assert_eq!(region[2], app_region[0]);

‎sw/host/tests/ownership/transfer_lib.rs‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,18 @@ impl HybridPair {
270270
}
271271
}
272272

273+
pub const OWNER_FLASH_ROM_EXT_START: u16 = 0;
274+
275+
#[cfg(not(feature = "ot_coverage_enabled"))]
276+
pub const OWNER_FLASH_ROM_EXT_SIZE: u16 = 32;
277+
#[cfg(feature = "ot_coverage_enabled")]
278+
pub const OWNER_FLASH_ROM_EXT_SIZE: u16 = 64;
279+
280+
pub const OWNER_FLASH_FILE_START: u16 = 224;
281+
pub const OWNER_FLASH_FILE_SIZE: u16 = 32;
282+
pub const OWNER_FLASH_OWNER_START: u16 = OWNER_FLASH_ROM_EXT_START + OWNER_FLASH_ROM_EXT_SIZE;
283+
pub const OWNER_FLASH_OWNER_SIZE: u16 = OWNER_FLASH_FILE_START - OWNER_FLASH_OWNER_START;
284+
273285
/// Prepares an OwnerBlock and sends it to the chip.
274286
#[allow(clippy::too_many_arguments)]
275287
pub fn create_owner<F>(
@@ -324,22 +336,62 @@ where
324336

325337
// Side A: 0-64K romext, 64-448K firmware, 448-512K filesystem.
326338
vec![
327-
OwnerFlashRegion::new(0, 32, config.rom_ext()),
328-
OwnerFlashRegion::new(32, 192, config.firmware()),
329-
OwnerFlashRegion::new(224, 32, config.filesystem()),
339+
OwnerFlashRegion::new(
340+
OWNER_FLASH_ROM_EXT_START,
341+
OWNER_FLASH_ROM_EXT_SIZE,
342+
config.rom_ext(),
343+
),
344+
OwnerFlashRegion::new(
345+
OWNER_FLASH_OWNER_START,
346+
OWNER_FLASH_OWNER_SIZE,
347+
config.firmware(),
348+
),
349+
OwnerFlashRegion::new(
350+
OWNER_FLASH_FILE_START,
351+
OWNER_FLASH_FILE_SIZE,
352+
config.filesystem(),
353+
),
330354
// Side B: 0-64K romext, 64-448K firmware, 448-512K filesystem.
331-
OwnerFlashRegion::new(256, 32, config.rom_ext()),
332-
OwnerFlashRegion::new(256 + 32, 192, config.firmware()),
333-
OwnerFlashRegion::new(256 + 224, 32, config.filesystem()),
355+
OwnerFlashRegion::new(
356+
256 + OWNER_FLASH_ROM_EXT_START,
357+
OWNER_FLASH_ROM_EXT_SIZE,
358+
config.rom_ext(),
359+
),
360+
OwnerFlashRegion::new(
361+
256 + OWNER_FLASH_OWNER_START,
362+
OWNER_FLASH_OWNER_SIZE,
363+
config.firmware(),
364+
),
365+
OwnerFlashRegion::new(
366+
256 + OWNER_FLASH_FILE_START,
367+
OWNER_FLASH_FILE_SIZE,
368+
config.filesystem(),
369+
),
334370
]
335371
} else {
336372
vec![
337373
// Side A: 64-448K firmware, 448-512K filesystem.
338-
OwnerFlashRegion::new(32, 192, config.firmware()),
339-
OwnerFlashRegion::new(224, 32, config.filesystem()),
374+
OwnerFlashRegion::new(
375+
OWNER_FLASH_OWNER_START,
376+
OWNER_FLASH_OWNER_SIZE,
377+
config.firmware(),
378+
),
379+
OwnerFlashRegion::new(
380+
OWNER_FLASH_FILE_START,
381+
OWNER_FLASH_FILE_SIZE,
382+
config.filesystem(),
383+
),
340384
// Side B: 64-448K firmware, 448-512K filesystem.
341-
OwnerFlashRegion::new(256 + 32, 192, config.firmware()),
342-
OwnerFlashRegion::new(256 + 224, 32, config.filesystem()),
385+
OwnerFlashRegion::new(
386+
256 + OWNER_FLASH_OWNER_START,
387+
OWNER_FLASH_OWNER_SIZE,
388+
config.firmware(),
389+
),
390+
OwnerFlashRegion::new(
391+
256 + OWNER_FLASH_FILE_START,
392+
OWNER_FLASH_FILE_SIZE,
393+
config.filesystem(),
394+
),
343395
]
344396
};
345397
owner
@@ -360,8 +412,8 @@ where
360412
}
361413
if cfg & CFG_RESCUE1 != 0 {
362414
let mut rescue = OwnerRescueConfig::all();
363-
rescue.start = 32;
364-
rescue.size = 192;
415+
rescue.start = OWNER_FLASH_OWNER_START;
416+
rescue.size = OWNER_FLASH_OWNER_SIZE;
365417
if cfg & CFG_RESCUE_RESTRICT != 0 {
366418
// Restrict one of the boot_svc commands in "restrict" mode.
367419
rescue

0 commit comments

Comments
 (0)