[PATCH] pci: mediatek: fix warning in msi.h

[PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

From: Frank Wunderlich <frank-w@public-files.de>

5.10 shows these warnings on bootup while enabling pcie
at least on bananapi-r2:

[    6.161730] WARNING: CPU: 2 PID: 73 at include/linux/msi.h:213 pci_msi_setup_
msi_irqs.constprop.0+0x78/0x80
....
[    6.724607] WARNING: CPU: 2 PID: 73 at include/linux/msi.h:219 free_msi_irqs+

fix this by selecting PCI_MSI_ARCH_FALLBACKS for MTK PCIe driver

Fixes: 077ee78e3928 ("PCI/MSI: Make arch_.*_msi_irq[s] fallbacks selectable")
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 drivers/pci/controller/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 64e2f5e379aa..8345de010186 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -238,6 +238,7 @@ config PCIE_MEDIATEK
 	depends on ARCH_MEDIATEK || COMPILE_TEST
 	depends on OF
 	depends on PCI_MSI_IRQ_DOMAIN
+	select PCI_MSI_ARCH_FALLBACKS
 	help
 	  Say Y here if you want to enable PCIe controller support on
 	  MediaTek SoCs.
-- 
2.25.1

Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

Frank,

On Sat, Oct 31 2020 at 15:03, Frank Wunderlich wrote:
> From: Frank Wunderlich <frank-w@public-files.de>
>
> 5.10 shows these warnings on bootup while enabling pcie
> at least on bananapi-r2:
>
> [    6.161730] WARNING: CPU: 2 PID: 73 at include/linux/msi.h:213 pci_msi_setup_
> msi_irqs.constprop.0+0x78/0x80
> ....
> [    6.724607] WARNING: CPU: 2 PID: 73 at include/linux/msi.h:219 free_msi_irqs+
>
> fix this by selecting PCI_MSI_ARCH_FALLBACKS for MTK PCIe driver

That's not a fix. It's just supressing the warning.

PCI_MSI_ARCH_FALLBACKS is only valid for

  1) Architectures which implement the fallbacks

  2) Outdated PCI controller drivers on architectures without #1 which
     implement the deprecated msi_controller mechanism. That is handled
     in the weak arch fallback implementation.

The mediatek PCIE driver does not qualify for #2. It's purely irq domain
based.

So there is something else going wrong. The PCI device which tries to
allocate MSIs does not have an irq domain associated which makes it run
into that warning.

If you enable PCI_MSI_ARCH_FALLBACKS then the MSI allocation fails
silently. So it's just papering over the underlying problem.

So it needs to be figured out why the domain association is not there.

Thanks,

        tglx

Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

Am 31. Oktober 2020 22:49:14 MEZ schrieb Thomas Gleixner <tglx@linutronix.de>:

>That's not a fix. It's just supressing the warning.

Ok sorry

>So it needs to be figured out why the domain association is not there.

It looks like for mt7623 there is no msi domain setup (done via mtk_pcie_setup_irq callback + mtk_pcie_init_irq_domain) in mtk pcie driver.

https://elixir.bootlin.com/linux/v5.10-rc1/source/drivers/pci/controller/pcie-mediatek.c#L1204

regards Frank

Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Sun, Nov 01 2020 at 10:25, Frank Wunderlich wrote:
> Am 31. Oktober 2020 22:49:14 MEZ schrieb Thomas Gleixner <tglx@linutronix.de>:
>
>>So it needs to be figured out why the domain association is not there.
>
> It looks like for mt7623 there is no msi domain setup (done via
> mtk_pcie_setup_irq callback + mtk_pcie_init_irq_domain) in mtk pcie
> driver.

Hrm. No idea how that is supposed to work. I defer that to the mt
wizards then.

Thanks,

        tglx

Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On Sun, 01 Nov 2020 09:25:04 +0000,
Frank Wunderlich <frank-w@public-files.de> wrote:
> 
> Am 31. Oktober 2020 22:49:14 MEZ schrieb Thomas Gleixner <tglx@linutronix.de>:
> 
> >That's not a fix. It's just supressing the warning.
> 
> Ok sorry
> 
> >So it needs to be figured out why the domain association is not there.
> 
> It looks like for mt7623 there is no msi domain setup (done via
> mtk_pcie_setup_irq callback + mtk_pcie_init_irq_domain) in mtk pcie
> driver.

Does this mean that this SoC never handled MSIs the first place? Which
would explain the warning, as there is no MSI domain registered for
the device, and we end-up falling back to arch_setup_msi_irqs().

If this system truly is unable to handle MSIs, one potential
workaround would be to register a PCI-MSI domain that would always
fail its allocation with -ENOSPC. It is really ugly, but would keep
the horror localised. See the patchlet below, which I can't test.

If this situation is more common than we expect, we may need something
in core code instead.

	M.

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cf4c18f0c25a..52758b546d40 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -151,6 +151,7 @@ struct mtk_pcie_port;
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
 	bool need_fix_device_id;
+	bool no_msi;
 	unsigned int device_id;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
