Re: [patch V2 43/46] genirq/msi: Provide and use msi_domain_set_default_info_flags()
On Thu, Aug 27 2020 at 09:17, Marc Zyngier wrote:
> On 2020-08-26 12:17, Thomas Gleixner wrote:
>> #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>> +void msi_domain_set_default_info_flags(struct msi_domain_info *info)
>> +{
>> +/* Required so that a device latches a valid MSI message on startup
>> */
>> +info->flags |= MSI_FLAG_ACTIVATE_EARLY;
>
> As far as I remember the story behind this flag (it's been a while),
> it was working around a PCI-specific issue, hence being located in
> the PCI code.
Yes. Some cards misbehave when there is no valid message programmed and
MSI is enabled.
> Now, the "program the MSI before enabling it" concept makes sense no
> matter what bus this is on, and I wonder why we are even keeping this
> flag around.
> Can't we just drop it together with the check in
> msi_domain_alloc_irqs()?
I'm fine with that.
Thanks,
tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V2 43/46] genirq/msi: Provide and use msi_domain_set_default_info_flags()
On 2020-08-26 12:17, Thomas Gleixner wrote:
MSI interrupts have some common flags which should be set not only for
PCI/MSI interrupts.
Move the PCI/MSI flag setting into a common function so it can be
reused.
Signed-off-by: Thomas Gleixner
---
V2: New patch
---
drivers/pci/msi.c |7 +--
include/linux/msi.h |1 +
kernel/irq/msi.c| 24
3 files changed, 26 insertions(+), 6 deletions(-)
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1469,12 +1469,7 @@ struct irq_domain *pci_msi_create_irq_do
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
pci_msi_domain_update_chip_ops(info);
- info->flags |= MSI_FLAG_ACTIVATE_EARLY;
- if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
- info->flags |= MSI_FLAG_MUST_REACTIVATE;
-
- /* PCI-MSI is oneshot-safe */
- info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
+ msi_domain_set_default_info_flags(info);
domain = msi_create_irq_domain(fwnode, info, parent);
if (!domain)
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -410,6 +410,7 @@ int platform_msi_domain_alloc(struct irq
void platform_msi_domain_free(struct irq_domain *domain, unsigned int
virq,
unsigned int nvec);
void *platform_msi_get_host_data(struct irq_domain *domain);
+void msi_domain_set_default_info_flags(struct msi_domain_info *info);
#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -70,6 +70,30 @@ void get_cached_msi_msg(unsigned int irq
EXPORT_SYMBOL_GPL(get_cached_msi_msg);
#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+void msi_domain_set_default_info_flags(struct msi_domain_info *info)
+{
+ /* Required so that a device latches a valid MSI message on startup
*/
+ info->flags |= MSI_FLAG_ACTIVATE_EARLY;
As far as I remember the story behind this flag (it's been a while),
it was working around a PCI-specific issue, hence being located in
the PCI code.
Now, the "program the MSI before enabling it" concept makes sense no
matter
what bus this is on, and I wonder why we are even keeping this flag
around.
Can't we just drop it together with the check in
msi_domain_alloc_irqs()?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[patch V2 43/46] genirq/msi: Provide and use msi_domain_set_default_info_flags()
MSI interrupts have some common flags which should be set not only for
PCI/MSI interrupts.
Move the PCI/MSI flag setting into a common function so it can be reused.
Signed-off-by: Thomas Gleixner
---
V2: New patch
---
drivers/pci/msi.c |7 +--
include/linux/msi.h |1 +
kernel/irq/msi.c| 24
3 files changed, 26 insertions(+), 6 deletions(-)
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1469,12 +1469,7 @@ struct irq_domain *pci_msi_create_irq_do
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
pci_msi_domain_update_chip_ops(info);
- info->flags |= MSI_FLAG_ACTIVATE_EARLY;
- if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
- info->flags |= MSI_FLAG_MUST_REACTIVATE;
-
- /* PCI-MSI is oneshot-safe */
- info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
+ msi_domain_set_default_info_flags(info);
domain = msi_create_irq_domain(fwnode, info, parent);
if (!domain)
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -410,6 +410,7 @@ int platform_msi_domain_alloc(struct irq
void platform_msi_domain_free(struct irq_domain *domain, unsigned int virq,
unsigned int nvec);
void *platform_msi_get_host_data(struct irq_domain *domain);
+void msi_domain_set_default_info_flags(struct msi_domain_info *info);
#endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -70,6 +70,30 @@ void get_cached_msi_msg(unsigned int irq
EXPORT_SYMBOL_GPL(get_cached_msi_msg);
#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+void msi_domain_set_default_info_flags(struct msi_domain_info *info)
+{
+ /* Required so that a device latches a valid MSI message on startup */
+ info->flags |= MSI_FLAG_ACTIVATE_EARLY;
+
+ /*
+* Interrupt reservation mode allows to stear the MSI message of an
+* inactive device to a special (usually spurious interrupt) target.
+* This allows to prevent interrupt vector exhaustion e.g. on x86.
+* But (PCI)MSI interrupts are activated early - see above - so the
+* interrupt request/startup sequence would not try to allocate a
+* usable vector which means that the device interupts would end
+* up on the special vector and issue spurious interrupt messages.
+* Setting the reactivation flag ensures that when the interrupt
+* is requested the activation is invoked again so that a real
+* vector can be allocated.
+*/
+ if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
+ info->flags |= MSI_FLAG_MUST_REACTIVATE;
+
+ /* MSI is oneshot-safe at least in theory */
+ info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
+}
+
static inline void irq_chip_write_msi_msg(struct irq_data *data,
struct msi_msg *msg)
{
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
