From: Paolo Bonzini <pbonzini@redhat.com>
To: Tiejun Chen <tiejun.chen@intel.com>
Cc: kvm@vger.kernel.org
Subject: Re: [RFC][PATCH 2/2] kvm: x86: vmx: move some vmx setting from vmx_init() to hardware_setup()
Date: Tue, 04 Nov 2014 18:13:59 +0100 [thread overview]
Message-ID: <54590957.2040200@redhat.com> (raw)
In-Reply-To: <1414462488-5873-2-git-send-email-tiejun.chen@intel.com>
On 28/10/2014 03:14, Tiejun Chen wrote:
> Instead of vmx_init(), actually it would make reasonable sense to do
> anything specific to vmx hardware setting in vmx_x86_ops->hardware_setup().
>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
Applied, thanks. This makes vmx_init and svm_init more similar, which
is nice.
Paolo
> ---
> arch/x86/kvm/vmx.c | 267 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 133 insertions(+), 134 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 799165e..b25a588 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5717,8 +5717,108 @@ static void update_ple_window_actual_max(void)
>
> static __init int hardware_setup(void)
> {
> - if (setup_vmcs_config(&vmcs_config) < 0)
> - return -EIO;
> + int r = -ENOMEM, i, msr;
> +
> + rdmsrl_safe(MSR_EFER, &host_efer);
> +
> + for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i)
> + kvm_define_shared_msr(i, vmx_msr_index[i]);
> +
> + vmx_io_bitmap_a = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_io_bitmap_a)
> + return r;
> +
> + vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_io_bitmap_b)
> + goto out;
> +
> + vmx_msr_bitmap_legacy = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_msr_bitmap_legacy)
> + goto out1;
> +
> + vmx_msr_bitmap_legacy_x2apic =
> + (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_msr_bitmap_legacy_x2apic)
> + goto out2;
> +
> + vmx_msr_bitmap_longmode = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_msr_bitmap_longmode)
> + goto out3;
> +
> + vmx_msr_bitmap_longmode_x2apic =
> + (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_msr_bitmap_longmode_x2apic)
> + goto out4;
> + vmx_vmread_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_vmread_bitmap)
> + goto out5;
> +
> + vmx_vmwrite_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> + if (!vmx_vmwrite_bitmap)
> + goto out6;
> +
> + memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
> + memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
> +
> + /*
> + * Allow direct access to the PC debug port (it is often used for I/O
> + * delays, but the vmexits simply slow things down).
> + */
> + memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
> + clear_bit(0x80, vmx_io_bitmap_a);
> +
> + memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
> +
> + memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
> + memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
> +
> + vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> + vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> + vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> + vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> + vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> + vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> + vmx_disable_intercept_for_msr(MSR_IA32_BNDCFGS, true);
> +
> + memcpy(vmx_msr_bitmap_legacy_x2apic,
> + vmx_msr_bitmap_legacy, PAGE_SIZE);
> + memcpy(vmx_msr_bitmap_longmode_x2apic,
> + vmx_msr_bitmap_longmode, PAGE_SIZE);
> +
> + if (enable_apicv) {
> + for (msr = 0x800; msr <= 0x8ff; msr++)
> + vmx_disable_intercept_msr_read_x2apic(msr);
> +
> + /* According SDM, in x2apic mode, the whole id reg is used.
> + * But in KVM, it only use the highest eight bits. Need to
> + * intercept it */
> + vmx_enable_intercept_msr_read_x2apic(0x802);
> + /* TMCCT */
> + vmx_enable_intercept_msr_read_x2apic(0x839);
> + /* TPR */
> + vmx_disable_intercept_msr_write_x2apic(0x808);
> + /* EOI */
> + vmx_disable_intercept_msr_write_x2apic(0x80b);
> + /* SELF-IPI */
> + vmx_disable_intercept_msr_write_x2apic(0x83f);
> + }
> +
> + if (enable_ept) {
> + kvm_mmu_set_mask_ptes(0ull,
> + (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull,
> + (enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull,
> + 0ull, VMX_EPT_EXECUTABLE_MASK);
> + ept_set_mmio_spte_mask();
> + kvm_enable_tdp();
> + } else
> + kvm_disable_tdp();
> +
> + update_ple_window_actual_max();
> +
> + if (setup_vmcs_config(&vmcs_config) < 0) {
> + r = -EIO;
> + goto out7;
> + }
>
> if (boot_cpu_has(X86_FEATURE_NX))
> kvm_enable_efer_bits(EFER_NX);
> @@ -5778,10 +5878,38 @@ static __init int hardware_setup(void)
> nested_vmx_setup_ctls_msrs();
>
> return alloc_kvm_area();
> +
> +out7:
> + free_page((unsigned long)vmx_vmwrite_bitmap);
> +out6:
> + free_page((unsigned long)vmx_vmread_bitmap);
> +out5:
> + free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic);
> +out4:
> + free_page((unsigned long)vmx_msr_bitmap_longmode);
> +out3:
> + free_page((unsigned long)vmx_msr_bitmap_legacy_x2apic);
> +out2:
> + free_page((unsigned long)vmx_msr_bitmap_legacy);
> +out1:
> + free_page((unsigned long)vmx_io_bitmap_b);
> +out:
> + free_page((unsigned long)vmx_io_bitmap_a);
> +
> + return r;
> }
>
> static __exit void hardware_unsetup(void)
> {
> + free_page((unsigned long)vmx_msr_bitmap_legacy_x2apic);
> + free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic);
> + free_page((unsigned long)vmx_msr_bitmap_legacy);
> + free_page((unsigned long)vmx_msr_bitmap_longmode);
> + free_page((unsigned long)vmx_io_bitmap_b);
> + free_page((unsigned long)vmx_io_bitmap_a);
> + free_page((unsigned long)vmx_vmwrite_bitmap);
> + free_page((unsigned long)vmx_vmread_bitmap);
> +
> free_kvm_area();
> }
>
> @@ -9180,150 +9308,21 @@ static struct kvm_x86_ops vmx_x86_ops = {
>
> static int __init vmx_init(void)
> {
> - int r, i, msr;
> -
> - rdmsrl_safe(MSR_EFER, &host_efer);
> -
> - for (i = 0; i < ARRAY_SIZE(vmx_msr_index); ++i)
> - kvm_define_shared_msr(i, vmx_msr_index[i]);
> -
> - vmx_io_bitmap_a = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_io_bitmap_a)
> - return -ENOMEM;
> -
> - r = -ENOMEM;
> -
> - vmx_io_bitmap_b = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_io_bitmap_b)
> - goto out;
> -
> - vmx_msr_bitmap_legacy = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_msr_bitmap_legacy)
> - goto out1;
> -
> - vmx_msr_bitmap_legacy_x2apic =
> - (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_msr_bitmap_legacy_x2apic)
> - goto out2;
> -
> - vmx_msr_bitmap_longmode = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_msr_bitmap_longmode)
> - goto out3;
> -
> - vmx_msr_bitmap_longmode_x2apic =
> - (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_msr_bitmap_longmode_x2apic)
> - goto out4;
> - vmx_vmread_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_vmread_bitmap)
> - goto out5;
> -
> - vmx_vmwrite_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> - if (!vmx_vmwrite_bitmap)
> - goto out6;
> -
> - memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
> - memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
> -
> - /*
> - * Allow direct access to the PC debug port (it is often used for I/O
> - * delays, but the vmexits simply slow things down).
> - */
> - memset(vmx_io_bitmap_a, 0xff, PAGE_SIZE);
> - clear_bit(0x80, vmx_io_bitmap_a);
> -
> - memset(vmx_io_bitmap_b, 0xff, PAGE_SIZE);
> -
> - memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
> - memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
> -
> - set_bit(0, vmx_vpid_bitmap); /* 0 is reserved for host */
> -
> - r = kvm_init(&vmx_x86_ops, sizeof(struct vcpu_vmx),
> - __alignof__(struct vcpu_vmx), THIS_MODULE);
> + int r = kvm_init(&vmx_x86_ops, sizeof(struct vcpu_vmx),
> + __alignof__(struct vcpu_vmx), THIS_MODULE);
> if (r)
> - goto out7;
> + return r;
>
> #ifdef CONFIG_KEXEC
> rcu_assign_pointer(crash_vmclear_loaded_vmcss,
> crash_vmclear_local_loaded_vmcss);
> #endif
>
> - vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> - vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> - vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> - vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> - vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> - vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> - vmx_disable_intercept_for_msr(MSR_IA32_BNDCFGS, true);
> -
> - memcpy(vmx_msr_bitmap_legacy_x2apic,
> - vmx_msr_bitmap_legacy, PAGE_SIZE);
> - memcpy(vmx_msr_bitmap_longmode_x2apic,
> - vmx_msr_bitmap_longmode, PAGE_SIZE);
> -
> - if (enable_apicv) {
> - for (msr = 0x800; msr <= 0x8ff; msr++)
> - vmx_disable_intercept_msr_read_x2apic(msr);
> -
> - /* According SDM, in x2apic mode, the whole id reg is used.
> - * But in KVM, it only use the highest eight bits. Need to
> - * intercept it */
> - vmx_enable_intercept_msr_read_x2apic(0x802);
> - /* TMCCT */
> - vmx_enable_intercept_msr_read_x2apic(0x839);
> - /* TPR */
> - vmx_disable_intercept_msr_write_x2apic(0x808);
> - /* EOI */
> - vmx_disable_intercept_msr_write_x2apic(0x80b);
> - /* SELF-IPI */
> - vmx_disable_intercept_msr_write_x2apic(0x83f);
> - }
> -
> - if (enable_ept) {
> - kvm_mmu_set_mask_ptes(0ull,
> - (enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull,
> - (enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull,
> - 0ull, VMX_EPT_EXECUTABLE_MASK);
> - ept_set_mmio_spte_mask();
> - kvm_enable_tdp();
> - } else
> - kvm_disable_tdp();
> -
> - update_ple_window_actual_max();
> -
> return 0;
> -
> -out7:
> - free_page((unsigned long)vmx_vmwrite_bitmap);
> -out6:
> - free_page((unsigned long)vmx_vmread_bitmap);
> -out5:
> - free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic);
> -out4:
> - free_page((unsigned long)vmx_msr_bitmap_longmode);
> -out3:
> - free_page((unsigned long)vmx_msr_bitmap_legacy_x2apic);
> -out2:
> - free_page((unsigned long)vmx_msr_bitmap_legacy);
> -out1:
> - free_page((unsigned long)vmx_io_bitmap_b);
> -out:
> - free_page((unsigned long)vmx_io_bitmap_a);
> - return r;
> }
>
> static void __exit vmx_exit(void)
> {
> - free_page((unsigned long)vmx_msr_bitmap_legacy_x2apic);
> - free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic);
> - free_page((unsigned long)vmx_msr_bitmap_legacy);
> - free_page((unsigned long)vmx_msr_bitmap_longmode);
> - free_page((unsigned long)vmx_io_bitmap_b);
> - free_page((unsigned long)vmx_io_bitmap_a);
> - free_page((unsigned long)vmx_vmwrite_bitmap);
> - free_page((unsigned long)vmx_vmread_bitmap);
> -
> #ifdef CONFIG_KEXEC
> RCU_INIT_POINTER(crash_vmclear_loaded_vmcss, NULL);
> synchronize_rcu();
>
prev parent reply other threads:[~2014-11-04 17:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-28 2:14 [RFC][PATCH 1/2] kvm: x86: vmx: move down hardware_setup() and hardware_unsetup() Tiejun Chen
2014-10-28 2:14 ` [RFC][PATCH 2/2] kvm: x86: vmx: move some vmx setting from vmx_init() to hardware_setup() Tiejun Chen
2014-11-04 17:13 ` Paolo Bonzini [this message]
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=54590957.2040200@redhat.com \
--to=pbonzini@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=tiejun.chen@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