Re: [PATCH 2/5] x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Wei Liu <wl@xen.org>, Kevin Tian <kevin.tian@intel.com>,
	Jun Nakajima <jun.nakajima@intel.com>
Subject: Re: [PATCH 2/5] x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware
Date: Tue, 21 Nov 2023 17:31:09 +0000	[thread overview]
Message-ID: <b75a4d31-260c-4377-b68c-c8153dfd948e@citrix.com> (raw)
In-Reply-To: <d63dfc54-cf89-44b7-ab7e-983037892833@suse.com>

On 21/11/2023 5:27 pm, Jan Beulich wrote:
> On 21.11.2023 17:24, Roger Pau Monné wrote:
>> On Thu, Nov 16, 2023 at 02:31:05PM +0100, Jan Beulich wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>>> @@ -2163,6 +2163,23 @@ int __init vmx_vmcs_init(void)
>>>  
>>>      if ( !ret )
>>>          register_keyhandler('v', vmcs_dump, "dump VT-x VMCSs", 1);
>>> +    else
>>> +    {
>>> +        setup_clear_cpu_cap(X86_FEATURE_VMX);
>>> +
>>> +        /*
>>> +         * _vmx_vcpu_up() may have made it past feature identification.
>>> +         * Make sure all dependent features are off as well.
>>> +         */
>>> +        vmx_basic_msr              = 0;
>>> +        vmx_pin_based_exec_control = 0;
>>> +        vmx_cpu_based_exec_control = 0;
>>> +        vmx_secondary_exec_control = 0;
>>> +        vmx_vmexit_control         = 0;
>>> +        vmx_vmentry_control        = 0;
>>> +        vmx_ept_vpid_cap           = 0;
>>> +        vmx_vmfunc                 = 0;
>> Are there really any usages of those variables if VMX is disabled in
>> CPUID?
> I wanted to be on the safe side, as to me the question was "Are there really
> _no_ uses anywhere of those variables if VMX is disabled in CPUID?" And I
> couldn't easily convince myself of this being the case, seeing how all of
> vmcs.h's cpu_has_* are defined (and I'm pretty sure we have uses outside of
> arch/x86/hvm/vmx/).

Before you commit, are you sure that VT-d will continue to be happy
using IOMMU superpages when the EPT features are cleared like this?

That's the only linkage I'm aware of that might cause issues.

~Andrew