@@ -435,6 +436,9 @@ static int mtk_pcie_irq_domain_alloc(struct irq_domain *domain, unsigned int vir
 	struct mtk_pcie_port *port = domain->host_data;
 	unsigned long bit;
 
+	if (port->pcie->soc->no_msi)
+		return -ENOSPC;
+
 	WARN_ON(nr_irqs != 1);
 	mutex_lock(&port->lock);
 
@@ -966,11 +970,13 @@ static int mtk_pcie_parse_port(struct mtk_pcie *pcie,
 	port->slot = slot;
 	port->pcie = pcie;
 
-	if (pcie->soc->setup_irq) {
+	if (pcie->soc->setup_irq)
 		err = pcie->soc->setup_irq(port, node);
-		if (err)
-			return err;
-	}
+	else
+		err = mtk_pcie_allocate_msi_domains(port);
+
+	if (err)
+		return err;
 
 	INIT_LIST_HEAD(&port->list);
 	list_add_tail(&port->list, &pcie->ports);
@@ -1173,6 +1179,7 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {
 };
 
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
+	.no_msi = true,
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
 };

-- 
Without deviation from the norm, progress is not possible.

Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

> Gesendet: Sonntag, 01. November 2020 um 12:43 Uhr
> Von: "Marc Zyngier" <maz@kernel.org>

> On Sun, 01 Nov 2020 09:25:04 +0000,
> Frank Wunderlich <frank-w@public-files.de> wrote:

> > It looks like for mt7623 there is no msi domain setup (done via
> > mtk_pcie_setup_irq callback + mtk_pcie_init_irq_domain) in mtk pcie
> > driver.
>
> Does this mean that this SoC never handled MSIs the first place? Which
> would explain the warning, as there is no MSI domain registered for
> the device, and we end-up falling back to arch_setup_msi_irqs().

i tried 5.10-rc1 (without my patch of course) on bananapi-r64 to check if driver
on mt7622 works better (this does setup a msi-domain in pcie-driver)....and i got no warning.
so mt7623 needs to create an msi-domain or handle it in the correct way e.g.
by returning -ENOSPC like in your code.

> If this system truly is unable to handle MSIs, one potential
> workaround would be to register a PCI-MSI domain that would always
> fail its allocation with -ENOSPC. It is really ugly, but would keep
> the horror localised. See the patchlet below, which I can't test.
>
> If this situation is more common than we expect, we may need something
> in core code instead.

thanks for your code-example, here we need a response from MTK (CC'd Chuanjia Liu)

regards Frank

Re: [PATCH] pci: mediatek: fix warning in msi.h

[Ryder Lee]

On Sun, 2020-11-01 at 11:43 +0000, Marc Zyngier wrote:
> On Sun, 01 Nov 2020 09:25:04 +0000,
> Frank Wunderlich <frank-w@public-files.de> wrote:
> > 
> > Am 31. Oktober 2020 22:49:14 MEZ schrieb Thomas Gleixner <tglx@linutronix.de>:
> > 
> > >That's not a fix. It's just supressing the warning.
> > 
> > Ok sorry
> > 
> > >So it needs to be figured out why the domain association is not there.
> > 
> > It looks like for mt7623 there is no msi domain setup (done via
> > mtk_pcie_setup_irq callback + mtk_pcie_init_irq_domain) in mtk pcie
> > driver.
> 
> Does this mean that this SoC never handled MSIs the first place? Which
> would explain the warning, as there is no MSI domain registered for
> the device, and we end-up falling back to arch_setup_msi_irqs().
> 
> If this system truly is unable to handle MSIs, one potential
> workaround would be to register a PCI-MSI domain that would always
> fail its allocation with -ENOSPC. It is really ugly, but would keep
> the horror localised. See the patchlet below, which I can't test.
> 
> If this situation is more common than we expect, we may need something
> in core code instead.
> 
> 	M.
> 
Yea, mt7623 (mtk_pcie_soc_v1) does not support MSI, so that's a way to
handle it.

@Frank, could you help to test it?

Ryder

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index cf4c18f0c25a..52758b546d40 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -151,6 +151,7 @@ struct mtk_pcie_port;
>  struct mtk_pcie_soc {
>  	bool need_fix_class_id;
>  	bool need_fix_device_id;
> +	bool no_msi;
>  	unsigned int device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
> @@ -435,6 +436,9 @@ static int mtk_pcie_irq_domain_alloc(struct irq_domain *domain, unsigned int vir
>  	struct mtk_pcie_port *port = domain->host_data;
>  	unsigned long bit;
>  
> +	if (port->pcie->soc->no_msi)
> +		return -ENOSPC;
> +
>  	WARN_ON(nr_irqs != 1);
>  	mutex_lock(&port->lock);
>  
> @@ -966,11 +970,13 @@ static int mtk_pcie_parse_port(struct mtk_pcie *pcie,
>  	port->slot = slot;
>  	port->pcie = pcie;
>  
> -	if (pcie->soc->setup_irq) {
> +	if (pcie->soc->setup_irq)
>  		err = pcie->soc->setup_irq(port, node);
> -		if (err)
> -			return err;
> -	}
> +	else
> +		err = mtk_pcie_allocate_msi_domains(port);
> +
> +	if (err)
> +		return err;
>  
>  	INIT_LIST_HEAD(&port->list);
>  	list_add_tail(&port->list, &pcie->ports);
> @@ -1173,6 +1179,7 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {
>  };
>  
>  static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
> +	.no_msi = true,
>  	.ops = &mtk_pcie_ops,
>  	.startup = mtk_pcie_startup_port,
>  };
> 

Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

> Gesendet: Sonntag, 01. November 2020 um 18:54 Uhr
> Von: "Ryder Lee" <ryder.lee@mediatek.com>

> Yea, mt7623 (mtk_pcie_soc_v1) does not support MSI, so that's a way to
> handle it.
>
> @Frank, could you help to test it?
>
> Ryder

compiles clean for mt7623/armhf and mt7622/aarch64 so far

at least bananapi-r2/mt7623 booting is clean now - no warning
pcie and sata/ahci seems still working as expected. I have a mt7615 card in pcie-slot (firmware-load and init without errors) and hdd connected to outer sata port (can access partitions witout errors)

booted r64 too, still see no warning, but have not yet connected hdd/pcie-device, but i guess this should not break anything here

so Marc, if you post the patch separately, you can add my tested-by ;) thank you for this

regards Frank

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On Sun, 01 Nov 2020 18:27:13 +0000,
Frank Wunderlich <frank-w@public-files.de> wrote:
> 
> > Gesendet: Sonntag, 01. November 2020 um 18:54 Uhr
> > Von: "Ryder Lee" <ryder.lee@mediatek.com>
> 
> > Yea, mt7623 (mtk_pcie_soc_v1) does not support MSI, so that's a way to
> > handle it.
> >
> > @Frank, could you help to test it?
> >
> > Ryder
> 
> compiles clean for mt7623/armhf and mt7622/aarch64 so far
> 
> at least bananapi-r2/mt7623 booting is clean now - no warning pcie
> and sata/ahci seems still working as expected. I have a mt7615 card
> in pcie-slot (firmware-load and init without errors) and hdd
> connected to outer sata port (can access partitions witout errors)
> 
> booted r64 too, still see no warning, but have not yet connected
> hdd/pcie-device, but i guess this should not break anything here
> 
> so Marc, if you post the patch separately, you can add my tested-by
> ;) thank you for this

Thinking of it a bit more, I think this is the wrong solution.

PCI MSIs are optional, and not a requirement. I can trivially spin a
VM with PCI devices and yet no MSI capability (yes, it is more
difficult with real HW), and this results in a bunch of warning, none
of which are actually indicative of anything being wrong.

I think the real fix is to get rid of the warnings altogether. If we
could detect that there should be an MSI domain associated with the
device and that it wasn't there, that'd be a good reason to scream.
But on its own, the absence of a MSI domain is not an indication of
anything being amiss.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Sun, Nov 01 2020 at 21:47, Marc Zyngier wrote:
> On Sun, 01 Nov 2020 18:27:13 +0000,
> Frank Wunderlich <frank-w@public-files.de> wrote:
> Thinking of it a bit more, I think this is the wrong solution.
>
> PCI MSIs are optional, and not a requirement. I can trivially spin a
> VM with PCI devices and yet no MSI capability (yes, it is more
> difficult with real HW), and this results in a bunch of warning, none
> of which are actually indicative of anything being wrong.

Well. No. 

The problem is that the device enumerates MSI capability, but the host
bridge is not proving support for MSI. 

The host bridge fails to mark the bus with PCI_BUS_FLAGS_NO_MSI. That's
the reason why this runs into this issue.

Something like the uncompiled hack below. I haven't found a way to hand
that down to the probe function.

Thanks,

        tglx
---
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cf4c18f0c25a..d91bdfea7329 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -143,6 +143,7 @@ struct mtk_pcie_port;
  * struct mtk_pcie_soc - differentiate between host generations
  * @need_fix_class_id: whether this host's class ID needed to be fixed or not
  * @need_fix_device_id: whether this host's device ID needed to be fixed or not
+ * @no_msi: Bridge has no MSI support
  * @device_id: device ID which this host need to be fixed
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
@@ -151,6 +152,7 @@ struct mtk_pcie_port;
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
 	bool need_fix_device_id;
+	bool no_msi;
 	unsigned int device_id;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
@@ -1089,6 +1091,9 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto put_resources;
 
+	if (!pcie->soc->no_msi)
+		host->bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
 	return 0;
 
 put_resources:
@@ -1173,6 +1178,7 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {
 };
 
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
+	.no_msi = true,
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
 };

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-01 22:27, Thomas Gleixner wrote:
> On Sun, Nov 01 2020 at 21:47, Marc Zyngier wrote:
>> On Sun, 01 Nov 2020 18:27:13 +0000,
>> Frank Wunderlich <frank-w@public-files.de> wrote:
>> Thinking of it a bit more, I think this is the wrong solution.
>> 
>> PCI MSIs are optional, and not a requirement. I can trivially spin a
>> VM with PCI devices and yet no MSI capability (yes, it is more
>> difficult with real HW), and this results in a bunch of warning, none
>> of which are actually indicative of anything being wrong.
> 
> Well. No.
> 
> The problem is that the device enumerates MSI capability, but the host
> bridge is not proving support for MSI.
> 
> The host bridge fails to mark the bus with PCI_BUS_FLAGS_NO_MSI. That's
> the reason why this runs into this issue.

Right, that's the piece I was missing, thanks for that.

However, that doesn't really address the issue when the host bridge and
the MSI widget are two separate entities, oblivious of each other (which
is a pretty common thing on the ARM side).

In this configuration, you can't really decide whether you have a MSI
domain in the host bridge driver (the association is done in the code
PCI code, after you have registered it with the core code), and by the
time you get a pointer to the bus, the endpoints have already been 
probed.

The following patch makes it work for me (GICv3 guest without an ITS)by
checking for the presence of an MSI domain at the point where we 
actually
perform this association, and before starting to scan for endpoints.

I *think* this should work for the MTK thingy, but someone needs to
go and check.

Thanks,

         M.

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4289030b0fff..bb363eb103a2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus 
*bus)
  		d = pci_host_bridge_msi_domain(b);

  	dev_set_msi_domain(&bus->dev, d);
+	if (!d)
+		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
  }

  static int pci_register_host_bridge(struct pci_host_bridge *bridge)

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

Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

looks good on bananapi-r2, no warning, pcie-card and hdd recognized

regards Frank


> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4289030b0fff..bb363eb103a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus
> *bus)
>   		d = pci_host_bridge_msi_domain(b);
>
>   	dev_set_msi_domain(&bus->dev, d);
> +	if (!d)
> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>   }
>
>   static int pci_register_host_bridge(struct pci_host_bridge *bridge)

Re: Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-02 11:56, Frank Wunderlich wrote:
> looks good on bananapi-r2, no warning, pcie-card and hdd recognized

Thanks for giving it a shot. Still needs a bit of tweaking, as I expect
it to break configurations that select CONFIG_PCI_MSI_ARCH_FALLBACKS
(we have to assume that MSIs can be handled until we hit the 
arch-specific
stuff).

There is also a small nit in the way we allow userspace to mess with
this flag via sysfs, and similar restrictions should probably apply.

Updated patch below.

         M.

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d15c881e2e7e..5bb1306162c7 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -387,10 +387,20 @@ static ssize_t msi_bus_store(struct device *dev, 
struct device_attribute *attr,
  		return count;
  	}

-	if (val)
+	if (val) {
+		/*
+		 * If there is no possibility for this bus to deal with
+		 * MSIs, then allowing them to be requested would lead to
+		 * the kernel complaining loudly. In this situation, don't
+		 * let userspace mess things up.
+		 */
+		if (!pci_bus_is_msi_capable(subordinate))
+			return -EINVAL;
+
  		subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
-	else
+	} else {
  		subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+	}

  	dev_info(&subordinate->dev, "MSI/MSI-X %s for future drivers of 
devices on this bus\n",
  		 val ? "allowed" : "disallowed");
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4289030b0fff..28861cc6435a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus 
*bus)
  		d = pci_host_bridge_msi_domain(b);

  	dev_set_msi_domain(&bus->dev, d);
