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.

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

[flash_ctrl/rtl] Enable SecFifoPtr for TL-UL FIFOs #23515

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

nasahlpa
Copy link
Member

@nasahlpa nasahlpa commented Jun 5, 2024

This commit enables the .Secure() option of the prim_fifo_sync FIFOs inside the tlul_adapter_sram used by the flash_ctrl module.

On an error, the intg_error_o signal will be flagged.

This commit enables the .Secure() option of the prim_fifo_sync
FIFOs inside the tlul_adapter_sram used by the flash_ctrl module.

On an error, the intg_error_o signal will be flagged.

Signed-off-by: Pascal Nasahl <[email protected]>
@nasahlpa nasahlpa marked this pull request as ready for review June 6, 2024 05:32
Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

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

Thanks @nasahlpa !
This enables an additional security feature we already had for the blocks we can use it.

Area impact should be super low because it concerns the very small fifo ctrs only.

Code change is small: parameter change to switch on transparent hardening; additional error signals; additional assertions

Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @nasahlpa !

@moidx moidx merged commit 574ece3 into lowRISC:master Jun 6, 2024
31 checks passed
nasahlpa added a commit to nasahlpa/opentitan that referenced this pull request Jun 12, 2024
With lowRISC#23515, the .Secure(1'b1) parameter was also
enabled for the u_tlul_adapter_sram_imem.u_reqfifo and
u_tlul_adapter_sram_imem.u_sramreqfifo. Hence, for the same reason
as explained in lowRISC#23627 we should also exclude FI
on these FIFOs.

In addition, lowRISC#23627 only excluded the FI tests for the
u_rptr. However, the same issue ('X all over the place) could also happen
when faulting the u_wptr of the according FIFO. Hence, this commit
also excludes FI for the u_wptr.

Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa added a commit to nasahlpa/opentitan that referenced this pull request Jun 12, 2024
With lowRISC#23515, the .Secure(1'b1) parameter was also
enabled for the u_tlul_adapter_sram_imem.u_reqfifo and
u_tlul_adapter_sram_imem.u_sramreqfifo. Hence, for the same reason
as explained in lowRISC#23627 we should also exclude FI
on these FIFOs.

In addition, lowRISC#23627 only excluded the FI tests for the
u_rptr. However, the same issue ('X all over the place) could also happen
when faulting the u_wptr of the according FIFO. Hence, this commit
also excludes FI for the u_wptr.

Signed-off-by: Pascal Nasahl <[email protected]>
rswarbrick pushed a commit that referenced this pull request Jun 12, 2024
With #23515, the .Secure(1'b1) parameter was also
enabled for the u_tlul_adapter_sram_imem.u_reqfifo and
u_tlul_adapter_sram_imem.u_sramreqfifo. Hence, for the same reason
as explained in #23627 we should also exclude FI
on these FIFOs.

In addition, #23627 only excluded the FI tests for the
u_rptr. However, the same issue ('X all over the place) could also happen
when faulting the u_wptr of the according FIFO. Hence, this commit
also excludes FI for the u_wptr.

Signed-off-by: Pascal Nasahl <[email protected]>
cfrantz pushed a commit to cfrantz/opentitan that referenced this pull request Jun 14, 2024
With lowRISC#23515, the .Secure(1'b1) parameter was also
enabled for the u_tlul_adapter_sram_imem.u_reqfifo and
u_tlul_adapter_sram_imem.u_sramreqfifo. Hence, for the same reason
as explained in lowRISC#23627 we should also exclude FI
on these FIFOs.

In addition, lowRISC#23627 only excluded the FI tests for the
u_rptr. However, the same issue ('X all over the place) could also happen
when faulting the u_wptr of the according FIFO. Hence, this commit
also excludes FI for the u_wptr.

Signed-off-by: Pascal Nasahl <[email protected]>
AlexJones0 pushed a commit to AlexJones0/opentitan that referenced this pull request Jul 8, 2024
With lowRISC#23515, the .Secure(1'b1) parameter was also
enabled for the u_tlul_adapter_sram_imem.u_reqfifo and
u_tlul_adapter_sram_imem.u_sramreqfifo. Hence, for the same reason
as explained in lowRISC#23627 we should also exclude FI
on these FIFOs.

In addition, lowRISC#23627 only excluded the FI tests for the
u_rptr. However, the same issue ('X all over the place) could also happen
when faulting the u_wptr of the according FIFO. Hence, this commit
also excludes FI for the u_wptr.

Signed-off-by: Pascal Nasahl <[email protected]>
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.

4 participants