| Message ID | 20250227150922.3965010-1-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] docs: fusa: Define the requirements for XEN_VERSION hypercall. | expand |
Hi Ayan, > On 27 Feb 2025, at 16:09, 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> > --- > 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. > + > +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. Cheers Bertrand > + > +Rationale: > + > +Comments: > + > +Covers: > + - `XenMkt~version_hypercall~1` > + > +Needs: > + - XenSwdgn > \ No newline at end of file > -- > 2.25.1 >
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@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> >> --- >> 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. > >> + >> +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 ? - Ayan > > Cheers > Bertrand > >> + >> +Rationale: >> + >> +Comments: >> + >> +Covers: >> + - `XenMkt~version_hypercall~1` >> + >> +Needs: >> + - XenSwdgn >> \ No newline at end of file >> -- >> 2.25.1 >>
Hi Ayan, > On 27 Feb 2025, at 20:29, Ayan Kumar Halder <ayankuma@amd.com> 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@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> >>> --- >>> 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
Hi, 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@amd.com> > --- > 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). > +> +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. > + > +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? > +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? > + > +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
On 28/02/2025 07:54, Bertrand Marquis wrote: > Hi Ayan, Hi Bertrand, > >> On 27 Feb 2025, at 20:29, Ayan Kumar Halder <ayankuma@amd.com> 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@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> >>>> --- >>>> 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. Yes, I will place it under hypercall.rst. > > 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. Agreed. And an example design requirement which will be linked is :- Xen shall return -EFAULT if it encounters an error while copying the extraversion string to domain's buffer. Is this what you have in mind ? > > At the end you should have a test to trigger each error condition and that test will be linked to the design req corresponding. Ack. - Ayan > > Cheers > Bertrand > >> - Ayan >> >>> Cheers >>> Bertrand >>> >>>> + >>>> +Rationale: >>>> + >>>> +Comments: >>>> + >>>> +Covers: >>>> + - `XenMkt~version_hypercall~1` >>>> + >>>> +Needs: >>>> + - XenSwdgn >>>> \ No newline at end of file >>>> -- >>>> 2.25.1 >
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@amd.com> >> --- >> 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 >
Hi Ayan, > On 28 Feb 2025, at 12:07, Ayan Kumar Halder <ayankuma@amd.com> wrote: > > > On 28/02/2025 07:54, Bertrand Marquis wrote: >> Hi Ayan, > Hi Bertrand, >> >>> On 27 Feb 2025, at 20:29, Ayan Kumar Halder <ayankuma@amd.com> 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@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> >>>>> --- >>>>> 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. > Yes, I will place it under hypercall.rst. >> >> 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. > > Agreed. > > And an example design requirement which will be linked is :- > > Xen shall return -EFAULT if it encounters an error while copying the extraversion string to domain's buffer. > > Is this what you have in mind ? Yes it is. But is it the only error that can be return by the hypercall ? Bertrand > >> >> At the end you should have a test to trigger each error condition and that test will be linked to the design req corresponding. > > Ack. > > - Ayan > >> >> Cheers >> Bertrand >> >>> - Ayan >>> >>>> Cheers >>>> Bertrand >>>> >>>>> + >>>>> +Rationale: >>>>> + >>>>> +Comments: >>>>> + >>>>> +Covers: >>>>> + - `XenMkt~version_hypercall~1` >>>>> + >>>>> +Needs: >>>>> + - XenSwdgn >>>>> \ No newline at end of file >>>>> -- >>>>> 2.25.1
Hi Ayan, > On 28 Feb 2025, at 12:51, Ayan Kumar Halder <ayankuma@amd.com> wrote: > > > 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@amd.com> >>> --- >>> 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. Yes we should mention the 5 registers as those are the one supported by Xen and also the register where the hypercall number is passed as mentioned after Cheers Bertrand > >> >>> + >>> +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
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. + +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. + +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
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> --- 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