+	if (!pci_bus_is_msi_capable(bus))
+		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
  }

  static int pci_register_host_bridge(struct pci_host_bridge *bridge)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 22207a79762c..6aadb863dff4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2333,6 +2333,12 @@ pci_host_bridge_acpi_msi_domain(struct pci_bus 
*bus) { return NULL; }
  static inline bool pci_pr3_present(struct pci_dev *pdev) { return 
false; }
  #endif

+static inline bool pci_bus_is_msi_capable(struct pci_bus *bus)
+{
+	return (IS_ENABLED(CONFIG_PCI_MSI_ARCH_FALLBACKS) ||
+		dev_get_msi_domain(&bus->dev));
+}
+
  #ifdef CONFIG_EEH
  static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
  {

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

Aw: Re: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

> Gesendet: Montag, 02. November 2020 um 14:58 Uhr
> Von: "Marc Zyngier" <maz@kernel.org>

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d15c881e2e7e..5bb1306162c7 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -387,10 +387,20 @@ static ssize_t msi_bus_store(struct device *dev,
> struct device_attribute *attr,
>   		return count;
>   	}
>
> -	if (val)
> +	if (val) {
> +		/*
> +		 * If there is no possibility for this bus to deal with
> +		 * MSIs, then allowing them to be requested would lead to
> +		 * the kernel complaining loudly. In this situation, don't
> +		 * let userspace mess things up.
> +		 */
> +		if (!pci_bus_is_msi_capable(subordinate))
> +			return -EINVAL;
> +
>   		subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
> -	else
> +	} else {
>   		subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
> +	}
>
>   	dev_info(&subordinate->dev, "MSI/MSI-X %s for future drivers of
> devices on this bus\n",
>   		 val ? "allowed" : "disallowed");
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4289030b0fff..28861cc6435a 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus
> *bus)
>   		d = pci_host_bridge_msi_domain(b);
>
>   	dev_set_msi_domain(&bus->dev, d);
> +	if (!pci_bus_is_msi_capable(bus))
> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>   }
>
>   static int pci_register_host_bridge(struct pci_host_bridge *bridge)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 22207a79762c..6aadb863dff4 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2333,6 +2333,12 @@ pci_host_bridge_acpi_msi_domain(struct pci_bus
> *bus) { return NULL; }
>   static inline bool pci_pr3_present(struct pci_dev *pdev) { return
> false; }
>   #endif
>
> +static inline bool pci_bus_is_msi_capable(struct pci_bus *bus)
> +{
> +	return (IS_ENABLED(CONFIG_PCI_MSI_ARCH_FALLBACKS) ||
> +		dev_get_msi_domain(&bus->dev));
> +}
> +
>   #ifdef CONFIG_EEH
>   static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>   {

Hi,

this Patch seems to work well too...no warning, pcie-card and hdd recognized

regards Frank

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Mon, Nov 02 2020 at 11:30, Marc Zyngier wrote:
> On 2020-11-01 22:27, Thomas Gleixner wrote:
> The following patch makes it work for me (GICv3 guest without an ITS)by
> checking for the presence of an MSI domain at the point where we 
> actually
> perform this association, and before starting to scan for endpoints.
>
> I *think* this should work for the MTK thingy, but someone needs to
> go and check.
>
> Thanks,
>
>          M.
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4289030b0fff..bb363eb103a2 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus 
> *bus)
>   		d = pci_host_bridge_msi_domain(b);
>
>   	dev_set_msi_domain(&bus->dev, d);
> +	if (!d)
> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;

Hrm, that might break legacy setups (no irqdomain support). I'd rather
prefer to explicitly tell the pci core at host registration time.

Bjorn?

Thanks,

        tglx

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Mon, Nov 02 2020 at 17:16, Thomas Gleixner wrote:
> On Mon, Nov 02 2020 at 11:30, Marc Zyngier wrote:
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus 
>> *bus)
>>   		d = pci_host_bridge_msi_domain(b);
>>
>>   	dev_set_msi_domain(&bus->dev, d);
>> +	if (!d)
>> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>
> Hrm, that might break legacy setups (no irqdomain support). I'd rather
> prefer to explicitly tell the pci core at host registration time.

s/might break/ breaks /     Just validated :)

So we really need some other solution and removing the warning is not an
option. If MSI is enabled then we want to get a warning when a PCI
device has no MSI domain associated. Explicitly expressing the PCIE
brigde misfeature of not supporting MSI is way better than silently
returning an error code which is swallowed anyway.

Whatever the preferred way is via flags at host probe time or flagging
it post probe I don't care much as long as it is consistent.

Thanks,

        tglx

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-02 22:18, Thomas Gleixner wrote:
> On Mon, Nov 02 2020 at 17:16, Thomas Gleixner wrote:
>> On Mon, Nov 02 2020 at 11:30, Marc Zyngier wrote:
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus
>>> *bus)
>>>   		d = pci_host_bridge_msi_domain(b);
>>> 
>>>   	dev_set_msi_domain(&bus->dev, d);
>>> +	if (!d)
>>> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>> 
>> Hrm, that might break legacy setups (no irqdomain support). I'd rather
>> prefer to explicitly tell the pci core at host registration time.
> 
> s/might break/ breaks /     Just validated :)

For my own edification, can you point me to the failing case?

> So we really need some other solution and removing the warning is not 
> an
> option. If MSI is enabled then we want to get a warning when a PCI
> device has no MSI domain associated. Explicitly expressing the PCIE
> brigde misfeature of not supporting MSI is way better than silently
> returning an error code which is swallowed anyway.

I don't disagree here, though the PCI_MSI_ARCH_FALLBACKS mechanism
makes it more difficult to establish.

> Whatever the preferred way is via flags at host probe time or flagging
> it post probe I don't care much as long as it is consistent.

Host probe time is going to require some changes in the core PCI api,
as everything that checks for a MSI domain is based on the pci_bus
structure, which is only allocated much later.

I'll have a think.

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

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Tue, Nov 03 2020 at 09:54, Marc Zyngier wrote:
> On 2020-11-02 22:18, Thomas Gleixner wrote:
>> On Mon, Nov 02 2020 at 17:16, Thomas Gleixner wrote:
>>> On Mon, Nov 02 2020 at 11:30, Marc Zyngier wrote:
>>>> --- a/drivers/pci/probe.c
>>>> +++ b/drivers/pci/probe.c
>>>> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct pci_bus
>>>> *bus)
>>>>   		d = pci_host_bridge_msi_domain(b);
>>>> 
>>>>   	dev_set_msi_domain(&bus->dev, d);
>>>> +	if (!d)
>>>> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>>> 
>>> Hrm, that might break legacy setups (no irqdomain support). I'd rather
>>> prefer to explicitly tell the pci core at host registration time.
>> 
>> s/might break/ breaks /     Just validated :)
>
> For my own edification, can you point me to the failing case?

Any architecture which selects PCI_MSI_ARCH_FALLBACKS and does not have
irqdomain support runs into:

	if (!d)
		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;

which in turn makes pci_msi_supported() return 0 and consequently makes
pci_enable_msi[x]() fail.

Thanks,

        tglx

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-03 10:16, Thomas Gleixner wrote:
> On Tue, Nov 03 2020 at 09:54, Marc Zyngier wrote:
>> On 2020-11-02 22:18, Thomas Gleixner wrote:
>>> On Mon, Nov 02 2020 at 17:16, Thomas Gleixner wrote:
>>>> On Mon, Nov 02 2020 at 11:30, Marc Zyngier wrote:
>>>>> --- a/drivers/pci/probe.c
>>>>> +++ b/drivers/pci/probe.c
>>>>> @@ -871,6 +871,8 @@ static void pci_set_bus_msi_domain(struct 
>>>>> pci_bus
>>>>> *bus)
>>>>>   		d = pci_host_bridge_msi_domain(b);
>>>>> 
>>>>>   	dev_set_msi_domain(&bus->dev, d);
>>>>> +	if (!d)
>>>>> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>>>> 
>>>> Hrm, that might break legacy setups (no irqdomain support). I'd 
>>>> rather
>>>> prefer to explicitly tell the pci core at host registration time.
>>> 
>>> s/might break/ breaks /     Just validated :)
>> 
>> For my own edification, can you point me to the failing case?
> 
> Any architecture which selects PCI_MSI_ARCH_FALLBACKS and does not have
> irqdomain support runs into:
> 
> 	if (!d)
> 		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
> 
> which in turn makes pci_msi_supported() return 0 and consequently makes
> pci_enable_msi[x]() fail.

I pointer that out in [1], together with a potential fix. Not sure if
anything else breaks though.

Thanks,

         M.

[1] 
https://lore.kernel.org/r/336d6588567949029c52ecfbb87660c1@kernel.org/
-- 
Jazz is not dead. It just smells funny...

Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

> Gesendet: Dienstag, 03. November 2020 um 11:16 Uhr
> Von: "Thomas Gleixner" <tglx@linutronix.de>
> Any architecture which selects PCI_MSI_ARCH_FALLBACKS and does not have
> irqdomain support runs into:
>
> 	if (!d)
> 		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>
> which in turn makes pci_msi_supported() return 0 and consequently makes
> pci_enable_msi[x]() fail.

i'm not that deep into this, but just my thoughts...you are the experts :)

checking for PCI_MSI_ARCH_FALLBACKS here does not help?

