x86: irq_comm: Add check for RH bit in kvm_set_msi_irq

---
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

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 <[email protected]>

---
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(-)

arch/x86/kvm/irq_comm.c +14 -2

index 72298b3..f2887ea 100644
@@ -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,

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 <[email protected]>

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):

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.

Quoted 28 lines

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

Quoted 4 lines

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.

Quoted 14 lines

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);

Quoted 3 lines

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

Quoted 28 lines

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

Quoted 4 lines

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.

Quoted 14 lines

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);

Quoted 3 lines

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

Quoted 10 lines

Quoted 9 lines

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 :)

Quoted 5 lines

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?

2015-03-18 22:09-0300, Marcelo Tosatti:

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

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.)

Quoted 5 lines

Quoted 5 lines

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.

Quoted 4 lines

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/288883 :

2015-03-19 16:51-0600, James Sullivan:

Quoted 6 lines

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?)

Quoted 4 lines

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/288883 :

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.)

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.

Quoted 10 lines · Radim Krčmář

2015-03-19 16:51-0600, James Sullivan:

Quoted 6 lines

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').

Quoted 8 lines

Quoted 4 lines

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/288883 :

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.

Quoted 6 lines

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.

Quoted 4 lines

Can you also check if RH=1 does something to delivery mode?

Thanks.

Sure, I'll look into that as well.

-James

Quoted 9 lines · Radim Krčmář

Quoted 8 lines

2015-03-19 16:51-0600, James Sullivan:

Quoted 6 lines

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

Quoted 4 lines

Quoted 2 lines

Great! (What CPU family was that?)

This was on Intel x86_64 (Core i5-3210m, 'Ivy Bridge').

Quoted 7 lines

Quoted 7 lines

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.

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.

2015-03-20 11:50-0600, James Sullivan:

Quoted 10 lines · James Sullivan

Quoted 9 lines · Radim Krčmář

Quoted 8 lines

2015-03-19 16:51-0600, James Sullivan:

Quoted 6 lines

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

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

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.)

Quoted 8 lines

Quoted 7 lines

Quoted 7 lines

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?

Quoted 3 lines

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.)

Quoted 3 lines

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.

Quoted 2 lines

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 ;)

Quoted 4 lines

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?

2015-03-23 16:46-0600, James Sullivan:

Quoted 7 lines · Radim Krčmář

Quoted 4 lines

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.

arch/x86/include/asm/msidef.h +1 -0

index 4cc48af23fef..2270e459186b 100644
@@ -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)

arch/x86/kernel/apic/msi.c +0 -0

index d6ba2d660dc5..4f71737c34eb 100644

2015-03-23 16:46-0600, James Sullivan:

Quoted 7 lines · Radim Krčmář

Quoted 4 lines

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.

arch/x86/include/asm/msidef.h +1 -0

index 4cc48af23fef..2270e459186b 100644
@@ -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)

arch/x86/kernel/apic/msi.c +0 -0

index d6ba2d660dc5..4f71737c34eb 100644

Quoted 4 lines

2015-03-23 16:46-0600, James Sullivan:

Quoted 1 line · Radim Krčmář

I tested it and Linux got a lot of unexpected NMIs, so the emulation in your latest patch looks correct.

Good to hear, thanks for testing that.

Any other tweaks you think are necessary for the patch set?

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/288883 :

Quoted 7 lines

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

Quoted 4 lines

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/288883 :

So this means this patch is not necessary, right?