Re: [PATCH v1 2/2] docs: fusa: Add the requirements for some of the commands of XEN_VERSION

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

On Wed, 29 Jan 2025 at 12:49, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:

Hi Julien,

Welcome back :-)

I am not fully back yet but I have a bit of spare time to go through xen-devel :).

> On 29 Jan 2025, at 16:33, Julien Grall <julien.grall.oss@xxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, 14 Jan 2025 at 16:50, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx> wrote:
> We have written the requirements for some of the commands of the XEN_VERSION
> hypercall.
>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
>  .../design-reqs/arm64/version_hypercall.rst   | 33 ++++++++
>  .../reqs/design-reqs/version_hypercall.rst    | 65 +++++++++++++++
>  docs/fusa/reqs/index.rst                      |  2 +
>  .../reqs/product-reqs/version_hypercall.rst   | 82 +++++++++++++++++++
>  4 files changed, 182 insertions(+)
>  create mode 100644 docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
>  create mode 100644 docs/fusa/reqs/design-reqs/version_hypercall.rst
>
> diff --git a/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst b/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> new file mode 100644
> index 0000000000..1dad2b84c2
> --- /dev/null
> +++ b/docs/fusa/reqs/design-reqs/arm64/version_hypercall.rst
> @@ -0,0 +1,33 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Capabilities
> +------------
> +
> +`XenSwdgn~arm64_capabilities~1`
> +
> +Description:
> +Xen shall have a internal constant string storing "xen-3.0-aarch64".
>
> Can you explain why we need to specify how Xen is storing the string? At least to me this feels a bit overkill. What matters is what/how the VM is seen.

This is a design requirement and as such it should be testable so it would be easier to have something saying:
Xen shall have a constant named XXX storing YYY.

Reading this, would it be better to tie to the variable in the makefile? This would be closer to how a user would set it and how one would test it.

Just saying "an internal constant" seem a bit limited here and not saying much that could be tested easily.

Why do you think this would be an overkill ? do you expect the constant name to change a lot ?

I don’t expect the constant name to change. It is more that this is an internal implementation quite far to how the user would set it (see above).

Cheers,

I would see more as an overkill the fact that the value is stored in a requirement.

Cheers
Bertrand