something like this:

#ifndef PCI_MSI_ARCH_FALLBACKS
	if (!d)
		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
#endif

imho pci_enable_msi[x]() does not do anything if there is no msi-support (or does not get called), so maybe check the PCI_BUS_FLAGS_NO_MSI before the call (if this is inside core) or inside the enable-function (if called from outside)....

or is this the issue marc talkes about (called before flag is set)?

regards Frank

Re: Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

Frank,

On Wed, Nov 04 2020 at 17:49, Frank Wunderlich wrote:
>> Von: "Thomas Gleixner" <tglx@linutronix.de>
>> Any architecture which selects PCI_MSI_ARCH_FALLBACKS and does not have
>> irqdomain support runs into:
>>
>> 	if (!d)
>> 		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
>>
>> which in turn makes pci_msi_supported() return 0 and consequently makes
>> pci_enable_msi[x]() fail.
>
> i'm not that deep into this, but just my thoughts...you are the experts :)
>
> checking for PCI_MSI_ARCH_FALLBACKS here does not help?
>
> something like this:
>
> #ifndef PCI_MSI_ARCH_FALLBACKS
> 	if (!d)
> 		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
> #endif

TBH, that's butt ugly. So after staring long enough into the PCI code I
came up with a way to transport that information to the probe code.

That allows a particular device to say 'I can't do MSI' and at the same
time keeps the warning machinery intact which tells us that a particular
host controller driver is broken.

Uncompiled and untested as usual :)

Thanks,

        tglx
---
 drivers/pci/controller/pcie-mediatek.c |    4 ++++
 drivers/pci/probe.c                    |    3 +++
 include/linux/pci.h                    |    1 +
 3 files changed, 8 insertions(+)

--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -143,6 +143,7 @@ struct mtk_pcie_port;
  * struct mtk_pcie_soc - differentiate between host generations
  * @need_fix_class_id: whether this host's class ID needed to be fixed or not
  * @need_fix_device_id: whether this host's device ID needed to be fixed or not
