[PATCH v5 00/12] SVE feature for arm guests

Luca Fancellu posted 12 patches 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230412094938.2693890-1-luca.fancellu@arm.com
Test gitlab-ci failed
There is a newer version of this series
CHANGELOG.md                                  |   5 +
docs/man/xl.cfg.5.pod.in                      |  15 ++
docs/misc/arm/device-tree/booting.txt         |  11 +
docs/misc/xen-command-line.pandoc             |  18 +-
tools/golang/xenlight/helpers.gen.go          |   4 +
tools/golang/xenlight/types.gen.go            |  24 +++
tools/include/libxl.h                         |  11 +
.../include/xen-tools/arm-arch-capabilities.h |  28 +++
tools/include/xen-tools/common-macros.h       |   2 +
tools/libs/light/libxl.c                      |   1 +
tools/libs/light/libxl_arm.c                  |  28 +++
tools/libs/light/libxl_internal.h             |   1 -
tools/libs/light/libxl_types.idl              |  23 +++
tools/ocaml/libs/xc/xenctrl.ml                |   4 +-
tools/ocaml/libs/xc/xenctrl.mli               |   4 +-
tools/ocaml/libs/xc/xenctrl_stubs.c           |   8 +-
tools/python/xen/lowlevel/xc/xc.c             |   8 +-
tools/xl/xl_info.c                            |   8 +
tools/xl/xl_parse.c                           |   8 +
xen/arch/arm/Kconfig                          |  10 +-
xen/arch/arm/arm64/Makefile                   |   1 +
xen/arch/arm/arm64/cpufeature.c               |   7 +-
xen/arch/arm/arm64/sve-asm.S                  | 189 ++++++++++++++++++
xen/arch/arm/arm64/sve.c                      | 141 +++++++++++++
xen/arch/arm/arm64/vfp.c                      |  79 ++++----
xen/arch/arm/arm64/vsysreg.c                  |  39 +++-
xen/arch/arm/cpufeature.c                     |   6 +-
xen/arch/arm/domain.c                         |  43 +++-
xen/arch/arm/domain_build.c                   |  56 ++++++
xen/arch/arm/include/asm/arm64/sve.h          |  83 ++++++++
xen/arch/arm/include/asm/arm64/sysregs.h      |   4 +
xen/arch/arm/include/asm/arm64/vfp.h          |  10 +
xen/arch/arm/include/asm/cpufeature.h         |  14 ++
xen/arch/arm/include/asm/domain.h             |   8 +
xen/arch/arm/include/asm/processor.h          |   3 +
xen/arch/arm/setup.c                          |   5 +-
xen/arch/arm/sysctl.c                         |   4 +
xen/arch/arm/traps.c                          |  40 +++-
xen/arch/x86/dom0_build.c                     |  48 ++---
xen/common/domain.c                           |  23 +++
xen/common/kernel.c                           |  25 +++
xen/include/public/arch-arm.h                 |   2 +
xen/include/public/domctl.h                   |   2 +-
xen/include/public/sysctl.h                   |   4 +
xen/include/xen/domain.h                      |   1 +
xen/include/xen/lib.h                         |  10 +
46 files changed, 963 insertions(+), 105 deletions(-)
create mode 100644 tools/include/xen-tools/arm-arch-capabilities.h
create mode 100644 xen/arch/arm/arm64/sve-asm.S
create mode 100644 xen/arch/arm/arm64/sve.c
create mode 100644 xen/arch/arm/include/asm/arm64/sve.h
[PATCH v5 00/12] SVE feature for arm guests
Posted by Luca Fancellu 1 year ago
This serie is introducing the possibility for Dom0 and DomU guests to use
sve/sve2 instructions.

SVE feature introduces new instruction and registers to improve performances on
floating point operations.

The SVE feature is advertised using the ID_AA64PFR0_EL1 register, SVE field, and
when available the ID_AA64ZFR0_EL1 register provides additional information
about the implemented version and other SVE feature.

