0.1 Review Comments/Questions: Clarification Requests for Chapter 5 "I/O MTT extension"

[ram2532]

1. Page 28, third paragraph refers to "listed fields" and "unlisted fields". I assume it is referring to the list of fields provided for SET_ENTRY in first paragraph. But, I think the document should be explicit about which fields apply to each "OP" rather than leaving it implied.
2. Page 30, Table 10 has a SRC_IDM column numbered 0 to 3. The text above the Table says that SRC_IDM=NAPOT which is encoding 2. So, I assume the table shows examples of NAPOT encodings. So, what does the SRC_IDM column mean?
3. Also regarding Table 10 and the text above it - it says SRC_ID is set to DEVID. I assume that refers to encoding 1 - Filter by device ID - defined in Table 8, but the term "DEVID" is not defined. So, I have to infer that meaning rather than it being explicitly defined. Can the table be updated to define DEVID or the text changed to refer to "Filter by device ID"?
4. Page 30, Table 11 uses the term "TEE_LIM". I don't know what that is. Should it be defined here or point to a reference?
5. Page 31, 2nd paragraph, I infer that the value written into the PPN field requirements are the same as those define on page 13, 2nd paragraph for the mttp PNN field. Can that be made explicit?
6. More generally, I think that this chapter would benefit by providing a cohesive description of how S/W uses the SET_ENTRY "OP" to construct a table of "rules" which maps devices to SDID, IOMMU ID and MTT base address and how this table is used by SDCL.
7. Regarding mapping a devices to SDID/IOMMU/MTT base mapping - One may need to use multiple ruleids to map multiple devices to the same SDID/IOMMU/MTT base. A programming error could incorrectly specify the two devices with the same SDID but different MTT bases. or vice versa What does the spec say about that? Should H/W catch that and report an error in the control.STATUS? Or does that lead to UNSPECIFIED beahavior?

[ved-rivos]

1. Reworded to clarify in Clarifications to IOMTT #82. The section 5.5 and 5.6 call out which fields of the operands used by each operation.
2. These typos have been addressed in v0.1 editorial change suggestions #67
3. Updated Table 10 caption in Clarifications to IOMTT #82
4. TEE_LIM is a typo. Has been addressed in v0.1 editorial change suggestions #67
5. Updated in Clarifications to IOMTT #82
6. The specifications and examples are explanatory in this case.
7. When two devices are specified to use same SDID but different MTT then either of the MTT may be used for enforcing the access. This is clarified in Clarifications to IOMTT #82

[SiFiveHolland]

7. Regarding mapping a devices to SDID/IOMMU/MTT base mapping - One may need to use multiple ruleids to map multiple devices to the same SDID/IOMMU/MTT base. A programming error could incorrectly specify the two devices with the same SDID but different MTT bases. or vice versa What does the spec say about that? Should H/W catch that and report an error in the control.STATUS? Or does that lead to UNSPECIFIED beahavior?

Is the implication here that the I/O MTT can share cache entries across rules with the same SDID, so we can't know which MTT PPN was used to fill the cache? If so, this sharing needs to be stated explicitly, and the description of MTTINVAL needs to be updated. Right now the spec says:

If the PPNV field is 0, the MTTINVAL operation affects all entries from the MTT associated with RULEID.

But it sounds like what you really want is:

If the PPNV field is 0, the MTTINVAL operation affects all entries with an SDID equal to the SDID in the rule given by RULEID.

(and we should probably clarify that the scoping to a particular rule or SDID applies even when PPNV == 1.)