Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754470AbbLVTxF (ORCPT ); Tue, 22 Dec 2015 14:53:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47269 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752037AbbLVTxD (ORCPT ); Tue, 22 Dec 2015 14:53:03 -0500 Date: Tue, 22 Dec 2015 20:52:59 +0100 From: "rkrcmar@redhat.com" To: "Wu, Feng" Cc: Yang Zhang , "pbonzini@redhat.com" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "Jiang Liu (jiang.liu@linux.intel.com)" Subject: Re: [PATCH v2 1/2] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts Message-ID: <20151222195259.GC12725@potion.redhat.com> References: <1450229853-3886-1-git-send-email-feng.wu@intel.com> <1450229853-3886-2-git-send-email-feng.wu@intel.com> <567759F2.5080809@gmail.com> <56775E9A.5030108@gmail.com> <5678F268.5070801@gmail.com> <5678F81C.5050309@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3260 Lines: 75 2015-12-22 07:19+0000, Wu, Feng: >> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com] >> On 2015/12/22 14:59, Wu, Feng wrote: >> >> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com] >> >>>>>> On 2015/12/16 9:37, Feng Wu wrote: >> >>>>>>> + for_each_set_bit(i, &bitmap, 16) { >> >>>>>>> + if (!dst[i] >> >>>>>> && !kvm_lapic_enabled(dst[i]->vcpu)) { >> >>>>>> >> >>>>>> It should be or(||) not and (&&). >> >>>>> >> >>>>> Oh, you are right! My negligence! Thanks for pointing this out, Yang! >> >>>> >> >>>> btw, i think the kvm_lapic_enabled check is wrong here? Why need it here? >> >>> >> >>> If the lapic is not enabled, I think we cannot recognize it as a candidate, can >> >> we? >> >>> Maybe Radim can confirm this, Radim, what is your option? SDM 10.6.2.2 Logical Destination Mode: For both configurations of logical destination mode, when combined with lowest priority delivery mode, software is responsible for ensuring that all of the local APICs included in or addressed by the IPI or I/O subsystem interrupt are present and enabled to receive the interrupt. The case is undefined if some targeted LAPICs weren't hardware enabled as no interrupts can be delivered to hardware disabled LAPIC, so we can check for hardware enabled. It's not obvious if "enabled to receive the interrupt" means hardware or software enabled, but lowest priority cannot deliver NMI/INIT/..., so checking for software enabled doesn't restrict any valid uses either. so ... KVM only musn't blow up when encountering this situation :) The current code seems correct, but redundant. Just for reference, KVM now does: - check for software enabled LAPIC since patch aefd18f01ee8 ("KVM: x86: In DM_LOWEST, only deliver interrupts to vcpus with enabled LAPIC's") - check only for hardware enabled LAPIC in the fast path, since 1e08ec4a130e ("KVM: optimize apic interrupt delivery")) (v1 was arguable better, I pointed the need for enabled LAPIC in v1 only from looking at one KVM function, sorry.) >> >> Lapic can be disable by hw or sw. Here we only need to check the hw is >> >> enough which is already covered while injecting the interrupt into >> >> guest. I remember we(Glab, Macelo and me) have discussed it several ago, >> >> but i cannot find the mail thread. >> >> > >> > But if the lapic is disabled by software, we cannot still inject interrupts to >> > it, can we? >> >> Yes, We cannot inject the normal interrupt. But this already covered by >> current logic and add a check here seems meaningless. Conversely, it may >> do bad thing.. >> > > Let's wait for Radim/Paolo's opinions about this. I'd pick whatever results in less code: this time it seems like checking for hardware enabled LAPIC in both paths (implicitly in the fast path). Maybe it can be done better, I haven't given it much thought. We should revert aefd18f01ee8 at the same time, so our PI/non-PI slow paths won't diverge -- I hope it wasn't fixing a bug :) I'll review the series tomorrow, thanks for your patience. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/