New registers added by the SVE feature are Z0-Z31, P0-P15, FFR, ZCR_ELx.

Z0-Z31 are scalable vector register whose size is implementation defined and
goes from 128 bits to maximum 2048, the term vector length will be used to refer
to this quantity.
P0-P15 are predicate registers and the size is the vector length divided by 8,
same size is the FFR (First Fault Register).
ZCR_ELx is a register that can control and restrict the maximum vector length
used by the <x> exception level and all the lower exception levels, so for
example EL3 can restrict the vector length usable by EL3,2,1,0.

The platform has a maximum implemented vector length, so for every value
written in ZCR register, if this value is above the implemented length, then the
lower value will be used. The RDVL instruction can be used to check what vector
length is the HW using after setting ZCR.

For an SVE guest, the V0-V31 registers are part of the Z0-Z31, so there is no
need to save them separately, saving Z0-Z31 will save implicitly also V0-V31.

SVE usage can be trapped using a flag in CPTR_EL2, hence in this serie the
register is added to the domain state, to be able to trap only the guests that
are not allowed to use SVE.

This serie is introducing a command line parameter to enable Dom0 to use SVE and
to set its maximum vector length that by default is 0 which means the guest is
not allowed to use SVE. Values from 128 to 2048 mean the guest can use SVE with
the selected value used as maximum allowed vector length (which could be lower
if the implemented one is lower).
For DomUs, an XL parameter with the same way of use is introduced and a dom0less
DTB binding is created.

The context switch is the most critical part because there can be big registers
to be saved, in this serie an easy approach is used and the context is
saved/restored every time for the guests that are allowed to use SVE.

Luca Fancellu (12):
  xen/arm: enable SVE extension for Xen
  xen/arm: add SVE vector length field to the domain
  xen/arm: Expose SVE feature to the guest
  xen/arm: add SVE exception class handling
  arm/sve: save/restore SVE context switch
  xen/common: add dom0 xen command line argument for Arm
  xen: enable Dom0 to use SVE feature
  xen/physinfo: encode Arm SVE vector length in arch_capabilities
  tools: add physinfo arch_capabilities handling for Arm
  xen/tools: add sve parameter in XL configuration
  xen/arm: add sve property for dom0less domUs
  xen/changelog: Add SVE and "dom0" options to the changelog for Arm

 CHANGELOG.md                                  |   5 +
 docs/man/xl.cfg.5.pod.in                      |  15 ++
 docs/misc/arm/device-tree/booting.txt         |  11 +
 docs/misc/xen-command-line.pandoc             |  18 +-
 tools/golang/xenlight/helpers.gen.go          |   4 +
 tools/golang/xenlight/types.gen.go            |  24 +++
 tools/include/libxl.h                         |  11 +
 .../include/xen-tools/arm-arch-capabilities.h |  28 +++
 tools/include/xen-tools/common-macros.h       |   2 +
 tools/libs/light/libxl.c                      |   1 +
 tools/libs/light/libxl_arm.c                  |  28 +++
 tools/libs/light/libxl_internal.h             |   1 -
 tools/libs/light/libxl_types.idl              |  23 +++
 tools/ocaml/libs/xc/xenctrl.ml                |   4 +-
 tools/ocaml/libs/xc/xenctrl.mli               |   4 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c           |   8 +-
 tools/python/xen/lowlevel/xc/xc.c             |   8 +-
 tools/xl/xl_info.c                            |   8 +
 tools/xl/xl_parse.c                           |   8 +
 xen/arch/arm/Kconfig                          |  10 +-
 xen/arch/arm/arm64/Makefile                   |   1 +
 xen/arch/arm/arm64/cpufeature.c               |   7 +-
 xen/arch/arm/arm64/sve-asm.S                  | 189 ++++++++++++++++++
 xen/arch/arm/arm64/sve.c                      | 141 +++++++++++++
 xen/arch/arm/arm64/vfp.c                      |  79 ++++----
 xen/arch/arm/arm64/vsysreg.c                  |  39 +++-
 xen/arch/arm/cpufeature.c                     |   6 +-
 xen/arch/arm/domain.c                         |  43 +++-
 xen/arch/arm/domain_build.c                   |  56 ++++++
 xen/arch/arm/include/asm/arm64/sve.h          |  83 ++++++++
 xen/arch/arm/include/asm/arm64/sysregs.h      |   4 +
 xen/arch/arm/include/asm/arm64/vfp.h          |  10 +
 xen/arch/arm/include/asm/cpufeature.h         |  14 ++
 xen/arch/arm/include/asm/domain.h             |   8 +
 xen/arch/arm/include/asm/processor.h          |   3 +
 xen/arch/arm/setup.c                          |   5 +-
 xen/arch/arm/sysctl.c                         |   4 +
 xen/arch/arm/traps.c                          |  40 +++-
 xen/arch/x86/dom0_build.c                     |  48 ++---
 xen/common/domain.c                           |  23 +++
 xen/common/kernel.c                           |  25 +++
 xen/include/public/arch-arm.h                 |   2 +
 xen/include/public/domctl.h                   |   2 +-
 xen/include/public/sysctl.h                   |   4 +
 xen/include/xen/domain.h                      |   1 +
 xen/include/xen/lib.h                         |  10 +
 46 files changed, 963 insertions(+), 105 deletions(-)
 create mode 100644 tools/include/xen-tools/arm-arch-capabilities.h
 create mode 100644 xen/arch/arm/arm64/sve-asm.S
 create mode 100644 xen/arch/arm/arm64/sve.c
 create mode 100644 xen/arch/arm/include/asm/arm64/sve.h