+ * @no_msi: Bridge has no MSI support
  * @device_id: device ID which this host need to be fixed
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
@@ -151,6 +152,7 @@ struct mtk_pcie_port;
 struct mtk_pcie_soc {
 	bool need_fix_class_id;
 	bool need_fix_device_id;
+	bool no_msi;
 	unsigned int device_id;
 	struct pci_ops *ops;
 	int (*startup)(struct mtk_pcie_port *port);
@@ -1084,6 +1086,7 @@ static int mtk_pcie_probe(struct platfor
 
 	host->ops = pcie->soc->ops;
 	host->sysdata = pcie;
+	host->no_msi = pcie->soc->no_msi;
 
 	err = pci_host_probe(host);
 	if (err)
@@ -1173,6 +1176,7 @@ static const struct dev_pm_ops mtk_pcie_
 };
 
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
+	.no_msi = true,
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
 };
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -889,6 +889,9 @@ static int pci_register_host_bridge(stru
 	if (!bus)
 		return -ENOMEM;
 
+	if (bridge->no_msi)
+		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
 	bridge->bus = bus;
 
 	/* Temporarily move resources off the list */
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -545,6 +545,7 @@ struct pci_host_bridge {
 	unsigned int	native_dpc:1;		/* OS may use PCIe DPC */
 	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
+	unsigned int	no_msi:1;		/* Bridge has no MSI support */
 
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,

Re: Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-04 23:14, Thomas Gleixner wrote:

[...]

> TBH, that's butt ugly. So after staring long enough into the PCI code I
> came up with a way to transport that information to the probe code.
> 
> That allows a particular device to say 'I can't do MSI' and at the same
> time keeps the warning machinery intact which tells us that a 
> particular
> host controller driver is broken.
> 
> Uncompiled and untested as usual :)
> 
> Thanks,
> 
>         tglx
> ---
>  drivers/pci/controller/pcie-mediatek.c |    4 ++++
>  drivers/pci/probe.c                    |    3 +++
>  include/linux/pci.h                    |    1 +
>  3 files changed, 8 insertions(+)
> 
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -143,6 +143,7 @@ struct mtk_pcie_port;
>   * struct mtk_pcie_soc - differentiate between host generations
>   * @need_fix_class_id: whether this host's class ID needed to be fixed 
> or not
>   * @need_fix_device_id: whether this host's device ID needed to be 
> fixed or not
> + * @no_msi: Bridge has no MSI support
>   * @device_id: device ID which this host need to be fixed
>   * @ops: pointer to configuration access functions
>   * @startup: pointer to controller setting functions
> @@ -151,6 +152,7 @@ struct mtk_pcie_port;
>  struct mtk_pcie_soc {
>  	bool need_fix_class_id;
>  	bool need_fix_device_id;
> +	bool no_msi;
>  	unsigned int device_id;
>  	struct pci_ops *ops;
>  	int (*startup)(struct mtk_pcie_port *port);
> @@ -1084,6 +1086,7 @@ static int mtk_pcie_probe(struct platfor
> 
>  	host->ops = pcie->soc->ops;
>  	host->sysdata = pcie;
> +	host->no_msi = pcie->soc->no_msi;
> 
>  	err = pci_host_probe(host);
>  	if (err)
> @@ -1173,6 +1176,7 @@ static const struct dev_pm_ops mtk_pcie_
>  };
> 
>  static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
> +	.no_msi = true,
>  	.ops = &mtk_pcie_ops,
>  	.startup = mtk_pcie_startup_port,
>  };
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -889,6 +889,9 @@ static int pci_register_host_bridge(stru
>  	if (!bus)
>  		return -ENOMEM;
> 
> +	if (bridge->no_msi)
> +		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
> +
>  	bridge->bus = bus;
> 
>  	/* Temporarily move resources off the list */
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -545,6 +545,7 @@ struct pci_host_bridge {
>  	unsigned int	native_dpc:1;		/* OS may use PCIe DPC */
>  	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
>  	unsigned int	size_windows:1;		/* Enable root bus sizing */
> +	unsigned int	no_msi:1;		/* Bridge has no MSI support */
> 
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,

If that's the direction of travel, we also need something like this
for configuration where the host bridge relies on an external MSI block
that uses MSI domains (boot-tested in a GICv3 guest).

         M.

diff --git a/drivers/pci/controller/pci-host-common.c 
b/drivers/pci/controller/pci-host-common.c
index 6ce34a1deecb..603f6fbbe68a 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -77,6 +77,7 @@ int pci_host_common_probe(struct platform_device 
*pdev)

  	bridge->sysdata = cfg;
  	bridge->ops = (struct pci_ops *)&ops->pci_ops;
+	bridge->msi_domain = true;

  	platform_set_drvdata(pdev, bridge);

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 16fb150fbb8d..f421b2869bca 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -889,9 +889,6 @@ static int pci_register_host_bridge(struct 
pci_host_bridge *bridge)
  	if (!bus)
  		return -ENOMEM;

-	if (bridge->no_msi)
-		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
-
  	bridge->bus = bus;

  	/* Temporarily move resources off the list */
@@ -928,6 +925,9 @@ static int pci_register_host_bridge(struct 
pci_host_bridge *bridge)
  	device_enable_async_suspend(bus->bridge);
  	pci_set_bus_of_node(bus);
  	pci_set_bus_msi_domain(bus);
+	if (bridge->no_msi ||
+	    (bridge->msi_domain && !bus->dev.msi_domain))
+		bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;

  	if (!parent)
  		set_dev_node(bus->bridge, pcibus_to_node(bus));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c2a0c1d471d6..81f72fd46e06 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -546,6 +546,7 @@ struct pci_host_bridge {
  	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
  	unsigned int	size_windows:1;		/* Enable root bus sizing */
  	unsigned int	no_msi:1;		/* Bridge has no MSI support */
+	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */

  	/* Resource alignment requirements */
  	resource_size_t (*align_resource)(struct pci_dev *dev,

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

Aw: Re: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

Marc's Patch based on thomas' last one also seems to work well for r2, again no warning, PCI and AHCI (connected to pcie bus) working

regards Frank

Re: Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Thu, Nov 05 2020 at 09:20, Marc Zyngier wrote:
> On 2020-11-04 23:14, Thomas Gleixner wrote:
>>  	/* Resource alignment requirements */
>>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>
> If that's the direction of travel, we also need something like this
> for configuration where the host bridge relies on an external MSI block
> that uses MSI domains (boot-tested in a GICv3 guest).

Some more context would be helpful. Brain fails to decode the logic
here.

Re: Aw: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-05 23:00, Thomas Gleixner wrote:
> On Thu, Nov 05 2020 at 09:20, Marc Zyngier wrote:
>> On 2020-11-04 23:14, Thomas Gleixner wrote:
>>>  	/* Resource alignment requirements */
>>>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>> 
>> If that's the direction of travel, we also need something like this
>> for configuration where the host bridge relies on an external MSI 
>> block
>> that uses MSI domains (boot-tested in a GICv3 guest).
> 
> Some more context would be helpful. Brain fails to decode the logic
> here.

OK, let me try again.

The MSI controller, which is the thing that deals with MSIs in the 
system
(GICv2m, GICv3-ITS, and a number of others), is optional, is not part of 
the
host bridge (it has nothing to do with PCI at all), and the bridge 
driver has
absolutely no idea  whether:

- there is something that provides MSI or not
- that something has successfully been initialised or not (which 
translates
   into an MSI domain being present or not)

This is the case for most ARM systems, and all KVM/arm guests. Booting a 
VM
without MSIs is absolutely trivial, and actually makes sense for some of 
the
smaller guests.

In these conditions, your no_msi attribute doesn't work as is: we can't 
decide
on its value at probe time without extracting all of the OF/ACPI logic 
that
deals with MSI domains from the core code, and making it available to 
the host
bridge drivers for systems that follow that model.

Using the flow you insist on requires parsing the topology twice:

- once to find out whether there is actually a MSI provider registered
   for the host bridge in order to set the no_msi flag

- once to actually be able to store the domain into the pci_bus 
structure,
   as it isn't available at probe time.

My last suggestion is to indicate to the core code that there is a 
*possible*
MSI controller available in the form of a MSI domain. This is still 
suboptimal
compared to checking the presence an MSI domain in core code (my initial
suggestion), but the fallback stuff gets in the way (though I still 
think it
can be made to work).

Anyway, this was my last attempt at addressing the problem. Most people
won't see it. The couple of drivers that require the fallback hack are
usually selected in distro kernels, and do a good job hiding the error.

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

Aw: Re: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

Hi,

any new state here?

last fix works, but i have not seen it approved by anyone for merge or sent as separate Patch

regards Frank

Aw: Re: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

Hi,

5.11-rc1 is also affected, here is the relevant part of trace from my bananapi-r2:

[    5.792659] mtk-pcie 1a140000.pcie: PCI host bridge to bus 0000:00
[    5.798879] pci_bus 0000:00: root bus resource [bus 00-ff]
[    5.804412] pci_bus 0000:00: root bus resource [io  0x0000-0xffff] (bus address [0x1a160000-0x1a16ffff])
[    5.813928] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    5.820876] pci 0000:00:00.0: [14c3:0801] type 01 class 0x060400
[    5.826915] pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
[    5.833294] pci 0000:00:00.0: supports D1
[    5.837315] pci 0000:00:00.0: PME# supported from D0 D1 D3hot
[    5.843550] pci 0000:00:01.0: [14c3:0801] type 01 class 0x060400
[    5.849598] pci 0000:00:01.0: reg 0x14: [mem 0x00000000-0x0000ffff]
[    5.855995] pci 0000:00:01.0: supports D1
[    5.860016] pci 0000:00:01.0: PME# supported from D0 D1 D3hot
[    5.868688] PCI: bus0: Fast back to back transfers disabled
[    5.874325] pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    5.882392] pci 0000:00:01.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    5.890648] pci 0000:01:00.0: [14c3:7615] type 00 class 0x000280
[    5.896700] pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x000fffff 64bit]
[    5.933326] PCI: bus1: Fast back to back transfers disabled
[    5.938930] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    5.945814] pci 0000:02:00.0: [1b21:0611] type 00 class 0x010185
[    5.951889] pci 0000:02:00.0: reg 0x10: initial BAR value 0x00000000 invalid
[    5.958949] pci 0000:02:00.0: reg 0x10: [io  size 0x0008]
[    5.964382] pci 0000:02:00.0: reg 0x14: initial BAR value 0x00000000 invalid
[    5.971454] pci 0000:02:00.0: reg 0x14: [io  size 0x0004]
[    5.976869] pci 0000:02:00.0: reg 0x18: initial BAR value 0x00000000 invalid
[    5.983938] pci 0000:02:00.0: reg 0x18: [io  size 0x0008]
[    5.989352] pci 0000:02:00.0: reg 0x1c: initial BAR value 0x00000000 invalid
[    5.996424] pci 0000:02:00.0: reg 0x1c: [io  size 0x0004]
[    6.001854] pci 0000:02:00.0: reg 0x20: initial BAR value 0x00000000 invalid
[    6.008910] pci 0000:02:00.0: reg 0x20: [io  size 0x0010]
[    6.014337] pci 0000:02:00.0: reg 0x24: [mem 0x00000000-0x000001ff]
[    6.020635] pci 0000:02:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
[    6.053312] PCI: bus2: Fast back to back transfers disabled
[    6.058912] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 02
[    6.065622] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    6.072448] pci 0000:00:01.0: BAR 8: assigned [mem 0x60100000-0x601fffff]
[    6.079248] pci 0000:00:01.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
[    6.086500] pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
[    6.093318] pci 0000:00:01.0: BAR 1: assigned [mem 0x60310000-0x6031ffff]
[    6.100120] pci 0000:00:01.0: BAR 7: assigned [io  0x1000-0x1fff]
[    6.106255] pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x600fffff 64bit]
[    6.113609] pci 0000:00:00.0: PCI bridge to [bus 01]
[    6.118585] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    6.125405] pci 0000:02:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
[    6.132656] pci 0000:02:00.0: BAR 5: assigned [mem 0x60100000-0x601001ff]
[    6.139458] pci 0000:02:00.0: BAR 4: assigned [io  0x1000-0x100f]
[    6.145579] pci 0000:02:00.0: BAR 0: assigned [io  0x1010-0x1017]
[    6.151699] pci 0000:02:00.0: BAR 2: assigned [io  0x1018-0x101f]
[    6.157807] pci 0000:02:00.0: BAR 1: assigned [io  0x1020-0x1023]
[    6.163927] pci 0000:02:00.0: BAR 3: assigned [io  0x1024-0x1027]
[    6.170034] pci 0000:00:01.0: PCI bridge to [bus 02]
[    6.175035] pci 0000:00:01.0:   bridge window [io  0x1000-0x1fff]
[    6.181153] pci 0000:00:01.0:   bridge window [mem 0x60100000-0x601fffff]
[    6.187951] pci 0000:00:01.0:   bridge window [mem 0x60200000-0x602fffff pref]
[    6.195427] pcieport 0000:00:00.0: enabling device (0140 -> 0142)
[    6.201795] pcieport 0000:00:00.0: PME: Signaling with IRQ 258
[    6.208053] pcieport 0000:00:01.0: enabling device (0140 -> 0143)
[    6.214395] pcieport 0000:00:01.0: PME: Signaling with IRQ 252
[    6.220784] ahci 0000:02:00.0: version 3.0
[    6.220814] ahci 0000:02:00.0: enabling device (0140 -> 0143)
[    6.226705] ------------[ cut here ]------------
[    6.231380] WARNING: CPU: 0 PID: 33 at include/linux/msi.h:252 pci_msi_setup_msi_irqs.constprop.0+0x78/0x80
[    6.241167] Modules linked in:
[    6.244235] CPU: 0 PID: 33 Comm: kworker/0:1 Not tainted 5.11.0-rc1-bpi-r2-wifi #1
[    6.251816] Hardware name: Mediatek Cortex-A7 (Device Tree)
[    6.257395] Workqueue: events deferred_probe_work_func
[    6.262549] Backtrace:
[    6.264999] [<c0deb91c>] (dump_backtrace) from [<c0debce0>] (show_stack+0x20/0x24)
[    6.272598]  r7:000000fc r6:60000013 r5:00000000 r4:c15eff8c
[    6.278258] [<c0debcc0>] (show_stack) from [<c0def924>] (dump_stack+0xcc/0xe0)
[    6.285499] [<c0def858>] (dump_stack) from [<c0126c40>] (__warn+0xfc/0x114)
[    6.292477]  r7:000000fc r6:c061e14c r5:00000009 r4:c129b5b0
[    6.298137] [<c0126b44>] (__warn) from [<c0dec3a8>] (warn_slowpath_fmt+0x74/0xd0)
[    6.305641]  r7:c061e14c r6:000000fc r5:c129b5b0 r4:00000000
[    6.311301] [<c0dec338>] (warn_slowpath_fmt) from [<c061e14c>] (pci_msi_setup_msi_irqs.constprop.0+0x78/0x80)
[    6.321241]  r8:00000001 r7:00000000 r6:00000001 r5:c30f1800 r4:c318f800
[    6.327944] [<c061e0d4>] (pci_msi_setup_msi_irqs.constprop.0) from [<c061e7c8>] (__pci_enable_msi_range+0x224/0x39c)
[    6.338483] [<c061e5a4>] (__pci_enable_msi_range) from [<c061f218>] (pci_alloc_irq_vectors_affinity+0x114/0x158)
[    6.348680]  r10:c16815f8 r9:c23b1ae0 r8:00000001 r7:00000000 r6:c30f1800 r5:00000001
[    6.356515]  r4:00000002
[    6.359048] [<c061f104>] (pci_alloc_irq_vectors_affinity) from [<c088195c>] (ahci_init_one+0x500/0x958)
[    6.368468]  r9:00000000 r8:c318e200 r7:00000005 r6:c308c940 r5:c30f1800 r4:00000000
[    6.376215] [<c088145c>] (ahci_init_one) from [<c0614d70>] (pci_device_probe+0xb8/0x14c)
[    6.384331]  r10:c16815f8 r9:c1611408 r8:c1611438 r7:00000000 r6:c30f1800 r5:c0fb66a8
[    6.392166]  r4:c30f1880
[    6.394699] [<c0614cb8>] (pci_device_probe) from [<c07176e4>] (really_probe+0x220/0x50c)
[    6.402813]  r9:c1611438 r8:00000000 r7:c16f9ec0 r6:00000000 r5:c16f9eb8 r4:c30f1880
[    6.410560] [<c07174c4>] (really_probe) from [<c0717a58>] (driver_probe_device+0x88/0x1fc)
[    6.418849]  r10:00000000 r9:c31790c0 r8:c16815f8 r7:c30f1880 r6:c23b1cac r5:c1611438
[    6.426684]  r4:c30f1880
[    6.429217] [<c07179d0>] (driver_probe_device) from [<c0717f58>] (__device_attach_driver+0xa8/0x114)
[    6.438374]  r9:c31790c0 r8:c2336410 r7:c30f1880 r6:c23b1cac r5:c1611438 r4:00000001
[    6.446121] [<c0717eb0>] (__device_attach_driver) from [<c07151ac>] (bus_for_each_drv+0x94/0xe4)
[    6.454928]  r7:c30efa40 r6:c0717eb0 r5:c23b1cac r4:00000000
[    6.460589] [<c0715118>] (bus_for_each_drv) from [<c071740c>] (__device_attach+0x104/0x19c)
[    6.468960]  r6:00000000 r5:c30f18c4 r4:c30f1880
[    6.473577] [<c0717308>] (__device_attach) from [<c07174c0>] (device_attach+0x1c/0x20)
[    6.481515]  r6:c238d000 r5:c30f1880 r4:c30f1800
[    6.486132] [<c07174a4>] (device_attach) from [<c0606300>] (pci_bus_add_device+0x54/0x94)
[    6.494326] [<c06062ac>] (pci_bus_add_device) from [<c060637c>] (pci_bus_add_devices+0x3c/0x80)
[    6.503042]  r5:c238d014 r4:c30f1800
[    6.506617] [<c0606340>] (pci_bus_add_devices) from [<c06063b0>] (pci_bus_add_devices+0x70/0x80)
[    6.515422]  r7:c30efa40 r6:c238ec00 r5:c238ec14 r4:c30f0800
[    6.521082] [<c0606340>] (pci_bus_add_devices) from [<c060a5a4>] (pci_host_probe+0x50/0xa0)
[    6.529454]  r7:c30efa40 r6:c238ec00 r5:c238ec0c r4:00000000
[    6.535114] [<c060a554>] (pci_host_probe) from [<c062d234>] (mtk_pcie_probe+0x188/0x250)
[    6.543228]  r7:c30efa40 r6:c30efa4c r5:c30ef800 r4:c30d4b80
[    6.548889] [<c062d0ac>] (mtk_pcie_probe) from [<c071a7b4>] (platform_probe+0x6c/0xc8)
[    6.556830]  r10:c16815f8 r9:c15f8c9c r8:00000000 r7:c16f9ec0 r6:c15f8c9c r5:c2336410
[    6.564667]  r4:00000000 r3:c062d0ac
[    6.568242] [<c071a748>] (platform_probe) from [<c07176e4>] (really_probe+0x220/0x50c)
[    6.576181]  r7:c16f9ec0 r6:00000000 r5:c16f9eb8 r4:c2336410
[    6.581842] [<c07174c4>] (really_probe) from [<c0717a58>] (driver_probe_device+0x88/0x1fc)
[    6.590130]  r10:c2336410 r9:c12b6910 r8:c16815f8 r7:c2336410 r6:c23b1e7c r5:c15f8c9c
[    6.597965]  r4:c2336410
[    6.600498] [<c07179d0>] (driver_probe_device) from [<c0717f58>] (__device_attach_driver+0xa8/0x114)
[    6.609655]  r9:c12b6910 r8:c16815f8 r7:c2336410 r6:c23b1e7c r5:c15f8c9c r4:00000001
[    6.617401] [<c0717eb0>] (__device_attach_driver) from [<c07151ac>] (bus_for_each_drv+0x94/0xe4)
[    6.626209]  r7:c1606130 r6:c0717eb0 r5:c23b1e7c r4:00000000
[    6.631870] [<c0715118>] (bus_for_each_drv) from [<c071740c>] (__device_attach+0x104/0x19c)
[    6.640241]  r6:00000001 r5:c2336454 r4:c2336410
[    6.644859] [<c0717308>] (__device_attach) from [<c0717fe0>] (device_initial_probe+0x1c/0x20)
[    6.653405]  r6:c2336410 r5:c16063d8 r4:c2315e54
[    6.658022] [<c0717fc4>] (device_initial_probe) from [<c071635c>] (bus_probe_device+0x94/0x9c)
[    6.666652] [<c07162c8>] (bus_probe_device) from [<c0716924>] (deferred_probe_work_func+0x9c/0xe0)
[    6.675633]  r7:c1606130 r6:c160611c r5:c160611c r4:c2315e54
[    6.681294] [<c0716888>] (deferred_probe_work_func) from [<c0144ec0>] (process_one_work+0x1c8/0x530)
[    6.690452]  r10:00000000 r9:c1673d00 r8:00000000 r7:df59b900 r6:df598740 r5:c2383a80
[    6.698287]  r4:c160615c r3:c0716888
[    6.701862] [<c0144cf8>] (process_one_work) from [<c014545c>] (worker_thread+0x234/0x534)
[    6.710062]  r10:df598740 r9:00000008 r8:c1503d00 r7:df598758 r6:c2383a94 r5:df598740
[    6.717897]  r4:c2383a80
[    6.720430] [<c0145228>] (worker_thread) from [<c014d8dc>] (kthread+0x130/0x158)
[    6.727847]  r10:c22410a4 r9:c2383a80 r8:c0145228 r7:c23b0000 r6:c214de8c r5:c23995c0
[    6.735682]  r4:c2241080
[    6.738216] [<c014d7ac>] (kthread) from [<c0100150>] (ret_from_fork+0x14/0x24)
[    6.745453] Exception stack(0xc23b1fb0 to 0xc23b1ff8)
[    6.750512] 1fa0:                                     00000000 00000000 00000000 00000000
[    6.758699] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    6.766885] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    6.773508]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c014d7ac
[    6.781344]  r4:c23995c0 r3:00000001
[    6.784944] ---[ end trace b6842aa211313a7f ]---
[    6.789572] ------------[ cut here ]------------
[    6.794205] WARNING: CPU: 0 PID: 33 at include/linux/msi.h:258 free_msi_irqs+0x1c8/0x1cc
[    6.802327] Modules linked in:
[    6.805388] CPU: 0 PID: 33 Comm: kworker/0:1 Tainted: G        W         5.11.0-rc1-bpi-r2-wifi #1
[    6.814358] Hardware name: Mediatek Cortex-A7 (Device Tree)
[    6.819935] Workqueue: events deferred_probe_work_func
[    6.825085] Backtrace:
[    6.827534] [<c0deb91c>] (dump_backtrace) from [<c0debce0>] (show_stack+0x20/0x24)
[    6.835130]  r7:00000102 r6:60000013 r5:00000000 r4:c15eff8c
[    6.840790] [<c0debcc0>] (show_stack) from [<c0def924>] (dump_stack+0xcc/0xe0)
[    6.848030] [<c0def858>] (dump_stack) from [<c0126c40>] (__warn+0xfc/0x114)
[    6.855008]  r7:00000102 r6:c061e31c r5:00000009 r4:c129b5b0
[    6.860668] [<c0126b44>] (__warn) from [<c0dec3a8>] (warn_slowpath_fmt+0x74/0xd0)
[    6.868171]  r7:c061e31c r6:00000102 r5:c129b5b0 r4:00000000
[    6.873831] [<c0dec338>] (warn_slowpath_fmt) from [<c061e31c>] (free_msi_irqs+0x1c8/0x1cc)
[    6.882119]  r8:00000001 r7:c30f1800 r6:c30f19dc r5:c30f1800 r4:c30f19dc
[    6.888821] [<c061e154>] (free_msi_irqs) from [<c061e870>] (__pci_enable_msi_range+0x2cc/0x39c)
[    6.897541]  r10:00000001 r9:ffffffed r8:00000001 r7:00000000 r6:00000001 r5:c30f1800
[    6.905377]  r4:c318f800 r3:00000000
[    6.908951] [<c061e5a4>] (__pci_enable_msi_range) from [<c061f218>] (pci_alloc_irq_vectors_affinity+0x114/0x158)
[    6.919149]  r10:c16815f8 r9:c23b1ae0 r8:00000001 r7:00000000 r6:c30f1800 r5:00000001
[    6.926984]  r4:00000002
[    6.929517] [<c061f104>] (pci_alloc_irq_vectors_affinity) from [<c088195c>] (ahci_init_one+0x500/0x958)
[    6.938936]  r9:00000000 r8:c318e200 r7:00000005 r6:c308c940 r5:c30f1800 r4:00000000
[    6.946683] [<c088145c>] (ahci_init_one) from [<c0614d70>] (pci_device_probe+0xb8/0x14c)
[    6.954798]  r10:c16815f8 r9:c1611408 r8:c1611438 r7:00000000 r6:c30f1800 r5:c0fb66a8
[    6.962634]  r4:c30f1880
[    6.965166] [<c0614cb8>] (pci_device_probe) from [<c07176e4>] (really_probe+0x220/0x50c)
[    6.973280]  r9:c1611438 r8:00000000 r7:c16f9ec0 r6:00000000 r5:c16f9eb8 r4:c30f1880
[    6.981027] [<c07174c4>] (really_probe) from [<c0717a58>] (driver_probe_device+0x88/0x1fc)
[    6.989316]  r10:00000000 r9:c31790c0 r8:c16815f8 r7:c30f1880 r6:c23b1cac r5:c1611438
[    6.997151]  r4:c30f1880
[    6.999683] [<c07179d0>] (driver_probe_device) from [<c0717f58>] (__device_attach_driver+0xa8/0x114)
[    7.008840]  r9:c31790c0 r8:c2336410 r7:c30f1880 r6:c23b1cac r5:c1611438 r4:00000001
[    7.016587] [<c0717eb0>] (__device_attach_driver) from [<c07151ac>] (bus_for_each_drv+0x94/0xe4)
[    7.025395]  r7:c30efa40 r6:c0717eb0 r5:c23b1cac r4:00000000
[    7.031055] [<c0715118>] (bus_for_each_drv) from [<c071740c>] (__device_attach+0x104/0x19c)
[    7.039427]  r6:00000000 r5:c30f18c4 r4:c30f1880
[    7.044043] [<c0717308>] (__device_attach) from [<c07174c0>] (device_attach+0x1c/0x20)
[    7.051980]  r6:c238d000 r5:c30f1880 r4:c30f1800
[    7.056598] [<c07174a4>] (device_attach) from [<c0606300>] (pci_bus_add_device+0x54/0x94)
[    7.064793] [<c06062ac>] (pci_bus_add_device) from [<c060637c>] (pci_bus_add_devices+0x3c/0x80)
[    7.073509]  r5:c238d014 r4:c30f1800
[    7.077084] [<c0606340>] (pci_bus_add_devices) from [<c06063b0>] (pci_bus_add_devices+0x70/0x80)
[    7.085889]  r7:c30efa40 r6:c238ec00 r5:c238ec14 r4:c30f0800
[    7.091549] [<c0606340>] (pci_bus_add_devices) from [<c060a5a4>] (pci_host_probe+0x50/0xa0)
[    7.099920]  r7:c30efa40 r6:c238ec00 r5:c238ec0c r4:00000000
[    7.105581] [<c060a554>] (pci_host_probe) from [<c062d234>] (mtk_pcie_probe+0x188/0x250)
[    7.113694]  r7:c30efa40 r6:c30efa4c r5:c30ef800 r4:c30d4b80
[    7.119354] [<c062d0ac>] (mtk_pcie_probe) from [<c071a7b4>] (platform_probe+0x6c/0xc8)
[    7.127296]  r10:c16815f8 r9:c15f8c9c r8:00000000 r7:c16f9ec0 r6:c15f8c9c r5:c2336410
[    7.135132]  r4:00000000 r3:c062d0ac
[    7.138706] [<c071a748>] (platform_probe) from [<c07176e4>] (really_probe+0x220/0x50c)
[    7.146646]  r7:c16f9ec0 r6:00000000 r5:c16f9eb8 r4:c2336410
[    7.152306] [<c07174c4>] (really_probe) from [<c0717a58>] (driver_probe_device+0x88/0x1fc)
[    7.160594]  r10:c2336410 r9:c12b6910 r8:c16815f8 r7:c2336410 r6:c23b1e7c r5:c15f8c9c
[    7.168430]  r4:c2336410
[    7.170962] [<c07179d0>] (driver_probe_device) from [<c0717f58>] (__device_attach_driver+0xa8/0x114)
[    7.180119]  r9:c12b6910 r8:c16815f8 r7:c2336410 r6:c23b1e7c r5:c15f8c9c r4:00000001
[    7.187866] [<c0717eb0>] (__device_attach_driver) from [<c07151ac>] (bus_for_each_drv+0x94/0xe4)
[    7.196672]  r7:c1606130 r6:c0717eb0 r5:c23b1e7c r4:00000000
[    7.202333] [<c0715118>] (bus_for_each_drv) from [<c071740c>] (__device_attach+0x104/0x19c)
[    7.210704]  r6:00000001 r5:c2336454 r4:c2336410
[    7.215321] [<c0717308>] (__device_attach) from [<c0717fe0>] (device_initial_probe+0x1c/0x20)
[    7.223867]  r6:c2336410 r5:c16063d8 r4:c2315e54
[    7.228484] [<c0717fc4>] (device_initial_probe) from [<c071635c>] (bus_probe_device+0x94/0x9c)
[    7.237114] [<c07162c8>] (bus_probe_device) from [<c0716924>] (deferred_probe_work_func+0x9c/0xe0)
[    7.246095]  r7:c1606130 r6:c160611c r5:c160611c r4:c2315e54
[    7.251756] [<c0716888>] (deferred_probe_work_func) from [<c0144ec0>] (process_one_work+0x1c8/0x530)
[    7.260912]  r10:00000000 r9:c1673d00 r8:00000000 r7:df59b900 r6:df598740 r5:c2383a80
[    7.268748]  r4:c160615c r3:c0716888
[    7.272323] [<c0144cf8>] (process_one_work) from [<c014545c>] (worker_thread+0x234/0x534)
[    7.280523]  r10:df598740 r9:00000008 r8:c1503d00 r7:df598758 r6:c2383a94 r5:df598740
[    7.288358]  r4:c2383a80
[    7.290890] [<c0145228>] (worker_thread) from [<c014d8dc>] (kthread+0x130/0x158)
[    7.298306]  r10:c22410a4 r9:c2383a80 r8:c0145228 r7:c23b0000 r6:c214de8c r5:c23995c0
[    7.306141]  r4:c2241080
[    7.308674] [<c014d7ac>] (kthread) from [<c0100150>] (ret_from_fork+0x14/0x24)
[    7.315910] Exception stack(0xc23b1fb0 to 0xc23b1ff8)
[    7.320968] 1fa0:                                     00000000 00000000 00000000 00000000
[    7.329155] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    7.337341] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    7.343964]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c014d7ac
[    7.351799]  r4:c23995c0 r3:00000001
[    7.355410] ---[ end trace b6842aa211313a80 ]---

