Re: [PATCH v3 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 9 May 2025, at 13:24, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> 
> Define the requirements which are common for all the commands for XEN_VERSION
> hypercall.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> Changes from -
> 
> v1 - 1. Fixed `XenProd~version_hyp_ret_val~1` requirement as Xen does not 
> return
> 0 for success in all the cases.
> 2. Reworded the requirements so as to write them from Xen's perspective (not
> domain's perspective).
> 
> v2 - 1. Specified the register details.
> 2. Specified the type of buffer. 
> 
> .../fusa/reqs/design-reqs/arm64/hypercall.rst | 58 +++++++++++++++++++
> docs/fusa/reqs/index.rst                      |  2 +
> docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
> .../reqs/product-reqs/version_hypercall.rst   | 43 ++++++++++++++
> 4 files changed, 119 insertions(+)
> create mode 100644 docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> create mode 100644 docs/fusa/reqs/product-reqs/version_hypercall.rst
> 
> diff --git a/docs/fusa/reqs/design-reqs/arm64/hypercall.rst 
> b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> new file mode 100644
> index 0000000000..f00b0b00f9
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> @@ -0,0 +1,58 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Hypercall
> +=========
> +
> +Instruction
> +-----------
> +
> +`XenSwdgn~arm64_hyp_instr~1`
> +
> +Description:
> +Xen shall treat domain hypercall exception as hypercall requests.

This really reads weirdly.
Maybe: Xen shall treat domain hvc instruction execution as hypercall requests.

Then you can add a comment to explain that this is detected through a specific 
exception.

Also this is not completely true as hvc is also used in other scenarios:
- PSCI calls
- Firmware calls

So i would put the 0xEA1 value as part of the requirement.

> +
> +Rationale:
> +
> +Comments:
> +Hypercall is one of the communication mechanism between Xen and domains.
> +Domains use hypercalls for various requests to Xen.
> +Domains use 'hvc #0xEA1' instruction to invoke hypercalls.
> +
> +Covers:
> + - `XenProd~version_hyp_first_param~1`
> + - `XenProd~version_hyp_second_param~1`
> +
> +Parameters
> +----------
> +
> +`XenSwdgn~arm64_hyp_param~1`
> +
> +Description:
> +Xen shall use the first five cpu core registers to obtain the arguments for
> +domain hypercall requests.

Why not say x0 to x4 registers instead ? You use x16 after anyway

> +
> +Rationale:
> +
> +Comments:
> +Xen shall read the first register for the first argument, second register for
> +the second argument and so on.
> +
> +Covers:
> + - `XenProd~version_hyp_first_param~1`
> + - `XenProd~version_hyp_second_param~1`
> +
> +Hypercall number
> +----------------
> +
> +`XenSwdgn~arm64_hyp_num~1`
> +
> +Description:
> +Xen shall read x16 to obtain the hypercall number.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_first_param~1`
> + - `XenProd~version_hyp_second_param~1`
> diff --git a/docs/fusa/reqs/index.rst b/docs/fusa/reqs/index.rst
> index 1088a51d52..d8683edce7 100644
> --- a/docs/fusa/reqs/index.rst
> +++ b/docs/fusa/reqs/index.rst
> @@ -10,5 +10,7 @@ Requirements documentation
>    market-reqs/reqs
>    product-reqs/reqs
>    product-reqs/arm64/reqs
> +   product-reqs/version_hypercall
>    design-reqs/arm64/generic-timer
>    design-reqs/arm64/sbsa-uart
> +   design-reqs/arm64/hypercall
> diff --git a/docs/fusa/reqs/market-reqs/reqs.rst 
> b/docs/fusa/reqs/market-reqs/reqs.rst
> index 2d297ecc13..0e29fe5362 100644
> --- a/docs/fusa/reqs/market-reqs/reqs.rst
> +++ b/docs/fusa/reqs/market-reqs/reqs.rst
> @@ -79,3 +79,19 @@ Comments:
> 
> Needs:
>  - XenProd
> +
> +Version hypercall
> +-----------------
> +
> +`XenMkt~version_hypercall~1`
> +
> +Description:
> +Xen shall provide an interface for the domains to retrieve Xen's version, 
> type
> +and compilation information.

I would say hypercall instead of interface here

> +
> +Rationale:
> +
> +Comments:
> +
> +Needs:
> + - XenProd
> diff --git a/docs/fusa/reqs/product-reqs/version_hypercall.rst 
> b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> new file mode 100644
> index 0000000000..400d51bbeb
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> @@ -0,0 +1,43 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Version hypercall
> +=================
> +
> +First Parameter
> +---------------
> +
> +`XenProd~version_hyp_first_param~1`
> +
> +Description:
> +Xen shall treat the first argument (as an integer) to denote the command 
> number
> +for the hypercall.

I would be more precise and say x0 value.

Also "integer" is unspecified, use a more specific type definition (32/64 bit 
signed/unsigned integer).

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Second Parameter
> +----------------
> +
> +`XenProd~version_hyp_second_param~1`
> +
> +Description:
> +Xen shall treat the second argument as a virtual address (mapped as Normal
> +Inner Write-Back Outer Write-Back Inner-Shareable) to buffer in domain's
> +memory.

You should say "guest virtual address" to be more precise.

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

Cheers
Bertrand