[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 07/12] xen: enable Dom0 to use SVE feature





On 24/05/2023 11:05, Bertrand Marquis wrote:
Hi Luca,

Hi,


On 23 May 2023, at 09:43, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:

Add a command line parameter to allow Dom0 the use of SVE resources,
the command line parameter sve=<integer>, sub argument of dom0=,
controls the feature on this domain and sets the maximum SVE vector
length for Dom0.

Add a new function, parse_signed_integer(), to parse an integer
command line argument.

Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

with ...

---
Changes from v6:
- Fixed case for e==NULL in parse_signed_integer, drop parenthesis
   from if conditions, delete inline sve_domctl_vl_param and rely on
   DCE from the compiler (Jan)
- Drop parenthesis from opt_dom0_sve (Julien)
- Do not continue if 'sve' is in command line args but
   CONFIG_ARM64_SVE is not selected:
   https://lore.kernel.org/all/7614AE25-F59D-430A-9C3E-30B1CE0E1580@xxxxxxx/
Changes from v5:
- stop the domain if VL error occurs (Julien, Bertrand)
- update the documentation
- Rename sve_sanitize_vl_param to sve_domctl_vl_param to
   mark the fact that we are sanitizing a parameter coming from
   the user before encoding it into sve_vl in domctl structure.
   (suggestion from Bertrand in a separate discussion)
- update comment in parse_signed_integer, return boolean in
   sve_domctl_vl_param (Jan).
Changes from v4:
- Negative values as user param means max supported HW VL (Jan)
- update documentation, make use of no_config_param(), rename
   parse_integer into parse_signed_integer and take long long *,
   also put a comment on the -2 return condition, update
   declaration comment to reflect the modifications (Jan)
Changes from v3:
- Don't use fixed len types when not needed (Jan)
- renamed domainconfig_encode_vl to sve_encode_vl
- Use a sub argument of dom0= to enable the feature (Jan)
- Add parse_integer() function
Changes from v2:
- xen_domctl_createdomain field has changed into sve_vl and its
   value now is the VL / 128, create an helper function for that.
Changes from v1:
- No changes
Changes from RFC:
- Changed docs to explain that the domain won't be created if the
   requested vector length is above the supported one from the
   platform.
---
docs/misc/xen-command-line.pandoc    | 20 ++++++++++++++++++--
xen/arch/arm/arm64/sve.c             | 20 ++++++++++++++++++++
xen/arch/arm/domain_build.c          | 26 ++++++++++++++++++++++++++
xen/arch/arm/include/asm/arm64/sve.h | 10 ++++++++++
xen/common/kernel.c                  | 28 ++++++++++++++++++++++++++++
xen/include/xen/lib.h                | 10 ++++++++++
6 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index e0b89b7d3319..47e5b4eb6199 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -777,9 +777,9 @@ Specify the bit width of the DMA heap.

### dom0
     = List of [ pv | pvh, shadow=<bool>, verbose=<bool>,
-                cpuid-faulting=<bool>, msr-relaxed=<bool> ]
+                cpuid-faulting=<bool>, msr-relaxed=<bool> ] (x86)

-    Applicability: x86
+    = List of [ sve=<integer> ] (Arm)

Controls for how dom0 is constructed on x86 systems.

@@ -838,6 +838,22 @@ Controls for how dom0 is constructed on x86 systems.

     If using this option is necessary to fix an issue, please report a bug.

+Enables features on dom0 on Arm systems.
+
+*   The `sve` integer parameter enables Arm SVE usage for Dom0 domain and sets

NIT: "Domain" is bit redundant here.

+    the maximum SVE vector length, the option is applicable only to AArch64
+    guests.

Here i would just remove "guests", just AArch64 is enough.
I am ok if you choose to use "AArch64 Dom0 kernels"

So far we have no use of AArch64 in our documentation. We have a few use of Arm64 (with various uppercase).

In the code base, we seem to have a mix of AArch64 and Arm64. At the moment, I am not going to ask for consistency in the code. But we should aim to not introduce inconsistency in the documentation.

I don't have a strong opinion whether we should use aarch64 or arm64. My only request is to be consistent.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.