regards Frank

Aw: Re: Re: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Frank Wunderlich]

Hi,

is there any new state?

kernel test robot reports the following problem (i do not see it when compiling for my arm/arm64 devices):

ARCH=i386

drivers/pci/probe.c: In function 'pci_register_host_bridge':
>> drivers/pci/probe.c:930:39: error: 'struct device' has no member named 'msi_domain'; did you mean 'pm_domain'?
930 | (bridge->msi_domain && !bus->dev.msi_domain))

and yes...msi_domain is only member of pci_host_bridge (include/linux/pci.h)

why do you check for msi_domain in pci_bus->dev

regards Frank

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Tue, Nov 03 2020 at 09:54, Marc Zyngier wrote:
> On 2020-11-02 22:18, Thomas Gleixner wrote:
>> So we really need some other solution and removing the warning is not 
>> an option. If MSI is enabled then we want to get a warning when a PCI
>> device has no MSI domain associated. Explicitly expressing the PCIE
>> brigde misfeature of not supporting MSI is way better than silently
>> returning an error code which is swallowed anyway.
>
> I don't disagree here, though the PCI_MSI_ARCH_FALLBACKS mechanism
> makes it more difficult to establish.

Only for the few leftovers which implement msi_controller, i.e.

drivers/pci/controller/pci-hyperv.c
drivers/pci/controller/pci-tegra.c
drivers/pci/controller/pcie-rcar-host.c
drivers/pci/controller/pcie-xilinx.c

