From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752118AbcGAMqI (ORCPT ); Fri, 1 Jul 2016 08:46:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47675 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751969AbcGAMqG (ORCPT ); Fri, 1 Jul 2016 08:46:06 -0400 Date: Fri, 1 Jul 2016 14:39:42 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, "Lan, Tianyu" , Igor Mammedov , Jan Kiszka , Peter Xu Subject: Re: [PATCH v1 02/11] KVM: x86: add kvm_apic_map_get_dest_lapic Message-ID: <20160701123941.GA27840@potion> References: <20160630205429.16480-1-rkrcmar@redhat.com> <20160630205429.16480-3-rkrcmar@redhat.com> <7f5a8277-f905-e6a2-fba8-7f859d300b66@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7f5a8277-f905-e6a2-fba8-7f859d300b66@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 01 Jul 2016 12:39:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-07-01 09:57+0200, Paolo Bonzini: > On 30/06/2016 22:54, Radim Krčmář wrote: >> kvm_irq_delivery_to_apic_fast and kvm_intr_is_single_vcpu_fast both >> compute the interrupt destination. Factor the code. >> >> 'struct kvm_lapic **dst = NULL' had to be added to silence GCC. >> GCC might complain about potential NULL access in the future, because it >> missed conditions that avoided uninitialized uses of dst. >> >> Signed-off-by: Radim Krčmář >> --- >> v1: improved comment for kvm_apic_map_get_dest_lapic() [Peter] >> >> arch/x86/kvm/lapic.c | 241 ++++++++++++++++++++++----------------------------- >> 1 file changed, 103 insertions(+), 138 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 22a6474af220..238b87b068db 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -671,14 +671,98 @@ static void kvm_apic_disabled_lapic_found(struct kvm *kvm) >> } >> } >> >> +/* Return true if the interrupt can be handled by using *bitmap as index mask >> + * for valid destinations in *dst array. >> + * Return false if kvm_apic_map_get_dest_lapic did nothing useful. >> + * Note: we may have zero kvm_lapic destinations when we return true, which >> + * means that the interrupt should be dropped. In this case, *bitmap would be >> + * zero and *dst undefined. >> + */ >> +static inline bool kvm_apic_map_get_dest_lapic(struct kvm *kvm, struct kvm_lapic *src, >> + struct kvm_lapic_irq *irq, struct kvm_apic_map *map, >> + struct kvm_lapic ***dst, unsigned long *bitmap) >> +{ >> + int i, lowest; >> + bool x2apic_ipi; >> + u16 cid; >> + >> + if (irq->shorthand == APIC_DEST_SELF) { >> + *dst = &src; > > This is not valid, &src dies as soon as you leave the function. You > need to pass &src into kvm_apic_map_get_dest_lapic and here do something > like Indeed, that would have been bad. > if (irq->shorthand) { > if (irq->shorthand == APIC_DEST_SELF && p_src) { > *dst = p_src; > *bitmap = 1; > return true; > } > return false; > } > > If it's not too hard, I'd like to have patch 4 before this one. Sure, thanks.