* [PATCH] powerpc: pseries: Round up MSI-X requests
@ 2012-06-03 23:15 Anton Blanchard
2012-06-04 6:43 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Anton Blanchard @ 2012-06-03 23:15 UTC (permalink / raw)
To: benh, paulus, michael; +Cc: linuxppc-dev
The pseries firmware currently refuses any non power of two MSI-X
request. Unfortunately most network drivers end up asking for that
because they want a power of two for RX queues and one or two extra
for everything else.
This patch rounds up the firmware request to the next power of two
if the quota allows it.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-build/arch/powerpc/platforms/pseries/msi.c
===================================================================
--- linux-build.orig/arch/powerpc/platforms/pseries/msi.c 2012-06-03 20:49:29.082280031 +1000
+++ linux-build/arch/powerpc/platforms/pseries/msi.c 2012-06-04 09:06:55.909732276 +1000
@@ -402,6 +402,18 @@ static int rtas_setup_msi_irqs(struct pc
return -EINVAL;
/*
+ * Firmware currently refuse any non power of two allocation
+ * so we round up if the quota will allow it.
+ */
+ if (type == PCI_CAP_ID_MSIX) {
+ int m = roundup_pow_of_two(nvec);
+ int quota = msi_quota_for_device(pdev, m);
+
+ if (quota >= m)
+ nvec = m;
+ }
+
+ /*
* Try the new more explicit firmware interface, if that fails fall
* back to the old interface. The old interface is known to never
* return MSI-Xs.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc: pseries: Round up MSI-X requests 2012-06-03 23:15 [PATCH] powerpc: pseries: Round up MSI-X requests Anton Blanchard @ 2012-06-04 6:43 ` Michael Ellerman 2012-06-04 6:54 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Michael Ellerman @ 2012-06-04 6:43 UTC (permalink / raw) To: Anton Blanchard; +Cc: paulus, linuxppc-dev On Mon, 2012-06-04 at 09:15 +1000, Anton Blanchard wrote: > The pseries firmware currently refuses any non power of two MSI-X > request. Unfortunately most network drivers end up asking for that > because they want a power of two for RX queues and one or two extra > for everything else. > > This patch rounds up the firmware request to the next power of two > if the quota allows it. There is some chance this will result in breakage because the driver asks for N - and assumes that is what was allocated - and the device is configured for > N. But that's a hypothetical, and we know the current approach sucks because it will result in many drivers falling back to a single interrupt. I think this is the least-worst approach in light of the FW limitations, and we can always add quirks in here if we really have to. Paul is the pseries maintainer so he gets to ACK or NAK it, but from an MSI point of view it gets my +1. cheers ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: pseries: Round up MSI-X requests 2012-06-04 6:43 ` Michael Ellerman @ 2012-06-04 6:54 ` Benjamin Herrenschmidt 2012-06-05 2:47 ` Anton Blanchard 0 siblings, 1 reply; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-06-04 6:54 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, paulus, Anton Blanchard On Mon, 2012-06-04 at 16:43 +1000, Michael Ellerman wrote: > There is some chance this will result in breakage because the driver > asks for N - and assumes that is what was allocated - and the device is > configured for > N. We can fix that. We can whack the configuration back with N, just know that we have "allocated" > N. > But that's a hypothetical, and we know the current approach sucks > because it will result in many drivers falling back to a single > interrupt. > > I think this is the least-worst approach in light of the FW limitations, > and we can always add quirks in here if we really have to. > > Paul is the pseries maintainer so he gets to ACK or NAK it, but from an > MSI point of view it gets my +1. > Cheers, Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: pseries: Round up MSI-X requests 2012-06-04 6:54 ` Benjamin Herrenschmidt @ 2012-06-05 2:47 ` Anton Blanchard 2012-06-13 5:18 ` Michael Ellerman 0 siblings, 1 reply; 6+ messages in thread From: Anton Blanchard @ 2012-06-05 2:47 UTC (permalink / raw) To: Benjamin Herrenschmidt, Michael Ellerman; +Cc: linuxppc-dev, paulus Hi, > On Mon, 2012-06-04 at 16:43 +1000, Michael Ellerman wrote: > > There is some chance this will result in breakage because the driver > > asks for N - and assumes that is what was allocated - and the > > device is configured for > N. > > We can fix that. We can whack the configuration back with N, just know > that we have "allocated" > N. I agree we don't want to be giving back a larger value than requested. There's only one place that can happen in theory and since firmware only returns power of two values I dont think it will happen in practise. Even so do we want to do something like this (as yet untested)? If the rounded up request fails we retry with the original request. The pseries msi free code just sets our vectors to 0 so it doesn't need to know how many were originally allocated. Anton -- [PATCH] powerpc: pseries: Round up MSI-X requests The pseries firmware currently refuses any non power of two MSI-X request. Unfortunately most network drivers end up asking for that because they want a power of two for RX queues and one or two extra for everything else. This patch rounds up the firmware request to the next power of two if the quota allows it. If this fails we fall back to using the original request size. Signed-off-by: Anton Blanchard <anton@samba.org> --- Index: linux-build/arch/powerpc/platforms/pseries/msi.c =================================================================== --- linux-build.orig/arch/powerpc/platforms/pseries/msi.c 2012-06-04 15:58:48.095833249 +1000 +++ linux-build/arch/powerpc/platforms/pseries/msi.c 2012-06-05 12:15:58.711074499 +1000 @@ -387,12 +387,13 @@ static int check_msix_entries(struct pci return 0; } -static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type) +static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type) { struct pci_dn *pdn; int hwirq, virq, i, rc; struct msi_desc *entry; struct msi_msg msg; + int nvec = nvec_in; pdn = get_pdn(pdev); if (!pdn) @@ -402,10 +403,23 @@ static int rtas_setup_msi_irqs(struct pc return -EINVAL; /* + * Firmware currently refuse any non power of two allocation + * so we round up if the quota will allow it. + */ + if (type == PCI_CAP_ID_MSIX) { + int m = roundup_pow_of_two(nvec); + int quota = msi_quota_for_device(pdev, m); + + if (quota >= m) + nvec = m; + } + + /* * Try the new more explicit firmware interface, if that fails fall * back to the old interface. The old interface is known to never * return MSI-Xs. */ +again: if (type == PCI_CAP_ID_MSI) { rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec); @@ -417,6 +431,10 @@ static int rtas_setup_msi_irqs(struct pc rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec); if (rc != nvec) { + if (nvec != nvec_in) { + nvec = nvec_in; + goto again; + } pr_debug("rtas_msi: rtas_change_msi() failed\n"); return rc; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: pseries: Round up MSI-X requests 2012-06-05 2:47 ` Anton Blanchard @ 2012-06-13 5:18 ` Michael Ellerman 2012-06-13 5:25 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 6+ messages in thread From: Michael Ellerman @ 2012-06-13 5:18 UTC (permalink / raw) To: Anton Blanchard; +Cc: linuxppc-dev, paulus On Tue, 2012-06-05 at 12:47 +1000, Anton Blanchard wrote: > Hi, > > > On Mon, 2012-06-04 at 16:43 +1000, Michael Ellerman wrote: > > > There is some chance this will result in breakage because the driver > > > asks for N - and assumes that is what was allocated - and the > > > device is configured for > N. > > > > We can fix that. We can whack the configuration back with N, just know > > that we have "allocated" > N. I think whacking config space is more likely to break something than just configuring more than the driver asked for. > I agree we don't want to be giving back a larger value than requested. > There's only one place that can happen in theory and since firmware only > returns power of two values I dont think it will happen in practise. Don't follow you here. > Even so do we want to do something like this (as yet untested)? If the > rounded up request fails we retry with the original request. Yes we do. Had a chance to test it? > The pseries msi free code just sets our vectors to 0 so it doesn't need > to know how many were originally allocated. Yep, looks like it will cope. We only create virqs for what's in pdev->msi_list, which will be what the driver originally asked for, and we free all those by walking the list again. So the fact that firmware allocated a few extra for us is OK, we have nothing extra to cleanup. cheers ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: pseries: Round up MSI-X requests 2012-06-13 5:18 ` Michael Ellerman @ 2012-06-13 5:25 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 6+ messages in thread From: Benjamin Herrenschmidt @ 2012-06-13 5:25 UTC (permalink / raw) To: Michael Ellerman; +Cc: paulus, linuxppc-dev, Anton Blanchard On Wed, 2012-06-13 at 15:18 +1000, Michael Ellerman wrote: > On Tue, 2012-06-05 at 12:47 +1000, Anton Blanchard wrote: > > Hi, > > > > > On Mon, 2012-06-04 at 16:43 +1000, Michael Ellerman wrote: > > > > There is some chance this will result in breakage because the driver > > > > asks for N - and assumes that is what was allocated - and the > > > > device is configured for > N. > > > > > > We can fix that. We can whack the configuration back with N, just know > > > that we have "allocated" > N. > > I think whacking config space is more likely to break something than > just configuring more than the driver asked for. How so ? I tend to disagree here.. . > > I agree we don't want to be giving back a larger value than requested. > > There's only one place that can happen in theory and since firmware only > > returns power of two values I dont think it will happen in practise. > > Don't follow you here. > > > Even so do we want to do something like this (as yet untested)? If the > > rounded up request fails we retry with the original request. > > Yes we do. Had a chance to test it? > > > The pseries msi free code just sets our vectors to 0 so it doesn't need > > to know how many were originally allocated. > > Yep, looks like it will cope. > > We only create virqs for what's in pdev->msi_list, which will be what > the driver originally asked for, and we free all those by walking the > list again. So the fact that firmware allocated a few extra for us is > OK, we have nothing extra to cleanup. The firmware interface is busted anyway. We want to be able to enable/allocate individual MSI-X at runtime, we need to get a new interface sorted. Ben. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-13 5:25 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-06-03 23:15 [PATCH] powerpc: pseries: Round up MSI-X requests Anton Blanchard 2012-06-04 6:43 ` Michael Ellerman 2012-06-04 6:54 ` Benjamin Herrenschmidt 2012-06-05 2:47 ` Anton Blanchard 2012-06-13 5:18 ` Michael Ellerman 2012-06-13 5:25 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).