diff mbox series

docs: fusa: Add Assumption of Use (AOU)

Message ID 20240906101318.1419954-1-ayan.kumar.halder@amd.com (mailing list archive)
State Superseded
Headers show
Series docs: fusa: Add Assumption of Use (AOU) | expand

Commit Message

Ayan Kumar Halder Sept. 6, 2024, 10:13 a.m. UTC
From: Michal Orzel <michal.orzel@amd.com>

AOU are the assumptions Xen relies on other components (eg 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.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++++++++
 docs/fusa/reqs/intro.rst                      | 10 ++++++++++
 2 files changed, 29 insertions(+)

Comments

Julien Grall Sept. 8, 2024, 9:05 p.m. UTC | #1
Hi Ayan,

On 06/09/2024 11:13, Ayan Kumar Halder wrote:
> From: Michal Orzel <michal.orzel@amd.com>
> 
> AOU are the assumptions Xen relies on other components (eg platform, domains)

Searching online, I think the abbrevition is AoU rather than AOU. This 
would also match how we abbreviate in Xen (IOW if we use a lower case 
letter from the expanded name, then the letter in the acronym is also 
lower case).

> 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.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
>   .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++++++++
>   docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> index f2a0cd7fb8..9df87cf4e0 100644
> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains the system
> +timer frequency.

The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be 
invalid. This seems to contradict the Assumption of Use. Can you explain 
the difference?

> +
> +Rationale:

This seems to be a bit odd to have an empty section. Can you explain why?

> +
> +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

Cheers,

[1] 
https://lore.kernel.org/all/20240829113120.3980270-1-ayan.kumar.halder@amd.com/
Michal Orzel Sept. 9, 2024, 8:56 a.m. UTC | #2
Hi Julien,

On 08/09/2024 23:05, Julien Grall wrote:
> 
> 
> Hi Ayan,
> 
> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>> From: Michal Orzel <michal.orzel@amd.com>
>>
>> AOU are the assumptions Xen relies on other components (eg platform, domains)
> 
> Searching online, I think the abbrevition is AoU rather than AOU. This
> would also match how we abbreviate in Xen (IOW if we use a lower case
> letter from the expanded name, then the letter in the acronym is also
> lower case).
> 
>> 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.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>   .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++++++++
>>   docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>   2 files changed, 29 insertions(+)
>>
>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>> index f2a0cd7fb8..9df87cf4e0 100644
>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains the system
>> +timer frequency.
> 
> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
It is merged:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c

> invalid. This seems to contradict the Assumption of Use. Can you explain
> the difference?
The requirement you refer to is written from a domain perspective and is about Xen exposing the frequency
to domains via CNTFRQ and/or dt property. In case of a presence of dt property in the host dtb, Xen could for instance decide
to emulate CNTFRQ instead of relying on the domain to parse the dt at runtime.

The AoU on the platform (hw/firmware/bootloader) is written from Xen perspective and is about the platform
exposing the correct frequency via register. This is Xen expected behavior on the platform. In other words, the platform should
expose the correct frequency via register.

However, even if the frequency in the register is correct, the host dtb might still contain the "clock-frequency" property.
Even though it is stated in the bindings that this property is about overriding broken firmware (which in our case is not),
in real life scenarios, the property can still be there and can match the register. This is because the host dtb is created by the user, not platform.
Even in Linux dts base, you will find lots of examples of device trees with this property. How can Linux know if the firmware
has been fixed already or not? This is why we want to give possibility for a user *not* platform to provide a dt property.



> 
>> +
>> +Rationale:
> 
> This seems to be a bit odd to have an empty section. Can you explain why?
That's the format we decided to go with. It's been documented in docs/fusa/reqs/intro.rst.
While AFAICT it is not strictly required for OFT, in the future we can decide to write our own parser to
present the requirements in a nicer form that OFT exporter. Then, it will be easier for use if each
requirement defines the same fields (I agree it's a matter of opinion but that's what we decided to use).

> 
>> +
>> +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
> 
> Cheers,
> 
> [1]
> https://lore.kernel.org/all/20240829113120.3980270-1-ayan.kumar.halder@amd.com/
> 
> --
> Julien Grall

~Michal
Julien Grall Sept. 9, 2024, 9:11 a.m. UTC | #3
On 09/09/2024 09:56, Michal Orzel wrote:
> Hi Julien,
> 
> On 08/09/2024 23:05, Julien Grall wrote:
>>
>>
>> Hi Ayan,
>>
>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>> From: Michal Orzel <michal.orzel@amd.com>
>>>
>>> AOU are the assumptions Xen relies on other components (eg platform, domains)
>>
>> Searching online, I think the abbrevition is AoU rather than AOU. This
>> would also match how we abbreviate in Xen (IOW if we use a lower case
>> letter from the expanded name, then the letter in the acronym is also
>> lower case).
>>
>>> 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.
>>>
>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> ---
>>>    .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++++++++
>>>    docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>    2 files changed, 29 insertions(+)
>>>
>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>> index f2a0cd7fb8..9df87cf4e0 100644
>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains the system
>>> +timer frequency.
>>
>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
> It is merged:
> https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
> 
>> invalid. This seems to contradict the Assumption of Use. Can you explain
>> the difference?
> The requirement you refer to is written from a domain perspective and is about Xen exposing the frequency
> to domains via CNTFRQ and/or dt property. In case of a presence of dt property in the host dtb, Xen could for instance decide
> to emulate CNTFRQ instead of relying on the domain to parse the dt at runtime.

AFAICT, you can't trap CNTFRQ access. So what you suggest would not work.

> 
> The AoU on the platform (hw/firmware/bootloader) is written from Xen perspective and is about the platform
> exposing the correct frequency via register. This is Xen expected behavior on the platform. In other words, the platform should
> expose the correct frequency via register.

Xen is able to deal with broken CNTFRQ_EL0. So I don't understand why we 
we would want to make an assumption that it shall not be broken. What do 
you gain?


> 
> 
> 
>>
>>> +
>>> +Rationale:
>>
>> This seems to be a bit odd to have an empty section. Can you explain why?
> That's the format we decided to go with. It's been documented in docs/fusa/reqs/intro.rst.
> While AFAICT it is not strictly required for OFT, in the future we can decide to write our own parser to
> present the requirements in a nicer form that OFT exporter. Then, it will be easier for use if each
> requirement defines the same fields (I agree it's a matter of opinion but that's what we decided to use).

So this is explaining why you decided to add a section "Rationale", but 
this doesn't explain why you left it empty. Surely, if you write an 
assumption, you want to explain why.

Cheers,
Ayan Kumar Halder Sept. 9, 2024, 9:50 a.m. UTC | #4
Hi Julien,

On 09/09/2024 10:11, Julien Grall wrote:
>
>
> On 09/09/2024 09:56, Michal Orzel wrote:
>> Hi Julien,
>>
>> On 08/09/2024 23:05, Julien Grall wrote:
>>>
>>>
>>> Hi Ayan,
>>>
>>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>>> From: Michal Orzel <michal.orzel@amd.com>
>>>>
>>>> AOU are the assumptions Xen relies on other components (eg 
>>>> platform, domains)
>>>
>>> Searching online, I think the abbrevition is AoU rather than AOU. This
>>> would also match how we abbreviate in Xen (IOW if we use a lower case
>>> letter from the expanded name, then the letter in the acronym is also
>>> lower case).
>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>>> ---
>>>>    .../reqs/design-reqs/arm64/generic-timer.rst  | 19 
>>>> +++++++++++++++++++
>>>>    docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>>    2 files changed, 29 insertions(+)
>>>>
>>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst 
>>>> b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>> index f2a0cd7fb8..9df87cf4e0 100644
>>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains 
>>>> the system
>>>> +timer frequency.
>>>
>>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
>> It is merged:
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c 
>>
>>
>>> invalid. This seems to contradict the Assumption of Use. Can you 
>>> explain
>>> the difference?
>> The requirement you refer to is written from a domain perspective and 
>> is about Xen exposing the frequency
>> to domains via CNTFRQ and/or dt property. In case of a presence of dt 
>> property in the host dtb, Xen could for instance decide
>> to emulate CNTFRQ instead of relying on the domain to parse the dt at 
>> runtime.
>
> AFAICT, you can't trap CNTFRQ access. So what you suggest would not work.
>
>>
>> The AoU on the platform (hw/firmware/bootloader) is written from Xen 
>> perspective and is about the platform
>> exposing the correct frequency via register. This is Xen expected 
>> behavior on the platform. In other words, the platform should
>> expose the correct frequency via register.
>
> Xen is able to deal with broken CNTFRQ_EL0. 
Yes, this is correct if the user provides "clock-frequency" dt property.
> So I don't understand why we we would want to make an assumption that 
> it shall not be broken. What do you gain?

Refer 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml

```

Use of this property is strongly discouraged; fix your firmware unless 
absolutely impossible.

```

We wish to put the onus on the platform/firmware provider to program the 
register correctly. Otherwise, we will have to document it somewhere 
(may be safety manual) that User needs to provide the "clock-frequency" 
dt property.

We wish to put as little responsibility on the user as 
possible(especially when the dt documentation discourages it as well).

>
>
>>
>>
>>
>>>
>>>> +
>>>> +Rationale:
>>>
>>> This seems to be a bit odd to have an empty section. Can you explain 
>>> why?
>> That's the format we decided to go with. It's been documented in 
>> docs/fusa/reqs/intro.rst.
>> While AFAICT it is not strictly required for OFT, in the future we 
>> can decide to write our own parser to
>> present the requirements in a nicer form that OFT exporter. Then, it 
>> will be easier for use if each
>> requirement defines the same fields (I agree it's a matter of opinion 
>> but that's what we decided to use).
>
> So this is explaining why you decided to add a section "Rationale", 
> but this doesn't explain why you left it empty. Surely, if you write 
> an assumption, you want to explain why.

Agreed.

If you are fine with my rationale (explained before), I can put the 
exact same words.

- Ayan
Julien Grall Sept. 9, 2024, 3:16 p.m. UTC | #5
On 09/09/2024 10:50, Ayan Kumar Halder wrote:
> On 09/09/2024 10:11, Julien Grall wrote:
>>
>>
>> On 09/09/2024 09:56, Michal Orzel wrote:
>>> Hi Julien,
>>>
>>> On 08/09/2024 23:05, Julien Grall wrote:
>>>>
>>>>
>>>> Hi Ayan,
>>>>
>>>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>>>> From: Michal Orzel <michal.orzel@amd.com>
>>>>>
>>>>> AOU are the assumptions Xen relies on other components (eg 
>>>>> platform, domains)
>>>>
>>>> Searching online, I think the abbrevition is AoU rather than AOU. This
>>>> would also match how we abbreviate in Xen (IOW if we use a lower case
>>>> letter from the expanded name, then the letter in the acronym is also
>>>> lower case).
>>>>
>>>>> 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.
>>>>>
>>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>>>> ---
>>>>>    .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++ 
>>>>> ++++++
>>>>>    docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>>>    2 files changed, 29 insertions(+)
>>>>>
>>>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/ 
>>>>> docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>> index f2a0cd7fb8..9df87cf4e0 100644
>>>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains 
>>>>> the system
>>>>> +timer frequency.
>>>>
>>>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
>>> It is merged:
>>> https://xenbits.xen.org/gitweb/? 
>>> p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
>>>
>>>> invalid. This seems to contradict the Assumption of Use. Can you 
>>>> explain
>>>> the difference?
>>> The requirement you refer to is written from a domain perspective and 
>>> is about Xen exposing the frequency
>>> to domains via CNTFRQ and/or dt property. In case of a presence of dt 
>>> property in the host dtb, Xen could for instance decide
>>> to emulate CNTFRQ instead of relying on the domain to parse the dt at 
>>> runtime.
>>
>> AFAICT, you can't trap CNTFRQ access. So what you suggest would not work.
>>
>>>
>>> The AoU on the platform (hw/firmware/bootloader) is written from Xen 
>>> perspective and is about the platform
>>> exposing the correct frequency via register. This is Xen expected 
>>> behavior on the platform. In other words, the platform should
>>> expose the correct frequency via register.
>>
>> Xen is able to deal with broken CNTFRQ_EL0. 
> Yes, this is correct if the user provides "clock-frequency" dt property.
>> So I don't understand why we we would want to make an assumption that 
>> it shall not be broken. What do you gain?
> 
> Refer https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/ 
> linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> 
> ```
> 
> Use of this property is strongly discouraged; fix your firmware unless 
> absolutely impossible.
> 
> ```
> 
> We wish to put the onus on the platform/firmware provider to program the 
> register correctly. Otherwise, we will have to document it somewhere 
> (may be safety manual) that User needs to provide the "clock-frequency" 
> dt property.

I think you will have to. The integrator may not have the possibility to 
modify the firmware.

> We wish to put as little responsibility on the user as 
> possible(especially when the dt documentation discourages it as well).

I think there are some contradiction with the requirements. I understand 
they are for the guest interface only but, if I am not mistaken, the 
only way the "clock-frequency" can be visible to the guest is because it 
is part of the host DT.

Furthermore, as soon as the property is specified, then Xen will ignore 
the value from CNTFRQ_EL0. So if we want to push the firmware vendor  to 
expose a valid CNTFREQ_EL0, then I don't understand why we would want to 
mention the property in the requirements.

Cheers,
Stefano Stabellini Sept. 9, 2024, 7:53 p.m. UTC | #6
On Mon, 9 Sep 2024, Julien Grall wrote:
> On 09/09/2024 10:50, Ayan Kumar Halder wrote:
> > On 09/09/2024 10:11, Julien Grall wrote:
> > > 
> > > 
> > > On 09/09/2024 09:56, Michal Orzel wrote:
> > > > Hi Julien,
> > > > 
> > > > On 08/09/2024 23:05, Julien Grall wrote:
> > > > > 
> > > > > 
> > > > > Hi Ayan,
> > > > > 
> > > > > On 06/09/2024 11:13, Ayan Kumar Halder wrote:
> > > > > > From: Michal Orzel <michal.orzel@amd.com>
> > > > > > 
> > > > > > AOU are the assumptions Xen relies on other components (eg platform,
> > > > > > domains)
> > > > > 
> > > > > Searching online, I think the abbrevition is AoU rather than AOU. This
> > > > > would also match how we abbreviate in Xen (IOW if we use a lower case
> > > > > letter from the expanded name, then the letter in the acronym is also
> > > > > lower case).
> > > > > 
> > > > > > 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.
> > > > > > 
> > > > > > Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> > > > > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> > > > > > ---
> > > > > >    .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++
> > > > > > ++++++
> > > > > >    docs/fusa/reqs/intro.rst                      | 10 ++++++++++
> > > > > >    2 files changed, 29 insertions(+)
> > > > > > 
> > > > > > diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/
> > > > > > docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> > > > > > index f2a0cd7fb8..9df87cf4e0 100644
> > > > > > --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> > > > > > +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
> > > > > > @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains
> > > > > > the system
> > > > > > +timer frequency.
> > > > > 
> > > > > The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
> > > > It is merged:
> > > > https://xenbits.xen.org/gitweb/?
> > > > p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
> > > > 
> > > > > invalid. This seems to contradict the Assumption of Use. Can you
> > > > > explain
> > > > > the difference?
> > > > The requirement you refer to is written from a domain perspective and is
> > > > about Xen exposing the frequency
> > > > to domains via CNTFRQ and/or dt property. In case of a presence of dt
> > > > property in the host dtb, Xen could for instance decide
> > > > to emulate CNTFRQ instead of relying on the domain to parse the dt at
> > > > runtime.
> > > 
> > > AFAICT, you can't trap CNTFRQ access. So what you suggest would not work.
> > > 
> > > > 
> > > > The AoU on the platform (hw/firmware/bootloader) is written from Xen
> > > > perspective and is about the platform
> > > > exposing the correct frequency via register. This is Xen expected
> > > > behavior on the platform. In other words, the platform should
> > > > expose the correct frequency via register.
> > > 
> > > Xen is able to deal with broken CNTFRQ_EL0. 
> > Yes, this is correct if the user provides "clock-frequency" dt property.
> > > So I don't understand why we we would want to make an assumption that it
> > > shall not be broken. What do you gain?
> > 
> > Refer https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
> > linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> > 
> > ```
> > 
> > Use of this property is strongly discouraged; fix your firmware unless
> > absolutely impossible.
> > 
> > ```
> > 
> > We wish to put the onus on the platform/firmware provider to program the
> > register correctly. Otherwise, we will have to document it somewhere (may be
> > safety manual) that User needs to provide the "clock-frequency" dt property.
> 
> I think you will have to. The integrator may not have the possibility to
> modify the firmware.

Without getting into the specifics of CNTFRQ_EL0 and clock-frequency,
given that this is one of the first AoUs, let me clarify the spirit of
the AoUs.

When we say that Xen is "safe" we mean that it went through thousands of
tests so we are sure that in this specific configuration it is as
bug-free as we can reasonably make it.

"in this specific configuration" is important. Changing the
configuration might expand the scope or invalidate some of the tests.
Think of moving from a board with GICv2 to GICv3 as an example (we are
actually targeting GICv3 for safety, so this is not a great example,
but just to explain the point.)

So the AoUs are the set of assumptions Xen has toward the rest of the
system to make sure Xen operates "safely", with the word "safely"
defined as above.

Of course, Xen could totally work on systems with different AoUs (see
the GICv2 vs GICv3 example) but it would be outside the safety
parameters. In a way, it is similar to "security supported": there are
a bunch of Xen features that should work fine but are outside of
"security support" for one reason or the other.

If a user wants to use Xen on a system that breaks one of the AoUs, they
can, but we wouldn't promise it is "safe". For instance, imagine a user
running Xen on a GICv3 system if the safe version of Xen only validated
the GICv2 driver. Similarly to "security support", sometimes it is a bit
of a judgement call and it could be argued either way.

In the specific case of CNTFRQ_EL0, if we think Xen can be "safe" on a
system with a broken CNTFRQ_EL0 (thanks to the clock-frequency DT
property or other mechanisms), then we can remove this from the AoU. We
would probably have to have a different AoU about the presence of
clock-frequency. Otherwise, if we think we cannot really promise that
Xen is "safe" if CNTFRQ_EL0 is broken, then it is better to leave as is.

Keep in mind that users interested in safety, they are very likely to be
interested in the safety-certification of the entire system, which
includes the hardware as well. It is very likely that users will choose
a safety-certified board, which I am guessing would have a working
CNTFRQ_EL0. This is just a guess, I don't know the relationship between
CNTFRQ_EL0 and achieving hardware safety certifications.
Julien Grall Sept. 9, 2024, 8:59 p.m. UTC | #7
Hi Stefano,

On 09/09/2024 20:53, Stefano Stabellini wrote:
> On Mon, 9 Sep 2024, Julien Grall wrote:
>> On 09/09/2024 10:50, Ayan Kumar Halder wrote:
>>> On 09/09/2024 10:11, Julien Grall wrote:
>>>>
>>>>
>>>> On 09/09/2024 09:56, Michal Orzel wrote:
>>>>> Hi Julien,
>>>>>
>>>>> On 08/09/2024 23:05, Julien Grall wrote:
>>>>>>
>>>>>>
>>>>>> Hi Ayan,
>>>>>>
>>>>>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>>>>>> From: Michal Orzel <michal.orzel@amd.com>
>>>>>>>
>>>>>>> AOU are the assumptions Xen relies on other components (eg platform,
>>>>>>> domains)
>>>>>>
>>>>>> Searching online, I think the abbrevition is AoU rather than AOU. This
>>>>>> would also match how we abbreviate in Xen (IOW if we use a lower case
>>>>>> letter from the expanded name, then the letter in the acronym is also
>>>>>> lower case).
>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>>>>>> ---
>>>>>>>     .../reqs/design-reqs/arm64/generic-timer.rst  | 19 +++++++++++++
>>>>>>> ++++++
>>>>>>>     docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>>>>>     2 files changed, 29 insertions(+)
>>>>>>>
>>>>>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/
>>>>>>> docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>> index f2a0cd7fb8..9df87cf4e0 100644
>>>>>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register contains
>>>>>>> the system
>>>>>>> +timer frequency.
>>>>>>
>>>>>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
>>>>> It is merged:
>>>>> https://xenbits.xen.org/gitweb/?
>>>>> p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
>>>>>
>>>>>> invalid. This seems to contradict the Assumption of Use. Can you
>>>>>> explain
>>>>>> the difference?
>>>>> The requirement you refer to is written from a domain perspective and is
>>>>> about Xen exposing the frequency
>>>>> to domains via CNTFRQ and/or dt property. In case of a presence of dt
>>>>> property in the host dtb, Xen could for instance decide
>>>>> to emulate CNTFRQ instead of relying on the domain to parse the dt at
>>>>> runtime.
>>>>
>>>> AFAICT, you can't trap CNTFRQ access. So what you suggest would not work.
>>>>
>>>>>
>>>>> The AoU on the platform (hw/firmware/bootloader) is written from Xen
>>>>> perspective and is about the platform
>>>>> exposing the correct frequency via register. This is Xen expected
>>>>> behavior on the platform. In other words, the platform should
>>>>> expose the correct frequency via register.
>>>>
>>>> Xen is able to deal with broken CNTFRQ_EL0.
>>> Yes, this is correct if the user provides "clock-frequency" dt property.
>>>> So I don't understand why we we would want to make an assumption that it
>>>> shall not be broken. What do you gain?
>>>
>>> Refer https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>> linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
>>>
>>> ```
>>>
>>> Use of this property is strongly discouraged; fix your firmware unless
>>> absolutely impossible.
>>>
>>> ```
>>>
>>> We wish to put the onus on the platform/firmware provider to program the
>>> register correctly. Otherwise, we will have to document it somewhere (may be
>>> safety manual) that User needs to provide the "clock-frequency" dt property.
>>
>> I think you will have to. The integrator may not have the possibility to
>> modify the firmware.
> 
> Without getting into the specifics of CNTFRQ_EL0 and clock-frequency,
> given that this is one of the first AoUs, let me clarify the spirit of
> the AoUs.
> 
> When we say that Xen is "safe" we mean that it went through thousands of
> tests so we are sure that in this specific configuration it is as
> bug-free as we can reasonably make it.
> 
> "in this specific configuration" is important. Changing the
> configuration might expand the scope or invalidate some of the tests.
> Think of moving from a board with GICv2 to GICv3 as an example (we are
> actually targeting GICv3 for safety, so this is not a great example,
> but just to explain the point.)
> 
> So the AoUs are the set of assumptions Xen has toward the rest of the
> system to make sure Xen operates "safely", with the word "safely"
> defined as above.
> 
> Of course, Xen could totally work on systems with different AoUs (see
> the GICv2 vs GICv3 example) but it would be outside the safety
> parameters. In a way, it is similar to "security supported": there are
> a bunch of Xen features that should work fine but are outside of
> "security support" for one reason or the other.
> 
> If a user wants to use Xen on a system that breaks one of the AoUs, they
> can, but we wouldn't promise it is "safe". For instance, imagine a user
> running Xen on a GICv3 system if the safe version of Xen only validated
> the GICv2 driver. Similarly to "security support", sometimes it is a bit
> of a judgement call and it could be argued either way.
> 
> In the specific case of CNTFRQ_EL0, if we think Xen can be "safe" on a
> system with a broken CNTFRQ_EL0 (thanks to the clock-frequency DT
> property or other mechanisms), then we can remove this from the AoU. We
> would probably have to have a different AoU about the presence of
> clock-frequency. Otherwise, if we think we cannot really promise that
> Xen is "safe" if CNTFRQ_EL0 is broken, then it is better to leave as is.
> 
> Keep in mind that users interested in safety, they are very likely to be
> interested in the safety-certification of the entire system, which
> includes the hardware as well. It is very likely that users will choose
> a safety-certified board, which I am guessing would have a working
> CNTFRQ_EL0. This is just a guess, I don't know the relationship between
> CNTFRQ_EL0 and achieving hardware safety certifications.

Thanks for your input. I am open with the idea to require CNTFRQ_EL0 to 
be valid. But I think we need some consistency across the safety docs.

In this case, I think it would be inconsistent to mention 
"clock-frequency" in the requirement.

Cheers,
Ayan Kumar Halder Sept. 10, 2024, 12:16 p.m. UTC | #8
Hi Julien,

On 09/09/2024 21:59, Julien Grall wrote:
> Hi Stefano,
>
> On 09/09/2024 20:53, Stefano Stabellini wrote:
>> On Mon, 9 Sep 2024, Julien Grall wrote:
>>> On 09/09/2024 10:50, Ayan Kumar Halder wrote:
>>>> On 09/09/2024 10:11, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 09/09/2024 09:56, Michal Orzel wrote:
>>>>>> Hi Julien,
>>>>>>
>>>>>> On 08/09/2024 23:05, Julien Grall wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Ayan,
>>>>>>>
>>>>>>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>>>>>>> From: Michal Orzel <michal.orzel@amd.com>
>>>>>>>>
>>>>>>>> AOU are the assumptions Xen relies on other components (eg 
>>>>>>>> platform,
>>>>>>>> domains)
>>>>>>>
>>>>>>> Searching online, I think the abbrevition is AoU rather than 
>>>>>>> AOU. This
>>>>>>> would also match how we abbreviate in Xen (IOW if we use a lower 
>>>>>>> case
>>>>>>> letter from the expanded name, then the letter in the acronym is 
>>>>>>> also
>>>>>>> lower case).
>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>>>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>>>>>>> ---
>>>>>>>>     .../reqs/design-reqs/arm64/generic-timer.rst  | 19 
>>>>>>>> +++++++++++++
>>>>>>>> ++++++
>>>>>>>>     docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>>>>>>     2 files changed, 29 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/
>>>>>>>> docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>> index f2a0cd7fb8..9df87cf4e0 100644
>>>>>>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register 
>>>>>>>> contains
>>>>>>>> the system
>>>>>>>> +timer frequency.
>>>>>>>
>>>>>>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
>>>>>> It is merged:
>>>>>> https://xenbits.xen.org/gitweb/?
>>>>>> p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
>>>>>>
>>>>>>> invalid. This seems to contradict the Assumption of Use. Can you
>>>>>>> explain
>>>>>>> the difference?
>>>>>> The requirement you refer to is written from a domain perspective 
>>>>>> and is
>>>>>> about Xen exposing the frequency
>>>>>> to domains via CNTFRQ and/or dt property. In case of a presence 
>>>>>> of dt
>>>>>> property in the host dtb, Xen could for instance decide
>>>>>> to emulate CNTFRQ instead of relying on the domain to parse the 
>>>>>> dt at
>>>>>> runtime.
>>>>>
>>>>> AFAICT, you can't trap CNTFRQ access. So what you suggest would 
>>>>> not work.
>>>>>
>>>>>>
>>>>>> The AoU on the platform (hw/firmware/bootloader) is written from Xen
>>>>>> perspective and is about the platform
>>>>>> exposing the correct frequency via register. This is Xen expected
>>>>>> behavior on the platform. In other words, the platform should
>>>>>> expose the correct frequency via register.
>>>>>
>>>>> Xen is able to deal with broken CNTFRQ_EL0.
>>>> Yes, this is correct if the user provides "clock-frequency" dt 
>>>> property.
>>>>> So I don't understand why we we would want to make an assumption 
>>>>> that it
>>>>> shall not be broken. What do you gain?
>>>>
>>>> Refer https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>>> linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml 
>>>>
>>>>
>>>> ```
>>>>
>>>> Use of this property is strongly discouraged; fix your firmware unless
>>>> absolutely impossible.
>>>>
>>>> ```
>>>>
>>>> We wish to put the onus on the platform/firmware provider to 
>>>> program the
>>>> register correctly. Otherwise, we will have to document it 
>>>> somewhere (may be
>>>> safety manual) that User needs to provide the "clock-frequency" dt 
>>>> property.
>>>
>>> I think you will have to. The integrator may not have the 
>>> possibility to
>>> modify the firmware.
>>
>> Without getting into the specifics of CNTFRQ_EL0 and clock-frequency,
>> given that this is one of the first AoUs, let me clarify the spirit of
>> the AoUs.
>>
>> When we say that Xen is "safe" we mean that it went through thousands of
>> tests so we are sure that in this specific configuration it is as
>> bug-free as we can reasonably make it.
>>
>> "in this specific configuration" is important. Changing the
>> configuration might expand the scope or invalidate some of the tests.
>> Think of moving from a board with GICv2 to GICv3 as an example (we are
>> actually targeting GICv3 for safety, so this is not a great example,
>> but just to explain the point.)
>>
>> So the AoUs are the set of assumptions Xen has toward the rest of the
>> system to make sure Xen operates "safely", with the word "safely"
>> defined as above.
>>
>> Of course, Xen could totally work on systems with different AoUs (see
>> the GICv2 vs GICv3 example) but it would be outside the safety
>> parameters. In a way, it is similar to "security supported": there are
>> a bunch of Xen features that should work fine but are outside of
>> "security support" for one reason or the other.
>>
>> If a user wants to use Xen on a system that breaks one of the AoUs, they
>> can, but we wouldn't promise it is "safe". For instance, imagine a user
>> running Xen on a GICv3 system if the safe version of Xen only validated
>> the GICv2 driver. Similarly to "security support", sometimes it is a bit
>> of a judgement call and it could be argued either way.
>>
>> In the specific case of CNTFRQ_EL0, if we think Xen can be "safe" on a
>> system with a broken CNTFRQ_EL0 (thanks to the clock-frequency DT
>> property or other mechanisms), then we can remove this from the AoU. We
>> would probably have to have a different AoU about the presence of
>> clock-frequency. Otherwise, if we think we cannot really promise that
>> Xen is "safe" if CNTFRQ_EL0 is broken, then it is better to leave as is.
>>
>> Keep in mind that users interested in safety, they are very likely to be
>> interested in the safety-certification of the entire system, which
>> includes the hardware as well. It is very likely that users will choose
>> a safety-certified board, which I am guessing would have a working
>> CNTFRQ_EL0. This is just a guess, I don't know the relationship between
>> CNTFRQ_EL0 and achieving hardware safety certifications.
>
> Thanks for your input. I am open with the idea to require CNTFRQ_EL0 
> to be valid. But I think we need some consistency across the safety docs.
Agreed.
>
> In this case, I think it would be inconsistent to mention 
> "clock-frequency" in the requirement.

Yes, I see your point now. So the requirement should just state.

"Xen shall expose the frequency of the system counter to the domains in
CNTFRQ_EL0".

The AoU will complement this requirement

"Underlying platform shall ensure that CNTFRQ_EL0 register contains the 
system timer frequency.".

It makes sense to me.

@Michal , any inputs.

- Ayan
Michal Orzel Sept. 10, 2024, 12:41 p.m. UTC | #9
On 10/09/2024 14:16, Ayan Kumar Halder wrote:
> Hi Julien,
> 
> On 09/09/2024 21:59, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 09/09/2024 20:53, Stefano Stabellini wrote:
>>> On Mon, 9 Sep 2024, Julien Grall wrote:
>>>> On 09/09/2024 10:50, Ayan Kumar Halder wrote:
>>>>> On 09/09/2024 10:11, Julien Grall wrote:
>>>>>>
>>>>>>
>>>>>> On 09/09/2024 09:56, Michal Orzel wrote:
>>>>>>> Hi Julien,
>>>>>>>
>>>>>>> On 08/09/2024 23:05, Julien Grall wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Ayan,
>>>>>>>>
>>>>>>>> On 06/09/2024 11:13, Ayan Kumar Halder wrote:
>>>>>>>>> From: Michal Orzel <michal.orzel@amd.com>
>>>>>>>>>
>>>>>>>>> AOU are the assumptions Xen relies on other components (eg 
>>>>>>>>> platform,
>>>>>>>>> domains)
>>>>>>>>
>>>>>>>> Searching online, I think the abbrevition is AoU rather than 
>>>>>>>> AOU. This
>>>>>>>> would also match how we abbreviate in Xen (IOW if we use a lower 
>>>>>>>> case
>>>>>>>> letter from the expanded name, then the letter in the acronym is 
>>>>>>>> also
>>>>>>>> lower case).
>>>>>>>>
>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>>>>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>>>>>>>> ---
>>>>>>>>>     .../reqs/design-reqs/arm64/generic-timer.rst  | 19 
>>>>>>>>> +++++++++++++
>>>>>>>>> ++++++
>>>>>>>>>     docs/fusa/reqs/intro.rst                      | 10 ++++++++++
>>>>>>>>>     2 files changed, 29 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/
>>>>>>>>> docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>>> index f2a0cd7fb8..9df87cf4e0 100644
>>>>>>>>> --- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>>> +++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
>>>>>>>>> @@ -116,6 +116,25 @@ 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 ensure that CNTFRQ_EL0 register 
>>>>>>>>> contains
>>>>>>>>> the system
>>>>>>>>> +timer frequency.
>>>>>>>>
>>>>>>>> The wording in [1] (not yet merged) implies that CNTFRQ_EL0 may be
>>>>>>> It is merged:
>>>>>>> https://xenbits.xen.org/gitweb/?
>>>>>>> p=xen.git;a=commit;h=51ad2c57a2d21b583a5944a0dc21c709af022f3c
>>>>>>>
>>>>>>>> invalid. This seems to contradict the Assumption of Use. Can you
>>>>>>>> explain
>>>>>>>> the difference?
>>>>>>> The requirement you refer to is written from a domain perspective 
>>>>>>> and is
>>>>>>> about Xen exposing the frequency
>>>>>>> to domains via CNTFRQ and/or dt property. In case of a presence 
>>>>>>> of dt
>>>>>>> property in the host dtb, Xen could for instance decide
>>>>>>> to emulate CNTFRQ instead of relying on the domain to parse the 
>>>>>>> dt at
>>>>>>> runtime.
>>>>>>
>>>>>> AFAICT, you can't trap CNTFRQ access. So what you suggest would 
>>>>>> not work.
>>>>>>
>>>>>>>
>>>>>>> The AoU on the platform (hw/firmware/bootloader) is written from Xen
>>>>>>> perspective and is about the platform
>>>>>>> exposing the correct frequency via register. This is Xen expected
>>>>>>> behavior on the platform. In other words, the platform should
>>>>>>> expose the correct frequency via register.
>>>>>>
>>>>>> Xen is able to deal with broken CNTFRQ_EL0.
>>>>> Yes, this is correct if the user provides "clock-frequency" dt 
>>>>> property.
>>>>>> So I don't understand why we we would want to make an assumption 
>>>>>> that it
>>>>>> shall not be broken. What do you gain?
>>>>>
>>>>> Refer https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>>>> linux.git/tree/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml 
>>>>>
>>>>>
>>>>> ```
>>>>>
>>>>> Use of this property is strongly discouraged; fix your firmware unless
>>>>> absolutely impossible.
>>>>>
>>>>> ```
>>>>>
>>>>> We wish to put the onus on the platform/firmware provider to 
>>>>> program the
>>>>> register correctly. Otherwise, we will have to document it 
>>>>> somewhere (may be
>>>>> safety manual) that User needs to provide the "clock-frequency" dt 
>>>>> property.
>>>>
>>>> I think you will have to. The integrator may not have the 
>>>> possibility to
>>>> modify the firmware.
>>>
>>> Without getting into the specifics of CNTFRQ_EL0 and clock-frequency,
>>> given that this is one of the first AoUs, let me clarify the spirit of
>>> the AoUs.
>>>
>>> When we say that Xen is "safe" we mean that it went through thousands of
>>> tests so we are sure that in this specific configuration it is as
>>> bug-free as we can reasonably make it.
>>>
>>> "in this specific configuration" is important. Changing the
>>> configuration might expand the scope or invalidate some of the tests.
>>> Think of moving from a board with GICv2 to GICv3 as an example (we are
>>> actually targeting GICv3 for safety, so this is not a great example,
>>> but just to explain the point.)
>>>
>>> So the AoUs are the set of assumptions Xen has toward the rest of the
>>> system to make sure Xen operates "safely", with the word "safely"
>>> defined as above.
>>>
>>> Of course, Xen could totally work on systems with different AoUs (see
>>> the GICv2 vs GICv3 example) but it would be outside the safety
>>> parameters. In a way, it is similar to "security supported": there are
>>> a bunch of Xen features that should work fine but are outside of
>>> "security support" for one reason or the other.
>>>
>>> If a user wants to use Xen on a system that breaks one of the AoUs, they
>>> can, but we wouldn't promise it is "safe". For instance, imagine a user
>>> running Xen on a GICv3 system if the safe version of Xen only validated
>>> the GICv2 driver. Similarly to "security support", sometimes it is a bit
>>> of a judgement call and it could be argued either way.
>>>
>>> In the specific case of CNTFRQ_EL0, if we think Xen can be "safe" on a
>>> system with a broken CNTFRQ_EL0 (thanks to the clock-frequency DT
>>> property or other mechanisms), then we can remove this from the AoU. We
>>> would probably have to have a different AoU about the presence of
>>> clock-frequency. Otherwise, if we think we cannot really promise that
>>> Xen is "safe" if CNTFRQ_EL0 is broken, then it is better to leave as is.
>>>
>>> Keep in mind that users interested in safety, they are very likely to be
>>> interested in the safety-certification of the entire system, which
>>> includes the hardware as well. It is very likely that users will choose
>>> a safety-certified board, which I am guessing would have a working
>>> CNTFRQ_EL0. This is just a guess, I don't know the relationship between
>>> CNTFRQ_EL0 and achieving hardware safety certifications.
>>
>> Thanks for your input. I am open with the idea to require CNTFRQ_EL0 
>> to be valid. But I think we need some consistency across the safety docs.
> Agreed.
>>
>> In this case, I think it would be inconsistent to mention 
>> "clock-frequency" in the requirement.
> 
> Yes, I see your point now. So the requirement should just state.
> 
> "Xen shall expose the frequency of the system counter to the domains in
> CNTFRQ_EL0".
> 
> The AoU will complement this requirement
> 
> "Underlying platform shall ensure that CNTFRQ_EL0 register contains the 
> system timer frequency.".
> 
> It makes sense to me.
> 
> @Michal , any inputs.
In other words, keep this AoU as is and remove the following:
"and/or domain device tree's "clock-frequency" property."
from the SSR. I'm ok with that.

~Michal
diff mbox series

Patch

diff --git a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
index f2a0cd7fb8..9df87cf4e0 100644
--- a/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
+++ b/docs/fusa/reqs/design-reqs/arm64/generic-timer.rst
@@ -116,6 +116,25 @@  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 ensure that CNTFRQ_EL0 register contains the system
+timer frequency.
+
+Rationale:
+
+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