-- 
2.34.1
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Bertrand Marquis 1 year ago
Hi Luca,

On this serie I would like to open a discussion on how to handle the vector size
and the corresponding command line / configuration / device tree parameters.

In general the user must either give a vector size it wants or has a solution to
just request the maximum supported size.

In the current implementation if a size bigger than the supported one is provided:
- we silently disable SVE for dom0
- we silently disable SVE for dom0less
- we do not create a guest when done through tools

This is not completely coherent and i think we should aim for a coherent behaviour
unless we have arguments for this status.

Is there any good reason to silently disable for Dom0 and dom0less only ?

I see some possible solutions here:

- modify parameter behaviour to use the supported size if parameter is bigger than
it. This would at least keep SVE enabled if a VM depends on it and could simplify
some of the handling by using 2048 to use the maximum supported size.

- coherently stop if the parameter value is not supported (including if sve is
not supported)

- always disable SVE if the parameter value is not supported.

To be honest I am not quite sure which solution is better but I am not happy with
the different kind of behaviour we have right now.

What are your thoughts ?

Regards
Bertrand
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Julien Grall 1 year ago

On 18/04/2023 14:13, Bertrand Marquis wrote:
> Hi Luca,

Hi,

> On this serie I would like to open a discussion on how to handle the vector size
> and the corresponding command line / configuration / device tree parameters.
> 
> In general the user must either give a vector size it wants or has a solution to
> just request the maximum supported size.
> 
> In the current implementation if a size bigger than the supported one is provided:
> - we silently disable SVE for dom0
> - we silently disable SVE for dom0less
> - we do not create a guest when done through tools
> 
> This is not completely coherent and i think we should aim for a coherent behaviour
> unless we have arguments for this status.

+1.

> Is there any good reason to silently disable for Dom0 and dom0less only ?
> 
> I see some possible solutions here:
> 
> - modify parameter behaviour to use the supported size if parameter is bigger than
> it. This would at least keep SVE enabled if a VM depends on it and could simplify
> some of the handling by using 2048 to use the maximum supported size.

