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

Define trap (interrupt/exception) handler delegation #83

Open
orangecms opened this issue Oct 15, 2023 · 25 comments
Open

Define trap (interrupt/exception) handler delegation #83

orangecms opened this issue Oct 15, 2023 · 25 comments

Comments

@orangecms
Copy link

At this point, it is more a less coincidental which trap handlers an SBI may or may not delegate to an OS.
While OpenSBI handles e.g. unaligned access, this is not to be expected in general.

Now comes the hard part:
Should we define a set of what exactly is / should be delegated, or at least say that the OS should be prepared for anything to be delegated? It is crucial for the OS to know. Otherwise, there are dozens of possible combos of (non-)delegations.

@andreiw
Copy link
Collaborator

andreiw commented Oct 16, 2023

Can we start by enumerating the full list and the default behavior seen in OpenSBI?

@orangecms
Copy link
Author

orangecms commented Oct 16, 2023

Sure; we can grab the list from https://github.com/riscv/riscv-isa-manual/blob/main/src/machine.adoc#machine-cause-register-mcause

For a starting point, I suggest splitting into interrupts and exceptions, respectively, and for earch, a table with the columns

  • handler value + name
  • OpenSBI (whether set; just an X or O or blank for "keep default")
  • oreboot SBI (X, O, blank)

Edit: It is tri-state since different cores may have different defaults, AIUI. Eventual notation tbd.
Edit 2: add another column for configurability, possibly at build time or runtime or both, at some point if and when implemented (e.g. considered for misaligned memory access in OpenSBI)

That is what I did locally as well during development. Plus add the reference to the ISA spec.

In the SBI spec, the SBI implementations are listed with their IDs. Those should also be referenced somewhere here for completeness.

I'd create a trap-handlers.adoc file and a link from the sbi.adoc. WDYT?

@adurbin-rivos
Copy link
Collaborator

I'd create a trap-handlers.adoc file and a link from the sbi.adoc. WDYT?

Sure. Go with that. However, I'm not sure it's really related to sbi per-se. It's more of how to configure m-mode delegation to s-mode, if m-mode is implemented.

@ved-rivos
Copy link
Collaborator

In what case does it matter whether the interrupt/exception is delegated or is handled M-first? Should not the OS expect to handle all defined S exceptions and interrupts - whether delegated or emulated?

@orangecms
Copy link
Author

In what case does it matter whether the interrupt/exception is delegated or is handled M-first? Should not the OS expect to handle all defined S exceptions and interrupts - whether delegated or emulated?

This is to make clear what to expect and what not, more elaborate:
#84 (comment)

@andreiw
Copy link
Collaborator

andreiw commented Apr 11, 2024

@ved-rivos @orangecms can we revisit this and reach a conclusion?

@andreiw andreiw added the question Further information is requested label Apr 11, 2024
@orangecms
Copy link
Author

@ved-rivos @orangecms can we revisit this and reach a conclusion?

That'd be great. I've literally just had another discussion around this the other day. While Linux now has the patches for handling unaligned access itself, they are currently not effective e.g. on JH7110 boards, where vendor firmware does not delegate the respective exceptions. So that potentially skews performance results, depending on the software.

@andreiw
Copy link
Collaborator

andreiw commented Apr 12, 2024

Also @atishp04

Do we want an SBI extension where an OS can negotiate delegations? I.e agree if a particular trap would go fw-only, fw-first or kernel-first.

That's probably more workable then attempting to pin down which traps can be delegated or not (or should be), as that's highly dependent on the system in question. It seems like it would also help solve the real issues you see with JH7110.

@TobiasSchaffner
Copy link

I would also expect to see all exceptions that can be handled by the OS to be delegated. I did some evaluations with preempt_rt and saw latencies of 300us on exceptions handled in machine mode. As this can be triggered by any user I fear that undelegated exceptions may be an issue.

@andreiw
Copy link
Collaborator

andreiw commented Apr 12, 2024

I would also expect to see all exceptions that can be handled by the OS to be delegated. I did some evaluations with preempt_rt and saw latencies of 300us on exceptions handled in machine mode. As this can be triggered by any user I fear that undelegated exceptions may be an issue.

@TobiasSchaffner, could this be achieved by having an OS /negotiate/ with the M-mode firmware on delegations it would like to have (via an SBI extension) - instead of simply mandating a list of delegations in a spec?

@atishp04
Copy link
Collaborator

We already have a SBI extension proposal close to the final stages. It started with misaligned delegation and expanded to other config behaviors for ISA (ADUE, CFI etc).
https://lists.riscv.org/g/tech-prs/topic/105041801#866

Please review and let us know if you feel some other exception should also be added to the fwft feature list.

@atishp04
Copy link
Collaborator

@TobiasSchaffner : Is this related to misaligned access exception or something else ?

@TobiasSchaffner
Copy link

@TobiasSchaffner, could this be achieved by having an OS /negotiate/ with the M-mode firmware on delegations it would like to have (via an SBI extension) - instead of simply mandating a list of delegations in a spec?

I think I am not the right person to answer this. At the moment I do not understand where this "firmware first" approach comes from and in which cases the OS would want to reject to handle exceptions that are handleable in S-Mode.

@TobiasSchaffner
Copy link

@TobiasSchaffner : Is this related to misaligned access exception or something else ?

@atishp04 The latencies can be triggered with any undelegated exception no matter if it is an unaligned memory access or e.g. and illegal instruction exception.

At the moment OpenSBI delegates:

  • Breakpoint
  • Environment Call
  • Fetch page fault
  • Load page fault
  • Store page fault
  • Misaligned instruction fetch

