From: Sean Christopherson <seanjc@google.com>
To: Zeng Guang <guang.zeng@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
kvm@vger.kernel.org, Dave Hansen <dave.hansen@linux.intel.com>,
Tony Luck <tony.luck@intel.com>,
Kan Liang <kan.liang@linux.intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
Kim Phillips <kim.phillips@amd.com>,
Jarkko Sakkinen <jarkko@kernel.org>,
Jethro Beekman <jethro@fortanix.com>,
Kai Huang <kai.huang@intel.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
Robert Hu <robert.hu@intel.com>, Gao Chao <chao.gao@intel.com>
Subject: Re: [PATCH v5 6/8] KVM: VMX: enable IPI virtualization
Date: Thu, 13 Jan 2022 21:47:53 +0000 [thread overview]
Message-ID: <YeCeCYY2UTL/T1Tv@google.com> (raw)
In-Reply-To: <20211231142849.611-7-guang.zeng@intel.com>
On Fri, Dec 31, 2021, Zeng Guang wrote:
> +/* Tertiary Processor-Based VM-Execution Controls, word 3 */
> +#define VMX_FEATURE_IPI_VIRT (3*32 + 4) /* "" Enable IPI virtualization */
> #endif /* _ASM_X86_VMXFEATURES_H */
> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> index 38d414f64e61..78b0525dd991 100644
> --- a/arch/x86/kvm/vmx/capabilities.h
> +++ b/arch/x86/kvm/vmx/capabilities.h
> @@ -12,6 +12,7 @@ extern bool __read_mostly enable_ept;
> extern bool __read_mostly enable_unrestricted_guest;
> extern bool __read_mostly enable_ept_ad_bits;
> extern bool __read_mostly enable_pml;
> +extern bool __read_mostly enable_ipiv;
> extern int __read_mostly pt_mode;
>
> #define PT_MODE_SYSTEM 0
> @@ -283,6 +284,12 @@ static inline bool cpu_has_vmx_apicv(void)
> cpu_has_vmx_posted_intr();
> }
>
> +static inline bool cpu_has_vmx_ipiv(void)
> +{
> + return vmcs_config.cpu_based_3rd_exec_ctrl &
> + TERTIARY_EXEC_IPI_VIRT;
Unnecessary newline, that fits on a single line.
> +}
> +
> static inline bool cpu_has_vmx_flexpriority(void)
> {
> return cpu_has_vmx_tpr_shadow() &&
> diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c
> index 1c94783b5a54..bd9c9a89726a 100644
> --- a/arch/x86/kvm/vmx/posted_intr.c
> +++ b/arch/x86/kvm/vmx/posted_intr.c
> @@ -85,11 +85,16 @@ static bool vmx_can_use_vtd_pi(struct kvm *kvm)
> irq_remapping_cap(IRQ_POSTING_CAP);
> }
>
> +static bool vmx_can_use_ipiv_pi(struct kvm *kvm)
> +{
> + return irqchip_in_kernel(kvm) && enable_apicv && enable_ipiv;
enable_ipiv should be cleared if !enable_apicv, i.e. the enable_apicv check
here should be unnecessary.
> +}
> +
> void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu)
> {
> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu);
>
> - if (!vmx_can_use_vtd_pi(vcpu->kvm))
> + if (!(vmx_can_use_ipiv_pi(vcpu->kvm) || vmx_can_use_vtd_pi(vcpu->kvm)))
Purely because I am beyond terrible at reading !(A || B) and !(A && B), can we
write this as:
if (!vmx_can_use_ipiv_pi(vcpu->kvm) && !vmx_can_use_vtd_pi(vcpu->kvm))
return;
Or better, add a helper. We could even drop vmx_can_use_ipiv_pi() altogether, e.g.
static bool vmx_can_use_posted_interrupts(struct kvm *kvm)
{
return irqchip_in_kernel(kvm) &&
(enable_ipiv || vmx_can_use_vtd_pi(kvm));
}
Or with both helpers:
static bool vmx_can_use_posted_interrupts(struct kvm *kvm)
{
return vmx_can_use_ipiv_pi(kvm) || vmx_can_use_vtd_pi(kvm);
}
I don't think I have a strong preference over whether or not to drop
vmx_can_use_ipiv_pi(). I think it's marginally easier to read with the extra
helper?
> return;
>
> /* Set SN when the vCPU is preempted */
> @@ -147,7 +152,7 @@ int pi_pre_block(struct kvm_vcpu *vcpu)
> struct pi_desc old, new;
> struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu);
>
> - if (!vmx_can_use_vtd_pi(vcpu->kvm))
> + if (!(vmx_can_use_ipiv_pi(vcpu->kvm) || vmx_can_use_vtd_pi(vcpu->kvm)))
> return 0;
>
> WARN_ON(irqs_disabled());
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 5716db9704c0..2e65464d6dee 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -104,6 +104,9 @@ module_param(fasteoi, bool, S_IRUGO);
>
> module_param(enable_apicv, bool, S_IRUGO);
>
> +bool __read_mostly enable_ipiv = true;
> +module_param(enable_ipiv, bool, 0444);
> +
> /*
> * If nested=1, nested virtualization is supported, i.e., guests may use
> * VMX and be a hypervisor for its own guests. If nested=0, guests may not
> @@ -224,6 +227,11 @@ static const struct {
> };
>
> #define L1D_CACHE_ORDER 4
> +
> +/* PID(Posted-Interrupt Descriptor)-pointer table entry is 64-bit long */
> +#define MAX_PID_TABLE_ORDER get_order(KVM_MAX_VCPU_IDS * sizeof(u64))
> +#define PID_TABLE_ENTRY_VALID 1
> +
> static void *vmx_l1d_flush_pages;
>
> static int vmx_setup_l1d_flush(enum vmx_l1d_flush_state l1tf)
> @@ -2504,7 +2512,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> }
>
> if (_cpu_based_exec_control & CPU_BASED_ACTIVATE_TERTIARY_CONTROLS) {
> - u64 opt3 = 0;
> + u64 opt3 = TERTIARY_EXEC_IPI_VIRT;
> u64 min3 = 0;
>
> if (adjust_vmx_controls_64(min3, opt3,
> @@ -3841,6 +3849,8 @@ static void vmx_update_msr_bitmap_x2apic(struct kvm_vcpu *vcpu)
> vmx_enable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_TMCCT), MSR_TYPE_RW);
> vmx_disable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_EOI), MSR_TYPE_W);
> vmx_disable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_SELF_IPI), MSR_TYPE_W);
> + vmx_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_ICR),
> + MSR_TYPE_RW, !enable_ipiv);
Please align this, e.g.
vmx_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_ICR),
MSR_TYPE_RW, !enable_ipiv);
though I think I'd actually prefer we do:
if (enable_ipiv)
vmx_disable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_ICR), MSR_TYPE_RW);
and just let it poke out. That makes it much more obvious that interception is
disabled when IPI virtualization is enabled. Using vmx_set_intercept_for_msr()
implies that it could go either way, but that's not true as vmx_reset_x2apic_msrs()
sets the bitmap to intercept all x2APIC MSRs.
> }
> }
>
next prev parent reply other threads:[~2022-01-13 21:48 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-31 14:28 [PATCH v5 0/8] IPI virtualization support for VM Zeng Guang
2021-12-31 14:28 ` [PATCH v5 1/8] x86/cpu: Add new VMX feature, Tertiary VM-Execution control Zeng Guang
2021-12-31 14:28 ` [PATCH v5 2/8] KVM: VMX: Extend BUILD_CONTROLS_SHADOW macro to support 64-bit variation Zeng Guang
2021-12-31 14:28 ` [PATCH v5 3/8] KVM: VMX: Detect Tertiary VM-Execution control when setup VMCS config Zeng Guang
2021-12-31 14:28 ` [PATCH v5 4/8] KVM: VMX: dump_vmcs() reports tertiary_exec_control field as well Zeng Guang
2022-01-13 21:03 ` Sean Christopherson
2022-01-14 4:19 ` Zeng Guang
2022-01-20 1:06 ` Sean Christopherson
2022-01-20 5:34 ` Zeng Guang
2021-12-31 14:28 ` [PATCH v5 5/8] KVM: x86: Support interrupt dispatch in x2APIC mode with APIC-write VM exit Zeng Guang
2022-01-13 21:29 ` Sean Christopherson
2022-01-14 7:52 ` Zeng Guang
2022-01-14 17:34 ` Sean Christopherson
2022-01-15 2:08 ` Zeng Guang
2022-01-18 0:44 ` Yuan Yao
2022-01-18 3:06 ` Zeng Guang
2022-01-18 18:17 ` Sean Christopherson
2022-01-19 2:48 ` Zeng Guang
2021-12-31 14:28 ` [PATCH v5 6/8] KVM: VMX: enable IPI virtualization Zeng Guang
2022-01-13 21:47 ` Sean Christopherson [this message]
2022-01-14 5:36 ` Zeng Guang
2021-12-31 14:28 ` [PATCH v5 7/8] KVM: VMX: Update PID-pointer table entry when APIC ID is changed Zeng Guang
2022-01-05 19:13 ` Tom Lendacky
2022-01-06 1:44 ` Zeng Guang
2022-01-06 14:06 ` Tom Lendacky
2022-01-07 8:05 ` Zeng Guang
2022-01-07 8:31 ` Maxim Levitsky
2022-01-10 7:45 ` Chao Gao
2022-01-10 22:24 ` Maxim Levitsky
2022-01-13 22:19 ` Sean Christopherson
2022-01-14 2:58 ` Chao Gao
2022-01-14 8:17 ` Maxim Levitsky
2022-01-17 3:17 ` Chao Gao
2022-02-02 23:23 ` Sean Christopherson
2022-02-03 20:22 ` Sean Christopherson
2022-02-23 6:10 ` Chao Gao
2022-02-23 10:26 ` Maxim Levitsky
2022-01-14 0:22 ` Yuan Yao
2021-12-31 14:28 ` [PATCH v5 8/8] KVM: VMX: Resize PID-ponter table on demand for IPI virtualization Zeng Guang
2022-01-13 22:09 ` Sean Christopherson
2022-01-14 15:59 ` Zeng Guang
2022-01-14 16:18 ` Sean Christopherson
2022-01-17 15:04 ` Zeng Guang
2022-01-18 17:15 ` Sean Christopherson
2022-01-19 7:55 ` Zeng Guang
2022-01-20 1:01 ` Sean Christopherson
2022-01-24 16:40 ` Zeng Guang
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=YeCeCYY2UTL/T1Tv@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=chao.gao@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=guang.zeng@intel.com \
--cc=hpa@zytor.com \
--cc=jarkko@kernel.org \
--cc=jethro@fortanix.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kai.huang@intel.com \
--cc=kan.liang@linux.intel.com \
--cc=kim.phillips@amd.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=robert.hu@intel.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/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).