My concern with this approach and the third one is the user may take 
some time to realize the problem in the xl.cfg. So...

> 
> - coherently stop if the parameter value is not supported (including if sve is
> not supported)

... this is my preferred approach because it would be clear that the 
value passed to Xen is bogus.

Cheers,

-- 
Julien Grall
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Jan Beulich 1 year ago
On 18.04.2023 16:25, Julien Grall wrote:
> On 18/04/2023 14:13, Bertrand Marquis wrote:
>> On this serie I would like to open a discussion on how to handle the vector size
>> and the corresponding command line / configuration / device tree parameters.
>>
>> In general the user must either give a vector size it wants or has a solution to
>> just request the maximum supported size.
>>
>> In the current implementation if a size bigger than the supported one is provided:
>> - we silently disable SVE for dom0
>> - we silently disable SVE for dom0less
>> - we do not create a guest when done through tools
>>
>> This is not completely coherent and i think we should aim for a coherent behaviour
>> unless we have arguments for this status.
> 
> +1.
> 
>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>
>> I see some possible solutions here:
>>
>> - modify parameter behaviour to use the supported size if parameter is bigger than
>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>> some of the handling by using 2048 to use the maximum supported size.
> 
> My concern with this approach and the third one is the user may take 
> some time to realize the problem in the xl.cfg. So...
> 
>>
>> - coherently stop if the parameter value is not supported (including if sve is
>> not supported)
> 
> ... this is my preferred approach because it would be clear that the 
> value passed to Xen is bogus.

I did say earlier on that this comes with its own downside of preventing
boot to complete for no real reason. It's all Arm code, so you're fine
to ignore me, but in similar situations elsewhere (sorry, don't recall a
concrete example off the top of my head) we've aimed to allow the system
to boot, for the admin to then take corrective action if/as needed.

Jan
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Bertrand Marquis 1 year ago
Hi Jan,

> On 19 Apr 2023, at 08:28, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 18.04.2023 16:25, Julien Grall wrote:
>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>> On this serie I would like to open a discussion on how to handle the vector size
>>> and the corresponding command line / configuration / device tree parameters.
>>> 
>>> In general the user must either give a vector size it wants or has a solution to
>>> just request the maximum supported size.
>>> 
>>> In the current implementation if a size bigger than the supported one is provided:
>>> - we silently disable SVE for dom0
>>> - we silently disable SVE for dom0less
>>> - we do not create a guest when done through tools
>>> 
>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>> unless we have arguments for this status.
>> 
>> +1.
>> 
>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>> 
>>> I see some possible solutions here:
>>> 
>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>> some of the handling by using 2048 to use the maximum supported size.
>> 
>> My concern with this approach and the third one is the user may take 
>> some time to realize the problem in the xl.cfg. So...
>> 
>>> 
>>> - coherently stop if the parameter value is not supported (including if sve is
>>> not supported)
>> 
>> ... this is my preferred approach because it would be clear that the 
>> value passed to Xen is bogus.
> 
> I did say earlier on that this comes with its own downside of preventing
> boot to complete for no real reason. It's all Arm code, so you're fine
> to ignore me, but in similar situations elsewhere (sorry, don't recall a
> concrete example off the top of my head) we've aimed to allow the system
> to boot, for the admin to then take corrective action if/as needed.

But a guest depending on the feature will just crash later when booting.
This is making the assumption that guests are all able to properly adapt
to different hardware possibilities. 
This might be the case when you have a full Linux but if you consider an
embedded use case, if something is activated due to command line parameters
or configuration ones, it should not be expected that those are ignored I think.

There are definitely 2 different needs here, maybe we need to have something
like a "strict" switch to allow both use cases ?

