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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Stefano,

> On 18 Nov 2020, at 00:50, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> 
> 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.

This is a great change that makes configuration more clear.

> 
> 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
> 
> - 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
> 
> - clarify the EXPERT one-line description

Should we also follow the scheme and add (EXPERT) in the text for expert 
options ?

and one small fix

> 
> [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
> 
> ---
> Changes in v2:
> - introduce UNSUPPORTED as a separate new option
> - don't switch all EXPERT options to UNSUPPORTED
> ---
> xen/Kconfig              | 11 ++++++++++-
> xen/arch/arm/Kconfig     | 10 +++++-----
> xen/arch/x86/Kconfig     |  8 ++++----
> xen/common/Kconfig       |  4 ++--
> xen/common/sched/Kconfig |  6 +++---
> 5 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/Kconfig b/xen/Kconfig
> index 34c318bfa2..59400c4788 100644
> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -34,8 +34,17 @@ config DEFCONFIG_LIST
>       option defconfig_list
>       default ARCH_DEFCONFIG
> 
> +config UNSUPPORTED
> +     bool "Configure UNSUPPORTED features"
> +     help
> +       This option allows unsupported Xen options to be enabled, which
> +       includes non-security-supported, experimental, and tech preview
> +       features as defined by SUPPORT.md. Xen binaries built with this
> +       option enabled are not security supported.
> +     default n
> +
> 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 f938dd21bd..5981e7380d 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
> @@ -79,7 +79,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
> @@ -89,7 +89,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
> @@ -106,7 +106,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 24868aa6ad..d4e20e9d31 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -102,8 +102,8 @@ config HVM
>         If unsure, say Y.
> 
> config XEN_SHSTK
> -     bool "Supervisor Shadow Stacks"
> -     depends on HAS_AS_CET_SS && EXPERT
> +     bool "Supervisor Shadow Stacks (UNSUPPORTED)"
> +     depends on HAS_AS_CET_SS && UNSUPPORTED

This one is not following the standard scheme with “if UNSUPPORTED"

Cheers
Bertrand

>       default y
>       ---help---
>         Control-flow Enforcement Technology (CET) is a set of features in
> @@ -165,7 +165,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---
> @@ -251,7 +251,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 3e2cf25088..beed507727 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -151,7 +151,7 @@ config KEXEC
>         If unsure, say Y.
> 
> config EFI_SET_VIRTUAL_ADDRESS_MAP
> -    bool "EFI: call SetVirtualAddressMap()" if EXPERT
> +    bool "EFI: call SetVirtualAddressMap() (UNSUPPORTED)" if UNSUPPORTED
>     ---help---
>       Call EFI SetVirtualAddressMap() runtime service to setup memory map for
>       further runtime services. According to UEFI spec, it isn't strictly
> @@ -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
>