All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] x86: irq_remapping: Fix the regression of hpet irq remapping
       [not found] ` <1410512753-6428-1-git-send-email-wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
@ 2014-09-12  9:02   ` Jiang Liu
       [not found]     ` <5412B69C.8000202-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Jiang Liu @ 2014-09-12  9:02 UTC (permalink / raw)
  To: Yijing Wang, Joerg Roedel
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse



On 2014/9/12 17:05, Yijing Wang wrote:
> Commit 71054d8841b4 introduced x86_msi_ops.setup_hpet_msi
> to setup hpet MSI irq when irq remapping enabled.
> This caused a regression of hpet MSI irq remapping.
> 
> Original code flow before commit 71054d8841b4:
> hpet_setup_msi_irq()
> 	arch_setup_hpet_msi()
> 		setup_hpet_msi_remapped()
> 			remap_ops->setup_hpet_msi()
> 				alloc_irte()
> 		msi_compose_msg()
> 		hpet_msi_write()
> 		...
> 
> Current code flow after commit 71054d8841b4:
> hpet_setup_msi_irq()
> 	x86_msi.setup_hpet_msi()
> 		setup_hpet_msi_remapped()
> 			intel_setup_hpet_msi()
> 				alloc_irte()
> 
> 
> 
> Currently, we only alloc_irte() for hpet MSI, but
> have not composed and wrote its msg...
> 
> Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/irq_remapping.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
> index 85eb212..3541c71 100644
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -12,6 +12,7 @@
>  #include <asm/processor.h>
>  #include <asm/x86_init.h>
>  #include <asm/apic.h>
> +#include <asm/hpet.h>
>  
>  #include "irq_remapping.h"
>  
> @@ -332,10 +333,16 @@ static int msi_setup_remapped_irq(struct pci_dev *pdev, unsigned int irq,
>  
>  static int setup_hpet_msi_remapped(unsigned int irq, unsigned int id)
>  {
> +	int ret;
> +
>  	if (!remap_ops || !remap_ops->setup_hpet_msi)
>  		return -ENODEV;
>  
> -	return remap_ops->setup_hpet_msi(irq, id);
> +	ret = remap_ops->setup_hpet_msi(irq, id);
> +	if (ret)
> +		return -EINVAL;
> +
> +	return default_setup_hpet_msi(irq, id);
>  }
Hi Yijing,
	How about move default_setup_hpet_msi(irq, id) into
intel_setup_hpet_msi and amd.setup_hpet_msi?
Regards!
Gerry
>  
>  void panic_if_irq_remap(const char *msg)
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH] x86: irq_remapping: Fix the regression of hpet irq remapping
@ 2014-09-12  9:05 Yijing Wang
       [not found] ` <1410512753-6428-1-git-send-email-wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Yijing Wang @ 2014-09-12  9:05 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	David Woodhouse, Jiang Liu

Commit 71054d8841b4 introduced x86_msi_ops.setup_hpet_msi
to setup hpet MSI irq when irq remapping enabled.
This caused a regression of hpet MSI irq remapping.

Original code flow before commit 71054d8841b4:
hpet_setup_msi_irq()
	arch_setup_hpet_msi()
		setup_hpet_msi_remapped()
			remap_ops->setup_hpet_msi()
				alloc_irte()
		msi_compose_msg()
		hpet_msi_write()
		...

Current code flow after commit 71054d8841b4:
hpet_setup_msi_irq()
	x86_msi.setup_hpet_msi()
		setup_hpet_msi_remapped()
			intel_setup_hpet_msi()
				alloc_irte()



Currently, we only alloc_irte() for hpet MSI, but
have not composed and wrote its msg...

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/irq_remapping.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 85eb212..3541c71 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -12,6 +12,7 @@
 #include <asm/processor.h>
 #include <asm/x86_init.h>
 #include <asm/apic.h>
+#include <asm/hpet.h>
 
 #include "irq_remapping.h"
 
