[v1] x86: Fix AMD_SVM and INTEL_VMX dependency

[PATCH] x86: Fix AMD_SVM and INTEL_VMX dependency

Posted by Michal Orzel 1 month, 4 weeks ago

Commit e3ed540f2e9f was meant to make AMD_SVM dependent on AMD and
INTEL_VMX on INTEL. Such dependency should be done using 'depends on'
and not 'if' next to prompt that deals only with the visibility of the
given Kconfig option. This makes it impossible to e.g. disable INTEL_VMX
when INTEL is disabled (option is hidden).

Fixes: e3ed540f2e9f ("x86/hvm: add HVM-specific Kconfig")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 xen/arch/x86/hvm/Kconfig | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/Kconfig b/xen/arch/x86/hvm/Kconfig
index b903764bda0d..c85889ea8642 100644
--- a/xen/arch/x86/hvm/Kconfig
+++ b/xen/arch/x86/hvm/Kconfig
@@ -16,7 +16,8 @@ menuconfig HVM
 if HVM
 
 config AMD_SVM
-	bool "AMD-V" if AMD && EXPERT
+	bool "AMD-V" if EXPERT
+	depends on AMD
 	default y
 	help
 	  Enables virtual machine extensions on platforms that implement the
@@ -25,7 +26,8 @@ config AMD_SVM
 	  If in doubt, say Y.
 
 config INTEL_VMX
-	bool "Intel VT-x" if INTEL && EXPERT
+	bool "Intel VT-x" if EXPERT
+	depends on INTEL
 	default y
 	select ARCH_VCPU_IOREQ_COMPLETION
 	help
-- 
2.43.0

Re: [PATCH] x86: Fix AMD_SVM and INTEL_VMX dependency

Posted by Jan Beulich 1 month, 4 weeks ago

On 01.09.2025 12:43, Michal Orzel wrote:
> Commit e3ed540f2e9f was meant to make AMD_SVM dependent on AMD and
> INTEL_VMX on INTEL. Such dependency should be done using 'depends on'
> and not 'if' next to prompt that deals only with the visibility of the
> given Kconfig option. This makes it impossible to e.g. disable INTEL_VMX
> when INTEL is disabled (option is hidden).

Hmm, yes, just that ...

> --- a/xen/arch/x86/hvm/Kconfig
> +++ b/xen/arch/x86/hvm/Kconfig
> @@ -16,7 +16,8 @@ menuconfig HVM
>  if HVM
>  
>  config AMD_SVM
> -	bool "AMD-V" if AMD && EXPERT
> +	bool "AMD-V" if EXPERT
> +	depends on AMD
>  	default y
>  	help
>  	  Enables virtual machine extensions on platforms that implement the
> @@ -25,7 +26,8 @@ config AMD_SVM
>  	  If in doubt, say Y.
>  
>  config INTEL_VMX
> -	bool "Intel VT-x" if INTEL && EXPERT
> +	bool "Intel VT-x" if EXPERT
> +	depends on INTEL
>  	default y
>  	select ARCH_VCPU_IOREQ_COMPLETION
>  	help

... now it becomes impossible to _enable_ INTEL_VMX when INTEL is disabled,
yet which may be of interest if you target some other vendor's VMX
implementation. Perhaps really we should have

config INTEL_VMX
	bool "Intel VT-x" if EXPERT
 	default INTEL

?

Jan

Re: [PATCH] x86: Fix AMD_SVM and INTEL_VMX dependency

Posted by Orzel, Michal 1 month, 4 weeks ago


