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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

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.

- Ayan