diff mbox

[v5] x86: irq_comm: Add check for RH bit in kvm_set_msi_irq

Message ID 20150324140301.GD21522@potion.brq.redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář March 24, 2015, 2:03 p.m. UTC 
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.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

James Sullivan March 24, 2015, 2:17 p.m. UTC | #1 
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Sullivan April 2, 2015, 10:08 p.m. UTC | #2 
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Radim Krčmář April 3, 2015, 10:11 a.m. UTC | #3 
2015-04-02 16:08-0600, James Sullivan:
> 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 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?

No, I thought it fell off my list because I reviewed it; will do so now.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/include/asm/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);
 }