On 01/09/2025 13:19, Jan Beulich wrote:
> On 01.09.2025 12:43, Michal Orzel wrote:
>> Commit e3ed540f2e9f was meant to make AMD_SVM dependent on AMD and
>> INTEL_VMX on INTEL. Such dependency should be done using 'depends on'
>> and not 'if' next to prompt that deals only with the visibility of the
>> given Kconfig option. This makes it impossible to e.g. disable INTEL_VMX
>> when INTEL is disabled (option is hidden).
> 
> Hmm, yes, just that ...
> 
>> --- a/xen/arch/x86/hvm/Kconfig
>> +++ b/xen/arch/x86/hvm/Kconfig
>> @@ -16,7 +16,8 @@ menuconfig HVM
>>  if HVM
>>  
>>  config AMD_SVM
>> -	bool "AMD-V" if AMD && EXPERT
>> +	bool "AMD-V" if EXPERT
>> +	depends on AMD
>>  	default y
>>  	help
>>  	  Enables virtual machine extensions on platforms that implement the
>> @@ -25,7 +26,8 @@ config AMD_SVM
>>  	  If in doubt, say Y.
>>  
>>  config INTEL_VMX
>> -	bool "Intel VT-x" if INTEL && EXPERT
>> +	bool "Intel VT-x" if EXPERT
>> +	depends on INTEL
>>  	default y
>>  	select ARCH_VCPU_IOREQ_COMPLETION
>>  	help
> 
> ... now it becomes impossible to _enable_ INTEL_VMX when INTEL is disabled,
> yet which may be of interest if you target some other vendor's VMX
> implementation. Perhaps really we should have
> 
> config INTEL_VMX
> 	bool "Intel VT-x" if EXPERT
>  	default INTEL
I did not think such configuration makes sense (VMX without INTEL)
which is in line with the last sentence from the mentioned commit.
I'm ok either way.

~Michal

Re: [PATCH] x86: Fix AMD_SVM and INTEL_VMX dependency

Posted by Jan Beulich 1 month, 4 weeks ago

On 01.09.2025 13:25, Orzel, Michal wrote:
> 
> 
> On 01/09/2025 13:19, Jan Beulich wrote:
>> On 01.09.2025 12:43, Michal Orzel wrote:
>>> Commit e3ed540f2e9f was meant to make AMD_SVM dependent on AMD and
>>> INTEL_VMX on INTEL. Such dependency should be done using 'depends on'
>>> and not 'if' next to prompt that deals only with the visibility of the
>>> given Kconfig option. This makes it impossible to e.g. disable INTEL_VMX
>>> when INTEL is disabled (option is hidden).
>>
>> Hmm, yes, just that ...
>>
>>> --- a/xen/arch/x86/hvm/Kconfig
>>> +++ b/xen/arch/x86/hvm/Kconfig
>>> @@ -16,7 +16,8 @@ menuconfig HVM
>>>  if HVM
>>>  
>>>  config AMD_SVM
>>> -	bool "AMD-V" if AMD && EXPERT
>>> +	bool "AMD-V" if EXPERT
>>> +	depends on AMD
>>>  	default y
>>>  	help
>>>  	  Enables virtual machine extensions on platforms that implement the
>>> @@ -25,7 +26,8 @@ config AMD_SVM
>>>  	  If in doubt, say Y.
>>>  
>>>  config INTEL_VMX
>>> -	bool "Intel VT-x" if INTEL && EXPERT
>>> +	bool "Intel VT-x" if EXPERT
>>> +	depends on INTEL
>>>  	default y
>>>  	select ARCH_VCPU_IOREQ_COMPLETION
>>>  	help
>>
>> ... now it becomes impossible to _enable_ INTEL_VMX when INTEL is disabled,
>> yet which may be of interest if you target some other vendor's VMX
>> implementation. Perhaps really we should have
>>
>> config INTEL_VMX
>> 	bool "Intel VT-x" if EXPERT
>>  	default INTEL
> I did not think such configuration makes sense (VMX without INTEL)
> which is in line with the last sentence from the mentioned commit.

Just like Hygon CPUs support AMD's SVM (which we cover), VIA's and Shanghai
iirc support VMX (which we don't cover very well, but which we also shouldn't
break knowingly).

Jan