| Message ID | 1479434244-10223-4-git-send-email-feng.wu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> From: Wu, Feng > Sent: Friday, November 18, 2016 9:57 AM > > We may hit the last ASSERT() in vmx_vcpu_block in the current code, > since vmx_vcpu_block() may get called before vmx_pi_switch_to() > has been installed or executed. Here We use cmpxchg to update > the NDST field, this can make sure we only update the NDST when > vmx_pi_switch_to() has not been called. So the NDST is in a > proper state in vmx_vcpu_block(). > > Suggested-by: Jan Beulich <JBeulich@suse.com> > Signed-off-by: Feng Wu <feng.wu@intel.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Acked-by: Kevin Tian <kevin.tian@intel.com>
> -----Original Message----- > From: Tian, Kevin > Sent: Friday, November 18, 2016 12:11 PM > To: Wu, Feng <feng.wu@intel.com>; xen-devel@lists.xen.org > Cc: jbeulich@suse.com; andrew.cooper3@citrix.com; > george.dunlap@eu.citrix.com; dario.faggioli@citrix.com > Subject: RE: [PATCH v8 3/7] VMX: Make sure PI is in proper state before install > the hooks > > > From: Wu, Feng > > Sent: Friday, November 18, 2016 9:57 AM > > > > We may hit the last ASSERT() in vmx_vcpu_block in the current code, > > since vmx_vcpu_block() may get called before vmx_pi_switch_to() > > has been installed or executed. Here We use cmpxchg to update > > the NDST field, this can make sure we only update the NDST when > > vmx_pi_switch_to() has not been called. So the NDST is in a > > proper state in vmx_vcpu_block(). > > > > Suggested-by: Jan Beulich <JBeulich@suse.com> > > Signed-off-by: Feng Wu <feng.wu@intel.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Kevin Tian <kevin.tian@intel.com> Thanks for the Ack! Thanks, Feng
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 1bd875a..e8e3616 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -956,16 +956,13 @@ void virtual_vmcs_vmwrite(const struct vcpu *v, u32 vmcs_encoding, u64 val) */ static void pi_desc_init(struct vcpu *v) { - uint32_t dest; - v->arch.hvm_vmx.pi_desc.nv = posted_intr_vector; - dest = cpu_physical_id(v->processor); - - if ( x2apic_enabled ) - v->arch.hvm_vmx.pi_desc.ndst = dest; - else - v->arch.hvm_vmx.pi_desc.ndst = MASK_INSR(dest, PI_xAPIC_NDST_MASK); + /* + * Mark NDST as invalid, then we can use this invalid value as a + * marker to whether update NDST or not in vmx_pi_hooks_assign(). + */ + v->arch.hvm_vmx.pi_desc.ndst = APIC_INVALID_DEST; } static int construct_vmcs(struct vcpu *v) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index a8dcabe..a1f7903 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -206,14 +206,39 @@ static void vmx_pi_do_resume(struct vcpu *v) /* This function is called when pcidevs_lock is held */ void vmx_pi_hooks_assign(struct domain *d) { + struct vcpu *v; + if ( !iommu_intpost || !has_hvm_container_domain(d) ) return; ASSERT(!d->arch.hvm_domain.vmx.vcpu_block); - d->arch.hvm_domain.vmx.vcpu_block = vmx_vcpu_block; + /* + * We carefully handle the timing here: + * - Install the context switch first + * - Then set the NDST field + * - Install the block and resume hooks in the end + * + * This can make sure the PI (especially the NDST feild) is + * in proper state when we call vmx_vcpu_block(). + */ d->arch.hvm_domain.vmx.pi_switch_from = vmx_pi_switch_from; d->arch.hvm_domain.vmx.pi_switch_to = vmx_pi_switch_to; + + for_each_vcpu ( d, v ) + { + unsigned int dest = cpu_physical_id(v->processor); + struct pi_desc *pi_desc = &v->arch.hvm_vmx.pi_desc; + + /* + * We don't need to update NDST if vmx_pi_switch_to() + * has already got called. + */ + (void)cmpxchg(&pi_desc->ndst, APIC_INVALID_DEST, + x2apic_enabled ? dest : MASK_INSR(dest, PI_xAPIC_NDST_MASK)); + } + + d->arch.hvm_domain.vmx.vcpu_block = vmx_vcpu_block; d->arch.hvm_domain.vmx.pi_do_resume = vmx_pi_do_resume; } diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index 4cdd9b1..2f0435c 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -573,6 +573,8 @@ void vmx_pi_per_cpu_init(unsigned int cpu); void vmx_pi_hooks_assign(struct domain *d); void vmx_pi_hooks_deassign(struct domain *d); +#define APIC_INVALID_DEST 0xffffffff + /* EPT violation qualifications definitions */ #define _EPT_READ_VIOLATION 0 #define EPT_READ_VIOLATION (1UL<<_EPT_READ_VIOLATION)