@@ -332,10 +333,16 @@ static int msi_setup_remapped_irq(struct pci_dev *pdev, unsigned int irq,
 
 static int setup_hpet_msi_remapped(unsigned int irq, unsigned int id)
 {
+	int ret;
+
 	if (!remap_ops || !remap_ops->setup_hpet_msi)
 		return -ENODEV;
 
-	return remap_ops->setup_hpet_msi(irq, id);
+	ret = remap_ops->setup_hpet_msi(irq, id);
+	if (ret)
+		return -EINVAL;
+
+	return default_setup_hpet_msi(irq, id);
 }
 
 void panic_if_irq_remap(const char *msg)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86: irq_remapping: Fix the regression of hpet irq remapping
       [not found]     ` <5412B69C.8000202-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2014-09-12  9:13       ` Yijing Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Yijing Wang @ 2014-09-12  9:13 UTC (permalink / raw)
  To: Jiang Liu, Joerg Roedel
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, David Woodhouse

On 2014/9/12 17:02, Jiang Liu wrote:
> 
> 
> On 2014/9/12 17:05, Yijing Wang wrote:
>> Commit 71054d8841b4 introduced x86_msi_ops.setup_hpet_msi
>> to setup hpet MSI irq when irq remapping enabled.
>> This caused a regression of hpet MSI irq remapping.
>>
>> Original code flow before commit 71054d8841b4:
>> hpet_setup_msi_irq()
>> 	arch_setup_hpet_msi()
>> 		setup_hpet_msi_remapped()
>> 			remap_ops->setup_hpet_msi()
>> 				alloc_irte()
>> 		msi_compose_msg()
>> 		hpet_msi_write()
>> 		...
>>
>> Current code flow after commit 71054d8841b4:
>> hpet_setup_msi_irq()
>> 	x86_msi.setup_hpet_msi()
>> 		setup_hpet_msi_remapped()
>> 			intel_setup_hpet_msi()
>> 				alloc_irte()
>>
>>
>>
>> Currently, we only alloc_irte() for hpet MSI, but
>> have not composed and wrote its msg...
>>
>> Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/iommu/irq_remapping.c |    9 ++++++++-
>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
>> index 85eb212..3541c71 100644
>> --- a/drivers/iommu/irq_remapping.c
>> +++ b/drivers/iommu/irq_remapping.c
>> @@ -12,6 +12,7 @@
>>  #include <asm/processor.h>
>>  #include <asm/x86_init.h>
>>  #include <asm/apic.h>
>> +#include <asm/hpet.h>
>>  
>>  #include "irq_remapping.h"
>>  
>> @@ -332,10 +333,16 @@ static int msi_setup_remapped_irq(struct pci_dev *pdev, unsigned int irq,
>>  
>>  static int setup_hpet_msi_remapped(unsigned int irq, unsigned int id)
>>  {
>> +	int ret;
>> +
>>  	if (!remap_ops || !remap_ops->setup_hpet_msi)
>>  		return -ENODEV;
>>  
>> -	return remap_ops->setup_hpet_msi(irq, id);
>> +	ret = remap_ops->setup_hpet_msi(irq, id);
>> +	if (ret)
>> +		return -EINVAL;
>> +
>> +	return default_setup_hpet_msi(irq, id);
>>  }
> Hi Yijing,
> 	How about move default_setup_hpet_msi(irq, id) into
> intel_setup_hpet_msi and amd.setup_hpet_msi?

Hmmm, because the common irq remapping code is placed at irq_remapping.c, so
what about change remap_ops->setup_hpet_msi to remap_ops->alloc_remapping_irq(),

  static int setup_hpet_msi_remapped(unsigned int irq, unsigned int id)
  {
 +	int ret;
 +
  	if (!remap_ops || !remap_ops->alloc_remapping_irq)
  		return -ENODEV;

 -	return remap_ops->alloc_remapping_irq(irq, id);
 +	ret = remap_ops->alloc_remapping_irq(irq, id);
 +	if (ret)
 +		return -EINVAL;
 +
 +	return default_setup_hpet_msi(irq, id);
  }

Thanks!
Yijing.

> Regards!
> Gerry
>>  
>>  void panic_if_irq_remap(const char *msg)
>>
> 
> .
> 


-- 
Thanks!
Yijing

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-12  9:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-12  9:05 [PATCH] x86: irq_remapping: Fix the regression of hpet irq remapping Yijing Wang
     [not found] ` <1410512753-6428-1-git-send-email-wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2014-09-12  9:02   ` Jiang Liu
     [not found]     ` <5412B69C.8000202-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-09-12  9:13       ` Yijing Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.