Skip to content

Conversation

AlexJones0
Copy link

These memcpys were accidentally using the size of the dereferenced byte array (i.e., always just 1 byte), and so were only copying the first byte of the HW_CFG device ID and manufacturing state that were being loaded from the OTP.

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

Shouldn't we add static_assert to ensure that for some reason the size of those arrays change, they cannot overflow the dedicated registers?

@AlexJones0
Copy link
Author

AlexJones0 commented Oct 3, 2025

Shouldn't we add static_assert to ensure that for some reason the size of those arrays change, they cannot overflow the dedicated registers?

Yes, I will add that to this PR.

Edit: Pushed that now. Not sure if there is a cleaner way to do this, feedback welcome :)

@AlexJones0 AlexJones0 force-pushed the hw_cfg_copy branch 2 times, most recently from d0405da to 8a1a937 Compare October 3, 2025 20:50
@rivos-eblot
Copy link

Edit: Pushed that now. Not sure if there is a cleaner way to do this, feedback welcome :)

As we use this idiom in several files, it might be worth to define something like:

#define OT_REG_NAME_IDX(_n_, _i_) (R_ ## _n_ ## _ ## _i_)
#define OT_REG_COUNT(_n_, _l_)    (OT_REG_NAME_IDX(_n_, _l_) - OT_REG_NAME_IDX(_n_, 0) + 1u)
#define OT_REG_SPAN(_n_, _l_)     (OT_REG_COUNT(_n_, _l_) * sizeof(uint32_t))

// example
#define LC_MANUF_STATE_WIDTH OT_REG_SPAN(MANUF_STATE, 7)
#define LC_DEVICE_ID_WIDTH   OT_REG_SPAN(DEVICE_ID, 7)

in ot_common.h?

AlexJones0 and others added 2 commits October 7, 2025 10:32
Adds macros for generic computing of the size of register spans for
OpenTitan multi registers, which can be used to e.g. make static
assertions to ensure that the size of macros matches the size of
buffers being used.

Co-authored-by: Emmanuel Blot <eblot@rivosinc.com>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These `memcpy`s were accidentally using the size of the dereferenced
byte array (i.e., always just 1 byte), and so were only copying the
first byte of the HW_CFG device ID and manufacturing state that were
being loaded from the OTP.

Also add some local calculations and static assertions to check that the
HW_CFG value sizes that are given do not become out of sync with the
size of the registers.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
@AlexJones0 AlexJones0 merged commit ab75982 into lowRISC:ot-9.2.0 Oct 7, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants