| Message ID | 20240714122405.27548-1-marek.vasut+renesas@mailbox.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v2,RFC] genirq/cpuhotplug, PCI/rcar-host: Silence set affinity failed warning | expand |
Marek! On Sun, Jul 14 2024 at 14:23, Marek Vasut wrote: > --- a/drivers/base/platform-msi.c > +++ b/drivers/base/platform-msi.c > @@ -100,7 +100,7 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info) > chip->irq_unmask = irq_chip_unmask_parent; > if (!chip->irq_eoi) > chip->irq_eoi = irq_chip_eoi_parent; > - if (!chip->irq_set_affinity) > + if (!chip->irq_set_affinity && !(info->flags & MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF)) > chip->irq_set_affinity = msi_domain_set_affinity; I'm not really a fan of this new flag. I'd rather leave MSI_FLAG_USE_DEF_CHIP_OPS alone and introduce a dedicated flag MSI_FLAG_NO_AFFINITY to tell the core code that these interrupts cannot be steered. Other than that this approach looks sane. Thanks, tglx
On 7/14/24 9:14 PM, Thomas Gleixner wrote: > Marek! Hello Thomas, > On Sun, Jul 14 2024 at 14:23, Marek Vasut wrote: >> --- a/drivers/base/platform-msi.c >> +++ b/drivers/base/platform-msi.c >> @@ -100,7 +100,7 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info) >> chip->irq_unmask = irq_chip_unmask_parent; >> if (!chip->irq_eoi) >> chip->irq_eoi = irq_chip_eoi_parent; >> - if (!chip->irq_set_affinity) >> + if (!chip->irq_set_affinity && !(info->flags & MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF)) >> chip->irq_set_affinity = msi_domain_set_affinity; > > I'm not really a fan of this new flag. > > I'd rather leave MSI_FLAG_USE_DEF_CHIP_OPS alone and introduce a > dedicated flag MSI_FLAG_NO_AFFINITY to tell the core code that these > interrupts cannot be steered. Other than that this approach looks sane. Should be fixed in V3. I also converted Tegra so far and sent it alongside V3. There are many more controllers to convert that will follow.
diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c index 11f5fdf65b9ef..91e8a9e3d9610 100644 --- a/drivers/base/platform-msi.c +++ b/drivers/base/platform-msi.c @@ -100,7 +100,7 @@ static void platform_msi_update_chip_ops(struct msi_domain_info *info) chip->irq_unmask = irq_chip_unmask_parent; if (!chip->irq_eoi) chip->irq_eoi = irq_chip_eoi_parent; - if (!chip->irq_set_affinity) + if (!chip->irq_set_affinity && !(info->flags & MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF)) chip->irq_set_affinity = msi_domain_set_affinity; if (!chip->irq_write_msi_msg) chip->irq_write_msi_msg = platform_msi_write_msg; diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index c01efc6ea64f6..e898732159c04 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -658,11 +658,6 @@ static void rcar_msi_irq_unmask(struct irq_data *d) spin_unlock_irqrestore(&msi->mask_lock, flags); } -static int rcar_msi_set_affinity(struct irq_data *d, const struct cpumask *mask, bool force) -{ - return -EINVAL; -} - static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) { struct rcar_msi *msi = irq_data_get_irq_chip_data(data); @@ -678,7 +673,6 @@ static struct irq_chip rcar_msi_bottom_chip = { .irq_ack = rcar_msi_irq_ack, .irq_mask = rcar_msi_irq_mask, .irq_unmask = rcar_msi_irq_unmask, - .irq_set_affinity = rcar_msi_set_affinity, .irq_compose_msi_msg = rcar_compose_msi_msg, }; @@ -725,7 +719,7 @@ static const struct irq_domain_ops rcar_msi_domain_ops = { }; static struct msi_domain_info rcar_msi_info = { - .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF | MSI_FLAG_MULTI_PCI_MSI), .chip = &rcar_msi_top_chip, }; diff --git a/include/linux/msi.h b/include/linux/msi.h index 26588da88bdd8..ab79d772fd43d 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -573,6 +573,8 @@ enum { MSI_FLAG_MSIX_CONTIGUOUS = (1 << 19), /* PCI/MSI-X vectors can be dynamically allocated/freed post MSI-X enable */ MSI_FLAG_PCI_MSIX_ALLOC_DYN = (1 << 20), + /* Same as MSI_FLAG_USE_DEF_CHIP_OPS , but does not set .irq_set_affinity */ + MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF = ((1 << 21) | MSI_FLAG_USE_DEF_CHIP_OPS), }; /** diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 2024f89baea4b..877365eeab262 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -816,7 +816,7 @@ static void msi_domain_update_chip_ops(struct msi_domain_info *info) struct irq_chip *chip = info->chip; BUG_ON(!chip || !chip->irq_mask || !chip->irq_unmask); - if (!chip->irq_set_affinity) + if (!chip->irq_set_affinity && !(info->flags & MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF)) chip->irq_set_affinity = msi_domain_set_affinity; }
This is an RFC patch, I am looking for input on the approach taken here. If the approach is sound, this patch would be split into proper patchset. Various PCIe controllers that mux MSIs onto single IRQ line produce these "IRQ%d: set affinity failed" warnings when entering suspend. This has been discussed before [1] [2] and an example test case is included at the end of this commit message. Controller drivers which create MSI IRQ domain with MSI_FLAG_USE_DEF_CHIP_OPS flag set and which do not override the .irq_set_affinity irqchip callback get assigned default .irq_set_affinity = msi_domain_set_affinity() callback. That is not desired on controllers where it is not possible to set affinity of each MSI IRQ line to a specific CPU core due to hardware limitation. Introduce flag MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF, which keeps .irq_set_affinity unset in case the controller driver did not assign the callback. This way, the migrate_one_irq() code in cpuhotplug.c can exit right away, without printing the aforementioned warning. The .irq_set_affinity implementations which only return -EINVAL can be removed from multiple controller drivers. ``` $ grep 25 /proc/interrupts 25: 0 0 0 0 0 0 0 0 PCIe MSI 0 Edge PCIe PME $ echo core > /sys/power/pm_test ; echo mem > /sys/power/state ... Disabling non-boot CPUs ... IRQ25: set affinity failed(-22). <---------- This is being silenced here psci: CPU7 killed (polled 4 ms) ... ``` [1] https://lore.kernel.org/all/d4a6eea3c5e33a3a4056885419df95a7@kernel.org/ [2] https://lore.kernel.org/all/5f4947b18bf381615a37aa81c2242477@kernel.org/ Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> --- Cc: "Krzysztof Wilczyński" <kw@linux.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Jisheng Zhang <Jisheng.Zhang@synaptics.com> Cc: Jon Hunter <jonathanh@nvidia.com> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org> Cc: Marc Zyngier <maz@kernel.org> Cc: Rob Herring <robh@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Cc: linux-pci@vger.kernel.org Cc: linux-renesas-soc@vger.kernel.org --- V2: - Introduce MSI_FLAG_USE_DEF_CHIP_OPS_NOAFF to inhibit assignment of msi_domain_set_affinity() --- drivers/base/platform-msi.c | 2 +- drivers/pci/controller/pcie-rcar-host.c | 8 +------- include/linux/msi.h | 2 ++ kernel/irq/msi.c | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-)