Re: [PATCH v3] xen: EXPERT clean-up and introduce UNSUPPORTED

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Stefano,

> On 23 Jan 2021, at 02:19, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> A recent thread [1] has exposed a couple of issues with our current way
> of handling EXPERT.
> 
> 1) It is not obvious that "Configure standard Xen features (expert
> users)" is actually the famous EXPERT we keep talking about on xen-devel
> 
> 2) It is not obvious when we need to enable EXPERT to get a specific
> feature
> 
> In particular if you want to enable ACPI support so that you can boot
> Xen on an ACPI platform, you have to enable EXPERT first. But searching
> through the kconfig menu it is really not clear (type '/' and "ACPI"):
> nothing in the description tells you that you need to enable EXPERT to
> get the option.
> 
> So this patch makes things easier by doing two things:
> 
> - introduce a new kconfig option UNSUPPORTED which is clearly to enable
>  UNSUPPORTED features as defined by SUPPORT.md

That’s a great change which will improve user experience.

> 
> - change EXPERT options to UNSUPPORTED where it makes sense: keep
>  depending on EXPERT for features made for experts
> 
> - tag unsupported features by adding (UNSUPPORTED) to the one-line
>  description
> 

Shouldn’t we add  (EXPERT) for expert options in the same way for coherency ?

Cheers
Bertrand

