From: Jim Mattson <jmattson@google.com>
To: Yang Weijiang <weijiang.yang@intel.com>
Cc: "kvm list" <kvm@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Sean Christopherson" <sean.j.christopherson@intel.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
"Radim Krčmář" <rkrcmar@redhat.com>,
yu.c.zhang@intel.com, alazar@bitdefender.com
Subject: Re: [PATCH v5 2/9] vmx: spp: Add control flags for Sub-Page Protection(SPP)
Date: Fri, 4 Oct 2019 13:48:34 -0700 [thread overview]
Message-ID: <CALMp9eSEkZiFq3RhTuJSUCx3WDJy4EfYHk7GDoN=MO9tRt4=hQ@mail.gmail.com> (raw)
In-Reply-To: <20190917085304.16987-3-weijiang.yang@intel.com>
On Tue, Sep 17, 2019 at 1:52 AM Yang Weijiang <weijiang.yang@intel.com> wrote:
>
> Check SPP capability in MSR_IA32_VMX_PROCBASED_CTLS2, its 23-bit
> indicates SPP capability. Enable SPP feature bit in CPU capabilities
> bitmap if it's supported.
>
> Co-developed-by: He Chen <he.chen@linux.intel.com>
> Signed-off-by: He Chen <he.chen@linux.intel.com>
> Co-developed-by: Zhang Yi <yi.z.zhang@linux.intel.com>
> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/vmx.h | 1 +
> arch/x86/kernel/cpu/intel.c | 4 ++++
> arch/x86/kvm/mmu.h | 2 ++
> arch/x86/kvm/vmx/capabilities.h | 5 +++++
> arch/x86/kvm/vmx/vmx.c | 10 ++++++++++
> 6 files changed, 23 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index e880f2408e29..ee2c76fdadf6 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -228,6 +228,7 @@
> #define X86_FEATURE_FLEXPRIORITY ( 8*32+ 2) /* Intel FlexPriority */
> #define X86_FEATURE_EPT ( 8*32+ 3) /* Intel Extended Page Table */
> #define X86_FEATURE_VPID ( 8*32+ 4) /* Intel Virtual Processor ID */
> +#define X86_FEATURE_SPP ( 8*32+ 5) /* Intel EPT-based Sub-Page Write Protection */
>
> #define X86_FEATURE_VMMCALL ( 8*32+15) /* Prefer VMMCALL to VMCALL */
> #define X86_FEATURE_XENPV ( 8*32+16) /* "" Xen paravirtual guest */
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index a39136b0d509..e1137807affc 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -68,6 +68,7 @@
> #define SECONDARY_EXEC_XSAVES 0x00100000
> #define SECONDARY_EXEC_PT_USE_GPA 0x01000000
> #define SECONDARY_EXEC_MODE_BASED_EPT_EXEC 0x00400000
> +#define SECONDARY_EXEC_ENABLE_SPP 0x00800000
> #define SECONDARY_EXEC_TSC_SCALING 0x02000000
>
> #define PIN_BASED_EXT_INTR_MASK 0x00000001
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 8d6d92ebeb54..27617e522f01 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -503,6 +503,7 @@ static void detect_vmx_virtcap(struct cpuinfo_x86 *c)
> #define X86_VMX_FEATURE_PROC_CTLS2_EPT 0x00000002
> #define X86_VMX_FEATURE_PROC_CTLS2_VPID 0x00000020
> #define x86_VMX_FEATURE_EPT_CAP_AD 0x00200000
> +#define X86_VMX_FEATURE_PROC_CTLS2_SPP 0x00800000
>
> u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
> u32 msr_vpid_cap, msr_ept_cap;
> @@ -513,6 +514,7 @@ static void detect_vmx_virtcap(struct cpuinfo_x86 *c)
> clear_cpu_cap(c, X86_FEATURE_EPT);
> clear_cpu_cap(c, X86_FEATURE_VPID);
> clear_cpu_cap(c, X86_FEATURE_EPT_AD);
> + clear_cpu_cap(c, X86_FEATURE_SPP);
>
> rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
> msr_ctl = vmx_msr_high | vmx_msr_low;
> @@ -536,6 +538,8 @@ static void detect_vmx_virtcap(struct cpuinfo_x86 *c)
> }
> if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
> set_cpu_cap(c, X86_FEATURE_VPID);
> + if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_SPP)
> + set_cpu_cap(c, X86_FEATURE_SPP);
SPP requires EPT, so this could be moved up next to the EPT_AD check.
In fact, I would suggest changing 'SPP' to 'EPT_SPP' to make it clear
that this feature is *EPT* sub-page permissions.
> }
> }
>
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 54c2a377795b..3c1423526a98 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -26,6 +26,8 @@
> #define PT_PAGE_SIZE_MASK (1ULL << PT_PAGE_SIZE_SHIFT)
> #define PT_PAT_MASK (1ULL << 7)
> #define PT_GLOBAL_MASK (1ULL << 8)
> +#define PT_SPP_SHIFT 61
> +#define PT_SPP_MASK (1ULL << PT_SPP_SHIFT)
Since these constants are only applicable to EPT, would it be more
appropriate to define them in paging_tmpl.h, under '#elif PTTYPE ==
PTTYPE_EPT'? If not, it seems that they should at least be renamed to
PT64_SPP_* for consistency with the other macros here.
> #define PT64_NX_SHIFT 63
> #define PT64_NX_MASK (1ULL << PT64_NX_SHIFT)
>
> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> index d6664ee3d127..e3bde7a32123 100644
> --- a/arch/x86/kvm/vmx/capabilities.h
> +++ b/arch/x86/kvm/vmx/capabilities.h
> @@ -241,6 +241,11 @@ static inline bool cpu_has_vmx_pml(void)
> return vmcs_config.cpu_based_2nd_exec_ctrl & SECONDARY_EXEC_ENABLE_PML;
> }
>
> +static inline bool cpu_has_vmx_ept_spp(void)
> +{
> + return vmcs_config.cpu_based_2nd_exec_ctrl & SECONDARY_EXEC_ENABLE_SPP;
> +}
> +
> static inline bool vmx_xsaves_supported(void)
> {
> return vmcs_config.cpu_based_2nd_exec_ctrl &
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index c030c96fc81a..8ecf9cb24879 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -60,6 +60,7 @@
> #include "vmcs12.h"
> #include "vmx.h"
> #include "x86.h"
> +#include "spp.h"
>
> MODULE_AUTHOR("Qumranet");
> MODULE_LICENSE("GPL");
> @@ -113,6 +114,7 @@ module_param_named(pml, enable_pml, bool, S_IRUGO);
>
> static bool __read_mostly dump_invalid_vmcs = 0;
> module_param(dump_invalid_vmcs, bool, 0644);
> +static bool __read_mostly spp_supported = 0;
>
> #define MSR_BITMAP_MODE_X2APIC 1
> #define MSR_BITMAP_MODE_X2APIC_APICV 2
> @@ -2279,6 +2281,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> SECONDARY_EXEC_RDSEED_EXITING |
> SECONDARY_EXEC_RDRAND_EXITING |
> SECONDARY_EXEC_ENABLE_PML |
> + SECONDARY_EXEC_ENABLE_SPP |
> SECONDARY_EXEC_TSC_SCALING |
> SECONDARY_EXEC_PT_USE_GPA |
> SECONDARY_EXEC_PT_CONCEAL_VMX |
> @@ -3931,6 +3934,9 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
> if (!enable_pml)
> exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>
> + if (!spp_supported)
> + exec_control &= ~SECONDARY_EXEC_ENABLE_SPP;
> +
> if (vmx_xsaves_supported()) {
> /* Exposing XSAVES only when XSAVE is exposed */
> bool xsaves_enabled =
> @@ -7521,6 +7527,10 @@ static __init int hardware_setup(void)
> if (!cpu_has_vmx_flexpriority())
> flexpriority_enabled = 0;
>
> + if (cpu_has_vmx_ept_spp() && enable_ept &&
> + boot_cpu_has(X86_FEATURE_SPP))
> + spp_supported = 1;
Don't cpu_has_vmx_ept_spp() and boot_cpu_has(X86_FEATURE_SPP) test
exactly the same thing?
> if (!cpu_has_virtual_nmis())
> enable_vnmi = 0;
>
> --
> 2.17.2
>
next prev parent reply other threads:[~2019-10-04 20:48 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-17 8:52 [PATCH v5 0/9] Enable Sub-page Write Protection Support Yang Weijiang
2019-09-17 8:52 ` [PATCH v5 1/9] Documentation: Introduce EPT based Subpage Protection Yang Weijiang
2019-10-11 20:31 ` Jim Mattson
2019-10-15 8:53 ` Yang Weijiang
2019-09-17 8:52 ` [PATCH v5 2/9] vmx: spp: Add control flags for Sub-Page Protection(SPP) Yang Weijiang
2019-10-04 20:48 ` Jim Mattson [this message]
2019-10-04 21:02 ` Sean Christopherson
2019-10-15 1:53 ` Yang Weijiang
2019-09-17 8:52 ` [PATCH v5 3/9] mmu: spp: Add SPP Table setup functions Yang Weijiang
2019-09-17 8:52 ` [PATCH v5 4/9] mmu: spp: Add functions to create/destroy SPP bitmap block Yang Weijiang
2019-09-17 8:53 ` [PATCH v5 5/9] mmu: spp: Introduce SPP {init,set,get} functions Yang Weijiang
2019-09-17 8:53 ` [PATCH v5 6/9] x86: spp: Introduce user-space SPP IOCTLs Yang Weijiang
2019-09-17 8:53 ` [PATCH v5 7/9] vmx: spp: Set up SPP paging table at vm-entry/exit Yang Weijiang
2019-09-17 10:56 ` kbuild test robot
2019-09-17 8:53 ` [PATCH v5 8/9] mmu: spp: Enable Lazy mode SPP protection Yang Weijiang
2019-09-17 8:53 ` [PATCH v5 9/9] mmu: spp: Handle SPP protected pages when VM memory changes Yang Weijiang
2019-09-17 12:59 ` [PATCH v5 0/9] Enable Sub-page Write Protection Support Konrad Rzeszutek Wilk
2019-09-17 16:24 ` Adalbert Lazăr
2019-10-09 2:17 ` Yang Weijiang
2019-10-10 21:42 ` Jim Mattson
2019-10-11 7:50 ` Yang Weijiang
2019-10-11 16:11 ` Jim Mattson
2019-10-22 6:19 ` Yang Weijiang
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='CALMp9eSEkZiFq3RhTuJSUCx3WDJy4EfYHk7GDoN=MO9tRt4=hQ@mail.gmail.com' \
--to=jmattson@google.com \
--cc=alazar@bitdefender.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=sean.j.christopherson@intel.com \
--cc=weijiang.yang@intel.com \
--cc=yu.c.zhang@intel.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).