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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Ayan,

> On 27 Feb 2025, at 20:29, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:
> 
> 
> On 27/02/2025 17:15, Bertrand Marquis wrote:
>> Hi Ayan,
> 
> Hi Bertrand,
> 
> I have just some clarifications.
> 
>> 
>>> On 27 Feb 2025, at 16:09, 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>
>>> ---
>>> 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.
>>> +
>>> +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.
>>> +
>>> +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..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.
>> You speak of argument here and parameter earlier.
>> I would rephrase to: the first argument of an hypercall exception as the 
>> command number for the hypercall.
> 
> Ack
> 
> Should I do this everywhere ?
> 
> s/parameter/argument
> 
> That would make it consistent.

Yes definitely 

> 
>> 
>>> +
>>> +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.
>>> +
>> Same here on argument vs parameter.
>> 
>>> +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.
>> This is a very imprecise req as it does not states what can fail and what 
>> should be returned exactly.
>> Do we want to be that generic ? if yes then this might be a requirement 
>> valid for any hypercall.
> 
> Yes, you are correct.
> 
> I am thinking of pushing this further up ie have this requirement at market 
> level and leave it **unlinked** to any product requirement.
> 
> IOW, we don't need to validate each and every error code returned by the 
> hypercall.
> 
> Or should we just drop this requirement ?

I think you should move this requirement and make it a generic one valid for 
all hypercalls.

Now at some point you might have to describe which error is caused by what 
problem as part of your hypercall interface definition.
ie: one generic product req and per hypercall design req describing the error 
cases.

At the end you should have a test to trigger each error condition and that test 
will be linked to the design req corresponding.

Cheers
Bertrand

> 
> - Ayan
> 
>> 
>> Cheers
>> Bertrand
>> 
>>> +
>>> +Rationale:
>>> +
>>> +Comments:
>>> +
>>> +Covers:
>>> + - `XenMkt~version_hypercall~1`
>>> +
>>> +Needs:
>>> + - XenSwdgn
>>> \ No newline at end of file
>>> -- 
>>> 2.25.1