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

Add trigger delegation spec (temporarily) #54

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Conversation

bcstrongx
Copy link
Collaborator

No description provided.

| Sdtrig Register | `sireg*` Field Modifications | `vsireg*` Field Modifications
| `tdata1` | M is read-only 0

VS and VU are read-only 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

S-mode should have the capability to set trigger for VS/VU mode. Is it pasted here by mistake?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

S-mode can enable triggers in VS/VU-mode via vsiselect/vsireg*. This approach is much better for supporting nested virtualization, because it means a hypervisor running in VS-mode will naturally trap when it attempts to configure triggers for a true guest (by writing vsiselect/vsireg*), which will allow the L0 hypervisor (running in HS-mode) to emulate the desired behavior.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a new non-normative comment to clarify this, see new text below the table.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with your statement on nested virtualization. Actually what I pointed out is that in sireg* column, VS and VU should be configurable instead of being "VS and VU are read-only 0".
Moreover, it might be worthwhile to clairify the relocation is applied to real CSR instead of programing ones.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure I follow. HS-mode can enable trigger match in S/U-mode via siselect/sireg, and/or enable trigger match in VS/VU-mode via direct access to vsiselect/vsireg. So it is intentional that VS and VU are ROZ in sireg, then are accessible only in vsireg. I'm not sure what you mean by "real CSR instead of programming ones". In case it's related to this: both siselect/sireg and vsiselect/vsireg are real physical CSRs, but from VS-mode only attempts to access siselect/sireg are redirected by the HW to vsiselect/vsireg.


While the privilege mode is M or S and `siselect` holds XXX, illegal instruction exceptions are raised for attempts to access `sireg5` or `sireg6`. Similarly, while the privilege mode is M or S and `vsiselect` holds XXX, illegal instruction exceptions are raised for attempts to access `vsireg5`, or `vsireg6`.

While the privilege mode is VS and `vsiselect` holds XXX, virtual instruction exceptions are raised for attempts to access `sireg5` (really `vsireg5`) or `sireg6` (really `vsireg6`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

vsiselect holds XXX -> siselect holds XXX?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, in VS-mode, accesses to sireg* are redirected to vsireg*, and accesses to siselect are redirected to vsiselect. So a guest OS will attempt to access siselect/sireg*, but it will actually access vsiselect/vsireg*.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I got your point. By "vsiselect holds XXX" you mean the real CSR holds XXX instead of the programming shadow CSR hold XXX. It is a little bit misleading here. Probably we could align the description with sireg*, like siselect (really vsiselect) holds XXX.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The phrasing of "siregX (really vsiregX)" refers to software accesses, since VS-mode software will attempt to access siregX, but the hardware will direct the access to vsiregX. When speaking of the state held by a register, it is vsiselect that must hold XXX, not siselect. siselect may (likely does) hold some different value, written by the hypervisor or other more privileged software. It's true that the value held by vsiselect may be the result of a VS-mode attempt to write siselect, but it could also be the result of a direct write to vsiselect by (H)S-mode or M-mode. Since this behavior doesn't depend on the method of access, but rather the result, only the end value is referenced here. Hope that makes sense.


If the H extension is implemented and `mstateen0`.TR=1, the `hstateen0`.TR bit controls access to Sdtrig state when V=1. `hstateen0`.TR is read-only 0 when `mstateen0`.TR=0.

When `mstateen0`.TR=1 and `hstateen0`.TR=1, VS-mode accesses to supervisor TR state behave as described in <<Supervisor and Virtual Supervisor Trigger Access>> above. When `mstateen0`.TR=1 and `hstateen0`.TR=0, attempts from VS-mode to access `sireg` (really `vsireg`) or `sireg[234]` (really `vsireg[234]`) while `vsiselect` holds XXX raise a virtual-instruction exception.
Copy link
Collaborator

Choose a reason for hiding this comment

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

vsiselect -> siselect

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See response above.

Copy link
Collaborator

@gokhankaplayan gokhankaplayan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@joxie joxie left a comment

Choose a reason for hiding this comment

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

LTGM

@@ -0,0 +1,105 @@
[[intro]]
== Introduction
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is mtriggerdeleg intentionally removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, instead delegation is all-or-none, and controlled by mstateen0.TR/hstateen0.TR.

riscv-smtdeleg-sstcfg.adoc Show resolved Hide resolved
@joxie joxie merged commit 069dbf9 into main Aug 12, 2024
1 check 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.

4 participants