Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/24/2015 08:03 AM, Radim Krčmář wrote: 2015-03-23 16:46-0600, James Sullivan: On 03/23/2015 03:13 PM, Radim Krčmář wrote: I meant if the delivery mode from data register isn't ignored with RH=1, and the message delivered as if lowest-priority was set there. (Decided by having something else than fixed or lowest-priority there.) Hmm, any thoughts on how I could test for that? Set the MSI data register's delivery mode to NMI/SMI/... The change below fails = hardware honors delivery mode. I tested it and Linux got a lot of unexpected NMIs, so the emulation in your latest patch looks correct. diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h index 4cc48af23fef..2270e459186b 100644 --- a/arch/x86/include/asm/msidef.h +++ b/arch/x86/include/asm/msidef.h @@ -17,6 +17,7 @@ #define MSI_DATA_DELIVERY_MODE_SHIFT 8 #define MSI_DATA_DELIVERY_FIXED (0 MSI_DATA_DELIVERY_MODE_SHIFT) #define MSI_DATA_DELIVERY_LOWPRI(1 MSI_DATA_DELIVERY_MODE_SHIFT) +#define MSI_DATA_DELIVERY_NMI (4 MSI_DATA_DELIVERY_MODE_SHIFT) #define MSI_DATA_LEVEL_SHIFT 14 #define MSI_DATA_LEVEL_DEASSERT(0 MSI_DATA_LEVEL_SHIFT) diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c index d6ba2d660dc5..4f71737c34eb 100644 --- a/arch/x86/kernel/apic/msi.c +++ b/arch/x86/kernel/apic/msi.c @@ -46,7 +46,7 @@ void native_compose_msi_msg(struct pci_dev *pdev, MSI_DATA_LEVEL_ASSERT | ((apic-irq_delivery_mode != dest_LowestPrio) ? MSI_DATA_DELIVERY_FIXED : - MSI_DATA_DELIVERY_LOWPRI) | + MSI_DATA_DELIVERY_NMI) | MSI_DATA_VECTOR(cfg-vector); } Good to hear, thanks for testing that. Any other tweaks you think are necessary for the patch set? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] kvm: x86: Implement handling of RH=1 for MSI delivery in KVM
On 03/17/2015 02:19 AM, Jan Kiszka wrote: On 2015-03-17 02:30, James Sullivan wrote: Changes Since v1: * Reworked patches into two commits: 1) [Patch v2 1/2] Extended struct kvm_lapic_irq with bool msi_redir_hint * Initialize msi_redir_hint = true in kvm_set_msi_irq when RH=1 * Initialize msi_redir_hint = false otherwise * Added value of msi_redir_hint to debug message dump of IRQ in apic_send_ipi 2) [Patch v2 2/2] Deliver to only lowest prio CPU if msi_redir_hint is true * Move kvm_is_dm_lowest_prio() - lapic.h, rename to kvm_lowest_prio_delivery, set condition to (APIC_DM_LOWPRI || msi_redir_hint) * Change check in kvm_irq_delivery_to_apic_fast() for APIC_DM_LOWPRI or msi_redir_hint to a check for kvm_is_dm_lowest_prio() Changes since v2: * Extend Patch 1/2 (kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery) with older patch 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. (5502fedb.3030...@gmail.com) This was done to decouple the patch dependency and to collect all efforts to implement RH bit handling into one submission. * Patch formatting This series of patches extends the KVM interrupt delivery mechanism to correctly account 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 in the KVM interrupt delivery mechanism. 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. In other words, the RH bit is ignored in physical destination mode, and when it is set in logical destination mode causes delivery to only apply to the lowest priority processor in the logical group. The IA32 manual is in slight contradiction with itself on this matter, but this patch agrees with this interpretation of the RH bit: https://software.intel.com/en-us/forums/topic/23 This patch has passed some rudimentary tests using an SMP QEMU guest and virtio sourced MSIs, but I haven't done experiments with passing through PCI hardware (intend to start working on this). Once the kernel side is settled, would you mind looking into equivalent support for QEMU's APIC as well? Just to keep it easy to switch between both modes without too many functional restrictions. Thanks! Jan Just an update- I've submitted this to the QEMU side of things, see https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05020.html -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/24/2015 08:03 AM, Radim Krčmář wrote: 2015-03-23 16:46-0600, James Sullivan: On 03/23/2015 03:13 PM, Radim Krčmář wrote: I meant if the delivery mode from data register isn't ignored with RH=1, and the message delivered as if lowest-priority was set there. (Decided by having something else than fixed or lowest-priority there.) Hmm, any thoughts on how I could test for that? Set the MSI data register's delivery mode to NMI/SMI/... The change below fails = hardware honors delivery mode. I tested it and Linux got a lot of unexpected NMIs, so the emulation in your latest patch looks correct. diff --git a/arch/x86/include/asm/msidef.h b/arch/x86/include/asm/msidef.h index 4cc48af23fef..2270e459186b 100644 --- a/arch/x86/include/asm/msidef.h +++ b/arch/x86/include/asm/msidef.h @@ -17,6 +17,7 @@ #define MSI_DATA_DELIVERY_MODE_SHIFT 8 #define MSI_DATA_DELIVERY_FIXED (0 MSI_DATA_DELIVERY_MODE_SHIFT) #define MSI_DATA_DELIVERY_LOWPRI(1 MSI_DATA_DELIVERY_MODE_SHIFT) +#define MSI_DATA_DELIVERY_NMI (4 MSI_DATA_DELIVERY_MODE_SHIFT) #define MSI_DATA_LEVEL_SHIFT 14 #define MSI_DATA_LEVEL_DEASSERT(0 MSI_DATA_LEVEL_SHIFT) diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c index d6ba2d660dc5..4f71737c34eb 100644 --- a/arch/x86/kernel/apic/msi.c +++ b/arch/x86/kernel/apic/msi.c @@ -46,7 +46,7 @@ void native_compose_msi_msg(struct pci_dev *pdev, MSI_DATA_LEVEL_ASSERT | ((apic-irq_delivery_mode != dest_LowestPrio) ? MSI_DATA_DELIVERY_FIXED : - MSI_DATA_DELIVERY_LOWPRI) | + MSI_DATA_DELIVERY_NMI) | MSI_DATA_VECTOR(cfg-vector); } Great, thanks. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/23/2015 03:13 PM, Radim Krčmář wrote: 2015-03-20 11:50-0600, James Sullivan: On 03/20/2015 09:22 AM, James Sullivan wrote: On 03/20/2015 09:15 AM, Radim Krčmář wrote: 2015-03-19 16:51-0600, James Sullivan: I played around with native_compose_msi_msg and discovered the following: * dm=0, rh=0 = Physical Destination Mode * dm=0, rh=1 = Failed delivery * dm=1, rh=0 = Logical Destination Mode, No Redirection * dm=1, rh=1 = Logical Destination Mode, Redirection Great! (What CPU family was that?) This was on Intel x86_64 (Core i5-3210m, 'Ivy Bridge'). Thanks, it's possible that the behavior of chipsets changed since the report on Intel's forum ... (Lowest priority behaved differently before QPI, so it might coincide.) I'm still wondering about last sentence from that link, the parenthesised part to be exact, The reference to the APIC ID being 0xff is because 0xff is broadcast and lowest priority (what the RH bit really is for X86) is illegal with broadcast. Can you also check if RH=1 does something to delivery mode? I haven't seen any changes in the MSI Data Register for any values of RH, but I don't have a great sample size (one machine with one set of PCI devices), so if anyone else can confirm that I would appreciate it. I meant if the delivery mode from data register isn't ignored with RH=1, and the message delivered as if lowest-priority was set there. (Decided by having something else than fixed or lowest-priority there.) Hmm, any thoughts on how I could test for that? Worth noting that low prio delivery was used across the board for my PCI devices regardless of RH=1 or 0, so it doesn't seem to be de facto the case that the RH bit's only purpose is for lowprio delivery on x86. Yeah, afaik, it can be done with lowest priority delivery mode on ia64 too, so I have a hard time finding RH's intended purpose. Again, need to have some more PCI devices to test against to confirm anything. It's impossible to test everything, and there is no conflict if we have at most one data point ;) Very true :) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/20/2015 09:15 AM, Radim Krčmář wrote: 2015-03-19 16:51-0600, James Sullivan: I played around with native_compose_msi_msg and discovered the following: * dm=0, rh=0 = Physical Destination Mode * dm=0, rh=1 = Failed delivery * dm=1, rh=0 = Logical Destination Mode, No Redirection * dm=1, rh=1 = Logical Destination Mode, Redirection Great! (What CPU family was that?) This was on Intel x86_64 (Core i5-3210m, 'Ivy Bridge'). So it seems to be the case that logical destination mode is used whenever DM=1, regardless of RH. Furthermore, the case where DM=0 and RH=1 is undefined, as was indicated in the closing response to the thread in https://software.intel.com/en-us/forums/topic/23 : DM=0+RH=1 might be defined to fail, but I think it's acceptable to treat it as undefined. (Deliver them in KVM if it improves something.) My thoughts as well. I'm still wondering about last sentence from that link, the parenthesised part to be exact, The reference to the APIC ID being 0xff is because 0xff is broadcast and lowest priority (what the RH bit really is for X86) is illegal with broadcast. Can you also check if RH=1 does something to delivery mode? Thanks. Sure, I'll look into that as well. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/20/2015 09:22 AM, James Sullivan wrote: On 03/20/2015 09:15 AM, Radim Krčmář wrote: 2015-03-19 16:51-0600, James Sullivan: I played around with native_compose_msi_msg and discovered the following: * dm=0, rh=0 = Physical Destination Mode * dm=0, rh=1 = Failed delivery * dm=1, rh=0 = Logical Destination Mode, No Redirection * dm=1, rh=1 = Logical Destination Mode, Redirection Great! (What CPU family was that?) This was on Intel x86_64 (Core i5-3210m, 'Ivy Bridge'). So it seems to be the case that logical destination mode is used whenever DM=1, regardless of RH. Furthermore, the case where DM=0 and RH=1 is undefined, as was indicated in the closing response to the thread in https://software.intel.com/en-us/forums/topic/23 : DM=0+RH=1 might be defined to fail, but I think it's acceptable to treat it as undefined. (Deliver them in KVM if it improves something.) My thoughts as well. I'm still wondering about last sentence from that link, the parenthesised part to be exact, The reference to the APIC ID being 0xff is because 0xff is broadcast and lowest priority (what the RH bit really is for X86) is illegal with broadcast. Can you also check if RH=1 does something to delivery mode? Thanks. Sure, I'll look into that as well. -James I haven't seen any changes in the MSI Data Register for any values of RH, but I don't have a great sample size (one machine with one set of PCI devices), so if anyone else can confirm that I would appreciate it. Worth noting that low prio delivery was used across the board for my PCI devices regardless of RH=1 or 0, so it doesn't seem to be de facto the case that the RH bit's only purpose is for lowprio delivery on x86. Again, need to have some more PCI devices to test against to confirm anything. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/19/2015 07:00 AM, Radim Krčmář wrote: 2015-03-18 22:09-0300, Marcelo Tosatti: See native_compose_msi_msg: ((apic-irq_dest_mode == 0) ? MSI_ADDR_DEST_MODE_PHYSICAL : MSI_ADDR_DEST_MODE_LOGICAL) | ((apic-irq_delivery_mode != dest_LowestPrio) ? MSI_ADDR_REDIRECTION_CPU : MSI_ADDR_REDIRECTION_LOWPRI) | So it does configure DM = MSI_ADDR_DEST_MODE_LOGICAL and RH = MSI_ADDR_REDIRECTION_LOWPRI. ...and yet this is a good counterexample against my argument :) (It could be just to make the code nicer ... the developer might have known how real hardware will handle it.) What I think I'll do is revert this particular change so that dest_mode is set independently of RH. While I'm not entirely convinced that this is the intended interpretation, I do think that consistency with the existing logic is probably desirable for the time being. If I can get closure on the matter I'll re-submit that change, but for the time being I will undo it. Just write MSI-X table entries on real hardware (say: modify native_compose_msi_msg or MSI-X equivalent), with all RH/DM combinations, and see what behaviour is comes up? I second this idea. (We'd also get to know how RH interacts with delivery mode.) I played around with native_compose_msi_msg and discovered the following: * dm=0, rh=0 = Physical Destination Mode * dm=0, rh=1 = Failed delivery * dm=1, rh=0 = Logical Destination Mode, No Redirection * dm=1, rh=1 = Logical Destination Mode, Redirection So it seems to be the case that logical destination mode is used whenever DM=1, regardless of RH. Furthermore, the case where DM=0 and RH=1 is undefined, as was indicated in the closing response to the thread in https://software.intel.com/en-us/forums/topic/23 : ...X86 supports logical mode but does not support redirection of physical mode interrupts. I think they should have just said RH=1 is not defined in physical mode, but instead they are trying to tell you what the restrictions are if you set it on. The default config for PCI devices seems to be DM=1,RH=1. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery
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.jame...@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.3030...@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
Changes since v3:
* Reverted logic for setting dest_mode to RH=1/DM=1 (see
20150318225225.ga8...@amt.cnet; this is for consistency with
the interpretation of DM and RH in native_compose_msi_msg())
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/ioapic.c | 1 +
arch/x86/kvm/irq_comm.c | 3 ++-
arch/x86/kvm/lapic.c| 6 --
arch/x86/kvm/x86.c | 1 +
5 files changed, 9 insertions(+), 3 deletions(-)
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..80c10af 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -106,9 +106,10 @@ static inline void kvm_set_msi_irq(struct
kvm_kernel_irq_routing_entry *e,
irq-dest_mode = (1 MSI_ADDR_DEST_MODE_SHIFT) e-msi.address_lo;
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);
--
2.3.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] kvm: x86: Deliver MSI IRQ to only lowest prio cpu if msi_redir_hint is true
An MSI interrupt should only be delivered to the lowest priority CPU
when it has RH=1, regardless of the delivery mode. Modified
kvm_is_dm_lowest_prio() to check for either irq-delivery_mode == APIC_DM_LOWPRI
or irq-msi_redir_hint.
Moved kvm_is_dm_lowest_prio() into lapic.h and renamed to
kvm_lowest_prio_delivery().
Changed a check in kvm_irq_delivery_to_apic_fast() from
irq-delivery_mode == APIC_DM_LOWPRI to kvm_is_dm_lowest_prio().
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
Changes since v1:
* Squashed a number of smaller commits into this one commit,
which implements MSI delivery to only the lowest-priority
CPU whenever RH=1 using the above helper
kvm_lowest_prio_delivery().
Changes since v2:
* Patch formatting
Changes since v3:
N/A
arch/x86/kvm/irq_comm.c | 11 ---
arch/x86/kvm/lapic.c| 3 +--
arch/x86/kvm/lapic.h| 6 ++
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 80c10af..9efff9e 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -31,6 +31,8 @@
#include ioapic.h
+#include lapic.h
+
static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
struct kvm *kvm, int irq_source_id, int level,
bool line_status)
@@ -48,11 +50,6 @@ static int kvm_set_ioapic_irq(struct
kvm_kernel_irq_routing_entry *e,
line_status);
}
-inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
-{
- return irq-delivery_mode == APIC_DM_LOWEST;
-}
-
int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
struct kvm_lapic_irq *irq, unsigned long *dest_map)
{
@@ -60,7 +57,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
struct kvm_vcpu *vcpu, *lowest = NULL;
if (irq-dest_mode == 0 irq-dest_id == 0xff
- kvm_is_dm_lowest_prio(irq)) {
+ kvm_lowest_prio_delivery(irq)) {
printk(KERN_INFO kvm: apic: phys broadcast and lowest prio\n);
irq-delivery_mode = APIC_DM_FIXED;
}
@@ -76,7 +73,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
irq-dest_id, irq-dest_mode))
continue;
- if (!kvm_is_dm_lowest_prio(irq)) {
+ if (!kvm_lowest_prio_delivery(irq)) {
if (r 0)
r = 0;
r += kvm_apic_set_irq(vcpu, irq, dest_map);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index a15c444..f8b21d5 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -701,8 +701,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct
kvm_lapic *src,
dst = map-logical_map[cid];
bitmap = apic_logical_id(map, mda);
-
- if (irq-delivery_mode == APIC_DM_LOWEST) {
+ if (kvm_lowest_prio_delivery(irq)) {
int l = -1;
for_each_set_bit(i, bitmap, 16) {
if (!dst[i])
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 0bc6c65..ed7e2fa 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -168,6 +168,12 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu
*vcpu)
return vcpu-arch.apic-pending_events;
}
+static inline bool kvm_lowest_prio_delivery(struct kvm_lapic_irq *irq)
+{
+ return (irq-delivery_mode == APIC_DM_LOWEST ||
+ irq-msi_redir_hint);
+}
+
bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
void wait_lapic_expire(struct kvm_vcpu *vcpu);
--
2.3.3
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/2] kvm: x86: Implement handling of RH=1 for MSI delivery in KVM
Changes Since v1: * Reworked patches into two commits: 1) [Patch v2 1/2] Extended struct kvm_lapic_irq with bool msi_redir_hint * Initialize msi_redir_hint = true in kvm_set_msi_irq when RH=1 * Initialize msi_redir_hint = false otherwise * Added value of msi_redir_hint to debug message dump of IRQ in apic_send_ipi 2) [Patch v2 2/2] Deliver to only lowest prio CPU if msi_redir_hint is true * Move kvm_is_dm_lowest_prio() - lapic.h, rename to kvm_lowest_prio_delivery, set condition to (APIC_DM_LOWPRI || msi_redir_hint) * Change check in kvm_irq_delivery_to_apic_fast() for APIC_DM_LOWPRI or msi_redir_hint to a check for kvm_is_dm_lowest_prio() Changes since v2: * Extend Patch 1/2 (kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery) with older patch 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. (5502fedb.3030...@gmail.com) This was done to decouple the patch dependency and to collect all efforts to implement RH bit handling into one submission. * Patch formatting Changes since v3: * Revert logic for setting dest_mode; irq-dest_mode is now set independently of RH=1. (See 20150318225225.ga8...@amt.cnet). The reason for this is to maintain consistenty with the interpretation of MSI destination mode selection in native_compose_msi_msg(). This series of patches extends the KVM interrupt delivery mechanism to correctly account 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 in the KVM interrupt delivery mechanism. 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. In other words, the RH bit is ignored in physical destination mode, and when it is set in logical destination mode causes delivery to only apply to the lowest priority processor in the logical group. The IA32 manual is in slight contradiction with itself on this matter, but this patch agrees with this interpretation of the RH bit: https://software.intel.com/en-us/forums/topic/23 This patch has passed some rudimentary tests using an SMP QEMU guest and virtio sourced MSIs, but I haven't done experiments with passing through PCI hardware (intend to start working on this). -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
The documentation states the following: * When RH is 0, the interrupt is directed to the processor listed in the Destination ID field. * If RH is 0, then the DM bit is ignored and the message is sent ahead independent of whether the physical or logical destination mode is used. However, from the POV of a device writing to memory to generate an MSI interrupt, there is no (or i can't see any) other information that can be used to infer logical or physical mode for the interrupt message. Before your patch: (dm, rh) = (0, 0) = irq-dest_mode = 0 (dm, rh) = (0, 1) = irq-dest_mode = 0 (dm, rh) = (1, 0) = irq-dest_mode = 1 (dm, rh) = (1, 1) = irq-dest_mode = 1 After your patch: (dm, rh) = (0, 0) = irq-dest_mode = 0 (dm, rh) = (0, 1) = irq-dest_mode = 0 (dm, rh) = (1, 0) = irq-dest_mode = 0 (dm, rh) = (1, 1) = irq-dest_mode = 1 Am i missing some explicit documentation that refers to (dm, rh) = (1, 0) = irq-dest_mode = 0 ? From the IA32 manual (Vol. 3, 10.11.2): * When RH is 0, the interrupt is directed to the processor listed in the Destination ID field. * When RH is 1 and the physical destination mode is used, the Destination ID field must not be set to FFH; it must point to a processor that is present and enabled to receive the interrupt. * When RH is 1 and the logical destination mode is active in a system using a flat addressing model, the Destination ID field must be set so that bits set to 1 identify processors that are present and enabled to receive the interrupt. * If RH is set to 1 and the logical destination mode is active in a system using cluster addressing model, then Destination ID field must not be set to FFH; the processors identified with this field must be present and enabled to receive the interrupt. My interpretation of this is that RH=0 indicates that the Dest. ID field contains an APIC ID, and as such destination mode is physical. When RH=1, depending on the value of DM, we either use physical or logical dest mode. The result of this is that logical dest mode is set just when RH=1/DM=1, as far as I understand. See native_compose_msi_msg: msg-address_lo = MSI_ADDR_BASE_LO | ((apic-irq_dest_mode == 0) ? MSI_ADDR_DEST_MODE_PHYSICAL : MSI_ADDR_DEST_MODE_LOGICAL) | ((apic-irq_delivery_mode != dest_LowestPrio) ? MSI_ADDR_REDIRECTION_CPU : MSI_ADDR_REDIRECTION_LOWPRI) | MSI_ADDR_DEST_ID(dest); So it does configure DM = MSI_ADDR_DEST_MODE_LOGICAL and RH = MSI_ADDR_REDIRECTION_LOWPRI. ...and yet this is a good counterexample against my argument :) What I think I'll do is revert this particular change so that dest_mode is set independently of RH. While I'm not entirely convinced that this is the intended interpretation, I do think that consistency with the existing logic is probably desirable for the time being. If I can get closure on the matter I'll re-submit that change, but for the time being I will undo it. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
N.B. This patch has been re-submitted in a larger patch, see 1426555822-3280-1-git-send-email-sullivan.jame...@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 sullivan.jame...@gmail.com --- 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, -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] kvm: x86: Implement handling of RH=1 for MSI delivery in KVM
On 03/17/2015 02:19 AM, Jan Kiszka wrote: Once the kernel side is settled, would you mind looking into equivalent support for QEMU's APIC as well? Just to keep it easy to switch between both modes without too many functional restrictions. Thanks! Jan Good idea! I'm hoping to participate in GSoC working on shunting some interrupt handling devices out of the KVM and into QEMU, so that's definitely something I'll start to look into. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery
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.jame...@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.3030...@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(-)
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
[PATCH v3 2/2] kvm: x86: Deliver MSI IRQ to only lowest prio cpu if msi_redir_hint is true
An MSI interrupt should only be delivered to the lowest priority CPU
when it has RH=1, regardless of the delivery mode. Modified
kvm_is_dm_lowest_prio() to check for either irq-delivery_mode == APIC_DM_LOWPRI
or irq-msi_redir_hint.
Moved kvm_is_dm_lowest_prio() into lapic.h and renamed to
kvm_lowest_prio_delivery().
Changed a check in kvm_irq_delivery_to_apic_fast() from
irq-delivery_mode == APIC_DM_LOWPRI to kvm_is_dm_lowest_prio().
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
Changes since v1:
* Squashed a number of smaller commits into this one commit,
which implements MSI delivery to only the lowest-priority
CPU whenever RH=1 using the above helper
kvm_lowest_prio_delivery().
Changes since v2:
* Patch formatting
arch/x86/kvm/irq_comm.c | 11 ---
arch/x86/kvm/lapic.c| 3 +--
arch/x86/kvm/lapic.h| 6 ++
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 5c33cca..69cdaab 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -31,6 +31,8 @@
#include ioapic.h
+#include lapic.h
+
static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
struct kvm *kvm, int irq_source_id, int level,
bool line_status)
@@ -48,11 +50,6 @@ static int kvm_set_ioapic_irq(struct
kvm_kernel_irq_routing_entry *e,
line_status);
}
-inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
-{
- return irq-delivery_mode == APIC_DM_LOWEST;
-}
-
int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
struct kvm_lapic_irq *irq, unsigned long *dest_map)
{
@@ -60,7 +57,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
struct kvm_vcpu *vcpu, *lowest = NULL;
if (irq-dest_mode == 0 irq-dest_id == 0xff
- kvm_is_dm_lowest_prio(irq)) {
+ kvm_lowest_prio_delivery(irq)) {
printk(KERN_INFO kvm: apic: phys broadcast and lowest prio\n);
irq-delivery_mode = APIC_DM_FIXED;
}
@@ -76,7 +73,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
irq-dest_id, irq-dest_mode))
continue;
- if (!kvm_is_dm_lowest_prio(irq)) {
+ if (!kvm_lowest_prio_delivery(irq)) {
if (r 0)
r = 0;
r += kvm_apic_set_irq(vcpu, irq, dest_map);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index a15c444..f8b21d5 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -701,8 +701,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct
kvm_lapic *src,
dst = map-logical_map[cid];
bitmap = apic_logical_id(map, mda);
-
- if (irq-delivery_mode == APIC_DM_LOWEST) {
+ if (kvm_lowest_prio_delivery(irq)) {
int l = -1;
for_each_set_bit(i, bitmap, 16) {
if (!dst[i])
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 0bc6c65..ed7e2fa 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -168,6 +168,12 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu
*vcpu)
return vcpu-arch.apic-pending_events;
}
+static inline bool kvm_lowest_prio_delivery(struct kvm_lapic_irq *irq)
+{
+ return (irq-delivery_mode == APIC_DM_LOWEST ||
+ irq-msi_redir_hint);
+}
+
bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
void wait_lapic_expire(struct kvm_vcpu *vcpu);
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/2] kvm: x86: Implement handling of RH=1 for MSI delivery in KVM
Changes Since v1: * Reworked patches into two commits: 1) [Patch v2 1/2] Extended struct kvm_lapic_irq with bool msi_redir_hint * Initialize msi_redir_hint = true in kvm_set_msi_irq when RH=1 * Initialize msi_redir_hint = false otherwise * Added value of msi_redir_hint to debug message dump of IRQ in apic_send_ipi 2) [Patch v2 2/2] Deliver to only lowest prio CPU if msi_redir_hint is true * Move kvm_is_dm_lowest_prio() - lapic.h, rename to kvm_lowest_prio_delivery, set condition to (APIC_DM_LOWPRI || msi_redir_hint) * Change check in kvm_irq_delivery_to_apic_fast() for APIC_DM_LOWPRI or msi_redir_hint to a check for kvm_is_dm_lowest_prio() Changes since v2: * Extend Patch 1/2 (kvm: x86: Extended struct kvm_lapic_irq with msi_redir_hint for MSI delivery) with older patch 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. (5502fedb.3030...@gmail.com) This was done to decouple the patch dependency and to collect all efforts to implement RH bit handling into one submission. * Patch formatting This series of patches extends the KVM interrupt delivery mechanism to correctly account 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 in the KVM interrupt delivery mechanism. 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. In other words, the RH bit is ignored in physical destination mode, and when it is set in logical destination mode causes delivery to only apply to the lowest priority processor in the logical group. The IA32 manual is in slight contradiction with itself on this matter, but this patch agrees with this interpretation of the RH bit: https://software.intel.com/en-us/forums/topic/23 This patch has passed some rudimentary tests using an SMP QEMU guest and virtio sourced MSIs, but I haven't done experiments with passing through PCI hardware (intend to start working on this). -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] Extended struct kvm_lapic_irq with bool msi_redir_hint for MSI delivery
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
The following changes are added in this patch:
* Initialize msi_redir_hint = true in kvm_set_msi_irq when RH=1
* Initialize msi_redir_hint = false otherwise
* Added value of msi_redir_hint to debug message dump of IRQ in
apic_send_ipi
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.
This patch relies on a past submission that adds a check to
kvm_set_msi_irq() for RH=1, setting APIC_DEST_LOGICAL only when
RH=1/DM=1 and otherwise defaulting to APIC_DEST_PHYSICAL.
(See 5502fedb.3030...@gmail.com)
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/ioapic.c | 1 +
arch/x86/kvm/irq_comm.c | 11 ++-
arch/x86/kvm/lapic.c| 6 --
arch/x86/kvm/x86.c | 1 +
5 files changed, 9 insertions(+), 11 deletions(-)
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 f2887ea..5c33cca 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -103,12 +103,6 @@ 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;
- /*
-* 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;
@@ -116,9 +110,8 @@ static inline void kvm_set_msi_irq(struct
kvm_kernel_irq_routing_entry *e,
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-msi_redir_hint = ((e-msi.address_lo
+MSI_ADDR_REDIRECTION_LOWPRI) 0);
irq-level = 1;
irq-shorthand = 0;
}
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);
--
2.3.1
--
To unsubscribe from this list: send
[PATCH v2 0/2] Implement handling of RH=1 for MSI delivery in KVM
Changes Since v1: * Reworked patches into two commits: 1) [Patch v2 1/2] Extended struct kvm_lapic_irq with bool msi_redir_hint * Initialize msi_redir_hint = true in kvm_set_msi_irq when RH=1 * Initialize msi_redir_hint = false otherwise * Added value of msi_redir_hint to debug message dump of IRQ in apic_send_ipi 2) [Patch v2 2/2] Deliver to only lowest prio CPU if msi_redir_hint is true * Move kvm_is_dm_lowest_prio() - lapic.h, rename to kvm_lowest_prio_delivery, set condition to (APIC_DM_LOWPRI || msi_redir_hint) * Change check in kvm_irq_delivery_to_apic_fast() for APIC_DM_LOWPRI or msi_redir_hint to a check for kvm_is_dm_lowest_prio() (The patch relies on the changes made in a prior patch that adds a check for the RH bit in kvm_set_msi_irq(); see 5502fedb.3030...@gmail.com) This series of patches extends the KVM interrupt delivery mechanism to correctly account 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 in the KVM interrupt delivery mechanism. 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. In other words, the RH bit is ignored in physical destination mode, and when it is set in logical destination mode causes delivery to only apply to the lowest priority processor in the logical group. The IA32 manual is in slight contradiction with itself on this matter, but this patch agrees with this interpretation of the RH bit: https://software.intel.com/en-us/forums/topic/23 This patch has passed some rudimentary tests using an SMP QEMU guest and virtio sourced MSIs, but I haven't done experiments with passing through PCI hardware (intend to start working on this). Let me know your thoughts. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] Deliver to only lowest prio cpu if msi_redir_hint is true
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
The following changes are added in this patch:
* Move kvm_is_dm_lowest_prio() - lapic.h, rename to
kvm_lowest_prio_delivery, set condition to
(APIC_DM_LOWPRI || msi_redir_hint)
* Change check in kvm_irq_delivery_to_apic_fast() for
APIC_DM_LOWPRI or msi_redir_hint to a check for
kvm_is_dm_lowest_prio()
Changes since v1:
* Squashed a number of smaller commits into this one commit,
which implements MSI delivery to only the lowest-priority
CPU whenever RH=1 using the above helper
kvm_lowest_prio_delivery().
This patch relies on a past submission that adds a check to
kvm_set_msi_irq() for RH=1, setting APIC_DEST_LOGICAL only when
RH=1/DM=1 and otherwise defaulting to APIC_DEST_PHYSICAL.
(See 5502fedb.3030...@gmail.com)
arch/x86/kvm/irq_comm.c | 11 ---
arch/x86/kvm/lapic.c| 3 +--
arch/x86/kvm/lapic.h| 6 ++
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 5c33cca..69cdaab 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -31,6 +31,8 @@
#include ioapic.h
+#include lapic.h
+
static int kvm_set_pic_irq(struct kvm_kernel_irq_routing_entry *e,
struct kvm *kvm, int irq_source_id, int level,
bool line_status)
@@ -48,11 +50,6 @@ static int kvm_set_ioapic_irq(struct
kvm_kernel_irq_routing_entry *e,
line_status);
}
-inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
-{
- return irq-delivery_mode == APIC_DM_LOWEST;
-}
-
int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
struct kvm_lapic_irq *irq, unsigned long *dest_map)
{
@@ -60,7 +57,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
struct kvm_vcpu *vcpu, *lowest = NULL;
if (irq-dest_mode == 0 irq-dest_id == 0xff
- kvm_is_dm_lowest_prio(irq)) {
+ kvm_lowest_prio_delivery(irq)) {
printk(KERN_INFO kvm: apic: phys broadcast and lowest prio\n);
irq-delivery_mode = APIC_DM_FIXED;
}
@@ -76,7 +73,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
irq-dest_id, irq-dest_mode))
continue;
- if (!kvm_is_dm_lowest_prio(irq)) {
+ if (!kvm_lowest_prio_delivery(irq)) {
if (r 0)
r = 0;
r += kvm_apic_set_irq(vcpu, irq, dest_map);
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index a15c444..f8b21d5 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -701,8 +701,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct
kvm_lapic *src,
dst = map-logical_map[cid];
bitmap = apic_logical_id(map, mda);
-
- if (irq-delivery_mode == APIC_DM_LOWEST) {
+ if (kvm_lowest_prio_delivery(irq)) {
int l = -1;
for_each_set_bit(i, bitmap, 16) {
if (!dst[i])
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 0bc6c65..ed7e2fa 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -168,6 +168,12 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu
*vcpu)
return vcpu-arch.apic-pending_events;
}
+static inline bool kvm_lowest_prio_delivery(struct kvm_lapic_irq *irq)
+{
+ return (irq-delivery_mode == APIC_DM_LOWEST ||
+ irq-msi_redir_hint);
+}
+
bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
void wait_lapic_expire(struct kvm_vcpu *vcpu);
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] Removed TODO in kvm_set_msi_irq
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/irq_comm.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 36d2ca3a..f993f2f 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -103,12 +103,6 @@ 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; - /* -* 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; -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] Extended kvm_lapic_irq struct with 'bool redir_hint' for MSI delivery
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
arch/x86/include/asm/kvm_host.h | 1 +
1 file changed, 1 insertion(+)
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 {
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] Set irq-msi_redir_hint = 1 in kvm_set_msi_irq if RH=1
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/irq_comm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index f2887ea..7e0f469 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -116,9 +116,8 @@ static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e, 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-msi_redir_hint = ((e-msi.address_lo +MSI_ADDR_REDIRECTION_LOWPRI) 0); irq-level = 1; irq-shorthand = 0; } -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] Set default value for msi_redir_hint=false in kvm_pv_kick_cpu_op
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/x86.c | 1 + 1 file changed, 1 insertion(+) 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); -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] Deliver to only low-prio cpu in kvm_irq_delivery_to_apic_fast when MSI RH=1
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
arch/x86/kvm/lapic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index c26afc9..c946470 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -702,7 +702,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct
kvm_lapic *src,
bitmap = apic_logical_id(map, mda);
- if (irq-delivery_mode == APIC_DM_LOWEST) {
+ if (irq-delivery_mode == APIC_DM_LOWEST ||
+ irq-msi_redir_hint) {
int l = -1;
for_each_set_bit(i, bitmap, 16) {
if (!dst[i])
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC 0/9] Implement handling of RH=1 for MSI delivery in KVM
This series of patches extends the KVM interrupt delivery mechanism to correctly account 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 in the KVM interrupt delivery mechanism. 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. In other words, the RH bit is ignored in physical destination mode, and when it is set in logical destination mode causes delivery to only apply to the lowest priority processor in the logical group. The IA32 manual is in slight contradiction with itself on this matter, but this patch agrees with this interpretation of the RH bit: https://software.intel.com/en-us/forums/topic/23 This patch has passed some rudimentary tests using an SMP QEMU guest and virtio sourced MSIs, but I haven't done experiments with passing through PCI hardware (intend to start working on this). Let me know your thoughts. -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] Print value of msi_redir_hint in debug dump of irq in apic_send_ipi
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/lapic.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index c946470..bced6d5 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -903,10 +903,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); } -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] Prevent delivery to non-lowest priority vcpus in kvm_irq_delivery_to_apic
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
arch/x86/kvm/irq_comm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 7e0f469..36d2ca3a 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -76,7 +76,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
kvm_lapic *src,
irq-dest_id, irq-dest_mode))
continue;
- if (!kvm_is_dm_lowest_prio(irq)) {
+ if (!kvm_is_dm_lowest_prio(irq) !irq-msi_redir_hint) {
if (r 0)
r = 0;
r += kvm_apic_set_irq(vcpu, irq, dest_map);
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] Set default value for msi_redir_hint=false in apic_send_ipi
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/lapic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index bd4e34d..c26afc9 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 -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] Set default value for msi_redir_hint=false in ioapic_service
Signed-off-by: James Sullivan sullivan.jame...@gmail.com --- arch/x86/kvm/ioapic.c | 1 + 1 file changed, 1 insertion(+) 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); -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v4] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/13/2015 08:39 AM, Radim Krčmář wrote: ... The warning message is very clever: - it contains the magical may qualifier and being protected only by RH=1 creates weird-looking code structure, but it is technically right 1) lowest-priority delivery may be set in msi.data, which avoids our otherwise incorrect behavior with RH=1/DM=1 2) RH=1/DM=0 can't deliver to multiple APICs (broadcast is forbidden), but real hardware may overwrite delivery mode from msi.data - being two lines apart adds to suspicion, yet it can be hint to those possible problems I only fear it is too clever :) For the error message, how does: kvm: MSI RH=1 unsupported, use low-priority delivery mode Sit with you? -James -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v4] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
Perfect, thanks for the feedback. I'll get v5 out shortly. On 03/13/2015 09:08 AM, Radim Krčmář wrote: 2015-03-13 08:47-0600, James Sullivan: On 03/13/2015 08:39 AM, Radim Krčmář wrote: ... The warning message is very clever: - it contains the magical may qualifier and being protected only by RH=1 creates weird-looking code structure, but it is technically right 1) lowest-priority delivery may be set in msi.data, which avoids our otherwise incorrect behavior with RH=1/DM=1 2) RH=1/DM=0 can't deliver to multiple APICs (broadcast is forbidden), but real hardware may overwrite delivery mode from msi.data - being two lines apart adds to suspicion, yet it can be hint to those possible problems I only fear it is too clever :) For the error message, how does: kvm: MSI RH=1 unsupported, use low-priority delivery mode Sit with you? I actually liked the former. New one doesn't say what is the impact of the error and the advice is not easy follow -- people usually have no idea what low-priority delivery mode is and nothing can be done outside of the guest. (I put the rant mainly for future reviewers; the alternative I had in was to warn only when DM=1.) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
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 sullivan.jame...@gmail.com --- 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, -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Patch v3] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
(Disregard past message- submitted the wrong patch file, apologies.) 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 sullivan.jame...@gmail.com --- 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) arch/x86/kvm/irq_comm.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 72298b3..35b4beb 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -103,12 +103,25 @@ 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 ^ + 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch v4] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
(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 sullivan.jame...@gmail.com --- 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Patch v3] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
This patch adds a check in kvm_set_msi_irq for RH=1. 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. 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 sullivan.jame...@gmail.com --- 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) arch/x86/kvm/irq_comm.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c index 72298b3..5975a3d 100644 --- a/arch/x86/kvm/irq_comm.c +++ b/arch/x86/kvm/irq_comm.c @@ -103,12 +103,25 @@ 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) ^ + (MS_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 + to multiple APICs 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
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 sullivan.jame...@gmail.com
---
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/11/2015 07:43 AM, Radim Krčmář wrote: 2015-03-10 16:39-0600, James Sullivan: On 03/10/2015 08:47 AM, Radim Krčmář wrote: + irq-dest_mode = phys ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL); (Should be APIC_DEST_LOGICAL. All works because it is a boolean and we only checked for APIC_DEST_PHYSICAL, which is 0.) Thank you, that should be as follows: irq-dest_mode = phys ? (APIC_DEST_PHYSICAL) : (APIC_DEST_LOGICAL); ? Yes, thanks. (No need for parentheses around macros, we agreed to cover that in definition and they don't make this more readable.) I'll tidy that up too. Your patch improves the situation, but removing the TODO is not warranted -- RH still doesn't do what it should: How does DM=1/RH=1 work on real hardware? (There seem to be interesting corner cases with irq-delivery_mode like APIC_DM_NMI.) The IA32 manual says that if DM=1/RH=1, we operate in logical destination mode similarly to other IPIs. I don't believe this patch introduces any invalid settings listed in section 10-21, Vol. 3, so this shouldn't create any weirdness. Quoting SDM Jan 2015, 10.11.1 Message Address Register Format: This bit [RH] indicates whether the message should be directed to the processor with the lowest interrupt priority among processors that can receive the interrupt. = it should behave like lowest priority delivery. Deliver to just one APIC -- we don't do that. KVM interrupt delivery functions can only specify lowest priority though delivery_mode and we would have to rework KVM if MSI can set something else in the delivery_mode (like NMI). I see-- That will pose issues when you want some particular semantics of one delivery mode (such as ignoring vector info in NMI) but want low-pri delivery. Short of writing an MSI specific delivery function and calling it in kvm_set_msi, I can't think of a way to get this. Do you think that's warranted? I'll document this and resubmit this patch with the TODO for now. Thanks for the insight. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
On 03/10/2015 08:47 AM, Radim Krčmář wrote:
...
+/*
+ * Set dest_mode to logical just in case both the RH and DM
+ * bits are set, 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 ? 0 : (MSI_ADDR_DEST_MODE_LOGICAL);
(Should be APIC_DEST_LOGICAL. All works because it is a boolean and we
only checked for APIC_DEST_PHYSICAL, which is 0.)
Thank you, that should be as follows:
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 */
RH bit still isn't deal with -- we do not use lowest-priority-like
delivery in logical destination mode ...
Just want to make sure I understand this comment-
Isn't low-pri delivery mode used in kvm_irq_delivery_to_apic_fast
when irq-dest_mode APIC_DEST_PHYSICAL (ie, logical)? (See below.)
Do you mean that this patch will interfere with this? As long as
irq-dest_mode is still APIC_DEST_LOGICAL this shouldn't change.
bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
{
.
if (irq-dest_mode == APIC_DEST_PHYSICAL) {
if (irq-dest_id = ARRAY_SIZE(map-phys_map))
goto out;
dst = map-phys_map[irq-dest_id];
} else {
u32 mda = irq-dest_id (32 - map-ldr_bits);
u16 cid = apic_cluster_id(map, mda);
if (cid = ARRAY_SIZE(map-logical_map))
goto out;
dst = map-logical_map[cid];
bitmap = apic_logical_id(map, mda);
if (irq-delivery_mode == APIC_DM_LOWEST) {
int l = -1;
for_each_set_bit(i, bitmap, 16) {
if (!dst[i])
continue;
if (l 0)
l = i;
else if (kvm_apic_compare_prio(dst[i]-vcpu,
dst[l]-vcpu) 0)
l = i;
}
bitmap = (l = 0) ? 1 l : 0;
}
}
.
}
How does DM=1/RH=1 work on real hardware?
(There seem to be interesting corner cases with irq-delivery_mode like
APIC_DM_NMI.)
The IA32 manual says that if DM=1/RH=1, we operate in logical destination mode
similarly to other IPIs. I don't believe this patch introduces any invalid
settings listed in section 10-21, Vol. 3, so this shouldn't create any
weirdness.
Thanks
-James
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq
Hi folks,
This is a small patch that implements logic to handle the RH bit
being set in the msi message address register. 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 logical just when both bits are set,
and physical otherwise.
Signed-off-by: James Sullivan sullivan.jame...@gmail.com
---
arch/x86/kvm/irq_comm.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
index 72298b3..26bbab8 100644
--- a/arch/x86/kvm/irq_comm.c
+++ b/arch/x86/kvm/irq_comm.c
@@ -97,18 +97,26 @@ 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 the RH and DM
+* bits are set, 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 ? 0 : (MSI_ADDR_DEST_MODE_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 */
}
int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
--
2.3.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