The architectures which select PCI_MSI_ARCH_FALLBACKS are:

arch/ia64/Kconfig:      select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
arch/mips/Kconfig:      select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
arch/powerpc/Kconfig:   select PCI_MSI_ARCH_FALLBACKS           if PCI_MSI
arch/s390/Kconfig:      select PCI_MSI_ARCH_FALLBACKS   if PCI_MSI
arch/sparc/Kconfig:     select PCI_MSI_ARCH_FALLBACKS if PCI_MSI

implement arch_setup_msi_irq() which makes it magically work :)

>> Whatever the preferred way is via flags at host probe time or flagging
>> it post probe I don't care much as long as it is consistent.
>
> Host probe time is going to require some changes in the core PCI api,
> as everything that checks for a MSI domain is based on the pci_bus
> structure, which is only allocated much later.

Yeah, it's nasty. One possible solution is to add flags or a callback to
pci_ops, but it's not pretty either.

I think we should go with the 'mark it after pci_host_probe()' hack for
5.10-rc. The real fix will be larger and go into 5.11.

Thoughts?

Thanks,

        tglx

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Marc Zyngier]

On 2020-11-03 10:31, Thomas Gleixner wrote:
> On Tue, Nov 03 2020 at 09:54, Marc Zyngier wrote:
>> On 2020-11-02 22:18, Thomas Gleixner wrote:
>>> So we really need some other solution and removing the warning is not
>>> an option. If MSI is enabled then we want to get a warning when a PCI
>>> device has no MSI domain associated. Explicitly expressing the PCIE
>>> brigde misfeature of not supporting MSI is way better than silently
>>> returning an error code which is swallowed anyway.
>> 
>> I don't disagree here, though the PCI_MSI_ARCH_FALLBACKS mechanism
>> makes it more difficult to establish.
> 
> Only for the few leftovers which implement msi_controller, i.e.
> 
> drivers/pci/controller/pci-hyperv.c
> drivers/pci/controller/pci-tegra.c
> drivers/pci/controller/pcie-rcar-host.c
> drivers/pci/controller/pcie-xilinx.c
> 
> The architectures which select PCI_MSI_ARCH_FALLBACKS are:
> 
> arch/ia64/Kconfig:      select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> arch/mips/Kconfig:      select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> arch/powerpc/Kconfig:   select PCI_MSI_ARCH_FALLBACKS           if 
> PCI_MSI
> arch/s390/Kconfig:      select PCI_MSI_ARCH_FALLBACKS   if PCI_MSI
> arch/sparc/Kconfig:     select PCI_MSI_ARCH_FALLBACKS if PCI_MSI
> 
> implement arch_setup_msi_irq() which makes it magically work :)
> 
>>> Whatever the preferred way is via flags at host probe time or 
>>> flagging
>>> it post probe I don't care much as long as it is consistent.
>> 
>> Host probe time is going to require some changes in the core PCI api,
>> as everything that checks for a MSI domain is based on the pci_bus
>> structure, which is only allocated much later.
> 
> Yeah, it's nasty. One possible solution is to add flags or a callback 
> to
> pci_ops, but it's not pretty either.
> 
> I think we should go with the 'mark it after pci_host_probe()' hack for
> 5.10-rc. The real fix will be larger and go into 5.11.
> 
> Thoughts?

