[Qemu-devel] [PATCH 0/3] apic: Implement handling for MSI redirection hint in IRQ delivery

[Qemu-devel] [PATCH 0/3] apic: Implement handling for MSI redirection hint in IRQ delivery

[James Sullivan]

This set of patches implements handling for the MSI redirection hint
bit. The RH bit is used in logical destination mode to indicate that
the delivery of the interrupt shall only be to the lowest priority
candidate LAPIC.

Currently, there is no handling of the MSI RH bit. This patch implements 
the following logic:

* DM=0, RH=*  : Physical destination mode. Interrupt is delivered to
                    the LAPIC with the matching APIC ID. (Subject to
                    the usual restrictions, i.e. no broadcast dest)
* DM=1, RH=0  : Logical destination mode without redirection. Interrupt
                    is delivered to all LAPICs in the logical group 
                    specified by the IRQ's destination map and delivery
                    mode.
* DM=1, RH=1  : Logical destination mode with redirection. Interrupt
                    is delivered only to the lowest priority LAPIC in the 
                    logical group specified by the dest map and the
                    delivery mode. Delivery semantics are otherwise
                    specified by the delivery_mode of the IRQ, which
                    is unchanged.

These changes reflect those made in the KVM in
http://www.spinics.net/lists/kvm/msg114915.html ("kvm: x86: Implement
handling of RH=1 for MSI delivery in KVM"), which are under review but
are largely settled in terms of functionality.


James Sullivan (3):
  apic: Added helper function apic_match_dest,
    apic_match_[physical,logical]_dest
  apic: Set and pass in RH bit for MSI interrupts
  apic: Implement handling of RH=1 for MSI interrupt delivery

 hw/intc/apic.c         | 70 +++++++++++++++++++++++++++++++++++++-------------
 hw/intc/ioapic.c       |  2 +-
 include/hw/i386/apic.h |  3 ++-
 trace-events           |  2 +-
 4 files changed, 56 insertions(+), 21 deletions(-)

-- 
2.3.3

[Qemu-devel] [PATCH 1/3] apic: Added helper function apic_match_dest, apic_match_[physical, logical]_dest

[James Sullivan]

Added three helper functions apic_match_dest(),
apic_match_physical_dest(), and apic_match_logical_dest() which
can be used to determine if a logical or physical APIC ID match a
given LAPIC under a given dest_mode. This does not account for shorthand.

Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
---
 hw/intc/apic.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 0f97b47..a5b5990 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -217,6 +217,32 @@ static void apic_external_nmi(APICCommonState *s)
     }\
 }
 
+static bool apic_match_physical_dest(struct APICCommonState *apic, uint8_t dest)
+{
+    return (dest == 0xff || apic->id == dest);
+}
+
+static bool apic_match_logical_dest(struct APICCommonState *apic, uint8_t dest)
+{
+    if (apic->dest_mode == 0xf) {
+        return (dest & apic->log_dest) > 0;
+    } else if (apic->dest_mode == 0) {
+        return ((dest & 0xf0) == (apic->log_dest & 0xf0)) &&
+                (dest & apic->log_dest & 0x0f) > 0;
+    }
+    return false;
+}
+
+static bool apic_match_dest(struct APICCommonState *apic, uint8_t dest_mode,
+                     uint8_t dest)
+{
+    if (dest_mode == 0) {
+        return apic_match_physical_dest(apic, dest);
+    } else {
+        return apic_match_logical_dest(apic, dest);
+    }
+}
+
 static void apic_bus_deliver(const uint32_t *deliver_bitmask,
                              uint8_t delivery_mode, uint8_t vector_num,
                              uint8_t trigger_mode)
-- 
2.3.3

[Qemu-devel] [PATCH 2/3] apic: Set and pass in RH bit for MSI interrupts

[James Sullivan]

In apic_send_msi(), set msi_redir_hint to 0x1 when RH=1 in the
MSI Address Register.

Added an argument for msi_redir_hint to apic_deliver_irq(), and
changed calls to the function accordingly (using 0 as a default value
for non-MSI interrupts).

Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
---
 hw/intc/apic.c         | 10 ++++++----
 hw/intc/ioapic.c       |  2 +-
 include/hw/i386/apic.h |  3 ++-
 trace-events           |  2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a5b5990..97dd649 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -306,12 +306,13 @@ static void apic_bus_deliver(const uint32_t *deliver_bitmask,
 }
 
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
-                      uint8_t vector_num, uint8_t trigger_mode)
+                      uint8_t vector_num, uint8_t trigger_mode,
+                      uint8_t msi_redir_hint)
 {
     uint32_t deliver_bitmask[MAX_APIC_WORDS];
 
     trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
-                           trigger_mode);
+                           trigger_mode, msi_redir_hint);
 
     apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
     apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
