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

From: Jan Beulich <jbeulich@suse.com>
To: "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>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 2/5] x86/HVM: hide SVM/VMX when their enabling is prohibited by firmware
Date: Wed, 22 Nov 2023 08:52:41 +0100	[thread overview]
Message-ID: <33f43b2b-985b-4417-ba27-b7691deaa8b8@suse.com> (raw)
In-Reply-To: <b75a4d31-260c-4377-b68c-c8153dfd948e@citrix.com>

On 21.11.2023 18:31, Andrew Cooper wrote:
> 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?

There aren't going to be HVM guests in the case the clearing above happens.
And the only thing that happens when vtd_ept_page_compatible() returns
false is that page table sharing is suppressed, which is relevant only for
HVM guests.

Jan