ARC Feedback

[ved-rivos]

• 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 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”

[ved-rivos]

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.

[ved-rivos]

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.

[ved-rivos]

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.

[ved-rivos]

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

[ved-rivos]

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

[ved-rivos]

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.

[ved-rivos]

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.

[ved-rivos]

Updates in PR #49 and PR #50