From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Sullivan Subject: [Patch v4] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq Date: Thu, 12 Mar 2015 21:08:54 -0600 Message-ID: <550254C6.6010103@gmail.com> References: <55022423.7050101@gmail.com> <5502506C.6000005@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: pbonzini@redhat.com, gleb@kernel.org, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= To: kvm@vger.kernel.org Return-path: Received: from mail-pa0-f42.google.com ([209.85.220.42]:41305 "EHLO mail-pa0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752095AbbCMDLF (ORCPT ); Thu, 12 Mar 2015 23:11:05 -0400 Received: by pablj1 with SMTP id lj1so25710373pab.8 for ; Thu, 12 Mar 2015 20:11:05 -0700 (PDT) In-Reply-To: <5502506C.6000005@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: (Correction of a rather obvious and embarrassing logical error in v3). 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 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..9514fed 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 may not correctly deliver MSIs " + "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, -- 2.3.1