Bertrand
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Jan Beulich 1 year ago
On 19.04.2023 09:31, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 19 Apr 2023, at 08:28, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 18.04.2023 16:25, Julien Grall wrote:
>>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>>> On this serie I would like to open a discussion on how to handle the vector size
>>>> and the corresponding command line / configuration / device tree parameters.
>>>>
>>>> In general the user must either give a vector size it wants or has a solution to
>>>> just request the maximum supported size.
>>>>
>>>> In the current implementation if a size bigger than the supported one is provided:
>>>> - we silently disable SVE for dom0
>>>> - we silently disable SVE for dom0less
>>>> - we do not create a guest when done through tools
>>>>
>>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>>> unless we have arguments for this status.
>>>
>>> +1.
>>>
>>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>>>
>>>> I see some possible solutions here:
>>>>
>>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>>> some of the handling by using 2048 to use the maximum supported size.
>>>
>>> My concern with this approach and the third one is the user may take 
>>> some time to realize the problem in the xl.cfg. So...
>>>
>>>>
>>>> - coherently stop if the parameter value is not supported (including if sve is
>>>> not supported)
>>>
>>> ... this is my preferred approach because it would be clear that the 
>>> value passed to Xen is bogus.
>>
>> I did say earlier on that this comes with its own downside of preventing
>> boot to complete for no real reason. It's all Arm code, so you're fine
>> to ignore me, but in similar situations elsewhere (sorry, don't recall a
>> concrete example off the top of my head) we've aimed to allow the system
>> to boot, for the admin to then take corrective action if/as needed.
> 
> But a guest depending on the feature will just crash later when booting.
> This is making the assumption that guests are all able to properly adapt
> to different hardware possibilities. 
> This might be the case when you have a full Linux but if you consider an
> embedded use case, if something is activated due to command line parameters
> or configuration ones, it should not be expected that those are ignored I think.
> 
> There are definitely 2 different needs here, maybe we need to have something
> like a "strict" switch to allow both use cases ?

Possibly. Yet along what I've said before - would you then also mean that to
fail boot upon encountering entirely unknown command line options?

Jan
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Bertrand Marquis 1 year ago
Hi Jan,

> On 19 Apr 2023, at 09:52, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 19.04.2023 09:31, Bertrand Marquis wrote:
>> Hi Jan,
>> 
>>> On 19 Apr 2023, at 08:28, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 18.04.2023 16:25, Julien Grall wrote:
>>>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>>>> On this serie I would like to open a discussion on how to handle the vector size
>>>>> and the corresponding command line / configuration / device tree parameters.
>>>>> 
>>>>> In general the user must either give a vector size it wants or has a solution to
>>>>> just request the maximum supported size.
>>>>> 
>>>>> In the current implementation if a size bigger than the supported one is provided:
>>>>> - we silently disable SVE for dom0
>>>>> - we silently disable SVE for dom0less
>>>>> - we do not create a guest when done through tools
>>>>> 
>>>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>>>> unless we have arguments for this status.
>>>> 
>>>> +1.
>>>> 
>>>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>>>> 
>>>>> I see some possible solutions here:
>>>>> 
>>>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>>>> some of the handling by using 2048 to use the maximum supported size.
>>>> 
>>>> My concern with this approach and the third one is the user may take 
>>>> some time to realize the problem in the xl.cfg. So...
>>>> 
>>>>> 
>>>>> - coherently stop if the parameter value is not supported (including if sve is
>>>>> not supported)
>>>> 
>>>> ... this is my preferred approach because it would be clear that the 
>>>> value passed to Xen is bogus.
>>> 
>>> I did say earlier on that this comes with its own downside of preventing
>>> boot to complete for no real reason. It's all Arm code, so you're fine
>>> to ignore me, but in similar situations elsewhere (sorry, don't recall a
>>> concrete example off the top of my head) we've aimed to allow the system
>>> to boot, for the admin to then take corrective action if/as needed.
>> 
>> But a guest depending on the feature will just crash later when booting.
>> This is making the assumption that guests are all able to properly adapt
>> to different hardware possibilities. 
>> This might be the case when you have a full Linux but if you consider an
>> embedded use case, if something is activated due to command line parameters
>> or configuration ones, it should not be expected that those are ignored I think.
>> 
>> There are definitely 2 different needs here, maybe we need to have something
>> like a "strict" switch to allow both use cases ?
> 
> Possibly. Yet along what I've said before - would you then also mean that to
> fail boot upon encountering entirely unknown command line options?

