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

[Michal Orzel <michal.orzel@xxxxxxx>]

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@xxxxxxx>
>>>>>>>>>
>>>>>>>>> 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@xxxxxxx>
>>>>>>>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
>>>>>>>>> ---
>>>>>>>>>     .../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