Re: [PATCH v4 03/20] xen/x86: remove "depends on !PV_SHIM_EXCLUSIVE"

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.05.2025 11:16, Penny Zheng wrote:
> Remove all "depends on !PV_SHIM_EXCLUSIVE" (also the functionally
> equivalent "if !...") in Kconfig file, since negative dependancy will badly
> affect allyesconfig. To make sure unchanging produced config file based
> on "pvshim_defconfig", we shall explicitly state according Kconfig is not set
> 
> Add "default y" for SHADOW_PAGING and TBOOT, otherwise we will have unset
> values when running make defconfig based on "x86_64_defconfig".

I fear I don't understand this, perhaps related to me also not seeing how ...

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -143,7 +143,7 @@ config XEN_IBT
>  
>  config SHADOW_PAGING
>       bool "Shadow Paging"
> -     default !PV_SHIM_EXCLUSIVE
> +     default y
>       depends on PV || HVM
>       help
>  
> @@ -175,7 +175,7 @@ config BIGMEM
>  config TBOOT
>       bool "Xen tboot support (UNSUPPORTED)"
>       depends on INTEL && UNSUPPORTED
> -     default !PV_SHIM_EXCLUSIVE
> +     default y
>       select CRYPTO
>       help
>         Allows support for Trusted Boot using the Intel(R) Trusted Execution

... these two fit with title and description. The justification for removing
the !PV_SHIM_EXCLUSIVE here is not "breaks allyesconfig".

> @@ -288,7 +288,6 @@ config PV_SHIM_EXCLUSIVE
>  
>         If unsure, say N.
>  
> -if !PV_SHIM_EXCLUSIVE
>  
>  config HYPERV_GUEST
>       bool "Hyper-V Guest"
> @@ -298,7 +297,6 @@ config HYPERV_GUEST
>  
>         If unsure, say N.
>  
> -endif
>  
>  config REQUIRE_NX
>       bool "Require NX (No eXecute) support"

Please don't leave double blank lines.

> --- a/xen/arch/x86/configs/pvshim_defconfig
> +++ b/xen/arch/x86/configs/pvshim_defconfig
> @@ -26,3 +26,8 @@ CONFIG_EXPERT=y
>  # CONFIG_INTEL_IOMMU is not set
>  # CONFIG_DEBUG is not set
>  # CONFIG_GDBSX is not set
> +# CONFIG_SHADOW_PAGING is not set
> +# CONFIG_TBOOT is not set
> +# HYPERV_HYPERV_GUEST is not set

This one doesn't look right, simply by its name.

> +# CONFIG_HVM is not set
> +# CONFIG_VGA is not set

Just to mention it - I'm unsure whether adding such at the end isn't going
to cause issues. But maybe I'm paranoid ...

> --- a/xen/drivers/video/Kconfig
> +++ b/xen/drivers/video/Kconfig
> @@ -3,10 +3,10 @@ config VIDEO
>       bool
>  
>  config VGA
> -     bool "VGA support" if !PV_SHIM_EXCLUSIVE
> +     bool "VGA support"
>       select VIDEO
>       depends on X86
> -     default y if !PV_SHIM_EXCLUSIVE
> +     default y
>       help
>         Enable VGA output for the Xen hypervisor.

Like above, this change also doesn't really fit with title and description.

Jan