I think this should depend:
- completely unknow: we can ignore
- not supported (sve while sve is not supported by the platform or Xen): we should not ignore

I agree that one could use custom command line arguments for lots of reasons
(in linux you can do that and get them back from /proc for example) but silently
ignoring a parameter that is known to Xen i do not think we should do.

I think in most cases, one could think its system is correctly running but could get
problems later (or in some cases even not have any) so having a clear error at the
beginning is a lot more clear.

Bertrand
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Julien Grall 1 year ago

On 19/04/2023 09:20, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 19 Apr 2023, at 09:52, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 19.04.2023 09:31, Bertrand Marquis wrote:
>>> Hi Jan,
>>>
>>>> On 19 Apr 2023, at 08:28, Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 18.04.2023 16:25, Julien Grall wrote:
>>>>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>>>>> On this serie I would like to open a discussion on how to handle the vector size
>>>>>> and the corresponding command line / configuration / device tree parameters.
>>>>>>
>>>>>> In general the user must either give a vector size it wants or has a solution to
>>>>>> just request the maximum supported size.
>>>>>>
>>>>>> In the current implementation if a size bigger than the supported one is provided:
>>>>>> - we silently disable SVE for dom0
>>>>>> - we silently disable SVE for dom0less
>>>>>> - we do not create a guest when done through tools
>>>>>>
>>>>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>>>>> unless we have arguments for this status.
>>>>>
>>>>> +1.
>>>>>
>>>>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>>>>>
>>>>>> I see some possible solutions here:
>>>>>>
>>>>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>>>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>>>>> some of the handling by using 2048 to use the maximum supported size.
>>>>>
>>>>> My concern with this approach and the third one is the user may take
>>>>> some time to realize the problem in the xl.cfg. So...
>>>>>
>>>>>>
>>>>>> - coherently stop if the parameter value is not supported (including if sve is
>>>>>> not supported)
>>>>>
>>>>> ... this is my preferred approach because it would be clear that the
>>>>> value passed to Xen is bogus.
>>>>
>>>> I did say earlier on that this comes with its own downside of preventing
>>>> boot to complete for no real reason. It's all Arm code, so you're fine
>>>> to ignore me, but in similar situations elsewhere (sorry, don't recall a
>>>> concrete example off the top of my head) we've aimed to allow the system
>>>> to boot, for the admin to then take corrective action if/as needed.
>>>
>>> But a guest depending on the feature will just crash later when booting.
>>> This is making the assumption that guests are all able to properly adapt
>>> to different hardware possibilities.
>>> This might be the case when you have a full Linux but if you consider an
>>> embedded use case, if something is activated due to command line parameters
>>> or configuration ones, it should not be expected that those are ignored I think.
>>>
>>> There are definitely 2 different needs here, maybe we need to have something
>>> like a "strict" switch to allow both use cases ?
>>
>> Possibly. Yet along what I've said before - would you then also mean that to
>> fail boot upon encountering entirely unknown command line options?
> 
> I think this should depend:
> - completely unknow: we can ignore
> - not supported (sve while sve is not supported by the platform or Xen): we should not ignore
> 
> I agree that one could use custom command line arguments for lots of reasons
> (in linux you can do that and get them back from /proc for example) but silently
> ignoring a parameter that is known to Xen i do not think we should do.
> 
> I think in most cases, one could think its system is correctly running but could get
> problems later (or in some cases even not have any) so having a clear error at the
> beginning is a lot more clear.