See: https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_hart.c#L195

@andreiw
Copy link
Collaborator

andreiw commented Apr 12, 2024

@TobiasSchaffner, could this be achieved by having an OS /negotiate/ with the M-mode firmware on delegations it would like to have (via an SBI extension) - instead of simply mandating a list of delegations in a spec?

I think I am not the right person to answer this. At the moment I do not understand where this "firmware first" approach comes from and in which cases the OS would want to reject to handle exceptions that are handleable in S-Mode.

Consider that SBI may provide some unimpl instruction emulation. That's "firmware first". Presumably anything not handled would be injected for handling into the OS (I guess there's no point not to inject, but it looks like some implementations don't delegate for some traps period). A "kernel first" approach would be a contact that the OS takes full responsibility for all illegal insn handling and SBI impl does none of it.

I don't think an OS might want to reject handling exceptions. I think it's more about a kernel just might not care enough and be okay with whatever SBI impl does. Hence negotiating for each desired trap instead of mandating (in a spec) that everything is delegated.

N.B.: thinking more about it, it's probably not that interesting to add "illegal instruction" trap to FWFT, unless it is to specifically work around some broken extension emulation, at which point maybe fix the firmware.

@andreiw
Copy link
Collaborator

andreiw commented Apr 12, 2024

We already have a SBI extension proposal close to the final stages. It started with misaligned delegation and expanded to other config behaviors for ISA (ADUE, CFI etc). https://lists.riscv.org/g/tech-prs/topic/105041801#866

Please review and let us know if you feel some other exception should also be added to the fwft feature list.

@atishp04

Oh neat. Can we add a dependency on FWFT now or should we wait?

@ved-rivos
Copy link
Collaborator

ved-rivos commented Apr 12, 2024

An OS should have handling for all exceptions specified by the Priv. specification and can be delegated to S-mode. In some implementations M-mode may emulate some some of the exceptions. In other cases there may be firmware first handling. In both cases this should not lead to a functional difference to the S-mode software. I think of an implementation that handles unaligned load/store through emulation in M-mode as not very different than an implementation that handles unaligned load/store poorly in hardware - these just lead to performance issues and not functional issues. The BRS specification would not want to place performance requirements. So whether an exception is delegated or not should not be material unless it affects the functioning of the S-mode. Is there a functional - not performance - reason to mandate delegation or require implementation of FWFT?

@orangecms
Copy link
Author

Ved, we are aware of all that.

This is not about functional differences. It is about performance guarantees and expectations. I still do not understand why it is that unaligned access isn't delegated. You can just do it. All I'm asking for is clear statements. We are still at "be prepared for everything but do not expect anything". That is a non-binding contract.

My suggestion is to declare performance profiles, just like with the platform profiles. Call the one delegating everything possible the "high-perf" profile, and what OpenSBI will do the "negotiable" profile or something like that. Then it is clear what the OS/end user can expect. WDYT?

@orangecms
Copy link
Author

On

The BRS specification would not want to place performance requirements

Who decides that, why would that be? Would you want another working group to define performance profiles? If so, why?

@ved-rivos
Copy link
Collaborator

Who decides that, why would that be? Would you want another working group to define performance profiles? If so, why?

How would one define performance profiles? A machine may implement unaligned access in HW and take say a 1000 cycles to do the access. Another machine may do unaligned accesses in 3 cycles. Is the first machine then incompatible with risc-v and why is 3 cycles sufficient and not 1 cycle.

Performance is something that should be selected by the application. A system integrator building a network router or a laptop would pick the components needed for that application to meet the performance objectives of that application. What may be adequate performance for a 1 gigabit router may not be adequate for a terabit router. But BRS is applicable to both applications.

@andreiw
Copy link
Collaborator

andreiw commented Apr 13, 2024

Honestly I'd steer this in a different direction. I agree with Ved that the BRS doesn't concern itself with performance expectations, but in my opinion this isn't about performance, as we're not trying to mandate specific performance for anything in the spec. It's about functionality, in the sense that an OS may simply wish to provide its own functionality instead of relying on the firmware. It may choose to do so because it thinks it can do a better job, or as a possible workaround for bugs. In case of unaligned accesses, an OS may choose to disallow unaligned accesses entirely. In case of other delegations, avoid cycles spent in foreign non-interruptible code. Whatever the reason.

The FWFT allows an OS to potentially dial the level of low-level awareness it needs. Perhaps RAS is another such example, eventually, where an OS can choose kernel-first on a system where the default would be firmware-first.

@ved-rivos
Copy link
Collaborator

Agreed that the FWFT extension allows an OS to pick between kernel and OS first approach.

@andreiw
Copy link
Collaborator

andreiw commented Apr 17, 2024

@orangecms can you rework this PR (#84) to reference the still in progress FWFT extension as a required mechanism.

@andreiw andreiw added waiting-on-pr-or-ticket-close and removed question Further information is requested labels Apr 18, 2024
@atishp04
Copy link
Collaborator

We already have a SBI extension proposal close to the final stages. It started with misaligned delegation and expanded to other config behaviors for ISA (ADUE, CFI etc). https://lists.riscv.org/g/tech-prs/topic/105041801#866
Please review and let us know if you feel some other exception should also be added to the fwft feature list.

@atishp04

Oh neat. Can we add a dependency on FWFT now or should we wait?

FWFT will be ratified as a part of SBI v3.0 towards end of this year. As BRS is already in ARC review, I am not sure what's the policy for pointing to unratified specs.

@andreiw
Copy link
Collaborator

andreiw commented May 3, 2024

ok we can pend this on SBI 3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants