From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751918AbcAUUQl (ORCPT ); Thu, 21 Jan 2016 15:16:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55990 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750907AbcAUUQj (ORCPT ); Thu, 21 Jan 2016 15:16:39 -0500 Date: Thu, 21 Jan 2016 21:16:36 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Feng Wu Cc: pbonzini@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org Subject: Re: [PATCH v3 3/4] KVM: x86: Add lowest-priority support for vt-d posted-interrupts Message-ID: <20160121201635.GB14104@potion.brq.redhat.com> References: <1453254177-103002-1-git-send-email-feng.wu@intel.com> <1453254177-103002-4-git-send-email-feng.wu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1453254177-103002-4-git-send-email-feng.wu@intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-01-20 09:42+0800, Feng Wu: > Use vector-hashing to deliver lowest-priority interrupts for > VT-d posted-interrupts. This patch extends kvm_intr_is_single_vcpu() > to support lowest-priority handling. > > Signed-off-by: Feng Wu > --- > v3: > - Remove unnecessary check in fast irq delivery patch > - print a error message only once for each guest when we find hardware > disabled LAPIC during interrupt injection. > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > @@ -1316,8 +1316,8 @@ int x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa, u32 size); > bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu); > bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu); > > -bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq, > - struct kvm_vcpu **dest_vcpu); > +bool kvm_intr_can_posting(struct kvm *kvm, struct kvm_lapic_irq *irq, I prefer the original one; I think it's better to describe function than intent in names -- intention should be obvious from its use. > + struct kvm_vcpu **dest_vcpu); > > void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e, > struct kvm_lapic_irq *irq); > diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c > @@ -300,13 +300,13 @@ out: > return r; > } > > -bool kvm_intr_is_single_vcpu(struct kvm *kvm, struct kvm_lapic_irq *irq, > - struct kvm_vcpu **dest_vcpu) > +bool kvm_intr_can_posting(struct kvm *kvm, struct kvm_lapic_irq *irq, > + struct kvm_vcpu **dest_vcpu) > { > int i, r = 0; > struct kvm_vcpu *vcpu; > > - if (kvm_intr_is_single_vcpu_fast(kvm, irq, dest_vcpu)) > + if (kvm_intr_can_posting_fast(kvm, irq, dest_vcpu)) > return true; There is one pitfall: xAPIC flat logical broadcast returns false, but lowest priority is defined for it (practically isn't a broadcast) and the rest of this function doesn't check for lowest priority, so the interrupt won't be posted. We could modify our _fast functions to cover 0xff in flat logical, but ignoring this case isn't bad either ... it can happen only with 8 VCPU guests. I'd just comment that we did it on purpose. :) > > kvm_for_each_vcpu(i, vcpu, kvm) { [Comments for the rest mirror [2/4].]