FWIW, I agree with Bertrand.

Cheers,

-- 
Julien Grall
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Julien Grall 1 year ago
Hi,

Answering to Jan's and Bertrand's e-mail at the same time.

On 19/04/2023 08:31, Bertrand Marquis wrote:
>> On 19 Apr 2023, at 08:28, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 18.04.2023 16:25, Julien Grall wrote:
>>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>>> On this serie I would like to open a discussion on how to handle the vector size
>>>> and the corresponding command line / configuration / device tree parameters.
>>>>
>>>> In general the user must either give a vector size it wants or has a solution to
>>>> just request the maximum supported size.
>>>>
>>>> In the current implementation if a size bigger than the supported one is provided:
>>>> - we silently disable SVE for dom0
>>>> - we silently disable SVE for dom0less
>>>> - we do not create a guest when done through tools
>>>>
>>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>>> unless we have arguments for this status.
>>>
>>> +1.
>>>
>>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>>>
>>>> I see some possible solutions here:
>>>>
>>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>>> some of the handling by using 2048 to use the maximum supported size.
>>>
>>> My concern with this approach and the third one is the user may take
>>> some time to realize the problem in the xl.cfg. So...
>>>
>>>>
>>>> - coherently stop if the parameter value is not supported (including if sve is
>>>> not supported)
>>>
>>> ... this is my preferred approach because it would be clear that the
>>> value passed to Xen is bogus.
>>
>> I did say earlier on that this comes with its own downside of preventing
>> boot to complete for no real reason. It's all Arm code, so you're fine
>> to ignore me, but in similar situations elsewhere (sorry, don't recall a
>> concrete example off the top of my head) we've aimed to allow the system
>> to boot, for the admin to then take corrective action if/as needed.

It is not clear to me why it is necessary for the system to boot in 
order to take corrective action. In the case of GRUB, you would likely 
have a fallback for a Linux baremetal boot just in case Xen crash at 
boot. So you can use the fallback to boot and update your command line 
in grub.cfg.

I agree this is less efficient, but it would be easier for a user to 
find out there was an issue in the command line passed.

> 
> But a guest depending on the feature will just crash later when booting.
> This is making the assumption that guests are all able to properly adapt
> to different hardware possibilities.
> This might be the case when you have a full Linux but if you consider an
> embedded use case, if something is activated due to command line parameters
> or configuration ones, it should not be expected that those are ignored I think.
> 
> There are definitely 2 different needs here, maybe we need to have something
> like a "strict" switch to allow both use cases ?

At the mode, I am not in favor of introducing a switch to relax the 
check. It will add more code in Xen for a benefit that is not clear to 
me (see above).

Cheers,

-- 
Julien Grall
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Bertrand Marquis 1 year ago
Hi Julien,

> On 18 Apr 2023, at 16:25, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 18/04/2023 14:13, Bertrand Marquis wrote:
>> Hi Luca,
> 
> Hi,
> 
>> On this serie I would like to open a discussion on how to handle the vector size
>> and the corresponding command line / configuration / device tree parameters.
>> In general the user must either give a vector size it wants or has a solution to
>> just request the maximum supported size.
>> In the current implementation if a size bigger than the supported one is provided:
>> - we silently disable SVE for dom0
>> - we silently disable SVE for dom0less
>> - we do not create a guest when done through tools
>> This is not completely coherent and i think we should aim for a coherent behaviour
>> unless we have arguments for this status.
> 
> +1.
> 
>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>> I see some possible solutions here:
>> - modify parameter behaviour to use the supported size if parameter is bigger than
>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>> some of the handling by using 2048 to use the maximum supported size.
> 
> My concern with this approach and the third one is the user may take some time to realize the problem in the xl.cfg. So...

Good point

> 
>> - coherently stop if the parameter value is not supported (including if sve is
>> not supported)
> 
> ... this is my preferred approach because it would be clear that the value passed to Xen is bogus.

