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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 9 May 2025, at 13:24, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> 
> Define requirements for specific commands.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Changes from -
> 
> v1 - 1. Reworded the requirement so as to avoid mentioining variable names
> or hardcoded strings. Otherwise, one would need to change the requirement
> each time the code changes.
> 
> v2 - 1. Moved few changes to previous patch.
> 
> .../fusa/reqs/design-reqs/arm64/hypercall.rst | 15 ++++
> .../design-reqs/arm64/version_hypercall.rst   | 34 ++++++++
> .../reqs/design-reqs/version_hypercall.rst    | 82 ++++++++++++++++++
> docs/fusa/reqs/index.rst                      |  3 +
> docs/fusa/reqs/product-reqs/hypercall.rst     | 20 +++++
> .../reqs/product-reqs/version_hypercall.rst   | 83 +++++++++++++++++++
> 6 files changed, 237 insertions(+)
> create mode 100644 docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> create mode 100644 docs/fusa/reqs/design-reqs/version_hypercall.rst
> create mode 100644 docs/fusa/reqs/product-reqs/hypercall.rst
> 
> diff --git a/docs/fusa/reqs/design-reqs/arm64/hypercall.rst 
> b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> index f00b0b00f9..f58a9d50aa 100644
> --- a/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> @@ -56,3 +56,18 @@ Comments:
> Covers:
>  - `XenProd~version_hyp_first_param~1`
>  - `XenProd~version_hyp_second_param~1`
> +
> +Return value
> +------------
> +
> +`XenSwdgn~arm64_ret_val~1`
> +
> +Description:
> +Xen shall store the return value in first cpu core register.

use x0 instead.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~hyp_err_ret_val~1`
> 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..3aa12ea2c2
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> @@ -0,0 +1,34 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Capabilities
> +------------
> +
> +`XenSwdgn~arm64_capabilities~1`
> +
> +Description:
> +Xen shall have an internal constant string to denote that the cpu is running
> +in arm64 mode.

This is untestable if this is purely internal so this cannot be a requirement
I am not quite sure why you need this, can you explain ?

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_capabilities_cmd~1`
> +
> +Capabilities AArch32
> +--------------------
> +
> +`XenSwdgn~arm64_capabilities_aarch32~1`
> +
> +Description:
> +Xen shall have a internal constant string to denote that the cpu is running 
> in
> +arm32 mode.
> +
> +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..aac5896965
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/version_hypercall.rst
> @@ -0,0 +1,82 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Version
> +-------
> +
> +`XenSwdgn~version~1`
> +
> +Description:
> +Xen shall have a internal constant (XEN_VERSION) storing the version number
> +coming from the Makefile.

I really have doubts about this and the following one.

If this only goal is to say what should be returned in the XEN_VERSION hypcall 
you might
just need something saying it and mention this as a comment because you will 
have a very
hard time to test such a requirement.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_version_cmd~1`
> +
> +Subversion
> +----------
> +
> +`XenSwdgn~subversion~1`
> +
> +Description:
> +Xen shall have a internal constant (XEN_SUBVERSION) storing the sub version
> +number coming from the Makefile.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_version_cmd~1`
> +
> +Error copying buffer
> +--------------------
> +
> +`XenSwdgn~error_copy_buffer~1`
> +
> +Description:
> +Xen shall return -EFAULT if it is not able to copy data to domain's buffer.
> +
> +Rationale:
> +-EFAULT is one of the error code defined in
> +http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/errno.h.
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~hyp_err_ret_val~1`
> +
> +Extraversion
> +------------
> +
> +`XenSwdgn~extraversion~1`
> +
> +Description:
> +Xen shall have a internal constant (XEN_EXTRAVERSION) storing the 
> extraversion
> +coming from the build environment.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_extraversion_cmd~1`
> +
> +Changeset
> +---------
> +
> +`XenSwdgn~changeset~1`
> +
> +Description:
> +Xen shall have a internal constant string (XEN_CHANGESET) storing the date,
> +time and git hash of the last change made to Xen's codebase.
> +
> +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..de19b0cda2 100644
> --- a/docs/fusa/reqs/index.rst
> +++ b/docs/fusa/reqs/index.rst
> @@ -11,6 +11,9 @@ Requirements documentation
>    product-reqs/reqs
>    product-reqs/arm64/reqs
>    product-reqs/version_hypercall
> +   product-reqs/hypercall
>    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/hypercall.rst 
> b/docs/fusa/reqs/product-reqs/hypercall.rst
> new file mode 100644
> index 0000000000..b57b9acde8
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/hypercall.rst
> @@ -0,0 +1,20 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Error Return Value
> +------------------
> +
> +`XenProd~hyp_err_ret_val~1`
> +
> +Description:
> +In case the hypercall fails, Xen shall return one of the error codes defined
> +in 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/errno.h.

s/the/an/ hypercall otherwise it is not quite clear which hypercall you mean.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> diff --git a/docs/fusa/reqs/product-reqs/version_hypercall.rst 
> b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> index 400d51bbeb..2ef1c4f9ca 100644
> --- a/docs/fusa/reqs/product-reqs/version_hypercall.rst
> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> @@ -41,3 +41,86 @@ 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 register 0.
> +
> +Rationale:
> +
> +Comments:
> +Xen version is composed of major and minor number.

How Xen version is encoded should be a requirement, you can add this directly 
into
the definition of this one in fact i think.

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

Isn't it mandatory for xen to return aarch64 for arm64 ?
This could be turned into a requirement (easily testable).

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

Does this has a standard format ? if so it should be explained.

> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> -- 
> 2.25.1
> 

Cheers
Bertrand