> - clarify the EXPERT one-line description
> 
> [1] https://marc.info/?l=xen-devel&m=160333101228981
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> CC: andrew.cooper3@xxxxxxxxxx
> CC: george.dunlap@xxxxxxxxxx
> CC: iwj@xxxxxxxxxxxxxx
> CC: jbeulich@xxxxxxxx
> CC: julien@xxxxxxx
> CC: wl@xxxxxxx
> CC: Bertrand.Marquis@xxxxxxx
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> ---
> Changes in v3:
> - improve UNSUPPORTED text description
> - avoid changing XEN_SHSTK and EFI_SET_VIRTUAL_ADDRESS_MAP
> - update HVM_FEP to be UNSUPPORTED
> 
> Changes in v2:
> - introduce UNSUPPORTED
> - don't switch all EXPERT options to UNSUPPORTED
> 
> See as reference the v2 thread here:
> https://marc.info/?l=xen-devel&m=160566066013723
> ---
> xen/Kconfig              |  9 ++++++++-
> xen/arch/arm/Kconfig     | 10 +++++-----
> xen/arch/x86/Kconfig     |  6 +++---
> xen/common/Kconfig       |  2 +-
> xen/common/sched/Kconfig |  6 +++---
> 5 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/Kconfig b/xen/Kconfig
> index 34c318bfa2..4a3d988353 100644
> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -34,8 +34,15 @@ config DEFCONFIG_LIST
>       option defconfig_list
>       default ARCH_DEFCONFIG
> 
> +config UNSUPPORTED
> +     bool "Configure UNSUPPORTED features"
> +     help
> +       This option allows certain unsupported Xen options to be changed,
> +       which includes non-security-supported, experimental, and tech
> +       preview features as defined by SUPPORT.md.
> +
> config EXPERT
> -     bool "Configure standard Xen features (expert users)"
> +     bool "Configure EXPERT features"
>       help
>         This option allows certain base Xen options and settings
>         to be disabled or tweaked. This is for specialized environments
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index c3eb13ea73..cca76040e5 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -32,7 +32,7 @@ menu "Architecture Features"
> source "arch/Kconfig"
> 
> config ACPI
> -     bool "ACPI (Advanced Configuration and Power Interface) Support" if 
> EXPERT
> +     bool "ACPI (Advanced Configuration and Power Interface) Support 
> (UNSUPPORTED)" if UNSUPPORTED
>       depends on ARM_64
>       ---help---
> 
> @@ -49,7 +49,7 @@ config GICV3
>         If unsure, say Y
> 
> config HAS_ITS
> -        bool "GICv3 ITS MSI controller support" if EXPERT
> +        bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED
>         depends on GICV3 && !NEW_VGIC
> 
> config HVM
> @@ -77,7 +77,7 @@ config SBSA_VUART_CONSOLE
>         SBSA Generic UART implements a subset of ARM PL011 UART.
> 
> config ARM_SSBD
> -     bool "Speculative Store Bypass Disable" if EXPERT
> +     bool "Speculative Store Bypass Disable (UNSUPPORTED)" if UNSUPPORTED
>       depends on HAS_ALTERNATIVE
>       default y
>       help
> @@ -87,7 +87,7 @@ config ARM_SSBD
>         If unsure, say Y.
> 
> config HARDEN_BRANCH_PREDICTOR
> -     bool "Harden the branch predictor against aliasing attacks" if EXPERT
> +     bool "Harden the branch predictor against aliasing attacks 
> (UNSUPPORTED)" if UNSUPPORTED
>       default y
>       help
>         Speculation attacks against some high-performance processors rely on
> @@ -104,7 +104,7 @@ config HARDEN_BRANCH_PREDICTOR
>         If unsure, say Y.
> 
> config TEE
> -     bool "Enable TEE mediators support" if EXPERT
> +     bool "Enable TEE mediators support (UNSUPPORTED)" if UNSUPPORTED
>       default n
>       help
>         This option enables generic TEE mediators support. It allows guests
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 78f351f94b..302334d3e4 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -147,7 +147,7 @@ config BIGMEM
>         If unsure, say N.
> 
> config HVM_FEP
> -     bool "HVM Forced Emulation Prefix support" if EXPERT
> +     bool "HVM Forced Emulation Prefix support (UNSUPPORTED)" if UNSUPPORTED
>       default DEBUG
>       depends on HVM
>       ---help---
> @@ -166,7 +166,7 @@ config HVM_FEP
>         If unsure, say N.
> 
> config TBOOT
> -     bool "Xen tboot support" if EXPERT
> +     bool "Xen tboot support (UNSUPPORTED)" if UNSUPPORTED
>       default y if !PV_SHIM_EXCLUSIVE
>       select CRYPTO
>       ---help---
> @@ -252,7 +252,7 @@ config HYPERV_GUEST
> endif
> 
> config MEM_SHARING
> -     bool "Xen memory sharing support" if EXPERT
> +     bool "Xen memory sharing support (UNSUPPORTED)" if UNSUPPORTED
>       depends on HVM
> 
> endmenu
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index b5c91a1664..39451e8350 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -272,7 +272,7 @@ config LATE_HWDOM
>         If unsure, say N.
> 
> config ARGO
> -     bool "Argo: hypervisor-mediated interdomain communication" if EXPERT
> +     bool "Argo: hypervisor-mediated interdomain communication 
> (UNSUPPORTED)" if UNSUPPORTED
>       ---help---
>         Enables a hypercall for domains to ask the hypervisor to perform
>         data transfer of messages between domains.
> diff --git a/xen/common/sched/Kconfig b/xen/common/sched/Kconfig
> index 61231aacaa..94c9e20139 100644
> --- a/xen/common/sched/Kconfig
> +++ b/xen/common/sched/Kconfig
> @@ -15,7 +15,7 @@ config SCHED_CREDIT2
>         optimized for lower latency and higher VM density.
> 
> config SCHED_RTDS
> -     bool "RTDS scheduler support (EXPERIMENTAL)"
> +     bool "RTDS scheduler support (UNSUPPORTED)" if UNSUPPORTED
>       default y
>       ---help---
>         The RTDS scheduler is a soft and firm real-time scheduler for
> @@ -23,14 +23,14 @@ config SCHED_RTDS
>         in the cloud, and general low-latency workloads.
> 
> config SCHED_ARINC653
> -     bool "ARINC653 scheduler support (EXPERIMENTAL)"
> +     bool "ARINC653 scheduler support (UNSUPPORTED)" if UNSUPPORTED
>       default DEBUG
>       ---help---
>         The ARINC653 scheduler is a hard real-time scheduler for single
>         cores, targeted for avionics, drones, and medical devices.
> 
> config SCHED_NULL
> -     bool "Null scheduler support (EXPERIMENTAL)"
> +     bool "Null scheduler support (UNSUPPORTED)" if UNSUPPORTED
>       default y
>       ---help---
>         The null scheduler is a static, zero overhead scheduler,
> -- 
> 2.17.1
>