We can do that, although I worried that it isn't 100% reliable:

pci_host_probe() ends up calling pci_add_device(), and will start
probing devices if the endpoint drivers have already registered
with the core code, long before the flag gets set.

Here's what I've hacked together for a guest that doesn't have
any MSI capability:

diff --git a/drivers/pci/controller/pci-host-common.c 
b/drivers/pci/controller/pci-host-common.c
index 6ce34a1deecb..7dd5145cd38d 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -55,6 +55,7 @@ int pci_host_common_probe(struct platform_device 
*pdev)
  	struct pci_host_bridge *bridge;
  	struct pci_config_window *cfg;
  	const struct pci_ecam_ops *ops;
+	int ret;

  	ops = of_device_get_match_data(&pdev->dev);
  	if (!ops)
@@ -80,7 +81,10 @@ int pci_host_common_probe(struct platform_device 
*pdev)

  	platform_set_drvdata(pdev, bridge);

-	return pci_host_probe(bridge);
+	ret = pci_host_probe(bridge);
+	bridge->bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
+	return ret;
  }
  EXPORT_SYMBOL_GPL(pci_host_common_probe);

(plus another hack to get the host controller to initialise a bit
later, though building it as a module will achieve the same thing):

[    0.369114] 9pnet: Installing 9P2000 support
[    0.369807] mpls_gso: MPLS GSO support
[    0.370512] registered taskstats version 1
[    0.371204] Loading compiled-in X.509 certificates
[    0.371988] zswap: loaded using pool lzo/zbud
[    0.373041] pci-host-generic 40000000.pci: host bridge /pci ranges:
[    0.374045] pci-host-generic 40000000.pci:       IO 
0x0000000000..0x000000ffff -> 0x0000000000
[    0.375458] pci-host-generic 40000000.pci:      MEM 
0x0041000000..0x007fffffff -> 0x0041000000
[    0.376848] pci-host-generic 40000000.pci: ECAM at [mem 
0x40000000-0x40ffffff] for [bus 00]
[    0.378204] pci-host-generic 40000000.pci: PCI host bridge to bus 
0000:00
[    0.379316] pci_bus 0000:00: root bus resource [bus 00]
[    0.380146] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    0.381131] pci_bus 0000:00: root bus resource [mem 
0x41000000-0x7fffffff]
[    0.382267] pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
[    0.383369] pci 0000:00:00.0: reg 0x10: [io  0x6200-0x62ff]
[    0.384286] pci 0000:00:00.0: reg 0x14: [mem 0x41000000-0x410000ff]
[    0.385324] pci 0000:00:00.0: reg 0x18: [mem 0x41000200-0x410003ff]
[    0.386680] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
[    0.387778] pci 0000:00:01.0: reg 0x10: [io  0x6300-0x63ff]
[    0.388696] pci 0000:00:01.0: reg 0x14: [mem 0x41000400-0x410004ff]
[    0.389730] pci 0000:00:01.0: reg 0x18: [mem 0x41000600-0x410007ff]
[    0.391070] pci 0000:00:02.0: [1af4:1000] type 00 class 0x020000
[    0.392212] pci 0000:00:02.0: reg 0x10: [io  0x6400-0x64ff]
[    0.393137] pci 0000:00:02.0: reg 0x14: [mem 0x41000800-0x410008ff]
[    0.394163] pci 0000:00:02.0: reg 0x18: [mem 0x41000a00-0x41000bff]
[    0.395678] pci 0000:00:00.0: BAR 2: assigned [mem 
0x41000000-0x410001ff]
[    0.396762] pci 0000:00:01.0: BAR 2: assigned [mem 
0x41000200-0x410003ff]
[    0.397851] pci 0000:00:02.0: BAR 2: assigned [mem 
0x41000400-0x410005ff]
[    0.398934] pci 0000:00:00.0: BAR 0: assigned [io  0x1000-0x10ff]
[    0.400014] pci 0000:00:00.0: BAR 1: assigned [mem 
0x41000600-0x410006ff]
[    0.401105] pci 0000:00:01.0: BAR 0: assigned [io  0x1100-0x11ff]
[    0.402080] pci 0000:00:01.0: BAR 1: assigned [mem 
0x41000700-0x410007ff]
[    0.403185] pci 0000:00:02.0: BAR 0: assigned [io  0x1200-0x12ff]
[    0.404156] pci 0000:00:02.0: BAR 1: assigned [mem 
0x41000800-0x410008ff]
[    0.405344] virtio-pci 0000:00:00.0: virtio_pci: leaving for legacy 
driver
[    0.406569] ------------[ cut here ]------------
[    0.407347] WARNING: CPU: 1 PID: 1 at include/linux/msi.h:213 
__pci_enable_msix_range+0x680/0x720
[    0.408884] Modules linked in:
[    0.409429] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
5.10.0-rc2-dirty #2117
[    0.410646] Hardware name: linux,dummy-virt (DT)
[    0.411463] pstate: 60000005 (nZCv daif -PAN -UAO -TCO BTYPE=--)
[    0.412505] pc : __pci_enable_msix_range+0x680/0x720
[    0.413380] lr : __pci_enable_msix_range+0x394/0x720
[    0.414244] sp : ffffffc0119ab3d0
[    0.414830] x29: ffffffc0119ab3d0 x28: 0000000000000002
[    0.415766] x27: ffffff804107b0b8 x26: ffffff804107b000
[    0.416689] x25: ffffff80410acb00 x24: 0000000000000000
[    0.417623] x23: ffffff80410ac480 x22: ffffff804107b000
[    0.418549] x21: ffffff804107b2e8 x20: 0000000000000014
[    0.419482] x19: 0000000000000002 x18: 00000000fffffffc
[    0.420408] x17: 000000002ce4d3e3 x16: 00000000a4f09d66
[    0.421342] x15: 0000000000000020 x14: ffffffffffffffff
[    0.422268] x13: 0000000041001000 x12: ffffffc0119cf000
[    0.423201] x11: ffffffc010000000 x10: ffffffc0119cd000
[    0.424127] x9 : ffffffc010628ad4 x8 : 0000000000000000
[    0.425063] x7 : 0000000000000000 x6 : 000000000000003f
[    0.425989] x5 : 0000000000000040 x4 : ffffff804107b2e8
[    0.426914] x3 : 0000000000000004 x2 : 0000000000000000
[    0.427855] x1 : 0000000000000000 x0 : 0000000000000000
[    0.428788] Call trace:
[    0.429226]  __pci_enable_msix_range+0x680/0x720
[    0.430033]  pci_alloc_irq_vectors_affinity+0xcc/0x144
[    0.430932]  vp_find_vqs_msix+0xdc/0x414
[    0.431631]  vp_find_vqs+0x54/0x1c0
[    0.432245]  p9_virtio_probe+0xa8/0x374

This is admittedly a contrived example, but I'm not convinced it is
completely unlikely.

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

Re: Aw: Re: [PATCH] pci: mediatek: fix warning in msi.h

[Thomas Gleixner]

On Tue, Nov 03 2020 at 11:41, Marc Zyngier wrote:
> On 2020-11-03 10:31, Thomas Gleixner wrote:
> We can do that, although I worried that it isn't 100% reliable:
>
> pci_host_probe() ends up calling pci_add_device(), and will start
> probing devices if the endpoint drivers have already registered
> with the core code, long before the flag gets set.

Bah, you're right. What a mess.