diff mbox series

[v1,1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall.

Message ID 20250114195010.3409094-1-ayan.kumar.halder@amd.com (mailing list archive)
State New
Headers show
Series [v1,1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall. | expand

Commit Message

Ayan Kumar Halder Jan. 14, 2025, 7:50 p.m. UTC
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

Comments

Andrew Cooper Jan. 14, 2025, 8:15 p.m. UTC | #1
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
Ayan Kumar Halder Jan. 15, 2025, 12:23 p.m. UTC | #2
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
Bertrand Marquis Jan. 29, 2025, 8:27 a.m. UTC | #3
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
Ayan Kumar Halder Feb. 17, 2025, 4:26 p.m. UTC | #4
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
>
>
diff mbox series

Patch

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
+