LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] KVM: VMX: untangle VMXON revision_id setting when using eVMCS
Date: Fri, 6 Mar 2020 15:07:47 -0800	[thread overview]
Message-ID: <20200306230747.GA27868@linux.intel.com> (raw)
In-Reply-To: <908345f1-9bfd-004f-3ba6-0d6dce67d11e@oracle.com>

On Fri, Mar 06, 2020 at 02:20:13PM -0800, Krish Sadhukhan wrote:
> >@@ -2599,7 +2607,7 @@ void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs)
> >  int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs)
> >  {
> >-	loaded_vmcs->vmcs = alloc_vmcs(false);
> >+	loaded_vmcs->vmcs = alloc_vmcs(VMCS_REGION);
> >  	if (!loaded_vmcs->vmcs)
> >  		return -ENOMEM;
> >@@ -2652,25 +2660,13 @@ static __init int alloc_vmxon_regions(void)
> >  	for_each_possible_cpu(cpu) {
> >  		struct vmcs *vmcs;
> >-		vmcs = alloc_vmcs_cpu(false, cpu, GFP_KERNEL);
> >+		/* The VMXON region is really just a special type of VMCS. */
> 
> 
> Not sure if this is the right way to correlate the two.
> 
> AFAIU, the SDM calls VMXON region as a memory area that holds the VMCS data
> structure and it calls VMCS the data structure that is used by software to
> switch between VMX root-mode and not-root-mode. So VMXON is a memory area
> whereas VMCS is the structure of the data that resides in that memory area.
> 
> So if we follow this interpretation, your enum should rather look like,
> 
> enum vmcs_type {
> +    VMCS,
> +    EVMCS,
> +    SHADOW_VMCS

No (to the EVMCS suggestion), because this allocation needs to happen for
!eVMCS.  The SDM never explictly calls the VMXON region a VMCS, but it's
just being coy.  E.g. VMCLEAR doesn't fail if you point it at random
memory, but point it at the VMXON region and it yells.

We could call it VMXON_VMCS if that helps.  The SDM does call the memory
allocation for regular VMCSes a "VMCS region":

  A logical processor associates a region in memory with each VMCS. This
  region is called the VMCS region.

I don't think I've ever heard anyone differentiate that two though, i.e.
VMCS is used colloquially to mean both the data structure itself and the
memory region containing the data structure.

> >+		vmcs = alloc_vmcs_cpu(VMXON_REGION, cpu, GFP_KERNEL);
> >  		if (!vmcs) {
> >  			free_vmxon_regions();
> >  			return -ENOMEM;
> >  		}
> >-		/*
> >-		 * When eVMCS is enabled, alloc_vmcs_cpu() sets
> >-		 * vmcs->revision_id to KVM_EVMCS_VERSION instead of
> >-		 * revision_id reported by MSR_IA32_VMX_BASIC.
> >-		 *
> >-		 * However, even though not explicitly documented by
> >-		 * TLFS, VMXArea passed as VMXON argument should
> >-		 * still be marked with revision_id reported by
> >-		 * physical CPU.
> >-		 */
> >-		if (static_branch_unlikely(&enable_evmcs))
> >-			vmcs->hdr.revision_id = vmcs_config.revision_id;
> >-
> >  		per_cpu(vmxarea, cpu) = vmcs;
> >  	}
> >  	return 0;
> >diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> >index e64da06c7009..a5eb92638ac2 100644
> >--- a/arch/x86/kvm/vmx/vmx.h
> >+++ b/arch/x86/kvm/vmx/vmx.h
> >@@ -489,16 +489,22 @@ static inline struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu)
> >  	return &(to_vmx(vcpu)->pi_desc);
> >  }
> >-struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags);
> >+enum vmcs_type {
> >+	VMXON_REGION,
> >+	VMCS_REGION,
> >+	SHADOW_VMCS_REGION,
> >+};
> >+
> >+struct vmcs *alloc_vmcs_cpu(enum vmcs_type type, int cpu, gfp_t flags);
> >  void free_vmcs(struct vmcs *vmcs);
> >  int alloc_loaded_vmcs(struct loaded_vmcs *loaded_vmcs);
> >  void free_loaded_vmcs(struct loaded_vmcs *loaded_vmcs);
> >  void loaded_vmcs_init(struct loaded_vmcs *loaded_vmcs);
> >  void loaded_vmcs_clear(struct loaded_vmcs *loaded_vmcs);
> >-static inline struct vmcs *alloc_vmcs(bool shadow)
> >+static inline struct vmcs *alloc_vmcs(enum vmcs_type type)
> >  {
> >-	return alloc_vmcs_cpu(shadow, raw_smp_processor_id(),
> >+	return alloc_vmcs_cpu(type, raw_smp_processor_id(),
> >  			      GFP_KERNEL_ACCOUNT);
> >  }

  reply	other threads:[~2020-03-06 23:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 13:02 [PATCH v3 0/2] KVM: VMX: cleanup VMXON region allocation Vitaly Kuznetsov
2020-03-06 13:02 ` [PATCH v3 1/2] KVM: VMX: rename 'kvm_area' to 'vmxon_region' Vitaly Kuznetsov
2020-03-06 22:05   ` Krish Sadhukhan
2020-03-06 13:02 ` [PATCH v3 2/2] KVM: VMX: untangle VMXON revision_id setting when using eVMCS Vitaly Kuznetsov
2020-03-06 22:20   ` Krish Sadhukhan
2020-03-06 23:07     ` Sean Christopherson [this message]
2020-03-06 23:57       ` Krish Sadhukhan
2020-03-07  0:28         ` Sean Christopherson
2020-03-07  1:34           ` Krish Sadhukhan
2020-03-09  9:31           ` Vitaly Kuznetsov
2020-03-18 17:17             ` Vitaly Kuznetsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200306230747.GA27868@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jmattson@google.com \
    --cc=krish.sadhukhan@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --subject='Re: [PATCH v3 2/2] KVM: VMX: untangle VMXON revision_id setting when using eVMCS' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).