Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1590966 > unrolled thread

Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap Auger Eric <eric.auger@redhat.com> - 2017-03-02 11:30 +0100
    Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap Mian Yousaf Kaukab <yousaf.kaukab@suse.com> - 2017-03-02 13:30 +0100
      Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap Mian Yousaf Kaukab <yousaf.kaukab@suse.com> - 2017-03-02 14:40 +0100
      Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap Auger Eric <eric.auger@redhat.com> - 2017-03-02 15:10 +0100

#1590966 — Re: [PATCH 1/2] irqdomain: add empty irq_domain_check_msi_remap

Hi Mian Yousaf,

On 02/03/2017 11:01, Mian Yousaf Kaukab wrote:
> Fix following build error for s390:
> drivers/vfio/vfio_iommu_type1.c: In function 'vfio_iommu_type1_attach_group':
> drivers/vfio/vfio_iommu_type1.c:1290:25: error: implicit declaration of function 'irq_domain_check_msi_remap'
> 
> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
> ---
>  include/linux/irqdomain.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 188eced6813e..137817b08cdc 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -524,6 +524,10 @@ static inline struct irq_domain *irq_find_matching_fwnode(
>  {
>  	return NULL;
>  }
> +static inline bool irq_domain_check_msi_remap(void)
> +{
> +	return true;
By default you should rather return false, reporting there is no MSI
remapping capability on irq domain side. Besides thank you for the fix.

Best Regards

Eric
> +}
>  #endif /* !CONFIG_IRQ_DOMAIN */
>  
>  #endif /* _LINUX_IRQDOMAIN_H */
> 

[toc] | [next] | [standalone]

#1591036

On 03/02/2017 11:24 AM, Auger Eric wrote:
> Hi Mian Yousaf,
> 
> On 02/03/2017 11:01, Mian Yousaf Kaukab wrote:
>> Fix following build error for s390:
>> drivers/vfio/vfio_iommu_type1.c: In function 'vfio_iommu_type1_attach_group':
>> drivers/vfio/vfio_iommu_type1.c:1290:25: error: implicit declaration of function 'irq_domain_check_msi_remap'
>>
>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
>> ---
>>   include/linux/irqdomain.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index 188eced6813e..137817b08cdc 100644
>> --- a/include/linux/irqdomain.h
>> +++ b/include/linux/irqdomain.h
>> @@ -524,6 +524,10 @@ static inline struct irq_domain *irq_find_matching_fwnode(
>>   {
>>   	return NULL;
>>   }
>> +static inline bool irq_domain_check_msi_remap(void)
>> +{
>> +	return true;
> By default you should rather return false, reporting there is no MSI
> remapping capability on irq domain side. Besides thank you for the fix.
I choose to return true based on the function header comments of 
irq_domain_check_msi_remap. It says

"Return: false if any MSI irq domain does not support IRQ remapping, 
true otherwise (including if there is no MSI irq domain)"

So function should return true in case of no MSI irq domain. Have I miss 
understood this?

BR,
Yousaf

[toc] | [prev] | [next] | [standalone]

#1591081

On 03/02/2017 02:12 PM, Auger Eric wrote:
> Hi Yousaf,
> 
> On 02/03/2017 13:23, Mian Yousaf Kaukab wrote:
>> On 03/02/2017 11:24 AM, Auger Eric wrote:
>>> Hi Mian Yousaf,
>>>
>>> On 02/03/2017 11:01, Mian Yousaf Kaukab wrote:
>>>> Fix following build error for s390:
>>>> drivers/vfio/vfio_iommu_type1.c: In function
>>>> 'vfio_iommu_type1_attach_group':
>>>> drivers/vfio/vfio_iommu_type1.c:1290:25: error: implicit declaration
>>>> of function 'irq_domain_check_msi_remap'
>>>>
>>>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
>>>> ---
>>>>    include/linux/irqdomain.h | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>>> index 188eced6813e..137817b08cdc 100644
>>>> --- a/include/linux/irqdomain.h
>>>> +++ b/include/linux/irqdomain.h
>>>> @@ -524,6 +524,10 @@ static inline struct irq_domain
>>>> *irq_find_matching_fwnode(
>>>>    {
>>>>        return NULL;
>>>>    }
>>>> +static inline bool irq_domain_check_msi_remap(void)
>>>> +{
>>>> +    return true;
>>> By default you should rather return false, reporting there is no MSI
>>> remapping capability on irq domain side. Besides thank you for the fix.
>> I choose to return true based on the function header comments of
>> irq_domain_check_msi_remap. It says
>>
>> "Return: false if any MSI irq domain does not support IRQ remapping,
>> true otherwise (including if there is no MSI irq domain)"
>>
>> So function should return true in case of no MSI irq domain. Have I miss
>> understood this?
> This behavior is indeed mandated on ARM - where MSI are translated by
> the smmu - to allow safe device assignment if there is no MSI domain,
> ie. in this situation there is no risk an assigned device writes into an
> MSI doorbell.
> 
> As the function is not implemented at all in your case, personally I
> would rather be defensive though and return false. You were not able to
> check the capability.
OK Agree. I will send an update as soon as a decision is made on 2/2.

BR,
Yousaf

[toc] | [prev] | [next] | [standalone]

#1591114

Hi Yousaf,

On 02/03/2017 13:23, Mian Yousaf Kaukab wrote:
> On 03/02/2017 11:24 AM, Auger Eric wrote:
>> Hi Mian Yousaf,
>>
>> On 02/03/2017 11:01, Mian Yousaf Kaukab wrote:
>>> Fix following build error for s390:
>>> drivers/vfio/vfio_iommu_type1.c: In function
>>> 'vfio_iommu_type1_attach_group':
>>> drivers/vfio/vfio_iommu_type1.c:1290:25: error: implicit declaration
>>> of function 'irq_domain_check_msi_remap'
>>>
>>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
>>> ---
>>>   include/linux/irqdomain.h | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>> index 188eced6813e..137817b08cdc 100644
>>> --- a/include/linux/irqdomain.h
>>> +++ b/include/linux/irqdomain.h
>>> @@ -524,6 +524,10 @@ static inline struct irq_domain
>>> *irq_find_matching_fwnode(
>>>   {
>>>       return NULL;
>>>   }
>>> +static inline bool irq_domain_check_msi_remap(void)
>>> +{
>>> +    return true;
>> By default you should rather return false, reporting there is no MSI
>> remapping capability on irq domain side. Besides thank you for the fix.
> I choose to return true based on the function header comments of
> irq_domain_check_msi_remap. It says
> 
> "Return: false if any MSI irq domain does not support IRQ remapping,
> true otherwise (including if there is no MSI irq domain)"
> 
> So function should return true in case of no MSI irq domain. Have I miss
> understood this?
This behavior is indeed mandated on ARM - where MSI are translated by
the smmu - to allow safe device assignment if there is no MSI domain,
ie. in this situation there is no risk an assigned device writes into an
MSI doorbell.

As the function is not implemented at all in your case, personally I
would rather be defensive though and return false. You were not able to
check the capability.

Thanks

Eric
> 
> BR,
> Yousaf

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web