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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

On 28/02/2025 08:56, Julien Grall wrote:
> Hi,
Hi Julien/Bertrand,
> On 27/02/2025 15:09, Ayan Kumar Halder 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>
> > ---
> > Changes from -
> >
> > v1 - 1. Fixed `XenProd~version_hyp_ret_val~1` requirement as Xen does 
> > not return
> > 0 for success in all the cases.
> > 2. Reworded the requirements so as to write them from Xen's 
> > perspective (not
> > domain's perspective).
> >
> >   .../fusa/reqs/design-reqs/arm64/hypercall.rst | 55 +++++++++++++++++
> >   docs/fusa/reqs/index.rst                      |  2 +
> >   docs/fusa/reqs/market-reqs/reqs.rst           | 16 +++++
> >   .../reqs/product-reqs/version_hypercall.rst   | 61 +++++++++++++++++++
> >   4 files changed, 134 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..ffd883260c
> > --- /dev/null
> > +++ b/docs/fusa/reqs/design-reqs/arm64/hypercall.rst
> > @@ -0,0 +1,55 @@
> > +.. SPDX-License-Identifier: CC-BY-4.0
> > +
> > +Hypercall
> > +=========
> > +
> > +Instruction
> > +-----------
> > +
> > +`XenSwdgn~arm64_hyp_instr~1`
> > +
> > +Description:
> > +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.
> Are you trying to describe any hypercalls (e.g. SMCCC, Xen...) or just 
> the Xen one? If the latter, only "hvc #0xEA1" will be used for Xen 
> hypercalls. Other immediate/space will be used for something different 
> (i.e. #0 is used for SMCCC).
Yes, only the Xen one. I will mention "hvc #0xEA1".
> > +> +Covers:
> > + - `XenProd~version_hyp_first_param~1`
> > + - `XenProd~version_hyp_second_param~1`
> > +
> > +Parameters
> > +----------
> > +
> > +`XenSwdgn~arm64_hyp_param~1`
> > +
> > +Description:
> > +Xen shall use x0 to read the first parameter, x1 for second 
> > parameter and so
> > +on, for domain hypercall requests.
> This implies we are supporting a large number of parameters. However, 
> Xen is only support 5 arguments. So I would just list all the registers.
Xen shall use the first five cpu core registers to obtain the arguments 
for domain hypercall requests. Xen shall read the first register for the 
first argument, second register for the second argument and so on.
@Bertrand :- Does this look ok to you ? I deliberately changed from x0 
to first register so that this can be valid for both arm64 and arm32. 
Please comment.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenProd~version_hyp_first_param~1`
> > + - `XenProd~version_hyp_second_param~1`
> > +
> You don't seem to describe how the hypercall number is passed. Is this 
> intended?
Good catch. I will add a requirement.

Xen shall read x16 to obtain the hypercall number.

Xen shall read r12 to obtain the hypercall number.

> > +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..03221f70c3
> > --- /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:
> > +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:
> > +Xen shall treat the second argument as a virtual address to buffer 
> > in domain's
> > +memory.
> We don't support any VA. The VA will need to be mapped with specifc 
> attributes (see include/public/arch-arm.h). Should this be mentioned 
> in the requirement?
....as a virtual address (mapped as Normal Inner Write-Back Outer 
Write-Back Inner-Shareable) to buffer in domain's ....
- Ayan

> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~version_hypercall~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > +
> > +Return Value
> > +------------
> > +
> > +`XenProd~version_hyp_ret_val~1`
> > +
> > +Description:
> > +In case the hypercall fails, Xen shall return one of the error codes 
> > defined
> > +in 
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/errno.h.
> > +
> > +Rationale:
> > +
> > +Comments:
> > +
> > +Covers:
> > + - `XenMkt~version_hypercall~1`
> > +
> > +Needs:
> > + - XenSwdgn
> > \ No newline at end of file