From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Sullivan Subject: [PATCH v2] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq Date: Wed, 11 Mar 2015 09:05:26 -0600 Message-ID: <550059B6.6050708@gmail.com> References: <54FE7B9E.8020202@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: gleb@kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com To: kvm@vger.kernel.org Return-path: Received: from mail-pd0-f178.google.com ([209.85.192.178]:37630 "EHLO mail-pd0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752211AbbCKPHg (ORCPT ); Wed, 11 Mar 2015 11:07:36 -0400 Received: by pdbnh10 with SMTP id nh10so11899646pdb.4 for ; Wed, 11 Mar 2015 08:07:36 -0700 (PDT) In-Reply-To: <54FE7B9E.8020202@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: This is a revised patch to the previous submission, adding a check for RH=1/DM=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 is still used to specify the semantics of the delivery beyond its destination. We can't just set irq->delivery_mode to APIC_DM_LOWPRI if RH=1 since this squashes the other delivery semantics. I've documented this in the patch. A future fix may be to implement a separate interrupt delivery function for MSI interrupts in kvm_set_msi, which is a bit hacky but probably the only way to do this without modifying the kvm_lapic_irq struct. I'll write this up and see how it looks, unless there are major objections. Signed-off-by: James Sullivan --- arch/x86/kvm/irq_comm.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 72298b3..327a4b8 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -97,18 +97,34 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src, static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e, struct kvm_lapic_irq *irq) { + bool phys; trace_kvm_msi_set_irq(e->msi.address_lo, e->msi.data); irq->dest_id = (e->msi.address_lo & 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; + /* + * Set dest_mode to logical just in case both RH=1/DH=1, + * otherwise default to physical + */ + phys = ((e->msi.address_lo & (MSI_ADDR_REDIRECTION_LOWPRI | + MSI_ADDR_DEST_MODE_LOGICAL)) != + (MSI_ADDR_REDIRECTION_LOWPRI | + MSI_ADDR_DEST_MODE_LOGICAL)); + irq->dest_mode = phys ? APIC_DEST_PHYSICAL : APIC_DEST_LOGICAL; irq->trig_mode = (1 << MSI_DATA_TRIGGER_SHIFT) & e->msi.data; irq->delivery_mode = e->msi.data & 0x700; irq->level = 1; irq->shorthand = 0; - /* TODO Deal with RH bit of MSI message address */ + /* + * 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. However other delivery semantics are still + * specified by the delivery mode, so we can't default to + * APIC_DM_LOWPRI if RH=1. + */ } int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e, -- 2.3.1