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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

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@xxxxxxx> 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@xxxxxxx>
> > ---
> > .../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