xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

[PATCH] xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

Posted by Juergen Gross 3 years, 4 months ago

With CONFIG_PV_SHIM_EXCLUSIVE some sources required for CONFIG_HVM are
not built, so let CONFIG_HVM depend on !CONFIG_PV_SHIM_EXCLUSIVE.

Let CONFIG_HVM default to !CONFIG_PV_SHIM instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 24868aa6ad..0107cfa12f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -90,7 +90,8 @@ config PV_LINEAR_PT
          If unsure, say Y.
 
 config HVM
-	def_bool !PV_SHIM_EXCLUSIVE
+	depends on !PV_SHIM_EXCLUSIVE
+	def_bool !PV_SHIM
 	prompt "HVM support"
 	---help---
 	  Interfaces to support HVM domains.  HVM domains require hardware
-- 
2.26.2

Re: [PATCH] xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

Posted by Jan Beulich 3 years, 4 months ago

On 08.12.2020 14:51, Juergen Gross wrote:
> With CONFIG_PV_SHIM_EXCLUSIVE some sources required for CONFIG_HVM are
> not built, so let CONFIG_HVM depend on !CONFIG_PV_SHIM_EXCLUSIVE.
> 
> Let CONFIG_HVM default to !CONFIG_PV_SHIM instead.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  xen/arch/x86/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

See "x86/shim: don't permit HVM and PV_SHIM_EXCLUSIVE at the same
time" posted on Oct 19. I'd be fine switching to the !PV_SHIM
default you have here. But Andrew looks to be objecting to a
change like this, sadly without pointing out a good alternative
so far.

Jan

> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 24868aa6ad..0107cfa12f 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -90,7 +90,8 @@ config PV_LINEAR_PT
>           If unsure, say Y.
>  
>  config HVM
> -	def_bool !PV_SHIM_EXCLUSIVE
> +	depends on !PV_SHIM_EXCLUSIVE
> +	def_bool !PV_SHIM
>  	prompt "HVM support"
>  	---help---
>  	  Interfaces to support HVM domains.  HVM domains require hardware
>

Re: [PATCH] xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

Posted by Andrew Cooper 3 years, 4 months ago

On 08/12/2020 13:51, Juergen Gross wrote:
> With CONFIG_PV_SHIM_EXCLUSIVE some sources required for CONFIG_HVM are
> not built, so let CONFIG_HVM depend on !CONFIG_PV_SHIM_EXCLUSIVE.
>
> Let CONFIG_HVM default to !CONFIG_PV_SHIM instead.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

So while this will fix the randconfig failure, the statement isn't
true.  There are HVM codepaths which aren't even dead in shim-exclusive
mode.

The problem here is the way CONFIG_PV_SHIM_EXCLUSIVE abuses the Kconfig
system.  What is currently happening is that this option is trying to
enforce the pv shim defconfig in the dependency system.

We already have a defconfig, which is used in appropriate locations.  We
should not have two different things fighting over control.

This is the fault of c/s 8b5b49ceb3d which went in despite my
objections.  The change is not related to PV_SHIM_EXCLUSIVE - it is to
do with not supporting a control domain, which a) better describes what
it is actually doing, and b) has wider utility than PV Shim.

~Andrew

Re: [PATCH] xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

Posted by Jürgen Groß 3 years, 4 months ago

On 08.12.20 15:33, Andrew Cooper wrote:
> On 08/12/2020 13:51, Juergen Gross wrote:
>> With CONFIG_PV_SHIM_EXCLUSIVE some sources required for CONFIG_HVM are
>> not built, so let CONFIG_HVM depend on !CONFIG_PV_SHIM_EXCLUSIVE.
>>
>> Let CONFIG_HVM default to !CONFIG_PV_SHIM instead.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> So while this will fix the randconfig failure, the statement isn't
> true.  There are HVM codepaths which aren't even dead in shim-exclusive
> mode.

I only said that CONFIG_PV_SHIM_EXCLUSIVE disables building some
sources which are required for CONFIG_HVM, and this is certainly true.

> The problem here is the way CONFIG_PV_SHIM_EXCLUSIVE abuses the Kconfig
> system.  What is currently happening is that this option is trying to
> enforce the pv shim defconfig in the dependency system.
> 
> We already have a defconfig, which is used in appropriate locations.  We
> should not have two different things fighting over control.
> 
> This is the fault of c/s 8b5b49ceb3d which went in despite my
> objections.  The change is not related to PV_SHIM_EXCLUSIVE - it is to
> do with not supporting a control domain, which a) better describes what
> it is actually doing, and b) has wider utility than PV Shim.

Yes, maybe.

Random build failures are not nice, so in case there is no agreement
how to proceed I'd be in favor for fixing the fallout and then discuss
a proper solution.


Juergen

Re: [PATCH] xen: CONFIG_PV_SHIM_EXCLUSIVE and CONFIG_HVM are mutually exclusive

Posted by Jan Beulich 3 years, 4 months ago

On 08.12.2020 15:33, Andrew Cooper wrote:
> On 08/12/2020 13:51, Juergen Gross wrote:
>> With CONFIG_PV_SHIM_EXCLUSIVE some sources required for CONFIG_HVM are
>> not built, so let CONFIG_HVM depend on !CONFIG_PV_SHIM_EXCLUSIVE.
>>
>> Let CONFIG_HVM default to !CONFIG_PV_SHIM instead.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> So while this will fix the randconfig failure, the statement isn't
> true.  There are HVM codepaths which aren't even dead in shim-exclusive
> mode.
> 
> The problem here is the way CONFIG_PV_SHIM_EXCLUSIVE abuses the Kconfig
> system.  What is currently happening is that this option is trying to
> enforce the pv shim defconfig in the dependency system.
> 
> We already have a defconfig, which is used in appropriate locations.  We
> should not have two different things fighting over control.
> 
> This is the fault of c/s 8b5b49ceb3d which went in despite my
> objections.  The change is not related to PV_SHIM_EXCLUSIVE - it is to
> do with not supporting a control domain, which a) better describes what
> it is actually doing, and b) has wider utility than PV Shim.

Would you mind pointing me at where you had voiced objections
to that change? I've just searched both my inbox and the list
archives, without finding any. I only recall your objections
to the patch I sent later which is similar to Jürgen's. And
I'm quite certain I'd have stayed away from committing anything
while aware of unresolved objections, even if - more often than
not - this means waiting almost indefinitely, which I don't
appreciate as a way to deal with disagreement.

From what you further state, I derive that you'd like to see
e.g. !PV_SHIM_EXCLUSIVE be a dependency of a new CONTROL_DOMAIN
Kconfig setting? I'm not sure though I see how this would help
the situation (I'm not even sure what scope this control would
have: just domctl, or also sysctl, or additionally platform-op).
Nor do I see what's wrong with forcing HVM off in shim-exclusive
builds - there can't be HVM domains in such a configuration (but
I'm pretty sure I said so somewhere else already, without ever
hearing back, albeit it apparently wasn't on either of the
patches' threads according to my outbox).

Jan