brief review comments (0.1)

[sorear]

CTI_020 The time counter MUST appear to be always on and MUST appear to not lose its count across power states, including when the hart is powered off.

Given that all transitions between started and non-started hart states are mediated by the SBI, this seems to be a firmware requirement, unlike all of the other requirements in this spec which are clear hardware requirements.

Is this requirement intended to constrain behavior in states where all harts are powered off, such as during SBI system suspend?

IOM_020 ... This requirement does not apply to platform devices such as the APLIC or the IOMMU itself.

Under QEMU and Linux terminology, a "platform device" can be almost anything as long as it's not under a PCIe or similar bus. Are we using the wide definition here or is this intended to be a narrow cutout for things that the IOMMU requires itself to work?

IOM_250, ADR_020 ... MUST enforce the physical memory attribute checks and physical memory protection checks on memory accesses ...

The draft doesn't reference the IOPMP draft. Is "physical memory protection" for IOMMUs intended to be implementation-defined?

SPM_010 It is recommended that a cache with a capacity larger than 32 KiB be considered a significant cache.

An unfortunate size cutoff since it's right in the middle of the typical range for core-integrated L1 caches, which SoC integrators may not be able to modify.

[SiFiveHolland]

CTI_020 The time counter MUST appear to be always on and MUST appear to not lose its count across power states, including when the hart is powered off.

Given that all transitions between started and non-started hart states are mediated by the SBI, this seems to be a firmware requirement, unlike all of the other requirements in this spec which are clear hardware requirements.

Does this requirement imply that the Supervisor Timer Interrupt must be able to wake up the hart from those power states? (Failing to trigger the interrupt could be interpreted as the time counter being off.) If so, that would be a hardware requirement.

[ved-rivos]

In general the specification does not mandate an implementation method for the requirements. The specifications provides requirements on behavior observable by OS/hypervisors. Many SoC implementations may achieve the requirements outlined in this specification through a combination of hardware firmware. The firmware may execute in the machine mode of the application processor harts or in other controllers in the SoC such as a power management controller or a system management controller.

The CTI_020 is only requiring the time counter to not lose state. The specification provides implementation flexibility in achieving the requirement as observed by portable OS/hypervisors. The method employed should preserve the requirements outlined in CTI_010 and the Priv. architecture on having time synchronized across harts and being monotonic in nature. Whether a low power idle state will cause the timer context to be lost is unspecified but is discoverable by the OS using the ACPI _LPI object.

The terminology of platform devices is used to reference to the IOMMU and APLIC. It is not intended to include other devices in the form Qemu may be referencing them from the description. I will try to look for a better terminology for this. Please add your suggestions.

The physical memory protection mechanisms using PMP or IOPMP or World Guard or other mechanisms are to protect machine resources i.e. resources made otherwise inaccessible to the operating system. This specification is not requiring a specific mechanism to be used by the SoC as that is not observable by or interacted with by portable operating systems and hypervisors.

The SPM_010 is providing a recommendation on the sizes that should be considered as significant caches to provide system integrator the tools necessary for workload placement and tuning.

[ved-rivos]

I suggest removing the term "platform devices" and just leave the list of initiators to which this requirement does not apply. Update in PR #35