@@ -777,8 +778,9 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
     uint8_t dest_mode = (addr >> MSI_ADDR_DEST_MODE_SHIFT) & 0x1;
     uint8_t trigger_mode = (data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
     uint8_t delivery = (data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x7;
-    /* XXX: Ignore redirection hint. */
-    apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
+    uint8_t msi_redir_hint = (data >> MSI_ADDR_REDIRECTION_SHIFT) & 0x1;
+    apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode,
+                     msi_redir_hint);
 }
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index b527932..e69aa18 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -71,7 +71,7 @@ static void ioapic_service(IOAPICCommonState *s)
                     vector = entry & IOAPIC_VECTOR_MASK;
                 }
                 apic_deliver_irq(dest, dest_mode, delivery_mode,
-                                 vector, trig_mode);
+                                 vector, trig_mode, 0);
             }
         }
     }
diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index 1d48e02..1e15edd 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -5,7 +5,8 @@
 
 /* apic.c */
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
-                      uint8_t vector_num, uint8_t trigger_mode);
+                      uint8_t vector_num, uint8_t trigger_mode,
+                      uint8_t msi_redir_hint);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
 void apic_deliver_nmi(DeviceState *d);
diff --git a/trace-events b/trace-events
index 30eba92..b430cf9 100644
--- a/trace-events
+++ b/trace-events
@@ -158,7 +158,7 @@ apic_get_irq_delivered(int apic_irq_delivered) "returning coalescing %d"
 
 # hw/intc/apic.c
 apic_local_deliver(int vector, uint32_t lvt) "vector %d delivery mode %d"
-apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, uint8_t vector_num, uint8_t trigger_mode) "dest %d dest_mode %d delivery_mode %d vector %d trigger_mode %d"
+apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, uint8_t vector_num, uint8_t trigger_mode, uint8_t msi_redir_hint) "dest %d dest_mode %d delivery_mode %d vector %d trigger_mode %d msi_redir_hint %d"
 apic_mem_readl(uint64_t addr, uint32_t val)  "%"PRIx64" = %08x"
 apic_mem_writel(uint64_t addr, uint32_t val) "%"PRIx64" = %08x"
 
-- 
2.3.3

[Qemu-devel] [PATCH 3/3] apic: Implement handling of RH=1 for MSI interrupt delivery

[James Sullivan]

Added argument to apic_get_delivery_bitmask() for msi_redir_hint,
and changed calls to the function accordingly (using 0 as a default
value for non-MSI interrupts).

Modified the implementation of apic_get_delivery_bitmask() to account
for the RH bit of an MSI IRQ. The RH bit indicates that the message
should target only the lowest-priority processor among those specified
by the logical destination of the IRQ.

Signed-off-by: James Sullivan <sullivan.james.f@gmail.com>
---
 hw/intc/apic.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 97dd649..7ac60aa 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -37,7 +37,8 @@ static APICCommonState *local_apics[MAX_APICS + 1];
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
 static void apic_update_irq(APICCommonState *s);
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
-                                      uint8_t dest, uint8_t dest_mode);
+                                      uint8_t dest, uint8_t dest_mode,
+                                      uint8_t msi_redir_hint);
 
 /* Find first bit starting from msb */
 static int apic_fls_bit(uint32_t value)
@@ -314,7 +315,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
     trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
                            trigger_mode, msi_redir_hint);
 
-    apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
+    apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode, msi_redir_hint);
     apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
@@ -472,7 +473,8 @@ static int apic_find_dest(uint8_t dest)
 }
 
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
-                                      uint8_t dest, uint8_t dest_mode)
+                                      uint8_t dest, uint8_t dest_mode,
+                                      uint8_t msi_redir_hint)
 {
     APICCommonState *apic_iter;
     int i;
@@ -488,23 +490,27 @@ static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
         }
     } else {
         /* XXX: cluster mode */
+        int l = -1;
         memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t));
         for(i = 0; i < MAX_APICS; i++) {
             apic_iter = local_apics[i];
-            if (apic_iter) {
-                if (apic_iter->dest_mode == 0xf) {
-                    if (dest & apic_iter->log_dest)
-                        apic_set_bit(deliver_bitmask, i);
-                } else if (apic_iter->dest_mode == 0x0) {
-                    if ((dest & 0xf0) == (apic_iter->log_dest & 0xf0) &&
-                        (dest & apic_iter->log_dest & 0x0f)) {
-                        apic_set_bit(deliver_bitmask, i);
+            if (!apic_iter) {
+                break;
+            }
+            if (apic_match_dest(apic_iter, dest_mode, dest)) {
+                if (msi_redir_hint) {
+                    if (l < 0 ||
+                        apic_compare_prio(apic_iter, local_apics[l]) < 0) {
+                        l = i;
                     }
+                } else {
+                    apic_set_bit(deliver_bitmask, i);
                 }
-            } else {
-                break;
             }
         }
+        if (msi_redir_hint && l >= 0) {
+            apic_set_bit(deliver_bitmask, l);
+        }
     }
 }
 
@@ -537,7 +543,7 @@ static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
 
     switch (dest_shorthand) {
     case 0:
-        apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
+        apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode, 0);
         break;
     case 1:
         memset(deliver_bitmask, 0x00, sizeof(deliver_bitmask));
-- 
2.3.3