From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758154AbbA3NJu (ORCPT ); Fri, 30 Jan 2015 08:09:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59020 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754202AbbA3NJt (ORCPT ); Fri, 30 Jan 2015 08:09:49 -0500 Date: Fri, 30 Jan 2015 14:09:41 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Nadav Amit , Gleb Natapov Subject: Re: [PATCH 5/8] KVM: x86: use MDA for interrupt matching Message-ID: <20150130130941.GB28519@potion.redhat.com> References: <1422568135-28402-1-git-send-email-rkrcmar@redhat.com> <1422568135-28402-6-git-send-email-rkrcmar@redhat.com> <54CB48F5.9060503@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <54CB48F5.9060503@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2015-01-30 10:03+0100, Paolo Bonzini: > On 29/01/2015 22:48, Radim Krčmář wrote: > > +/* KVM APIC implementation has two quirks > > + * - dest always begins at 0 while xAPIC MDA has offset 24, > > + * - IOxAPIC messages have to be delivered (directly) to x2APIC. > > + */ > > +static u32 > > +kvm_apic_mda(unsigned int dest, struct kvm_lapic *ipi, bool x2apic_dest) > > Please pass two struct kvm_lapic, so that you can write > > bool ipi = source != NULL; > bool x2apic_mda = apic_x2apic_mode(ipi ? source : dest); > > Looks a little nicer to me at least. Definitely. > > +{ > > + bool x2apic_mda = ipi ? apic_x2apic_mode(ipi) : x2apic_dest; > > + > > + if (!ipi && dest == APIC_BROADCAST) > > + dest = X2APIC_BROADCAST; > > This works, but it is not super-clear that you are shifting left by 24 > here, and right in kvm_apic_broadcast(). What if you just make it > > if (!ipi && dest == APIC_BROADCAST && x2apic_mda) > return X2APIC_BROADCAST. It's better, it will be that way in v2 of [5-8/8], thanks. (I was using it twice in earlier iterations and didn't think again.)