From: Michal Orzel <michal.orzel@amd.com>

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@amd.com>
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
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.
+
+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.
+
+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.
+
 The following is the skeleton for a requirement.
 
 Title of the requirement
-- 
2.25.1

Hi Ayan,

> On 16 Sep 2024, at 14:18, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
> 
> From: Michal Orzel <michal.orzel@amd.com>
> 
> 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@amd.com>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> ---
> 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
> 

On 19/09/2024 13:01, Bertrand Marquis wrote:
> Hi Ayan,
Hi Bertrand,
>
>> On 16 Sep 2024, at 14:18, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> From: Michal Orzel <michal.orzel@amd.com>
>>
>> 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@amd.com>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>> ---
>> 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 think there is a typo.
> I would simplify a bit:
> Xen is making several assumptions on the status of the platform or on some
> functions being present and functional.
s/functional/functionality.
> 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 ?

It sounds ok.

- Ayan

Hi Ayan,

> On 23 Sep 2024, at 18:57, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> 
> On 19/09/2024 13:01, Bertrand Marquis wrote:
>> Hi Ayan,
> Hi Bertrand,
>> 
>>> On 16 Sep 2024, at 14:18, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>> 
>>> From: Michal Orzel <michal.orzel@amd.com>
>>> 
>>> 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@amd.com>
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>>> ---
>>> 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 think there is a typo.
>> I would simplify a bit:
>> Xen is making several assumptions on the status of the platform or on some
>> functions being present and functional.
> s/functional/functionality.

no that sounds weird, functional is right in the sentence i think.

>> 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 ?
> 
> It sounds ok.

Good
Cheers
Bertrand

> 
> - Ayan

On 24/09/2024 8:14 am, Bertrand Marquis wrote:
>>>> 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 think there is a typo.
>>> I would simplify a bit:
>>> Xen is making several assumptions on the status of the platform or on some
>>> functions being present and functional.
>> s/functional/functionality.
> no that sounds weird, functional is right in the sentence i think.

"functional" is correct grammar here.  "functionality" would not be.

However, "functions being present and functional" less than ideal.  I'd
suggest "present and working", "present and operational" instead, or to
rework the first "functions" into something else.

~Andrew

Hi Andrew,

> On 24 Sep 2024, at 10:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 24/09/2024 8:14 am, Bertrand Marquis wrote:
>>>>> 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 think there is a typo.
>>>> I would simplify a bit:
>>>> Xen is making several assumptions on the status of the platform or on some
>>>> functions being present and functional.
>>> s/functional/functionality.
>> no that sounds weird, functional is right in the sentence i think.
> 
> "functional" is correct grammar here.  "functionality" would not be.
> 
> However, "functions being present and functional" less than ideal.  I'd
> suggest "present and working", "present and operational" instead, or to
> rework the first "functions" into something else.

"Present and operational" sounds good to me as they might be working
but be deactivated by firmware for example.

Cheers
Bertrand

> 
> ~Andrew

On 24/09/2024 09:32, Bertrand Marquis wrote:
> Hi Andrew,
Hi Andrew/Bertrand,
>
>> On 24 Sep 2024, at 10:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 24/09/2024 8:14 am, Bertrand Marquis wrote:
>>>>>> 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 think there is a typo.
>>>>> I would simplify a bit:
>>>>> Xen is making several assumptions on the status of the platform or on some
>>>>> functions being present and functional.
>>>> s/functional/functionality.
>>> no that sounds weird, functional is right in the sentence i think.
>> "functional" is correct grammar here.  "functionality" would not be.
Yes, I have left it as "functional" in my v4. I saw this message after I 
had sent out the patch.
>>
>> However, "functions being present and functional" less than ideal.  I'd
>> suggest "present and working", "present and operational" instead, or to
>> rework the first "functions" into something else.
> "Present and operational" sounds good to me as they might be working
> but be deactivated by firmware for example.

I think the definition of AoU, seems clear to us.

Also any of the suggestions made by Andrew or Bertrand seems ok. (I 
don't have any opinion which one sounds better).

I will leave it to Stefano to fix the patch on commit (as suggested by 
Bertrand in v4).

- Ayan