Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

On 26/04/18 20:50, Thomas Gleixner wrote:
> On Mon, 23 Apr 2018, Marc Zyngier wrote:
> 
>> Nobody would be insane enough to try and use level triggered
>> MSIs on PCI, but let's make sure it doesn't happen. Also,
>> let's mandate that the irqchip backing the platform MSI domain
>> is providing an irq_set_type method (or the whole thing would
>> be a bit useless).
>>
>> Signed-off-by: Marc Zyngier 
>> ---
>>  drivers/base/platform-msi.c | 3 +++
>>  drivers/pci/msi.c   | 3 +++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
>> index 8e22073aeeed..3b9694a6beaa 100644
>> --- a/drivers/base/platform-msi.c
>> +++ b/drivers/base/platform-msi.c
>> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
>> msi_domain_info *info)
>>  chip->irq_set_affinity = msi_domain_set_affinity;
>>  if (!chip->irq_write_msi_msg)
>>  chip->irq_write_msi_msg = platform_msi_write_msg;
>> +if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
>> +!chip->irq_set_type))
> 
> Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
> in chip->flags being set instead?
That definitely makes sense. Thanks for the suggestion.

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

On 26/04/18 20:50, Thomas Gleixner wrote:
> On Mon, 23 Apr 2018, Marc Zyngier wrote:
> 
>> Nobody would be insane enough to try and use level triggered
>> MSIs on PCI, but let's make sure it doesn't happen. Also,
>> let's mandate that the irqchip backing the platform MSI domain
>> is providing an irq_set_type method (or the whole thing would
>> be a bit useless).
>>
>> Signed-off-by: Marc Zyngier 
>> ---
>>  drivers/base/platform-msi.c | 3 +++
>>  drivers/pci/msi.c   | 3 +++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
>> index 8e22073aeeed..3b9694a6beaa 100644
>> --- a/drivers/base/platform-msi.c
>> +++ b/drivers/base/platform-msi.c
>> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
>> msi_domain_info *info)
>>  chip->irq_set_affinity = msi_domain_set_affinity;
>>  if (!chip->irq_write_msi_msg)
>>  chip->irq_write_msi_msg = platform_msi_write_msg;
>> +if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
>> +!chip->irq_set_type))
> 
> Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
> in chip->flags being set instead?
That definitely makes sense. Thanks for the suggestion.

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Thomas Gleixner]

On Mon, 23 Apr 2018, Marc Zyngier wrote:

> Nobody would be insane enough to try and use level triggered
> MSIs on PCI, but let's make sure it doesn't happen. Also,
> let's mandate that the irqchip backing the platform MSI domain
> is providing an irq_set_type method (or the whole thing would
> be a bit useless).
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/base/platform-msi.c | 3 +++
>  drivers/pci/msi.c   | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 8e22073aeeed..3b9694a6beaa 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
> msi_domain_info *info)
>   chip->irq_set_affinity = msi_domain_set_affinity;
>   if (!chip->irq_write_msi_msg)
>   chip->irq_write_msi_msg = platform_msi_write_msg;
> + if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
> + !chip->irq_set_type))

Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
in chip->flags being set instead?

Thanks,

tglx

Re: [PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Thomas Gleixner]

On Mon, 23 Apr 2018, Marc Zyngier wrote:

> Nobody would be insane enough to try and use level triggered
> MSIs on PCI, but let's make sure it doesn't happen. Also,
> let's mandate that the irqchip backing the platform MSI domain
> is providing an irq_set_type method (or the whole thing would
> be a bit useless).
> 
> Signed-off-by: Marc Zyngier 
> ---
>  drivers/base/platform-msi.c | 3 +++
>  drivers/pci/msi.c   | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 8e22073aeeed..3b9694a6beaa 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
> msi_domain_info *info)
>   chip->irq_set_affinity = msi_domain_set_affinity;
>   if (!chip->irq_write_msi_msg)
>   chip->irq_write_msi_msg = platform_msi_write_msg;
> + if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
> + !chip->irq_set_type))

Shouldn't we just require something like IRQCHIP_SUPPORTS_MSI_LEVEL or such
in chip->flags being set instead?

Thanks,

tglx

[PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

Nobody would be insane enough to try and use level triggered
MSIs on PCI, but let's make sure it doesn't happen. Also,
let's mandate that the irqchip backing the platform MSI domain
is providing an irq_set_type method (or the whole thing would
be a bit useless).

Signed-off-by: Marc Zyngier 
---
 drivers/base/platform-msi.c | 3 +++
 drivers/pci/msi.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 8e22073aeeed..3b9694a6beaa 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
msi_domain_info *info)
chip->irq_set_affinity = msi_domain_set_affinity;
if (!chip->irq_write_msi_msg)
chip->irq_write_msi_msg = platform_msi_write_msg;
+   if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
+   !chip->irq_set_type))
+   info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
 }
 
 static void platform_msi_free_descs(struct device *dev, int base, int nvec)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 30250631efe7..f45b74fcc059 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1434,6 +1434,9 @@ struct irq_domain *pci_msi_create_irq_domain(struct 
fwnode_handle *fwnode,
 {
struct irq_domain *domain;
 
+   if (WARN_ON(info->flags & MSI_FLAG_LEVEL_CAPABLE))
+   info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
+
if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
pci_msi_domain_update_dom_ops(info);
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
-- 
2.14.2

[PATCH 2/7] genirq/msi: Limit level-triggered MSI to platform devices

[Marc Zyngier]

Nobody would be insane enough to try and use level triggered
MSIs on PCI, but let's make sure it doesn't happen. Also,
let's mandate that the irqchip backing the platform MSI domain
is providing an irq_set_type method (or the whole thing would
be a bit useless).

Signed-off-by: Marc Zyngier 
---
 drivers/base/platform-msi.c | 3 +++
 drivers/pci/msi.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 8e22073aeeed..3b9694a6beaa 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -101,6 +101,9 @@ static void platform_msi_update_chip_ops(struct 
msi_domain_info *info)
chip->irq_set_affinity = msi_domain_set_affinity;
if (!chip->irq_write_msi_msg)
chip->irq_write_msi_msg = platform_msi_write_msg;
+   if (WARN_ON((info->flags & MSI_FLAG_LEVEL_CAPABLE) &&
+   !chip->irq_set_type))
+   info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
 }
 
 static void platform_msi_free_descs(struct device *dev, int base, int nvec)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 30250631efe7..f45b74fcc059 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1434,6 +1434,9 @@ struct irq_domain *pci_msi_create_irq_domain(struct 
fwnode_handle *fwnode,
 {
struct irq_domain *domain;
 
+   if (WARN_ON(info->flags & MSI_FLAG_LEVEL_CAPABLE))
+   info->flags &= ~MSI_FLAG_LEVEL_CAPABLE;
+
if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
pci_msi_domain_update_dom_ops(info);
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
-- 
2.14.2