Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> 
> We have written the requirements for some of the commands of the XEN_VERSION
> hypercall.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> .../design-reqs/arm64/version_hypercall.rst   | 33 ++++++++
> .../reqs/design-reqs/version_hypercall.rst    | 65 +++++++++++++++
> docs/fusa/reqs/index.rst                      |  2 +
> .../reqs/product-reqs/version_hypercall.rst   | 82 +++++++++++++++++++
> 4 files changed, 182 insertions(+)
> create mode 100644 docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> create mode 100644 docs/fusa/reqs/design-reqs/version_hypercall.rst
> 
> diff --git a/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst 
> b/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> new file mode 100644
> index 0000000000..1dad2b84c2
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> @@ -0,0 +1,33 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Capabilities
> +------------
> +
> +`XenSwdgn~arm64_capabilities~1`
> +
> +Description:
> +Xen shall have a internal constant string storing "xen-3.0-aarch64".

I would rather not have a requirement that will need changing every time.
Could we turn this into a description and link this to where we store the 
version ?

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_capabilities_cmd~1`
> +
> +Capabilities AArch32
> +--------------------
> +
> +`XenSwdgn~arm64_capabilities_aarch32~1`
> +
> +Description:
> +Xen shall have a internal constant string storing "xen-3.0-armv7l" when it
> +detects that the cpu is running in AArch32 mode.
> +

Same here and also you have a "when" here and not in previous one.

> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_capabilities_cmd~1`
> +
> diff --git a/docs/fusa/reqs/design-reqs/version_hypercall.rst 
> b/docs/fusa/reqs/design-reqs/version_hypercall.rst
> new file mode 100644
> index 0000000000..8bb7a66576
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/version_hypercall.rst
> @@ -0,0 +1,65 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Version
> +-------
> +
> +`XenSwdgn~version~1`
> +
> +Description:
> +Xen shall have a internal constant storing the version number coming from the
> +Makefile.

If you go this far i think you should give the name of the constant.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_version_cmd~1`
> +
> +Subversion
> +----------
> +
> +`XenSwdgn~subversion~1`
> +
> +Description:
> +Xen shall have a internal constant storing the sub version number coming from
> +the Makefile.

Same here, please name it.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_version_cmd~1`
> +
> +Extraversion
> +------------
> +
> +`XenSwdgn~extraversion~1`
> +
> +Description:
> +Xen shall have a internal constant string storing the extraversion coming 
> from
> +the build environment.

Same here.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_extraversion_cmd~1`
> +
> +Changeset
> +---------
> +
> +`XenSwdgn~changeset~1`
> +
> +Description:
> +Xen shall have a internal constant string storing the date, time and git hash
> +of the last change made to Xen's codebase.

Same here.
Also i would use the comment here and in previous reqs to give an example.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_changeset_cmd~1`
> diff --git a/docs/fusa/reqs/index.rst b/docs/fusa/reqs/index.rst
> index d8683edce7..b85af19d19 100644
> --- a/docs/fusa/reqs/index.rst
> +++ b/docs/fusa/reqs/index.rst
> @@ -14,3 +14,5 @@ Requirements documentation
>    design-reqs/arm64/generic-timer
>    design-reqs/arm64/sbsa-uart
>    design-reqs/arm64/hypercall
> +   design-reqs/arm64/version_hypercall
> +   design-reqs/version_hypercall
> diff --git a/docs/fusa/reqs/product-reqs/version_hypercall.rst 
> b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> index fdb8da04e1..10bc7b6e87 100644
> --- a/docs/fusa/reqs/product-reqs/version_hypercall.rst
> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> @@ -59,3 +59,85 @@ Covers:
> Needs:
>  - XenSwdgn
> 
> +Version command
> +---------------
> +
> +`XenProd~version_hyp_version_cmd~1`
> +
> +Description:
> +Xen shall provide a command (num 0) for  hypercall (num 17) to retrieve Xen's
> +version in the domain's x0 register.

Somehow you will need a req saying that how and hypercall is specified in 
general
and then one req per hypercall:
Xen hypercall number 0  shall return the Xen version in register 0.
I would also prevent saying x0 which would make this aarch64 specific.

> +
> +Rationale:
> +
> +Comments:
> +Xen version is composed of major and minor number.

Can't we link to the requirement defining where the version is stored ?

> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Extraversion command
> +--------------------
> +
> +`XenProd~version_hyp_extraversion_cmd~1`
> +
> +Description:
> +Xen shall provide a command (num 1) for hypercall (num 17) to copy its
> +extraversion in the domain's buffer.
> +
> +Rationale:
> +
> +Comments:
> +Xen's extra version consists of a string passed with 'XEN_VENDORVERSION' 
> command
> +line parameter while building Xen.
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Capabilities command
> +--------------------
> +
> +`XenProd~version_hyp_capabilities_cmd~1`
> +
> +Description:
> +Xen shall provide a command (num 3) for hypercall (num 17) to copy its
> +capabilities to the domain's buffer.
> +
> +Rationale:
> +
> +Comments:
> +Capabilities related information is represented by char[1024].
> +For Arm64, the capabilities should contain "xen-3.0-aarch64" string.
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Changeset command
> +-----------------
> +
> +`XenProd~version_hyp_changeset_cmd~1`
> +
> +Description:
> +Xen shall provide a command (num 4) for hypercall (num 17) to copy changeset
> +to the domain's buffer.
> +
> +Rationale:
> +
> +Comments:
> +Changeset is string denoting the date, time and git hash of the last change
> +made to Xen's codebase.
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> -- 
> 2.25.1
> 

Cheers
Bertrand