From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Sullivan Subject: Re: [PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq Date: Wed, 11 Mar 2015 08:38:35 -0600 Message-ID: <5500536B.5000200@gmail.com> References: <54FE7B9E.8020202@gmail.com> <20150310144758.GA18573@potion.brq.redhat.com> <54FF72AE.3070507@gmail.com> <20150311134310.GB12854@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, gleb@kernel.org, pbonzini@redhat.com To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mail-pd0-f180.google.com ([209.85.192.180]:46216 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752962AbbCKOkr (ORCPT ); Wed, 11 Mar 2015 10:40:47 -0400 Received: by pdev10 with SMTP id v10so11641417pde.13 for ; Wed, 11 Mar 2015 07:40:46 -0700 (PDT) In-Reply-To: <20150311134310.GB12854@potion.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 03/11/2015 07:43 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > 2015-03-10 16:39-0600, James Sullivan: >> On 03/10/2015 08:47 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >>>> + irq->dest_mode =3D phys ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL); >>> >>> (Should be APIC_DEST_LOGICAL. All works because it is a boolean an= d we >>> only checked for APIC_DEST_PHYSICAL, which is 0.) >>> >> >> Thank you, that should be as follows: >> irq->dest_mode =3D phys ? (APIC_DEST_PHYSICAL) : (APIC_DEST_LOGI= CAL); >> ? > > Yes, thanks. (No need for parentheses around macros, we "agreed" to > cover that in definition and they don't make this more readable.) > I'll tidy that up too. > Your patch improves the situation, but removing the TODO is not > warranted -- RH still doesn't do what it should: > >>> How does DM=3D1/RH=3D1 work on real hardware? >>> (There seem to be interesting corner cases with irq->delivery_mode = like >>> APIC_DM_NMI.) >> >> The IA32 manual says that if DM=3D1/RH=3D1, we operate in logical de= stination mode >> similarly to other IPIs. I don't believe this patch introduces any i= nvalid >> settings listed in section 10-21, Vol. 3, so this shouldn't create a= ny weirdness. > > Quoting SDM Jan 2015, 10.11.1 Message Address Register Format: > This bit [RH] indicates whether the message should be directed to t= he > processor with the lowest interrupt priority among processors that = can > receive the interrupt. > > =3D> it should behave like lowest priority delivery. > Deliver to just one APIC -- we don't do that. > > KVM interrupt delivery functions can only specify lowest priority tho= ugh > delivery_mode and we would have to rework KVM if MSI can set somethin= g > else in the delivery_mode (like NMI). > I see-- That will pose issues when you want some particular semantics o= f one delivery mode (such as ignoring vector info in NMI) but want low-pri de= livery. Short of writing an MSI specific delivery function and calling it in kv= m_set_msi, I can't think of a way to get this. Do you think that's warranted? I'll document this and resubmit this patch with the TODO for now. Thank= s for the insight.