I agree: we should not silently ignore configuration parameters or try to "fix" them.

Cheers
Bertrand
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Luca Fancellu 1 year ago

> On 18 Apr 2023, at 15:38, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> 
> Hi Julien,
> 
>> On 18 Apr 2023, at 16:25, Julien Grall <julien@xen.org> wrote:
>> 
>> 
>> 
>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>> Hi Luca,
>> 
>> Hi,
>> 
>>> On this serie I would like to open a discussion on how to handle the vector size
>>> and the corresponding command line / configuration / device tree parameters.
>>> In general the user must either give a vector size it wants or has a solution to
>>> just request the maximum supported size.
>>> In the current implementation if a size bigger than the supported one is provided:
>>> - we silently disable SVE for dom0
>>> - we silently disable SVE for dom0less
>>> - we do not create a guest when done through tools
>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>> unless we have arguments for this status.
>> 
>> +1.
>> 
>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>> I see some possible solutions here:
>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>> some of the handling by using 2048 to use the maximum supported size.
>> 
>> My concern with this approach and the third one is the user may take some time to realize the problem in the xl.cfg. So...
> 
> Good point
> 
>> 
>>> - coherently stop if the parameter value is not supported (including if sve is
>>> not supported)
>> 
>> ... this is my preferred approach because it would be clear that the value passed to Xen is bogus.
> 
> I agree: we should not silently ignore configuration parameters or try to "fix" them.

Hi Bertrand and Julien,

Ok I will update the serie to stop the domain creation if the parameter supplied is wrong or SVE is not supported by the platform.

> 
> Cheers
> Bertrand
Re: [PATCH v5 00/12] SVE feature for arm guests
Posted by Bertrand Marquis 1 year ago
Hi Luca,

> On 18 Apr 2023, at 16:41, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
> 
> 
>> On 18 Apr 2023, at 15:38, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
>> 
>> Hi Julien,
>> 
>>> On 18 Apr 2023, at 16:25, Julien Grall <julien@xen.org> wrote:
>>> 
>>> 
>>> 
>>> On 18/04/2023 14:13, Bertrand Marquis wrote:
>>>> Hi Luca,
>>> 
>>> Hi,
>>> 
>>>> On this serie I would like to open a discussion on how to handle the vector size
>>>> and the corresponding command line / configuration / device tree parameters.
>>>> In general the user must either give a vector size it wants or has a solution to
>>>> just request the maximum supported size.
>>>> In the current implementation if a size bigger than the supported one is provided:
>>>> - we silently disable SVE for dom0
>>>> - we silently disable SVE for dom0less
>>>> - we do not create a guest when done through tools
>>>> This is not completely coherent and i think we should aim for a coherent behaviour
>>>> unless we have arguments for this status.
>>> 
>>> +1.
>>> 
>>>> Is there any good reason to silently disable for Dom0 and dom0less only ?
>>>> I see some possible solutions here:
>>>> - modify parameter behaviour to use the supported size if parameter is bigger than
>>>> it. This would at least keep SVE enabled if a VM depends on it and could simplify
>>>> some of the handling by using 2048 to use the maximum supported size.
>>> 
>>> My concern with this approach and the third one is the user may take some time to realize the problem in the xl.cfg. So...
>> 
>> Good point
>> 
>>> 
>>>> - coherently stop if the parameter value is not supported (including if sve is
>>>> not supported)
>>> 
>>> ... this is my preferred approach because it would be clear that the value passed to Xen is bogus.
>> 
>> I agree: we should not silently ignore configuration parameters or try to "fix" them.
> 
> Hi Bertrand and Julien,
> 
> Ok I will update the serie to stop the domain creation if the parameter supplied is wrong or SVE is not supported by the platform.

Thanks

Bertrand

> 
>> 
>> Cheers
>> Bertrand