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

Traps_Interrupts_Exceptions.rst: update chapter #1291

Merged

Conversation

ASintzoff
Copy link
Contributor

No description provided.

@ASintzoff ASintzoff requested a review from zarubaf as a code owner June 30, 2023 15:32
@github-actions
Copy link
Contributor

❌ failed run, report available here.

Copy link
Contributor

@jquevremont jquevremont left a comment

Choose a reason for hiding this comment

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

Mostly minor changes requested.

On one hand, interrupts are occuring independently of the instructions
(mainly raised by peripherals or debug module).
On the other hand, an instruction may raise exceptions synchronously.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to mention that U-mode delegation of interrupts and exceptions is not supported.

Copy link
Contributor Author

@ASintzoff ASintzoff Jul 20, 2023

Choose a reason for hiding this comment

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

AFAIK, RISC-V privilege spec does not support (even optionally) U-mode delegation. So, I'm not sure it is useful to mention that in our manual.
Whatever, I add "S-mode" few lines below to precise what is the lower privilege mode.

------------------
CSRs having an effect on the core behaviour when a trap occurs are:

* ``mstatus`` and ``sstatus``: several fields are read like interrupt enable (MIE, SIE)
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the context. Who / what entity reads?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased


* instruction access fault

* physical address fault
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you be more specific? E.g. error response from the memory bus.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased with PMP permissions


* load access fault

* access to PMP region without read permissions
Copy link
Contributor

Choose a reason for hiding this comment

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

What about write and execute permissions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For load, only read permissions


* store/AMO access fault

* access to PMP region without write permissions
Copy link
Contributor

Choose a reason for hiding this comment

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

What about write and execute permissions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For store, only write permissions


Modified CSRs
-------------
* ``mstatus``: several fields are updated like interrupt enable (MIE, SIE), modify privilege (MPRV)
Copy link
Contributor

Choose a reason for hiding this comment

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

Updated by the core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sentence added.

-------------
* ``mstatus``: several fields are updated like interrupt enable (MIE, SIE), modify privilege (MPRV)

Interrupts
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an introductory sentence, such as "These input signals of the CVA6 asynchronously trigger interrupts"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interrupts are already introduced at the beginning of the chapter.

Comment on lines +117 to +126
* external interrupt: ``irq_i`` signal
* software interrupt (inter-processor interrupt): ``ipi_i`` signal
* timer interrupt: ``time_irq_i`` signal
* debug interrupt: ``debug_req_i`` signal
Copy link
Contributor

Choose a reason for hiding this comment

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

How can the programmer identify the source of the interrupt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks to exception code field in mcause CSR. Added in the document.

* timer interrupt: ``time_irq_i`` signal
* debug interrupt: ``debug_req_i`` signal

These signals are level sensitive.
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this sentence. But is it clear for a programmer? May I suggest to add a second sentence: "The core will immediately interrupt again as long as the input signal stays in its active state"?


Wait for Interrupt
==================
* CVA6 implementation: WFI stalls the core. The instruction is not available in U-mode (raise illegal instruction exception). Such exception is also raised when ``TW=1`` in ``mstatus``.
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor: add `` `` around WFI

@JeanRochCoulon JeanRochCoulon merged commit 9b4b6ab into openhwgroup:master Jul 20, 2023
6 checks passed
@JeanRochCoulon JeanRochCoulon deleted the traps_user_manual branch July 20, 2023 08:56
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