riscv / riscv-smmtt

This specification will define the RISC-V privilege ISA extensions required to support Supervisor Domain isolation for multi-tenant security use cases e.g. confidential-computing, trusted platform services, fault isolation and so on.
https://jira.riscv.org/browse/RVG-65
Creative Commons Attribution 4.0 International
43 stars 17 forks source link

QC v0.1 review comments #72

Closed eckhard-delfs-qualcomm closed 1 month ago

eckhard-delfs-qualcomm commented 2 months ago
ved-rivos commented 1 month ago

We think it would help to add an example which clarifies the relation between range size and PPN field encoding.

Example of encoding address ranges using S field is included in PR #83

The chosen term "limit" is ambiguous, as it does not indicate the type of limitation.

The term "limiting access" is well defined. But to avoid confusion it is reworded as "restrict a supervisor domain's access" in PR #83.

While use of PMP to restrict the access to exclusively to a supervisor domain is the expected use case, the PMP or MTT by itself does not enforce such an exclusivity - a RDSM may allow access from multiple supervisor domains using suitable PMP/MTT configurations.

What is the reset value of NCBLKS for a QRI if the CONFIG_QRI_LIMIIT operation is not performed for that QRI? Is this implementation defined?

The previous paragraph specifies this: "By default all resources in the capacity and bandwidth controllers may be allocated using any of the QRI."

ved-rivos commented 1 month ago

Last paragraph. It is not specified if switching mttp.MODE from Bare to any other and vice versa mode takes effect immediately or requires execution of an MFENCE.SPA instruction.

A MFENCE.SPA should be required. The last paragraph specifies that "even if the old or new mode is Bare"

ved-rivos commented 1 month ago

This section needs more clarity on the precise operation of the fencing instruction when any of {rs1, rs2} is/are not zero.

I think the following text could be added:

eckhard-delfs-qualcomm commented 1 month ago

This section needs more clarity on the precise operation of the fencing instruction when any of {rs1, rs2} is/are not zero.

I think the following text could be added:

  • If rs1=x0 and rs2=x0, the fence orders all reads and writes to the MTT for all supervisor domain address spaces.
  • If rs1=x0 and rs2!=x0, the fence orders all reads and writes to the MTT for the supervisor domain address space identified by the SDID in rs2.
  • If rs1!=x0 and rs2=x0, the fence orders all reads and writes made to the MTT that correspond to the physical address in rs1, for all supervisor domain address spaces.
  • If rs1!=x0 and rs2!=x0, the fence orders all reads and writes made to the MTT that correspond to the physical address in rs1, for the supervisor domain address space identified by the SDID in rs2.

Looks good to me. Thanks!

eckhard-delfs-qualcomm commented 1 month ago

Last paragraph. It is not specified if switching mttp.MODE from Bare to any other and vice versa mode takes effect immediately or requires execution of an MFENCE.SPA instruction.

A MFENCE.SPA should be required. The last paragraph specifies that "even if the old or new mode is Bare"

Correct. Did not spot that statement before.

andrewdellow commented 1 month ago

The chosen term "limit" is ambiguous, as it does not indicate the type of limitation.

The term "limiting access" is well defined. But to avoid confusion it is reworded as "restrict a supervisor domain's access" in PR #83.

While use of PMP to restrict the access to exclusively to a supervisor domain is the expected use case, the PMP or MTT by itself does not enforce such an exclusivity - a RDSM may allow access from multiple supervisor domains using suitable PMP/MTT configurations.

The PMP and MTT by themselves don't enforce anything other than the intention of the RDSM, so is the intention here to provide a suggested use case, a recommendation, a definition ? maybe it is better to say the RDSM can employ the MTT and/or PMP to restrict a supervisor domain’s access to the memory mapped register interface of an interrupt controller, for example the RDSM may limit access to the memory-mapped register interface of an interrupt controller exclusively to a single supervisor domain, preventing access by other domains

ved-rivos commented 1 month ago

I agree that this sentence is not material to the specification. It's stating the obvious for what an RDSM might do. I will remove the sentence.

eckhard-delfs-qualcomm commented 1 month ago

All comments addressed - thanks, @ved-rivos