QC v0.1 review comments

[eckhard-delfs-qualcomm]

• Chapter 3. Smsdid: Supervisor Domain Identifier and Protection Register
  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.

• Chapter 3.2 M-mode Supervisor Domain Fence Instruction
  This section needs more clarity on the precise operation of the fencing instruction when any of {rs1, rs2} is/are not zero.

• Chapter 5.6 Operand 1 register
  The section ends with statement: "If the initial low-order 0 bit position is denoted as x, the size of the range is computed as (1 << (12 + x + 1)). We think it would help to add an example which clarifies the relation between range size and PPN field encoding.

• Chapter 6: Smsdia: Supervisor Domain Interrupt Assignment
  Statement: The RDSM can employ the MTT and/or PMP to limit a supervisor domain’s access to the memorymapped
  register interface of the interrupt controller associated with it.
  The chosen term "limit" is ambiguous, as it does not indicate the type of limitation. Maybe: The RDSM can utilize the MTT and/or PMP to map the memory-mapped register interface of an interrupt controller exclusively to a single supervisor domain, preventing access by other domains.

• Chapter 8.4 Supervisor Domain QoS Register Interfaces (QRI)
  The fifth paragraph states: "The CONFIG_QRI_LIMIT operation may be requested once following reset."
  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?

[ved-rivos]

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]

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]

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.

[eckhard-delfs-qualcomm]

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]

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]

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]

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]

All comments addressed - thanks, @ved-rivos