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

Machine mode indicator #174

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

szbieg
Copy link

@szbieg szbieg commented Nov 28, 2023

Expose to the output ports whether the core is running in user or machine mode

solves #8

…hine mode

solves openhwgroup#8

Signed-off-by: Szymon Bieganski <szymon.bieganski@oss.nxp.com>
@christian-herber-nxp
Copy link

I am thinking if there shouldnt be two different signal. 1, the mode. 2, the effective mode.

privilege can be different for fetches and for data accesses, if mstatus.MPRV is set.

@szbieg
Copy link
Author

szbieg commented Nov 28, 2023

resolves #8

@davideschiavone
Copy link

I am thinking if there shouldnt be two different signal. 1, the mode. 2, the effective mode.

privilege can be different for fetches and for data accesses, if mstatus.MPRV is set.

hi @christian-herber-nxp , what do you mean effective mode or just mode?

when you change mode to be sure 100% nothing in flights (newer than the change mode instruction) you should clear the internal instruction buffer filled by the prefetcher, and before changing the mode, flush the pipeline (e.g. if there are in flights loads/stores)

The cv32e2 pipeline should not have anything inflight from the ld/st viewpoint as 2 pipeline stage, so no issues in that part, but it won't be true from the pre-fetching part - one simple solution would be to make this specific instruction JUMP to the next instruction (PC+4) as jumping fetches the instruction from the memory, hence, the new fetch will come with the updated privilege mode).

But this won't belong to this PR, it would rather be a bug (like bug: PMP did not allow for instruction XYZ to be executed, and it has been executed)

@christian-herber-nxp
Copy link

I am thinking if there shouldnt be two different signal. 1, the mode. 2, the effective mode.
privilege can be different for fetches and for data accesses, if mstatus.MPRV is set.

hi @christian-herber-nxp , what do you mean effective mode or just mode?

when you change mode to be sure 100% nothing in flights (newer than the change mode instruction) you should clear the internal instruction buffer filled by the prefetcher, and before changing the mode, flush the pipeline (e.g. if there are in flights loads/stores)

The cv32e2 pipeline should not have anything inflight from the ld/st viewpoint as 2 pipeline stage, so no issues in that part, but it won't be true from the pre-fetching part - one simple solution would be to make this specific instruction JUMP to the next instruction (PC+4) as jumping fetches the instruction from the memory, hence, the new fetch will come with the updated privilege mode).

But this won't belong to this PR, it would rather be a bug (like bug: PMP did not allow for instruction XYZ to be executed, and it has been executed)

I think we are not quite in sync. My point is, that if mstatus.MPRV = 1, loads/and stores use a different privilege level than instruction fetches. At this point, there is no PMP. The ibex PMP is integrated, so i would expect this solves it.

But exposing the privilege level to the busses is a more complicated thing, because you dont know what checks to expect.
Goal is to have the correct HPROT settings on the AHB, and for that you need to potentially know the effective privilege, if it applies.

@christian-herber-nxp
Copy link

I had a look into the OBI spec (probably should have done this before).
I think what we really need is simply adding the prot[] signal. This would solve it.

@christian-herber-nxp
Copy link

Conclusions from meeting

  1. We need a privilege mode exposed per OBI interface. Data accesses need to follow the effective privilege mode as per privileged specification, section Memory Privilege in mstatus Register
  2. The privilege mode should be exposed in an OBI compliant way

Definition from OBI spec:
image

szbieg and others added 3 commits December 1, 2023 14:33
Signed-off-by: Szymon Bieganski <szymon.bieganski@oss.nxp.com>
…the exposed signals

closes openhwgroup#175

Signed-off-by: Szymon Bieganski <szymon.bieganski@oss.nxp.com>
Copy link

@christian-herber-nxp christian-herber-nxp left a comment

Choose a reason for hiding this comment

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

change needed to:

  • have prot[] signal for each OBI interface
  • prot based on effective privilege level for the data interface

Signed-off-by: Szymon Bieganski <szymon.bieganski@oss.nxp.com>
@MikeOpenHWGroup
Copy link
Member

Hi @davideschiavone, can you review the latest update by @szbieg?

@davideschiavone
Copy link

  • have prot[] signal for each OBI interface
  • prot based on effective privilege level for the data interface

I still do not see what @christian-herber-nxp requested here:

have prot[] signal for each OBI interface
prot based on effective privilege level for the data interface

@szbieg
Copy link
Author

szbieg commented Mar 12, 2024

The aim of this task is to expose privilege (effective) level status. OBI interface will be connected to those exposed signals, and as such is already covered in task #70. @davideschiavone please clarify what exactly do you expect to see if not the exposed privilege mode.

@szbieg szbieg self-assigned this Mar 13, 2024
@szbieg szbieg linked an issue Apr 18, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants