All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
@ 2025-01-14 19:50 Ayan Kumar Halder
  2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-01-14 19:50 UTC (permalink / raw)
  To: xen-devel
  Cc: Ayan Kumar Halder, Stefano Stabellini, Bertrand Marquis,
	Michal Orzel, Artem Mygaiev

In the current patch, we have defined the requirements which are common for
all the commands.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 .../fusa/reqs/design-reqs/arm64/hypercall.rst | 52 ++++++++++++++++
 docs/fusa/reqs/index.rst                      |  2 +
 docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
 .../reqs/product-reqs/version_hypercall.rst   | 61 +++++++++++++++++++
 4 files changed, 131 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..66dbcc3026
--- /dev/null
+++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
@@ -0,0 +1,52 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Hypercall
+=========
+
+Instruction
+-----------
+
+`XenSwdgn~arm64_hyp_instr~1`
+
+Description:
+Domains shall use the Arm instruction 'hvc' to interact with Xen.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~version_hyp_first_param~1`
+ - `XenProd~version_hyp_second_param~1`
+
+Parameters
+----------
+
+`XenSwdgn~arm64_hyp_param~1`
+
+Description:
+Domains shall use register x0 to pass first parameter, x1 to pass second
+parameter and so on.
+
+Rationale:
+
+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 x0 register.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenProd~version_hyp_ret_val~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.
+
+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..fdb8da04e1
--- /dev/null
+++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
@@ -0,0 +1,61 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Version hypercall
+=================
+
+First Parameter
+---------------
+
+`XenProd~version_hyp_first_param~1`
+
+Description:
+Domain shall pass the first argument (as an integer) to denote the command
+number for the hypercall.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenMkt~version_hypercall~1`
+
+Needs:
+ - XenSwdgn
+
+Second Parameter
+----------------
+
+`XenProd~version_hyp_second_param~1`
+
+Description:
+Domain shall pass the second argument as a pointer to buffer in guest memory.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenMkt~version_hypercall~1`
+
+Needs:
+ - XenSwdgn
+
+Return Value
+------------
+
+`XenProd~version_hyp_ret_val~1`
+
+Description:
+Xen shall return 0 in case of success or one of the error codes as defined in
+https://man7.org/linux/man-pages/man3/errno.3.html.
+
+Rationale:
+
+Comments:
+
+Covers:
+ - `XenMkt~version_hypercall~1`
+
+Needs:
+ - XenSwdgn
+
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-14 19:50 [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Ayan Kumar Halder
@ 2025-01-14 19:50 ` Ayan Kumar Halder
  2025-01-29  8:33   ` Bertrand Marquis
  2025-01-29 15:33   ` Julien Grall
  2025-01-14 20:15 ` [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Andrew Cooper
  2025-01-29  8:27 ` Bertrand Marquis
  2 siblings, 2 replies; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-01-14 19:50 UTC (permalink / raw)
  To: xen-devel
  Cc: Ayan Kumar Halder, Stefano Stabellini, Bertrand Marquis,
	Michal Orzel, Artem Mygaiev

We have written the requirements for some of the commands of the XEN_VERSION
hypercall.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 .../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".
+
+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.
+
+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.
+
+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.
+
+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.
+
+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.
+
+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.
+
+Rationale:
+
+Comments:
+Xen version is composed of major and minor number.
+
+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



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
  2025-01-14 19:50 [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Ayan Kumar Halder
  2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
@ 2025-01-14 20:15 ` Andrew Cooper
  2025-01-15 12:23   ` Ayan Kumar Halder
  2025-01-29  8:27 ` Bertrand Marquis
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2025-01-14 20:15 UTC (permalink / raw)
  To: Ayan Kumar Halder, xen-devel
  Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Artem Mygaiev

On 14/01/2025 7:50 pm, Ayan Kumar Halder wrote:
> 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..fdb8da04e1
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> @@ -0,0 +1,61 @@
> +Return Value
> +------------
> +
> +`XenProd~version_hyp_ret_val~1`
> +
> +Description:
> +Xen shall return 0 in case of success or one of the error codes as defined in
> +https://man7.org/linux/man-pages/man3/errno.3.html.

Xen's errors live in public/errno.h

They share a lot in common with Linux (for historical reasons), but they
are critically not Linux errnos because Xen supports OSes which aren't
Linux.  xenstored for example sends errors as text rather than numbers.

Also, that's not the return value ABI of __HYPERVISOR_xen_version.  Some
subops return a positive value instead of 0 on success.

And if you're wondering "hey, isn't that ambiguous in extreme cases",
yes it is.  Xen's hypercall API/ABI are a disaster.

~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
  2025-01-14 20:15 ` [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Andrew Cooper
@ 2025-01-15 12:23   ` Ayan Kumar Halder
  0 siblings, 0 replies; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-01-15 12:23 UTC (permalink / raw)
  To: Andrew Cooper, Ayan Kumar Halder, xen-devel
  Cc: Stefano Stabellini, Bertrand Marquis, Michal Orzel, Artem Mygaiev

Hi Andrew,

On 14/01/2025 20:15, Andrew Cooper wrote:
> On 14/01/2025 7:50 pm, Ayan Kumar Halder wrote:
>> 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..fdb8da04e1
>> --- /dev/null
>> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
>> @@ -0,0 +1,61 @@
>> +Return Value
>> +------------
>> +
>> +`XenProd~version_hyp_ret_val~1`
>> +
>> +Description:
>> +Xen shall return 0 in case of success or one of the error codes as defined in
>> +https://man7.org/linux/man-pages/man3/errno.3.html.
> Xen's errors live in public/errno.h
Ack.
>
> They share a lot in common with Linux (for historical reasons), but they
> are critically not Linux errnos because Xen supports OSes which aren't
> Linux.  xenstored for example sends errors as text rather than numbers.
>
> Also, that's not the return value ABI of __HYPERVISOR_xen_version.  Some
> subops return a positive value instead of 0 on success.

Yes, my bad. 'XENVER_version' cmd will return the actual version number 
on success. I should reword this as

"Xen shall use the error codes defined in xen/include/public/errno.h , 
as a return value in case of failure."

And a separate requirement will be

"Xen shall return a non negative value in case of success."

Let me know if this sounds ok.

>
> And if you're wondering "hey, isn't that ambiguous in extreme cases",
> yes it is.  Xen's hypercall API/ABI are a disaster.

Ack. :)

- Ayan

>
> ~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
  2025-01-14 19:50 [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Ayan Kumar Halder
  2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
  2025-01-14 20:15 ` [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Andrew Cooper
@ 2025-01-29  8:27 ` Bertrand Marquis
  2025-02-17 16:26   ` Ayan Kumar Halder
  2 siblings, 1 reply; 14+ messages in thread
From: Bertrand Marquis @ 2025-01-29  8:27 UTC (permalink / raw)
  To: Ayan Kumar Halder
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Michal Orzel,
	Artem Mygaiev

Hi Ayan,

> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
> 
> In the current patch, we have defined the requirements which are common for
> all the commands.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
> .../fusa/reqs/design-reqs/arm64/hypercall.rst | 52 ++++++++++++++++
> docs/fusa/reqs/index.rst                      |  2 +
> docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
> .../reqs/product-reqs/version_hypercall.rst   | 61 +++++++++++++++++++
> 4 files changed, 131 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..66dbcc3026
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> @@ -0,0 +1,52 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Hypercall
> +=========
> +
> +Instruction
> +-----------
> +
> +`XenSwdgn~arm64_hyp_instr~1`
> +
> +Description:
> +Domains shall use the Arm instruction 'hvc' to interact with Xen.

Why are those requirements defining what "Domains" should do ?
Shouldn't we define them as what Xen shall do ?
Something around:
Xen shall treat Domain hypercall exceptions and hypercall requests from Domains.

Or something around this idea.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_first_param~1`
> + - `XenProd~version_hyp_second_param~1`
> +
> +Parameters
> +----------
> +
> +`XenSwdgn~arm64_hyp_param~1`
> +
> +Description:
> +Domains shall use register x0 to pass first parameter, x1 to pass second
> +parameter and so on.

Same

> +
> +Rationale:
> +
> +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 x0 register.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenProd~version_hyp_ret_val~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.
> +
> +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..fdb8da04e1
> --- /dev/null
> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
> @@ -0,0 +1,61 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Version hypercall
> +=================
> +
> +First Parameter
> +---------------
> +
> +`XenProd~version_hyp_first_param~1`
> +
> +Description:
> +Domain shall pass the first argument (as an integer) to denote the command
> +number for the hypercall.

Same here should be turned as Xen shall.

> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Second Parameter
> +----------------
> +
> +`XenProd~version_hyp_second_param~1`
> +
> +Description:
> +Domain shall pass the second argument as a pointer to buffer in guest memory.
> +

Ditto

> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> +Return Value
> +------------
> +
> +`XenProd~version_hyp_ret_val~1`
> +
> +Description:
> +Xen shall return 0 in case of success or one of the error codes as defined in
> +https://man7.org/linux/man-pages/man3/errno.3.html.
> +
> +Rationale:
> +
> +Comments:
> +
> +Covers:
> + - `XenMkt~version_hypercall~1`
> +
> +Needs:
> + - XenSwdgn
> +
> -- 
> 2.25.1
> 

Cheers
Bertrand




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
@ 2025-01-29  8:33   ` Bertrand Marquis
  2025-02-17 16:01     ` Ayan Kumar Halder
  2025-01-29 15:33   ` Julien Grall
  1 sibling, 1 reply; 14+ messages in thread
From: Bertrand Marquis @ 2025-01-29  8:33 UTC (permalink / raw)
  To: Ayan Kumar Halder
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Michal Orzel,
	Artem Mygaiev

Hi Ayan,

> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> 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@amd.com>
> ---
> .../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




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
  2025-01-29  8:33   ` Bertrand Marquis
@ 2025-01-29 15:33   ` Julien Grall
  2025-01-29 15:49     ` Bertrand Marquis
  1 sibling, 1 reply; 14+ messages in thread
From: Julien Grall @ 2025-01-29 15:33 UTC (permalink / raw)
  To: Ayan Kumar Halder
  Cc: xen-devel, Stefano Stabellini, Bertrand Marquis, Michal Orzel,
	Artem Mygaiev

[-- Attachment #1: Type: text/plain, Size: 5893 bytes --]

Hi,

On Tue, 14 Jan 2025 at 16:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com>
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@amd.com>
> ---
>  .../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".


Can you explain why we need to specify how Xen is storing the string? At
least to me this feels a bit overkill. What matters is what/how the VM is
seen.


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


> +
> +Rationale:
> +
> +Comments:
> +Xen version is composed of major and minor number.
> +
> +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.


Not really. It returns a truncated version if it is too large. You likely
want to describe command 11.



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

[-- Attachment #2: Type: text/html, Size: 8043 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-29 15:33   ` Julien Grall
@ 2025-01-29 15:49     ` Bertrand Marquis
  2025-01-29 20:20       ` Julien Grall
  0 siblings, 1 reply; 14+ messages in thread
From: Bertrand Marquis @ 2025-01-29 15:49 UTC (permalink / raw)
  To: Julien Grall
  Cc: Ayan Kumar Halder, xen-devel@lists.xenproject.org,
	Stefano Stabellini, Michal Orzel, Artem Mygaiev

Hi Julien,

Welcome back :-)

> On 29 Jan 2025, at 16:33, Julien Grall <julien.grall.oss@gmail.com> wrote:
> 
> Hi,
> 
> On Tue, 14 Jan 2025 at 16:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> 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@amd.com>
> ---
>  .../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".
> 
> Can you explain why we need to specify how Xen is storing the string? At least to me this feels a bit overkill. What matters is what/how the VM is seen.

This is a design requirement and as such it should be testable so it would be easier to have something saying:
Xen shall have a constant named XXX storing YYY.

Just saying "an internal constant" seem a bit limited here and not saying much that could be tested easily.

Why do you think this would be an overkill ? do you expect the constant name to change a lot ?
I would see more as an overkill the fact that the value is stored in a requirement.

Cheers
Bertrand



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-29 15:49     ` Bertrand Marquis
@ 2025-01-29 20:20       ` Julien Grall
  2025-01-30  7:21         ` Bertrand Marquis
  0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2025-01-29 20:20 UTC (permalink / raw)
  To: Bertrand Marquis
  Cc: Ayan Kumar Halder, xen-devel@lists.xenproject.org,
	Stefano Stabellini, Michal Orzel, Artem Mygaiev

[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]

On Wed, 29 Jan 2025 at 12:49, Bertrand Marquis <Bertrand.Marquis@arm.com>
wrote:

> Hi Julien,
>
> Welcome back :-)


I am not fully back yet but I have a bit of spare time to go through
xen-devel :).


>
> > On 29 Jan 2025, at 16:33, Julien Grall <julien.grall.oss@gmail.com>
> wrote:
> >
> > Hi,
> >
> > On Tue, 14 Jan 2025 at 16:50, Ayan Kumar Halder <
> ayan.kumar.halder@amd.com> 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@amd.com>
> > ---
> >  .../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".
> >
> > Can you explain why we need to specify how Xen is storing the string? At
> least to me this feels a bit overkill. What matters is what/how the VM is
> seen.
>
> This is a design requirement and as such it should be testable so it would
> be easier to have something saying:
> Xen shall have a constant named XXX storing YYY.


Reading this, would it be better to tie to the variable in the makefile?
This would be closer to how a user would set it and how one would test it.



>
> Just saying "an internal constant" seem a bit limited here and not saying
> much that could be tested easily.
>
> Why do you think this would be an overkill ? do you expect the constant
> name to change a lot ?


I don’t expect the constant name to change. It is more that this is an
internal implementation quite far to how the user would set it (see above).

Cheers,


> I would see more as an overkill the fact that the value is stored in a
> requirement.
>
> Cheers
> Bertrand
>
>

[-- Attachment #2: Type: text/html, Size: 4209 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-29 20:20       ` Julien Grall
@ 2025-01-30  7:21         ` Bertrand Marquis
  0 siblings, 0 replies; 14+ messages in thread
From: Bertrand Marquis @ 2025-01-30  7:21 UTC (permalink / raw)
  To: Julien Grall
  Cc: Ayan Kumar Halder, xen-devel@lists.xenproject.org,
	Stefano Stabellini, Michal Orzel, Artem Mygaiev

Hi Julien,

> On 29 Jan 2025, at 21:20, Julien Grall <julien.grall.oss@gmail.com> wrote:
> 
> 
> On Wed, 29 Jan 2025 at 12:49, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> Hi Julien,
> 
> Welcome back :-)
> 
> I am not fully back yet but I have a bit of spare time to go through xen-devel :).

Then enjoy your remaining free time to, nothing urgent on the ML ;-)

> 
> 
> 
> > On 29 Jan 2025, at 16:33, Julien Grall <julien.grall.oss@gmail.com> wrote:
> > 
> > Hi,
> > 
> > On Tue, 14 Jan 2025 at 16:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> 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@amd.com>
> > ---
> >  .../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".
> > 
> > Can you explain why we need to specify how Xen is storing the string? At least to me this feels a bit overkill. What matters is what/how the VM is seen.
> 
> This is a design requirement and as such it should be testable so it would be easier to have something saying:
> Xen shall have a constant named XXX storing YYY.
> 
> Reading this, would it be better to tie to the variable in the makefile? This would be closer to how a user would set it and how one would test it.

Definitely yes. The more direct the variable, the better it is.
As the Makefile variable is what we modify, I agree that this should point to it.

> 
> 
> 
> 
> Just saying "an internal constant" seem a bit limited here and not saying much that could be tested easily.
> 
> Why do you think this would be an overkill ? do you expect the constant name to change a lot ?
> 
> I don’t expect the constant name to change. It is more that this is an internal implementation quite far to how the user would set it (see above).

Agree and the Makefile variable seems the best way.
All in all, the design just need to say that it must be stored somewhere "bounded" to the source code so that a tester can check it.

Cheers
Bertrand

> 
> Cheers,
> 
> 
> I would see more as an overkill the fact that the value is stored in a requirement.
> 
> Cheers
> Bertrand



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-01-29  8:33   ` Bertrand Marquis
@ 2025-02-17 16:01     ` Ayan Kumar Halder
  2025-02-24 10:01       ` Bertrand Marquis
  0 siblings, 1 reply; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-02-17 16:01 UTC (permalink / raw)
  To: Bertrand Marquis, Ayan Kumar Halder
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Michal Orzel,
	Artem Mygaiev


On 29/01/2025 08:33, Bertrand Marquis wrote:
> Hi Ayan,

Hi Bertrand,

Apologies for the delay in response. I am working on v2 , but need some 
clarifications.

>
>> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> 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@amd.com>
>> ---
>> .../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 ?

I tried the follow the discussion between Julien and you. We do not get 
the version from the Makefile ie 3.0 is hardcoded.

So, does the following look ok

Xen shall have an internal constant string to denote that the cpu is 
running
in arm64 mode.
>
>> +
>> +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.
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..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.
Xen shall have a internal constant (XEN_VERSION) the version number 
coming from
the Makefile.
>
>> +
>> +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.
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`
>> +
>> +Extraversion
>> +------------
>> +
>> +`XenSwdgn~extraversion~1`
>> +
>> +Description:
>> +Xen shall have a internal constant string storing the extraversion coming from
>> +the build environment.
> Same here.
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 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.
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..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:

We have a market requirement, if this looks fine

Xen shall provide an interface for the domains to retrieve Xen's 
version, type
and compilation information.

> Xen hypercall number 0  shall return the Xen version in register 0.
> I would also prevent saying x0 which would make this aarch64 specific.
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.
> Can't we link to the requirement defining where the version is stored ?

Yes, this is linked to the design requirement

`XenSwdgn~version~1` and `XenSwdgn~subversion~1`
- Ayan

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
  2025-01-29  8:27 ` Bertrand Marquis
@ 2025-02-17 16:26   ` Ayan Kumar Halder
  2025-02-24 10:03     ` Bertrand Marquis
  0 siblings, 1 reply; 14+ messages in thread
From: Ayan Kumar Halder @ 2025-02-17 16:26 UTC (permalink / raw)
  To: Bertrand Marquis, Ayan Kumar Halder
  Cc: xen-devel@lists.xenproject.org, Stefano Stabellini, Michal Orzel,
	Artem Mygaiev


On 29/01/2025 08:27, Bertrand Marquis wrote:
> Hi Ayan,

Hi Bertrand,

I need some clarifications.

>
>> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>
>> In the current patch, we have defined the requirements which are common for
>> all the commands.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>> .../fusa/reqs/design-reqs/arm64/hypercall.rst | 52 ++++++++++++++++
>> docs/fusa/reqs/index.rst                      |  2 +
>> docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
>> .../reqs/product-reqs/version_hypercall.rst   | 61 +++++++++++++++++++
>> 4 files changed, 131 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..66dbcc3026
>> --- /dev/null
>> +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
>> @@ -0,0 +1,52 @@
>> +.. SPDX-License-Identifier: CC-BY-4.0
>> +
>> +Hypercall
>> +=========
>> +
>> +Instruction
>> +-----------
>> +
>> +`XenSwdgn~arm64_hyp_instr~1`
>> +
>> +Description:
>> +Domains shall use the Arm instruction 'hvc' to interact with Xen.
> Why are those requirements defining what "Domains" should do ?
> Shouldn't we define them as what Xen shall do ?
> Something around:
> Xen shall treat Domain hypercall exceptions and hypercall requests from Domains.
>
> Or something around this idea.
Xen shall treat domain hypercall exception as hypercall requests.
>
>> +
>> +Rationale:
>> +
>> +Comments:
Hypercall is one of the communication mechanism between Xen and domains.
Domains use hypercalls for various requests to Xen.
Domains use 'hvc' instruction to invoke hypercalls.
>> +
>> +Covers:
>> + - `XenProd~version_hyp_first_param~1`
>> + - `XenProd~version_hyp_second_param~1`
>> +
>> +Parameters
>> +----------
>> +
>> +`XenSwdgn~arm64_hyp_param~1`
>> +
>> +Description:
>> +Domains shall use register x0 to pass first parameter, x1 to pass second
>> +parameter and so on.
> Same
Xen shall use the register 0 to read the first parameter, register 1
for second parameter and so on, for domain hypercall requests.
>
>> +
>> +Rationale:
>> +
>> +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 x0 register.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenProd~version_hyp_ret_val~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.
>> +
>> +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..fdb8da04e1
>> --- /dev/null
>> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
>> @@ -0,0 +1,61 @@
>> +.. SPDX-License-Identifier: CC-BY-4.0
>> +
>> +Version hypercall
>> +=================
>> +
>> +First Parameter
>> +---------------
>> +
>> +`XenProd~version_hyp_first_param~1`
>> +
>> +Description:
>> +Domain shall pass the first argument (as an integer) to denote the command
>> +number for the hypercall.
> Same here should be turned as Xen shall.
Xen shall treat the first argument (as an integer) to denote the command 
number
for the hypercall.
>
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenMkt~version_hypercall~1`
>> +
>> +Needs:
>> + - XenSwdgn
>> +
>> +Second Parameter
>> +----------------
>> +
>> +`XenProd~version_hyp_second_param~1`
>> +
>> +Description:
>> +Domain shall pass the second argument as a pointer to buffer in guest memory.
>> +
> Ditto
Xen shall treat the second argument as a pointer to buffer in guest memory.
- Ayan
>
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenMkt~version_hypercall~1`
>> +
>> +Needs:
>> + - XenSwdgn
>> +
>> +Return Value
>> +------------
>> +
>> +`XenProd~version_hyp_ret_val~1`
>> +
>> +Description:
>> +Xen shall return 0 in case of success or one of the error codes as defined in
>> +https://man7.org/linux/man-pages/man3/errno.3.html.
>> +
>> +Rationale:
>> +
>> +Comments:
>> +
>> +Covers:
>> + - `XenMkt~version_hypercall~1`
>> +
>> +Needs:
>> + - XenSwdgn
>> +
>> -- 
>> 2.25.1
>>
> Cheers
> Bertrand
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION
  2025-02-17 16:01     ` Ayan Kumar Halder
@ 2025-02-24 10:01       ` Bertrand Marquis
  0 siblings, 0 replies; 14+ messages in thread
From: Bertrand Marquis @ 2025-02-24 10:01 UTC (permalink / raw)
  To: Ayan Kumar Halder
  Cc: Ayan Kumar Halder, xen-devel@lists.xenproject.org,
	Stefano Stabellini, Michal Orzel, Artem Mygaiev

Hi Ayan,

> On 17 Feb 2025, at 17:01, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> 
> On 29/01/2025 08:33, Bertrand Marquis wrote:
>> Hi Ayan,
> 
> Hi Bertrand,
> 
> Apologies for the delay in response. I am working on v2 , but need some clarifications.
> 
>> 
>>> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> 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@amd.com>
>>> ---
>>> .../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 ?
> 
> I tried the follow the discussion between Julien and you. We do not get the version from the Makefile ie 3.0 is hardcoded.
> 
> So, does the following look ok
> 
> Xen shall have an internal constant string to denote that the cpu is running
> in arm64 mode.

ok for me.

>> 
>>> +
>>> +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.
> Xen shall have a internal constant string to denote that the cpu is running in
> arm32 mode.

ok

>> 
>>> +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.
> Xen shall have a internal constant (XEN_VERSION) the version number coming from
> the Makefile.

you are missing a "storing"

>> 
>>> +
>>> +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.
> Xen shall have a internal constant (XEN_SUBVERSION) storing the sub version
> number coming from the Makefile.

ok

>> 
>>> +
>>> +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.
> Xen shall have a internal constant (XEN_EXTRAVERSION) storing the extraversion
> coming from the build environment.

ok

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

ok

>> 
>>> +
>>> +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:
> 
> We have a market requirement, if this looks fine
> 
> Xen shall provide an interface for the domains to retrieve Xen's version, type
> and compilation information.

ok

> 
>> Xen hypercall number 0  shall return the Xen version in register 0.
>> I would also prevent saying x0 which would make this aarch64 specific.
> Xen shall provide a command (num 0) for hypercall (num 17) to retrieve Xen's
> version in the domain's register 0.


ok

>> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +Xen version is composed of major and minor number.
>> Can't we link to the requirement defining where the version is stored ?
> 
> Yes, this is linked to the design requirement
> 
> `XenSwdgn~version~1` and `XenSwdgn~subversion~1`

Good

Bertrand

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




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.
  2025-02-17 16:26   ` Ayan Kumar Halder
@ 2025-02-24 10:03     ` Bertrand Marquis
  0 siblings, 0 replies; 14+ messages in thread
From: Bertrand Marquis @ 2025-02-24 10:03 UTC (permalink / raw)
  To: Ayan Kumar Halder
  Cc: Ayan Kumar Halder, xen-devel@lists.xenproject.org,
	Stefano Stabellini, Michal Orzel, Artem Mygaiev

Hi Ayan,

> On 17 Feb 2025, at 17:26, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> 
> On 29/01/2025 08:27, Bertrand Marquis wrote:
>> Hi Ayan,
> 
> Hi Bertrand,
> 
> I need some clarifications.
> 
>> 
>>> On 14 Jan 2025, at 20:50, Ayan Kumar Halder <ayan.kumar.halder@amd.com> wrote:
>>> 
>>> In the current patch, we have defined the requirements which are common for
>>> all the commands.
>>> 
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> ---
>>> .../fusa/reqs/design-reqs/arm64/hypercall.rst | 52 ++++++++++++++++
>>> docs/fusa/reqs/index.rst                      |  2 +
>>> docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
>>> .../reqs/product-reqs/version_hypercall.rst   | 61 +++++++++++++++++++
>>> 4 files changed, 131 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..66dbcc3026
>>> --- /dev/null
>>> +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
>>> @@ -0,0 +1,52 @@
>>> +.. SPDX-License-Identifier: CC-BY-4.0
>>> +
>>> +Hypercall
>>> +=========
>>> +
>>> +Instruction
>>> +-----------
>>> +
>>> +`XenSwdgn~arm64_hyp_instr~1`
>>> +
>>> +Description:
>>> +Domains shall use the Arm instruction 'hvc' to interact with Xen.
>> Why are those requirements defining what "Domains" should do ?
>> Shouldn't we define them as what Xen shall do ?
>> Something around:
>> Xen shall treat Domain hypercall exceptions and hypercall requests from Domains.
>> 
>> Or something around this idea.
> Xen shall treat domain hypercall exception as hypercall requests.

sounds good

>> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
> Hypercall is one of the communication mechanism between Xen and domains.
> Domains use hypercalls for various requests to Xen.
> Domains use 'hvc' instruction to invoke hypercalls.

ok

>>> +
>>> +Covers:
>>> + - `XenProd~version_hyp_first_param~1`
>>> + - `XenProd~version_hyp_second_param~1`
>>> +
>>> +Parameters
>>> +----------
>>> +
>>> +`XenSwdgn~arm64_hyp_param~1`
>>> +
>>> +Description:
>>> +Domains shall use register x0 to pass first parameter, x1 to pass second
>>> +parameter and so on.
>> Same
> Xen shall use the register 0 to read the first parameter, register 1
> for second parameter and so on, for domain hypercall requests.

ok

>> 
>>> +
>>> +Rationale:
>>> +
>>> +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 x0 register.
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenProd~version_hyp_ret_val~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.
>>> +
>>> +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..fdb8da04e1
>>> --- /dev/null
>>> +++ b/docs/fusa/reqs/product-reqs/version_hypercall.rst
>>> @@ -0,0 +1,61 @@
>>> +.. SPDX-License-Identifier: CC-BY-4.0
>>> +
>>> +Version hypercall
>>> +=================
>>> +
>>> +First Parameter
>>> +---------------
>>> +
>>> +`XenProd~version_hyp_first_param~1`
>>> +
>>> +Description:
>>> +Domain shall pass the first argument (as an integer) to denote the command
>>> +number for the hypercall.
>> Same here should be turned as Xen shall.
> Xen shall treat the first argument (as an integer) to denote the command number
> for the hypercall.

ok

>> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenMkt~version_hypercall~1`
>>> +
>>> +Needs:
>>> + - XenSwdgn
>>> +
>>> +Second Parameter
>>> +----------------
>>> +
>>> +`XenProd~version_hyp_second_param~1`
>>> +
>>> +Description:
>>> +Domain shall pass the second argument as a pointer to buffer in guest memory.
>>> +
>> Ditto
> Xen shall treat the second argument as a pointer to buffer in guest memory.

You might want to specify the addressing type (PA/IPA/VA) here.


Bertrand

> - Ayan
>> 
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenMkt~version_hypercall~1`
>>> +
>>> +Needs:
>>> + - XenSwdgn
>>> +
>>> +Return Value
>>> +------------
>>> +
>>> +`XenProd~version_hyp_ret_val~1`
>>> +
>>> +Description:
>>> +Xen shall return 0 in case of success or one of the error codes as defined in
>>> +https://man7.org/linux/man-pages/man3/errno.3.html.
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenMkt~version_hypercall~1`
>>> +
>>> +Needs:
>>> + - XenSwdgn
>>> +
>>> -- 
>>> 2.25.1
>>> 
>> Cheers
>> Bertrand




^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-02-24 10:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-14 19:50 [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Ayan Kumar Halder
2025-01-14 19:50 ` [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION Ayan Kumar Halder
2025-01-29  8:33   ` Bertrand Marquis
2025-02-17 16:01     ` Ayan Kumar Halder
2025-02-24 10:01       ` Bertrand Marquis
2025-01-29 15:33   ` Julien Grall
2025-01-29 15:49     ` Bertrand Marquis
2025-01-29 20:20       ` Julien Grall
2025-01-30  7:21         ` Bertrand Marquis
2025-01-14 20:15 ` [PATCH v1 1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall Andrew Cooper
2025-01-15 12:23   ` Ayan Kumar Halder
2025-01-29  8:27 ` Bertrand Marquis
2025-02-17 16:26   ` Ayan Kumar Halder
2025-02-24 10:03     ` Bertrand Marquis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.