From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Sullivan Subject: Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq Date: Tue, 17 Mar 2015 09:23:04 -0600 Message-ID: <550846D8.5050709@gmail.com> References: <5502FEDB.3030606@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: gleb@kernel.org, pbonzini@redhat.com To: kvm@vger.kernel.org Return-path: Received: from mail-pa0-f42.google.com ([209.85.220.42]:35131 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753291AbbCQPZQ (ORCPT ); Tue, 17 Mar 2015 11:25:16 -0400 Received: by pabyw6 with SMTP id yw6so12681299pab.2 for ; Tue, 17 Mar 2015 08:25:16 -0700 (PDT) In-Reply-To: <5502FEDB.3030606@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: N.B. This patch has been re-submitted in a larger patch, see <1426555822-3280-1-git-send-email-sullivan.james.f@gmail.com> (The new patch relies on changes made in this patch, and as such it makes more sense to bundle them) On 03/13/2015 09:14 AM, James Sullivan wrote: > This patch adds a check for RH=1 in kvm_set_msi_irq. Currently the > DM bit is the only thing used to decide irq->dest_mode (logical when DM > set, physical when unset). Documentation indicates that the DM bit will > be 'ignored' when the RH bit is unset, and physical destination mode is > used in this case. > > Fixed this to set irq->dest_mode to APIC_DEST_LOGICAL just in case both > RH=1/DM=1. > > This patch doesn't completely handle RH=1; if RH=1 then the delivery will behave > as in low priority mode (deliver the interrupt to only the lowest priority processor), > but the delivery mode may still used to specify the semantics of the delivery beyond > its destination. > > I will be trying and comparing a few options to handle this fully (extension of > struct kvm_lapic_irq, introduction of MSI specific delivery functions or helpers, > etc) and hope to have some patches to show in the near future. > > > Signed-off-by: James Sullivan > --- > Changes since v2: > * Added one time warning message when RH=1 > * Documented conflict between RH=1 and delivery mode > * Tidied code to check RH=1/DM=1 (remove bool phys, use if/else) > Changes since v3: > * Fixed logical error in RH=1/DM=1 check > * Aligned quotation blocks in multiline pr_warn_once argument > Changes since v4: > * Put error message string on single line > > arch/x86/kvm/irq_comm.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c > index 72298b3..f2887ea 100644 > --- a/arch/x86/kvm/irq_comm.c > +++ b/arch/x86/kvm/irq_comm.c > @@ -103,12 +103,24 @@ static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e, > MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT; > irq->vector = (e->msi.data & > MSI_DATA_VECTOR_MASK) >> MSI_DATA_VECTOR_SHIFT; > - irq->dest_mode = (1 << MSI_ADDR_DEST_MODE_SHIFT) & e->msi.address_lo; > + /* > + * TODO Deal with RH bit of MSI message address > + * IF RH=1, then MSI delivers only to the processor with the > + * lowest interrupt priority among processors that can receive > + * the interrupt. > + */ > + if ((e->msi.address_lo & MSI_ADDR_REDIRECTION_LOWPRI) && > + (e->msi.address_lo & MSI_ADDR_DEST_MODE_LOGICAL)) > + irq->dest_mode = APIC_DEST_LOGICAL; > + else > + irq->dest_mode = APIC_DEST_PHYSICAL; > irq->trig_mode = (1 << MSI_DATA_TRIGGER_SHIFT) & e->msi.data; > irq->delivery_mode = e->msi.data & 0x700; > + if (e->msi.address_lo & MSI_ADDR_REDIRECTION_LOWPRI) > + pr_warn_once( > + "kvm: MSIs may not be correctly delivered with RH set.\n"); > irq->level = 1; > irq->shorthand = 0; > - /* TODO Deal with RH bit of MSI message address */ > } > > int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, >