Re: [PATCH] x86: irq_remapping: Fix the regression of hpet irq remapping

[Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>]

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