Re: [PATCH v3] docs: fusa: Add Assumption of Use (AoU)

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 16 Sep 2024, at 14:18, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> 
> From: Michal Orzel <michal.orzel@xxxxxxx>
> 
> AoU are the assumptions that Xen relies on other components (eg platform
> platform, domains)
> to fulfill its requirements. In our case, platform means a combination
> of hardware, firmware and bootloader.
> 
> We have defined AoU in the intro.rst and added AoU for the generic
> timer.
> 
> Also, fixed a requirement to denote that Xen shall **not** expose the
> system counter frequency via the "clock-frequency" device tree property.
> The reason being the device tree documentation strongly discourages the
> use of this peoperty. Further if the "clock-frequency" is exposed, then
> it overrides the value programmed in the CNTFRQ_EL0 register.
> 
> So, the frequency shall be exposed via the CNTFRQ_EL0 register only and
> consequently there is an assumption on the platform to program the
> register correctly.
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
> ---
> Changes from :-
> 
> v1 - 1. Removed the part of requirement which states that Xen exposes the
> frequency of the system timer by reading the "clock-frequency" property.
> 
> 2. Added a rationale for AoU.
> 
> 3. Reworded the AoU.
> 
> v2 - 1. Reworded the commit message. Added R-b.
> 
> .../reqs/design-reqs/arm64/generic-timer.rst  | 24 ++++++++++++++++++-
> docs/fusa/reqs/intro.rst                      | 10 ++++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst 
> b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> index f2a0cd7fb8..86d84a3c40 100644
> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> @@ -30,7 +30,7 @@ Read system counter frequency
> 
> Description:
> Xen shall expose the frequency of the system counter to the domains in
> -CNTFRQ_EL0 register and/or domain device tree's "clock-frequency" property.
> +CNTFRQ_EL0 register.
> 
> Rationale:
> 
> @@ -116,6 +116,28 @@ Rationale:
> 
> Comments:
> 
> +Covers:
> + - `XenProd~emulated_timer~1`
> +
> +Assumption of Use on the Platform
> +=================================
> +
> +Expose system timer frequency via register
> +------------------------------------------
> +
> +`XenSwdgn~arm64_generic_timer_pf_program_cntfrq_el0~1`
> +
> +Description:
> +Underlying platform shall program CNTFRQ_EL0 register with the value of 
> system
> +timer frequency.

How about: CNTFRQ_EL0 register shall be programmed with the value of the system 
timer frequency.

It prevent to use "platform" which is quite undefined here.

> +
> +Rationale:
> +Xen reads the CNTFRQ_EL0 register to get the value of system timer frequency.
> +While there is a provision to get this value by reading the "clock-frequency"
> +dt property [2], the use of this property is strongly discouraged.

I would put the second sentence as a comment as only the first one is the 
rationale explaining
why we need it to be correct.

> +
> +Comments:
> +
> Covers:
>  - `XenProd~emulated_timer~1`
> 
> diff --git a/docs/fusa/reqs/intro.rst b/docs/fusa/reqs/intro.rst
> index 245a219ff2..aa85ff821c 100644
> --- a/docs/fusa/reqs/intro.rst
> +++ b/docs/fusa/reqs/intro.rst
> @@ -38,6 +38,16 @@ The requirements are linked using OpenFastTrace
> OpenFastTrace parses through the requirements and generates a traceability
> report.
> 
> +Assumption of Use
> +=================
> +
> +To fulfill one or more design requirements, there may be underlying 
> assumptions
> +on one or more components that Xen interacts with directly or indirectly. For
> +eg, there may be assumptions on the underlying platform (hardware + firmware 
> +
> +bootloader) to set certain registers, etc. The important thing here is that
> +anyone who validates these requirements, need to consider the assumption on 
> the
> +other components.

I would simplify a bit:
Xen is making several assumptions on the status of the platform or on some
functions being present and functional. For example, Xen might assume that
some registers are set.
Anybody who wants to use Xen must validate that the platform it is used on
(meaning the hardware and any software running before Xen like the firmware)
fulfils all the AoU described by Xen.

What do you think ?

Cheers
Bertrand

> +
> The following is the skeleton for a requirement.
> 
> Title of the requirement
> -- 
> 2.25.1
>