diff mbox

[v3,1/2] kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery

Message ID 1426555822-3280-2-git-send-email-sullivan.james.f@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

James Sullivan March 17, 2015, 1:30 a.m. UTC
In kvm_set_msi_irq(), the RH bit is currently ignored for
determining the destination mode of the MSI delivery, and only the
DM bit is used. Corrected this so that dest_mode is APIC_DEST_LOGICAL
only when RH=1/DM=1, and APIC_DEST_PHYSICAL otherwise.

Extended struct kvm_lapic_irq with bool msi_redir_hint, which will
be used to determine if the delivery of the MSI should target only
the lowest priority CPU in the logical group specified for delivery.
(In physical dest mode, the RH bit is not relevant). Initialized the value
of msi_redir_hint to true when RH=1 in kvm_set_msi_irq(), and initialized
to false in all other cases.

Added value of msi_redir_hint to a debug message dump of an IRQ in
apic_send_ipi().

Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
---
Changes since v1:
    * Squashed a number of smaller commits into this one commit,
        which adds and initializes the msi_redir_hint variable
        and extends existing debug messages to display its value.
Changes since v2:
    * Added old patch (<5502FEDB.3030606@gmail.com>) to set the value of
    dest_mode in kvm_set_msi_irq() to be APIC_DEST_LOGICAL only when RH=1/DM=1,
    and APIC_DEST_PHYSICAL otherwise. This decouples the dependency of 
    this patch set on the previous submission and collects all efforts
    to implement RH bit handling into one submission.
    * Patch formatting

 arch/x86/include/asm/kvm_host.h | 1 +
 arch/x86/kvm/ioapic.c           | 1 +
 arch/x86/kvm/irq_comm.c         | 9 +++++++--
 arch/x86/kvm/lapic.c            | 6 ++++--
 arch/x86/kvm/x86.c              | 1 +
 5 files changed, 14 insertions(+), 4 deletions(-)

Comments

Radim Krčmář March 17, 2015, 2:02 p.m. UTC | #1
2015-03-16 19:30-0600, James Sullivan:
> In kvm_set_msi_irq(), the RH bit is currently ignored for
> determining the destination mode of the MSI delivery, and only the
> DM bit is used. Corrected this so that dest_mode is APIC_DEST_LOGICAL
> only when RH=1/DM=1, and APIC_DEST_PHYSICAL otherwise.
> 
> Extended struct kvm_lapic_irq with bool msi_redir_hint, which will
> be used to determine if the delivery of the MSI should target only
> the lowest priority CPU in the logical group specified for delivery.
> (In physical dest mode, the RH bit is not relevant). Initialized the value
> of msi_redir_hint to true when RH=1 in kvm_set_msi_irq(), and initialized
> to false in all other cases.
> 
> Added value of msi_redir_hint to a debug message dump of an IRQ in
> apic_send_ipi().
> 
> Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
> ---

Reviewed-by: Radim Kr?má? <rkrcmar@redhat.com>

Thanks.

---
Btw. I often have comments ... if they are in parentheses, I have a
different opinion on some choice and would like to know why you chose a
that variant instead, because I might be missing something;
there is no need to send new revision or even answer them.

> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> @@ -103,12 +103,17 @@ static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
> +	irq->msi_redir_hint = ((e->msi.address_lo
> +		& MSI_ADDR_REDIRECTION_LOWPRI) > 0);

(- two extra tabs, or alignment, help to distinguish a continued line
   from indented next line
 - outer parentheses are useless
 - '!= 0' saves thinking about negative values
 - it would fit on one line if we didn't map to {0,1} explicitly;
   KVM style prefers explicit, though ...)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a236e39..77feaf4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -685,6 +685,7 @@  struct kvm_lapic_irq {
 	u32 trig_mode;
 	u32 shorthand;
 	u32 dest_id;
+	bool msi_redir_hint;
 };
 
 struct kvm_x86_ops {
diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index b1947e0..61f0874 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -347,6 +347,7 @@  static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
 	irqe.delivery_mode = entry->fields.delivery_mode << 8;
 	irqe.level = 1;
 	irqe.shorthand = 0;
+	irqe.msi_redir_hint = false;
 
 	if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
 		ioapic->irr &= ~(1 << irq);
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 72298b3..5c33cca 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -103,12 +103,17 @@  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;
+	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;
+	irq->msi_redir_hint = ((e->msi.address_lo
+		& MSI_ADDR_REDIRECTION_LOWPRI) > 0);
 	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,
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index bd4e34d..a15c444 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -892,6 +892,7 @@  static void apic_send_ipi(struct kvm_lapic *apic)
 	irq.level = icr_low & APIC_INT_ASSERT;
 	irq.trig_mode = icr_low & APIC_INT_LEVELTRIG;
 	irq.shorthand = icr_low & APIC_SHORT_MASK;
+	irq.msi_redir_hint = false;
 	if (apic_x2apic_mode(apic))
 		irq.dest_id = icr_high;
 	else
@@ -901,10 +902,11 @@  static void apic_send_ipi(struct kvm_lapic *apic)
 
 	apic_debug("icr_high 0x%x, icr_low 0x%x, "
 		   "short_hand 0x%x, dest 0x%x, trig_mode 0x%x, level 0x%x, "
-		   "dest_mode 0x%x, delivery_mode 0x%x, vector 0x%x\n",
+		   "dest_mode 0x%x, delivery_mode 0x%x, vector 0x%x, "
+		   "msi_redir_hint 0x%x\n",
 		   icr_high, icr_low, irq.shorthand, irq.dest_id,
 		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
-		   irq.vector);
+		   irq.vector, irq.msi_redir_hint);
 
 	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bd7a70b..03e9b09 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5902,6 +5902,7 @@  static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid)
 	lapic_irq.shorthand = 0;
 	lapic_irq.dest_mode = 0;
 	lapic_irq.dest_id = apicid;
+	lapic_irq.msi_redir_hint = false;
 
 	lapic_irq.delivery_mode = APIC_DM_REMRD;
 	kvm_irq_delivery_to_apic(kvm, 0, &lapic_irq, NULL);