Closed ved-rivos closed 1 week ago
IOM_300 mandates a 20-bit PASID, while IOM_170 allows 8-bit and 17-bit PASIDs. Recommend reconciling the conflict by removing IOM_170’s acceptance of 8- and 17-bit PASIDs.
IOM_170 requires that 20-bit PASID be supported but allows an implementation to support 8 and 17 bit PASID configurations in the IOMMU. IOM_300 mandates that the host bridge should not truncate the PASID from the PASID TLP that is input to the IOMMU. Thus they are not at conflict. The requirement IOM_300 enables the IOMMU to detect misconfigurations where a device may be programmed with a PASID wider than that configured in the pdtp.MODE
(8/17/20) in the IOMMU for that device. A clarification is included in IOM_300.
AER_080 Recommend clarifying what must not affect PCIe root port configuration registers (i.e., “affected by what”)
The requirement states that the configuration registers themselves must not be affected. The note following that requirement further clarifies why losing the root port configurations will cause issues with supporting hot-plug. This requirement has been placed because this is a commonly encountered integration issue.
MMS_030 The normative text in this requirement should clarify that address ranges corresponding to PCIe BARs with the Prefetchable bit set MAY have non-cacheable, idempotent, coherent, weakly-ordered PMAs, rather than placing an exception in the requirement’s non-normative section.
PCIe 6.0 does not have the concept of "Prefetchable" bit anymore. The concept of “Prefetchable” MMIO was originally needed to control PCI-PCI Bridges, which were allowed/encouraged to prefetch Memory Read data in prefetchable regions. MMIO that has read side-effects, e.g. locations that auto-increment when read, cannot safely be prefetched from, and so needed to be distinguished from regions that could. While this was intended to specifically focus on PCI behavior it has been a cause of confusion to system software developers and this was eventually fixed in PCIe 6.0 through "Removing Prefetchable Terminology ECN".
The MMS_030 specifically calls attention to “restricted programming model” to indicate the precautions provided by PCIe 6.0 specification when overriding the memory attribute of accesses made to device registers.
The spec is updated to include references to the ECN and explanation about the intent of "Prefetchable" to address such confusion.
IOM_190 Clarify why “at least 4 event counters” are required. Is there a rationale behind the number “4” here?
A typical performance analysis requires counting - translation requests, TLB misses, number of page walks, and possibly average page walk latency - for identifying performance bottlenecks at the top level. The count of 4 was arrived at based on review of HPM in IOMMU with the performance analysis SIG and based on experience with similar architectures. Some notes are in the minutes and follow on discussion. A note is now added to IOM_190
Several requirements appear to be duplicative of requirements in other specifications - in particular, the PCIe Specification. Consider removing these requirements from the RISC-V Server SoC Specification; instead, simply stating that normative requirements from certain referenced specifications such as the PCIe Base Specification 6.0 are incorporated here by reference. Examples: ACS_050; ECM_070; ECM_090; etc.
The ECM_090 was intended to draw attention to common integration mistakes such as ignoring ARI forwarding when making determination of when to convert type 1 to type 0 configuration requests. Added notes to further clarify.
Merged ECM_070 and ACS_050 into a single requirement on root complex integration - RCI_010
MSI_020 Unclear what the intention is behind this requirement and this may unintentionally conflict with IOMMU MSI configuration. Recommend removing it or clarifying it..
This is removed. The intent was to forbid needing vendor specific programming to route or handle MSIs as has been seen on some implementations.
IOM_250 This requirement leaves the details of the PMA and PMP checks vague. Recommend stating explicitly which PMA/PMP checks are intended to be performed here.
It is not possible to mandate which regions are protected regions for a platform and hence are covered by a PMP. The requirement has been clarified to state that the PMP checks are intended to restrict IOMMU accesses to the same memory that the software programming the hart can also access.
The IOMMU requires supporting its data structures and pages tables in main memory. It is clarified that supporting non main memory access for IOMMU data structures is not required.
Updates in PR #49 and PR #50
Several requirements appear to be duplicative of requirements in other specifications - in particular, the PCIe Specification. Consider removing these requirements from the RISC-V Server SoC Specification; instead, simply stating that normative requirements from certain referenced specifications such as the PCIe Base Specification 6.0 are incorporated here by reference.
The document contains several compound requirements, such as IOM_190 and ECM_020; consider decomposing these into multiple individual requirements.
SEC_010, SEC_020, SEC_030 The Server SoC specification Incorporates several sections from the RISC-V Security Model specification by reference as mandatory requirements; however, the Security Model specification specifically states that it is not intended to be normative. A normative specification should not incorporate sections of a non-normative specification. Either both specifications should be normative, or both should be non-normative. Recommend removing normative references to the Security Model Security Model specification specification until a version is available that has passed ARC and public review.
The Security Model specification is not on track to be frozen before or concurrently with the Server SoC specification. It seems unwise to have a frozen specification incorporating sections of a specification that has no clear line of sight to exit draft status.
Recommend removing references to the Security Model specification until a Security Model specification version is available that has passed ARC and public review.
SEC_060 The use of “such as” renders this requirement weak. Consider either clarifying what standards are acceptable (e.g., “security standards from national standards laboratories”), or explicitly listing the acceptable standards and removing the “such as”.
CTI_010, CTI_020 Clarify whether these requirements are also meant to apply to any comparators that may take the time CSR value as inputs, e.g., for any *timecmp registers.
CTI_020 This requirement should specify which power states the time CSR should continue to appear to function. In its current form, if strictly construed, the time CSR would need to appear to continue to count even when the entire system is powered off, which seems like an unnecessary requirement.
Recommend restricting the scope of the power states for which CTI_020 would apply. For example, the specification could state that this requirement applies to CPU complex power states, and that system power states (such as power-off or hibernation states) may not guarantee that the time CSR appears to be always-on.
IOM_170, IOM_300 IOM_300 mandates a 20-bit PASID, while IOM_170 allows 8-bit and 17-bit PASIDs. Recommend reconciling the conflict by removing IOM_170’s acceptance of 8- and 17-bit PASIDs.
IOM_190 Clarify why “at least 4 event counters” are required. Is there a rationale behind the number “4” here?
IOM_210 Clarify that this requirement refers to the “DBG field of the IOMMU capabilities register”
IOM_250 This requirement leaves the details of the PMA and PMP checks vague. Recommend stating explicitly which PMA/PMP checks are intended to be performed here.
MMS_030 The normative text in this requirement should clarify that address ranges corresponding to PCIe BARs with the Prefetchable bit set MAY have non-cacheable, idempotent, coherent, weakly-ordered PMAs, rather than placing an exception in the requirement’s non- normative section.
MMS_040, MMS_050 The language “MUST NOT lead to any other behavior”, if taken at face value, excludes other possibly useful actions. If the intention here is to forbid “hangs, deadlocks”, then consider rephrasing this as “MUST NOT lead to any abnormal behavior (hangs, deadlocks, etc.)”
MSI_020 Unclear what the intention is behind this requirement and this may unintentionally conflict with IOMMU MSI configuration. Recommend removing it or clarifying it..
PTM_030 Recommend clarifying that the usage of “time” at the end of this requirement refers to the RISC-V hart time CSR.
AER_080 Recommend clarifying what must not affect PCIe root port configuration registers (i.e., “affected by what”)
VSR_020 Clarify that this refers to PCIe configuration space registers
SID_020, MSI_030 The text in SID_020 referring to the INTx virtual wire interrupt signaling mechanism duplicates the MSI_030 requirement. Recommend removing the INTx-related text in SID_020.
SPM_010 The definition of “significant caches” is unclear. Recommend defining it, either here or in the glossary.
SPM_060 Consider replacing the initial “The” with “Any” for clarity
RAS_060 Clarify which “control register” this refers to
QOS_040 Consider pluralizing “the IOMMU”