diff mbox series

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

Message ID 20210123021950.1270-1-sstabellini@kernel.org (mailing list archive)
State New
Headers show
Series [v3] xen: EXPERT clean-up and introduce UNSUPPORTED | expand

Commit Message

Stefano Stabellini Jan. 23, 2021, 2:19 a.m. UTC
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

- 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

[1] https://marc.info/?l=xen-devel&m=160333101228981

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
CC: andrew.cooper3@citrix.com
CC: george.dunlap@citrix.com
CC: iwj@xenproject.org
CC: jbeulich@suse.com
CC: julien@xen.org
CC: wl@xen.org
CC: Bertrand.Marquis@arm.com

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
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(-)

Comments

Jan Beulich Jan. 25, 2021, 9:44 a.m. UTC | #1
On 23.01.2021 03:19, Stefano Stabellini wrote:
> --- 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.

And by implication anything not depending on UNSUPPORTED is
supported? I didn't think this was the case (some unsupported
code can't even be turned off via Kconfig), so I think this
needs clarifying here, so we won't end up with people
considering some feature supported which really isn't. That's
irrespective of the reference to 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

I'd like to suggest to move UNSUPPORTED past this one, to
then have that have "default EXPERT".

Jan
Bertrand Marquis Jan. 25, 2021, 10:38 a.m. UTC | #2
Hi Stefano,

> On 23 Jan 2021, at 02:19, Stefano Stabellini <sstabellini@kernel.org> 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@xilinx.com>
> CC: andrew.cooper3@citrix.com
> CC: george.dunlap@citrix.com
> CC: iwj@xenproject.org
> CC: jbeulich@suse.com
> CC: julien@xen.org
> CC: wl@xen.org
> CC: Bertrand.Marquis@arm.com
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
> 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
>
Stefano Stabellini Jan. 25, 2021, 9:18 p.m. UTC | #3
On Mon, 25 Jan 2021, Jan Beulich wrote:
> On 23.01.2021 03:19, Stefano Stabellini wrote:
> > --- 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.
> 
> And by implication anything not depending on UNSUPPORTED is
> supported? I didn't think this was the case (some unsupported
> code can't even be turned off via Kconfig), so I think this
> needs clarifying here, so we won't end up with people
> considering some feature supported which really isn't. That's
> irrespective of the reference to SUPPORT.md.

I'll clarify.


> >  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
> 
> I'd like to suggest to move UNSUPPORTED past this one, to
> then have that have "default EXPERT".

Sure, good idea.
Stefano Stabellini Jan. 25, 2021, 9:18 p.m. UTC | #4
On Mon, 25 Jan 2021, Bertrand Marquis wrote:
> Hi Stefano,
> 
> > On 23 Jan 2021, at 02:19, Stefano Stabellini <sstabellini@kernel.org> 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.

Thank you!


> > - 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 ?

I take you mean add "(EXPERT)" to the one-line description in kconfig. I
am OK with that, probably better as a second separate patch. I'll add
it.
diff mbox series

Patch

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,