Re: [E1000-devel] [PATCH] i40e: don't enable and init FCOE by default when do PF reset
On 2015/1/20 5:10, Dev, Vasu wrote:
-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Friday, January 16, 2015 7:01 PM
To: Kirsher, Jeffrey T
Cc: Dev, Vasu; Ethan Zhao; Ronciak, John; Brandeburg, Jesse; Allan, Bruce W;
Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick, Matthew;
Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset
Vasu,
What' your idea about the v2, any suggestion ? Jeff is looking forward
to
see it.
Jeff was asking for v2 in response to your last comment as disable FCOE as
default configuration as a temporary step but I think that is the fix and
user should n't enable FCoE until they have FCoE enabled X710 FCoE with
either fabric or VN2VN mode FCoE setup.
As a Linux distro, we don't know users have FCoE capable X710 or not,
so we couldn't disable the FCoE configuration
by default in the released kernel except FCoE is officially not
supported yet with X710, but if yes, fix those bugs I
mentioned is the only choice.
Thanks,
Ethan
Thanks,
Vasu
Thanks,
Ethan
On 2015/1/16 22:47, Jeff Kirsher wrote:
On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
Vasu,
OK, disable FCOE as default configuration as a temporary step to
make it work.
Sounds like I should expect a v2 coming, correct?
Thanks,
Ethan
On 2015/1/16 7:45, Dev, Vasu wrote:
-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg,
Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose,
Gregory V; Vick, Matthew; Williams, Mitch A; Parikh, Neerav; Linux
NICS; e1000- de...@lists.sourceforge.net; net...@vger.kernel.org;
linux- ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default
when do PF reset
Vasu,
On 2015/1/14 3:38, Dev, Vasu wrote:
-Original Message-
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void
i40e_reset_and_rebuild(struct i40e_pf *pf, bool reinit)
}
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
- ret = i40e_init_pf_fcoe(pf);
- if (ret)
- dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n,
ret);
+ if (pf-flags I40E_FLAG_FCOE_ENABLED) {
+ ret = i40e_init_pf_fcoe(pf);
Calling i40e_init_pf_fcoe() here conflicts with its
I40E_FLAG_FCOE_ENABLED pre-condition since
I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.
I don't think so, here ,i40e_reset_and_rebuild() is not the
only and the first place that i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.
i40e_probe()
--i40e_sw_init()
--i40e_init_pf_fcoe()
And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the
reset action is to be done.
It is set by i40e_init_pf_fcoe() and you are right that the
modified call flow
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED
anyway
which could have prevented calling i40e_init_pf_fcoe() as I
described above, so this is not an issue with the patch.
BTW, the reason I post this patch is that we hit a bug, after
setup vlan, the PF is enabled to FCOE.
Then that BUG would still remain un-fixed and calling
i40e_init_pf_fcoe()
under I40E_FLAG_FCOE_ENABLED flag really won't affect call flow to
fix anything. I mean I40E_FLAG_FCOE_ENABLED condition will be true
with pf-
hw.func_caps.fcoe == true and otherwise calling
i40e_init_pf_fcoe() simply
returns back early on after checking pf-hw.func_caps.fcoe ==
false, so how that bug is fixed here by added
I40E_FLAG_FCOE_ENABLED condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under
our test environment,
i40e_probe()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Or
i40e_reset_and_rebuild()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Under our test environment, the pf-hw.func_caps.fcoe is
true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic
test.
And then i40e_init_pf_fcoe() is to be called,
While if (!pf-hw.func_caps.fcoe) wouldn't return,
I said it would return with pf-hw.func_caps.fcoe == false in my last
response, more details below.
So pf
Re: [E1000-devel] [PATCH] i40e: don't enable and init FCOE by default when do PF reset
Vasu,
What' your idea about the v2, any suggestion ? Jeff is looking
forward to see it.
Thanks,
Ethan
On 2015/1/16 22:47, Jeff Kirsher wrote:
On Fri, 2015-01-16 at 09:48 +0800, ethan zhao wrote:
Vasu,
OK, disable FCOE as default configuration as a temporary step to
make it work.
Sounds like I should expect a v2 coming, correct?
Thanks,
Ethan
On 2015/1/16 7:45, Dev, Vasu wrote:
-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse;
Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset
Vasu,
On 2015/1/14 3:38, Dev, Vasu wrote:
-Original Message-
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
}
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
- ret = i40e_init_pf_fcoe(pf);
- if (ret)
- dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n,
ret);
+ if (pf-flags I40E_FLAG_FCOE_ENABLED) {
+ ret = i40e_init_pf_fcoe(pf);
Calling i40e_init_pf_fcoe() here conflicts with its
I40E_FLAG_FCOE_ENABLED pre-condition since
I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.
I don't think so, here ,i40e_reset_and_rebuild() is not the only
and the first place that i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.
i40e_probe()
--i40e_sw_init()
--i40e_init_pf_fcoe()
And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.
It is set by i40e_init_pf_fcoe() and you are right that the modified call
flow
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described
above,
so this is not an issue with the patch.
BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.
Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
under I40E_FLAG_FCOE_ENABLED flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED condition will be true with pf-
hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe()
simply
returns back early on after checking pf-hw.func_caps.fcoe == false, so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under our test
environment,
i40e_probe()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Or
i40e_reset_and_rebuild()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Under our test environment, the pf-hw.func_caps.fcoe is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
And then i40e_init_pf_fcoe() is to be called,
While if (!pf-hw.func_caps.fcoe) wouldn't return,
I said it would return with pf-hw.func_caps.fcoe == false in my last
response, more details below.
So pf-flags is set to I40E_FLAG_FCOE_ENABLED.
With my patch, i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.
Enable FCOE in i40e_probe() or not is another issue.
Nope since both cases we should do i40e_init_pf_fcoe() or don't based on
fcoe cap true or false.
I don't have much to add as I described before with the your patch that
calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED flag really
won't affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED
condition will be true with pf-hw.func_caps.fcoe == true and otherwise
calling i40e_init_pf_fcoe() simply returns back early on after checking
pf-hw.func_caps.fcoe == false.
May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE
disable as I suggested before and now it in upstream kernel or we can have
further off list discussion to fix the issue you are trying to fix with the
patch.
Thanks,
Vasu
Thanks,
Ethan
Jeff Kirsher should be getting out a patch queued by me which adds
I40E_FCoE Kbuild option, in that FCoE is disabled by default and
user could enable FCoE only if needed, that patch would do
Re: [E1000-devel] [PATCH] i40e: don't enable and init FCOE by default when do PF reset
Vasu,
OK, disable FCOE as default configuration as a temporary step to
make it work.
Thanks,
Ethan
On 2015/1/16 7:45, Dev, Vasu wrote:
-Original Message-
From: ethan zhao [mailto:ethan.z...@oracle.com]
Sent: Tuesday, January 13, 2015 6:41 PM
To: Dev, Vasu
Cc: Ethan Zhao; Ronciak, John; Kirsher, Jeffrey T; Brandeburg, Jesse; Allan,
Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Vick,
Matthew; Williams, Mitch A; Parikh, Neerav; Linux NICS; e1000-
de...@lists.sourceforge.net; net...@vger.kernel.org; linux-
ker...@vger.kernel.org; brian.m...@oracle.com
Subject: Re: [PATCH] i40e: don't enable and init FCOE by default when do PF
reset
Vasu,
On 2015/1/14 3:38, Dev, Vasu wrote:
-Original Message-
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
}
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
- ret = i40e_init_pf_fcoe(pf);
- if (ret)
- dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+ if (pf-flags I40E_FLAG_FCOE_ENABLED) {
+ ret = i40e_init_pf_fcoe(pf);
Calling i40e_init_pf_fcoe() here conflicts with its
I40E_FLAG_FCOE_ENABLED pre-condition since
I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe()
will never get called.
I don't think so, here ,i40e_reset_and_rebuild() is not the only
and the first place that i40e_init_pf_fcoe() is called, see
i40e_probe(), that is the first chance.
i40e_probe()
--i40e_sw_init()
--i40e_init_pf_fcoe()
And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset
action is to be done.
It is set by i40e_init_pf_fcoe() and you are right that the modified call
flow
by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.
BTW, the reason I post this patch is that we hit a bug, after setup
vlan, the PF is enabled to FCOE.
Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
under I40E_FLAG_FCOE_ENABLED flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED condition will be true with pf-
hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe() simply
returns back early on after checking pf-hw.func_caps.fcoe == false, so
how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under our test
environment,
i40e_probe()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Or
i40e_reset_and_rebuild()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Under our test environment, the pf-hw.func_caps.fcoe is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
And then i40e_init_pf_fcoe() is to be called,
While if (!pf-hw.func_caps.fcoe) wouldn't return,
I said it would return with pf-hw.func_caps.fcoe == false in my last
response, more details below.
So pf-flags is set to I40E_FLAG_FCOE_ENABLED.
With my patch, i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.
Enable FCOE in i40e_probe() or not is another issue.
Nope since both cases we should do i40e_init_pf_fcoe() or don't based on fcoe
cap true or false.
I don't have much to add as I described before with the your patch that
calling i40e_init_pf_fcoe() under I40E_FLAG_FCOE_ENABLED flag really won't
affect call flow to fix anything. I mean I40E_FLAG_FCOE_ENABLED condition
will be true with pf-hw.func_caps.fcoe == true and otherwise calling
i40e_init_pf_fcoe() simply returns back early on after checking
pf-hw.func_caps.fcoe == false.
May be I'm missing something, I guess next either go with CONFIG_I40E_FCOE
disable as I suggested before and now it in upstream kernel or we can have
further off list discussion to fix the issue you are trying to fix with the
patch.
Thanks,
Vasu
Thanks,
Ethan
Jeff Kirsher should be getting out a patch queued by me which adds
I40E_FCoE Kbuild option, in that FCoE is disabled by default and
user could enable FCoE only if needed, that patch would do same of
skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not
in default config.
The following patch will not fix the above issue -- configuration of
PF will be changed via reset.
How about the FCOE is configured and disabled by i40e_fcoe_disable
Re: [E1000-devel] [PATCH] i40e: don't enable and init FCOE by default when do PF reset
Vasu,
On 2015/1/14 3:38, Dev, Vasu wrote:
-Original Message-
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct
i40e_pf *pf, bool reinit)
}
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
- ret = i40e_init_pf_fcoe(pf);
- if (ret)
- dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+ if (pf-flags I40E_FLAG_FCOE_ENABLED) {
+ ret = i40e_init_pf_fcoe(pf);
Calling i40e_init_pf_fcoe() here conflicts with its
I40E_FLAG_FCOE_ENABLED pre-condition since I40E_FLAG_FCOE_ENABLED is
set by very same i40e_init_pf_fcoe(), in turn i40e_init_pf_fcoe() will never
get
called.
I don't think so, here ,i40e_reset_and_rebuild() is not the only and the
first
place that i40e_init_pf_fcoe() is called, see i40e_probe(), that is the
first
chance.
i40e_probe()
--i40e_sw_init()
--i40e_init_pf_fcoe()
And the I40E_FLAG_FCOE_ENABLED is possible be set by
i40e_fcoe_enable() or i40e_fcoe_disable() interface before the reset action
is
to be done.
It is set by i40e_init_pf_fcoe() and you are right that the modified call
flow by your patch won't impact setting of I40E_FLAG_FCOE_ENABLED anyway
which could have prevented calling i40e_init_pf_fcoe() as I described above,
so this is not an issue with the patch.
BTW, the reason I post this patch is that we hit a bug, after setup vlan,
the PF
is enabled to FCOE.
Then that BUG would still remain un-fixed and calling i40e_init_pf_fcoe()
under I40E_FLAG_FCOE_ENABLED flag really won't affect call flow to fix
anything. I mean I40E_FLAG_FCOE_ENABLED condition will be true with
pf-hw.func_caps.fcoe == true and otherwise calling i40e_init_pf_fcoe()
simply returns back early on after checking pf-hw.func_caps.fcoe == false,
so how that bug is fixed here by added I40E_FLAG_FCOE_ENABLED condition ?
What is the bug ?
The func_caps.fcoe is assigned by following call path, under our test
environment,
i40e_probe()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Or
i40e_reset_and_rebuild()
-i40e_get_capabilities()
-i40e_aq_discover_capabilities()
-i40e_parse_discover_capabilities()
Under our test environment, the pf-hw.func_caps.fcoe is true. so if
i40e_reset_and_rebuild() is called for VLAN setup, ethtool diagnostic test.
And then i40e_init_pf_fcoe() is to be called,
While if (!pf-hw.func_caps.fcoe) wouldn't return,
So pf-flags is set to I40E_FLAG_FCOE_ENABLED.
With my patch, i40e_init_pf_fcoe() is only called after
I40E_FLAG_FCOE_ENABLED is set before reset.
Enable FCOE in i40e_probe() or not is another issue.
Thanks,
Ethan
Jeff Kirsher should be getting out a patch queued by me which adds
I40E_FCoE Kbuild option, in that FCoE is disabled by default and user could
enable FCoE only if needed, that patch would do same of skipping
i40e_init_pf_fcoe() whether FCoE capability in device enabled or not in
default config.
The following patch will not fix the above issue -- configuration of PF will
be
changed via reset.
How about the FCOE is configured and disabled by i40e_fcoe_disable() ,
then reset happens ?
May be but if the BUG is due to FCoE being enabled then having it disabled in
config will avoid the bug for non FCoE config option and once bug is
understood then that has to be fixed for FCoE enabled config also as I asked
above.
Thanks Ethan for detailed response.
Vasu
From patchwork Wed Oct 2 23:26:08 2013
Content-Type: text/plain; charset=utf-8
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [net] i40e: adds FCoE configure option
Date: Thu, 03 Oct 2013 07:26:08 -
From: Vasu Dev vasu@intel.com
X-Patchwork-Id: 11797
Adds FCoE config option I40E_FCOE, so that FCoE can be enabled as
needed but otherwise have it disabled by default.
This also eliminate multiple FCoE config checks, instead now just one
config check for CONFIG_I40E_FCOE.
The I40E FCoE was added with 3.17 kernel and therefore this patch
shall be applied to stable 3.17 kernel also.
CC: sta...@vger.kernel.org
Signed-off-by: Vasu Dev vasu@intel.com
Tested-by: Jim Young jamesx.m.yo...@intel.com
---
drivers/net/ethernet/intel/Kconfig | 11 +++
drivers/net/ethernet/intel/i40e/Makefile |2 +-
drivers/net/ethernet/intel/i40e/i40e_osdep.h |4 ++--
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/Kconfig
b/drivers/net/ethernet/intel/Kconfig
index 5b8300a..4d61ef5 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -281,6 +281,17 @@ config I40E_DCB
If unsure, say N.
+config
[E1000-devel] [PATCH] i40e: don't enable and init FCOE by default when do PF reset
While do PF reset with function i40e_reset_and_rebuild(), it will
call i40e_init_pf_fcoe() by default if FCOE is defined, thus if the
PF is resetted, FCOE will be enabled whatever it was - enabled or
not.
Such bug might be hit when PF resumes from suspend, run diagnostic
test with ethtool, setup VLAN etc.
Passed building with v3.19-rc3.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a5f2660..a2572cc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6180,9 +6180,12 @@ static void i40e_reset_and_rebuild(struct i40e_pf *pf,
bool reinit)
}
#endif /* CONFIG_I40E_DCB */
#ifdef I40E_FCOE
- ret = i40e_init_pf_fcoe(pf);
- if (ret)
- dev_info(pf-pdev-dev, init_pf_fcoe failed: %d\n, ret);
+ if (pf-flags I40E_FLAG_FCOE_ENABLED) {
+ ret = i40e_init_pf_fcoe(pf);
+ if (ret)
+ dev_info(pf-pdev-dev,
+init_pf_fcoe failed: %d\n, ret);
+ }
#endif
/* do basic switch setup */
--
1.8.3.1
--
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH] ixgbe: delete one duplicate marcro definition of IXGBE_MAX_L2A_QUEUES
There is typo in ixgbe.h, two marcro definition of IXGBE_MAX_L2A_QUEUES to 4,
delete one, clear the compiler warning.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index ac9f214..bfb2c23 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -307,7 +307,6 @@ enum ixgbe_ring_f_enum {
#define MAX_RX_QUEUES (IXGBE_MAX_FDIR_INDICES + 1)
#define MAX_TX_QUEUES (IXGBE_MAX_FDIR_INDICES + 1)
#define IXGBE_MAX_L2A_QUEUES 4
-#define IXGBE_MAX_L2A_QUEUES 4
#define IXGBE_BAD_L2A_QUEUE 3
#define IXGBE_MAX_MACVLANS 31
#define IXGBE_MAX_DCBMACVLANS 8
--
1.7.1
--
Slashdot TV. Video for Nerds. Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] Hit tx unit hang when test igb 5.2.9.4 from sourceforge
Hi, folks, We hit following message on the console when test igb 5.2.9.4 download from sourceforge, because it is not a git repo, hard for me to bisect. this stops us to upgrade to ver 5.2.9.4. if more information needed helps to fix this issue, feel free to let me know. BTW, the prior version works well on the same box. http://sourceforge.net/projects/e1000/files/igb%20stable/5.2.9.4/igb-5.2.9.4.tar.gz/download Kernel version is 3.8.13, Some of the message FYI: -- Intel(R) Gigabit Ethernet Network Driver - version 5.2.9.4 Copyright (c) 2007-2014 Intel Corporation. igb :01:00.0: Intel(R) Gigabit Ethernet Network Connection igb :01:00.0: eth0: (PCIe:2.5GT/s:Width x2) igb :01:00.0 eth0: MAC: 00:21:28:e8:6a:54 igb :01:00.0: eth0: PBA No: 1040FF-0FF igb :01:00.0: LRO is disabled igb :01:00.0: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s) igb :01:00.1: Intel(R) Gigabit Ethernet Network Connection igb :01:00.1: eth1: (PCIe:2.5GT/s:Width x2) igb :01:00.1 eth1: MAC: 00:21:28:e8:6a:55 igb :01:00.1: eth1: PBA No: 1040FF-0FF igb :01:00.1: LRO is disabled igb :01:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s) igb :07:00.0: Intel(R) Gigabit Ethernet Network Connection igb :07:00.0: eth2: (PCIe:2.5GT/s:Width x2) igb :07:00.0 eth2: MAC: 00:21:28:e8:6a:56 igb :07:00.0: eth2: PBA No: 1040FF-0FF igb :07:00.0: LRO is disabled igb :07:00.0: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s) igb :07:00.1: Intel(R) Gigabit Ethernet Network Connection igb :07:00.1: eth3: (PCIe:2.5GT/s:Width x2) igb :07:00.1 eth3: MAC: 00:21:28:e8:6a:57 igb :07:00.1: eth3: PBA No: 1040FF-0FF igb :07:00.1: LRO is disabled igb :07:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s) -- Below are driver traces : igb :01:00.0: Detected Tx Unit Hang Tx Queue 0 TDH 0 TDT 1 next_to_use 1 next_to_clean0 buffer_info[next_to_clean] time_stamp fffc1f16 next_to_watch88065895c000 jiffies fffc3600 desc.status 168000 Oracle Linux Serigb :01:00.0: Detected Tx Unit Hang Tx Queue 7 TDH 0 TDT 1 next_to_use 1 next_to_clean0 buffer_info[next_to_clean] time_stamp fffc2606 next_to_watch88065a201000 jiffies fffc3600 desc.status a8000 ver release 6.5 Kernel 3.8.13-43.el6uek.dev.x86_64 on an x86_64 /@ ca-ostest432.us.oracle.com login: igb :01:00.0: Detected Tx Unit Hang/ Tx Queue 0 TDH 0 TDT 1 next_to_use 1 next_to_clean0 buffer_info[next_to_clean] time_stamp fffc1f16 next_to_watch88065895c000 jiffies fffc3dd0 desc.status 168000 igb :01:00.0: Detected Tx Unit Hang Tx Queue 7 TDH 0 TDT 1 next_to_use 1 next_to_clean0 buffer_info[next_to_clean] time_stamp fffc2606 next_to_watch88065a201000 jiffies fffc3f11 desc.status a8000 [ cut here ] WARNING: at net/sched/sch_generic.c:254 dev_watchdog+0x273/0x280() Hardware name: SUN FIRE X4170 M2 SERVER NETDEV WATCHDOG: eth0 (igb): transmit queue 0 timed out Modules linked in: ebtable_nat ebtables ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_CHECKSUM iptable_mangle tun bridge stp llc cpufreq_ondemand ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 iTCO_wdt iTCO_vendor_support coretemp acpi_cpufreq freq_table mperf intel_powerclamp kvm_intel kvm crc32c_intel ghash_clmulni_intel microcode pcspkr i2c_i801 i2c_core lpc_ich mfd_core ioatdma e1000e i7core_edac edac_core ixgbe sg igb hwmon dca ext4 jbd2 mbcache sr_mod cdrom sd_mod crc_t10dif aesni_intel ablk_helper cryptd lrw aes_x86_64 xts gf128mul usb_storage ahci libahci megaraid_sas dm_mirror dm_region_hash dm_log dm_mod Pid: 0, comm: swapper/9 Not tainted 3.8.13-43.el6uek.dev.x86_64 #2 Call Trace: IRQ [8105d67f] warn_slowpath_common+0x7f/0xc0 [8105d776] warn_slowpath_fmt+0x46/0x50 [8107a32d] ? insert_work+0x4d/0x60 [814e9b73] dev_watchdog+0x273/0x280 [8107b2f0] ? __queue_work+0x3e0/0x3e0 [814e9900] ? __netdev_watchdog_up+0x80/0x80 [8106ef09] call_timer_fn+0x49/0x120 [8106f521] run_timer_softirq+0x241/0x2b0 [814e9900] ? __netdev_watchdog_up+0x80/0x80 [8101c789] ? read_tsc+0x9/0x20 [81066247] __do_softirq+0xd7/0x240 [810b7f74] ? clockevents_program_event+0x74/0x120 [81087dde] ? hrtimer_interrupt+0x13e/0x240
[E1000-devel] [PATCH] ixgbe: remove useless bd_number from adapter struct
Because bd_number is not useful anymore, so remove it from adapter struct, or
if keep it, we have to fix the boards driven counter bug in ixgbe_remove() and
ixgbe_probe() only for trival debug purpose -- other output is enough.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h |2 --
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |4
2 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index c6c4ca7..238a74f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -752,8 +752,6 @@ struct ixgbe_adapter {
u8 __iomem *io_addr; /* Mainly for iounmap use */
u32 wol;
- u16 bd_number;
-
u16 eeprom_verh;
u16 eeprom_verl;
u16 eeprom_cap;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d62e7a2..1e25e8e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7929,7 +7929,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
struct ixgbe_adapter *adapter = NULL;
struct ixgbe_hw *hw;
const struct ixgbe_info *ii = ixgbe_info_tbl[ent-driver_data];
- static int cards_found;
int i, err, pci_using_dac, expected_gts;
unsigned int indices = MAX_TX_QUEUES;
u8 part_str[IXGBE_PBANUM_LENGTH];
@@ -8015,8 +8014,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
netdev-watchdog_timeo = 5 * HZ;
strncpy(netdev-name, pci_name(pdev), sizeof(netdev-name) - 1);
- adapter-bd_number = cards_found;
-
/* Setup hw api */
memcpy(hw-mac.ops, ii-mac_ops, sizeof(hw-mac.ops));
hw-mac.type = ii-mac;
@@ -8295,7 +8292,6 @@ skip_sriov:
ixgbe_add_sanmac_netdev(netdev);
e_dev_info(%s\n, ixgbe_default_device_descr);
- cards_found++;
#ifdef CONFIG_IXGBE_HWMON
if (ixgbe_sysfs_init(adapter))
--
1.7.1
--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls.
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH] ixgbevf: remove useless bd_number from struct ixgbevf_adapter
It is useless and buggy, just remove it.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h |1 -
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |4
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index a0a1de9..ba96cb5 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -385,7 +385,6 @@ struct ixgbevf_adapter {
/* structs defined in ixgbe_vf.h */
struct ixgbe_hw hw;
u16 msg_enable;
- u16 bd_number;
/* Interrupt Throttle Rate */
u32 eitr_param;
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index d0799e8..091b106 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3464,7 +3464,6 @@ static int ixgbevf_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
struct ixgbevf_adapter *adapter = NULL;
struct ixgbe_hw *hw = NULL;
const struct ixgbevf_info *ii = ixgbevf_info_tbl[ent-driver_data];
- static int cards_found;
int err, pci_using_dac;
err = pci_enable_device(pdev);
@@ -3525,8 +3524,6 @@ static int ixgbevf_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
ixgbevf_assign_netdev_ops(netdev);
- adapter-bd_number = cards_found;
-
/* Setup hw api */
memcpy(hw-mac.ops, ii-mac_ops, sizeof(hw-mac.ops));
hw-mac.type = ii-mac;
@@ -3601,7 +3598,6 @@ static int ixgbevf_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
hw_dbg(hw, MAC: %d\n, hw-mac.type);
hw_dbg(hw, Intel(R) 82599 Virtual Function\n);
- cards_found++;
return 0;
err_register:
--
1.7.1
--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls.
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH] ixgbe: remove useless bd_number from adapter struct
Thanks Brown, Aaron F On Tue, Jul 29, 2014 at 10:51 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- ow...@vger.kernel.org] On Behalf Of Ethan Zhao Sent: Monday, July 28, 2014 12:41 AM To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Wyborny, Carolyn; Skidmore, Donald C; Rose, Gregory V; Duyck, Alexander H; Ronciak, John; Williams, Mitch A Cc: Linux NICS; e1000-devel@lists.sourceforge.net; net...@vger.kernel.org; linux-ker...@vger.kernel.org; ethan.ker...@gmail.com; Ethan Zhao Subject: [PATCH] ixgbe: remove useless bd_number from adapter struct Because bd_number is not useful anymore, so remove it from adapter struct, or if keep it, we have to fix the boards driven counter bug in ixgbe_remove() and ixgbe_probe() only for trival debug purpose -- other output is enough. Thanks Ethan, I (briefly covering for Jeff Kirsher) have added this patch and your other one ofr ixgbevf to Jeff's internal tree. -- Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
On Mon, Jul 21, 2014 at 10:39 PM, Alex Williamson alex.william...@redhat.com wrote: On Mon, 2014-07-21 at 14:11 +, Yuval Mintz wrote: pci_iov_assign_device(), pci_iov_deassign_device() along with the existed one pci_vfs_assigned() They construct the VFs assignment management interface, perhaps it's the right place to add some sort of mechanism in order to prevent module removal once one of its VFs is assigned. [e.g., incrementing module reference count] On what module would the reference count be increased, the PF? The entire VF assigned interface is a hack to work around poor architectures like legacy KVM device assignment where there's no proper device owner for the VF. This is fixed by using VFIO instead and hopefully deprecating legacy KVM assignment. Thanks, To explain what Alex said, in another word, if VMs access VF via VFIO driver, the owner of the device is VFIO, in this case, if you unload PF driver, you still need to unload VFIO first, then unload PF driver. but the PF driver knows how to notify the VFIO driver to unload. But without VFIO like driver, for example some of current code will assign devcie (PF/VF) directly to KVM or XEN without driver in the middle. and the PF driver doesn't know how to unbind the assignment... I thought about perhaps incrementing the reference count of the PF's module [i.e., the module supplying the driver for the PF] directly, so THAT module won't be removable as long as the VF is assigned. Although I don't know whether the module is even accessible; Can you derive a pointer to a module from a net_device struct? A VF is not necessarily a net device. A pci_dev does have a physfn pointer though. I stand corrected. Is there anything inherently wrong about this approach, though? What does incrementing the module reference on the PF driver accomplish? We can still unbind the PF device from the driver. That's what this VF Unbind PF from its original driver should be OK. regarding some current hacked implementation that assign PF/VF to KVM/XEN without driver, unbing or removal to them is nightmare, maybe the best thing we could do is device is in using, please release... used flag is supposed to accomplish is preventing the PF from unbinding from the driver while VFs are in use, but as has been noted, it's a terrible hack. Yep. Ethan BTW, the below corporate disclaimer below is really inappropriate for the mailing list. Thanks, Alex -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
On Tue, Jul 22, 2014 at 6:41 AM, Alexander Duyck alexander.h.du...@intel.com wrote: On 07/21/2014 07:39 AM, Alex Williamson wrote: On Mon, 2014-07-21 at 14:11 +, Yuval Mintz wrote: pci_iov_assign_device(), pci_iov_deassign_device() along with the existed one pci_vfs_assigned() They construct the VFs assignment management interface, perhaps it's the right place to add some sort of mechanism in order to prevent module removal once one of its VFs is assigned. [e.g., incrementing module reference count] On what module would the reference count be increased, the PF? The entire VF assigned interface is a hack to work around poor architectures like legacy KVM device assignment where there's no proper device owner for the VF. This is fixed by using VFIO instead and hopefully deprecating legacy KVM assignment. Thanks, To explain what Alex said, in another word, if VMs access VF via VFIO driver, the owner of the device is VFIO, in this case, if you unload PF driver, you still need to unload VFIO first, then unload PF driver. but the PF driver knows how to notify the VFIO driver to unload. But without VFIO like driver, for example some of current code will assign devcie (PF/VF) directly to KVM or XEN without driver in the middle. and the PF driver doesn't know how to unbind the assignment... I thought about perhaps incrementing the reference count of the PF's module [i.e., the module supplying the driver for the PF] directly, so THAT module won't be removable as long as the VF is assigned. Although I don't know whether the module is even accessible; Can you derive a pointer to a module from a net_device struct? A VF is not necessarily a net device. A pci_dev does have a physfn pointer though. I stand corrected. Is there anything inherently wrong about this approach, though? What does incrementing the module reference on the PF driver accomplish? We can still unbind the PF device from the driver. That's what this VF used flag is supposed to accomplish is preventing the PF from unbinding from the driver while VFs are in use, but as has been noted, it's a terrible hack. Keep in mind there are also use cases such as what occurs in igb/ixgbe where the PF driver can be swapped out while the VFs are left in place. Nothing explicitly says we cannot unload the PF driver while the VFs are assigned, we just cannot free the VFs or disable SR-IOV while they are assigned. unbind one PF from the driver but keep the VF reside in it ? the meanwhile not shutdown the PF ? In that scenario we just force the link down on the link state down on the VFs while the PF is not present. I'm not sure how many people make use of the behavior but that is currently how we end up supporting it for igb/ixgbe. Roger ! One approach would be to use a reference count for the SR-IOV itself. I know in the case of the igb/ixgbe drivers we use the assigned number of VFs to determine if we can change the number of VFs allocated or not. I imagine you could probably just use the assigned count as a reference count and if it is non-zero prevent SR-IOV from being disabled. I am managing to figure out a way to cut out the VFs assigned checking before disable SR_IOV... ... Maybe via composing pseudo PCI driver for every user VF/PF assigned to them ? but does it work when PF swap out without touching VF as you mentioned ? Thanks, Ethan Thanks, Alex -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
On Tue, Jul 22, 2014 at 6:29 AM, Alexander Duyck
alexander.h.du...@intel.com wrote:
On 07/11/2014 05:30 AM, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
- struct pci_dev *vfdev;
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
- while (vfdev) {
- /* if we don't own it we don't care */
- if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
- /* if it is assigned we cannot release it */
- if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
- return true;
- }
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
-I40E_DEV_ID_VF,
-vfdev);
- }
+ if (pci_vfs_assigned(pdev))
+ return true;
return false;
}
This portion for i40e should be in one patch by itself. It shouldn't be
included in the bits below. Normally this would go through netdev. The
rest of this below would go through linux-pci.
Agree, thanks.
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_assign_device);
+
+/**
+ * pci_iov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_iov_deassign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
+
+/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
* @dev: the PCI PF device
* @numvfs: number that should be used for TotalVFs supported
The two functions above don't have anything to do with IOV. You can
direct assign a device that doesn't even support SR-IOV or MR-IOV. You
might be better off defining this as something like
pci_set_flag_assigned/pci_clear_flag_assigned. I would likely also
make them inline and possibly move them to pci.h since it would likely
result in less actual code after you consider the overhead to push
everything on the stack prior to making the call
Agree, if I still stand for this set of helpers.
diff --git a/drivers/xen/xen-pciback/pci_stub.c
b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..27e00d1 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);
- dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_iov_deassign_device(dev);
pci_dev_put(dev);
kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(dev-dev, reset device\n);
xen_pcibk_reset_device(dev);
- dev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_iov_assign_device(dev);
return 0;
config_release:
diff --git
[E1000-devel] [PATCH] i40e: use global pci_vfs_assigned() to replace local i40e_vfs_are_assigned()
There is global funcion pci_vfs_assigned(), so use it instead of composing
local one.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 31 +---
1 files changed, 1 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..ec59190 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -683,35 +683,6 @@ complete_reset:
wr32(hw, I40E_VFGEN_RSTAT1(vf-vf_id), I40E_VFR_VFACTIVE);
i40e_flush(hw);
}
-
-/**
- * i40e_vfs_are_assigned
- * @pf: pointer to the pf structure
- *
- * Determine if any VFs are assigned to VMs
- **/
-static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
-{
- struct pci_dev *pdev = pf-pdev;
- struct pci_dev *vfdev;
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
- while (vfdev) {
- /* if we don't own it we don't care */
- if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
- /* if it is assigned we cannot release it */
- if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
- return true;
- }
-
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
- }
-
- return false;
-}
#ifdef CONFIG_PCI_IOV
/**
@@ -815,7 +786,7 @@ void i40e_free_vfs(struct i40e_pf *pf)
kfree(pf-vf);
pf-vf = NULL;
- if (!i40e_vfs_are_assigned(pf)) {
+ if (!pci_vfs_assigned(pf-pdev)) {
pci_disable_sriov(pf-pdev);
/* Acknowledge VFLR for all VFS. Without this, VFs will fail to
* work correctly when SR-IOV gets re-enabled.
--
1.7.1
--
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
On Mon, Jul 21, 2014 at 9:58 AM, Alex Williamson alex.william...@redhat.com wrote: On Sun, 2014-07-20 at 14:48 +, Yuval Mintz wrote: On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote: On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote: This patch introduces two new device assignment functions pci_iov_assign_device(), pci_iov_deassign_device() along with the existed one pci_vfs_assigned() They construct the VFs assignment management interface, used to assign/ deassign device to VM and query the VFs reference counter. instead of direct manipulation of device flag. Sorry for the late intrusion into the conversation, and perhaps it's a bit unrelated, but given that you're creating a 'VF assignment management' perhaps it's the right place to add some sort of mechanism in order to prevent module removal once one of its VFs is assigned. [e.g., incrementing module reference count] At the moment [to the best of my knowledge] there's no mechanism preventing the 'not-so-bright' user from removing the driver, and no good will come out of it. On what module would the reference count be increased, the PF? The entire VF assigned interface is a hack to work around poor architectures like legacy KVM device assignment where there's no proper device owner for the VF. This is fixed by using VFIO instead and hopefully deprecating legacy KVM assignment. Thanks, To explain what Alex said, in another word, if VMs access VF via VFIO driver, the owner of the device is VFIO, in this case, if you unload PF driver, you still need to unload VFIO first, then unload PF driver. but the PF driver knows how to notify the VFIO driver to unload. But without VFIO like driver, for example some of current code will assign devcie (PF/VF) directly to KVM or XEN without driver in the middle. and the PF driver doesn't know how to unbind the assignment... Thanks, Ethan Alex -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
On Sun, Jul 20, 2014 at 10:48 PM, Yuval Mintz yuval.mi...@qlogic.com wrote: On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote: On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote: This patch introduces two new device assignment functions pci_iov_assign_device(), pci_iov_deassign_device() along with the existed one pci_vfs_assigned() They construct the VFs assignment management interface, used to assign/ deassign device to VM and query the VFs reference counter. instead of direct manipulation of device flag. Sorry for the late intrusion into the conversation, and perhaps it's a bit unrelated, but given that you're creating a 'VF assignment management' perhaps it's the right place to add some sort of mechanism in order to prevent module removal once one of its VFs is assigned. [e.g., incrementing module reference count] This patch set is used to make the current hacked code well formed and make the interface clear to refactory later. so we could keep the current direct assignment and woulnd't encounter the removal issue etc. in one word, manageable. At the moment [to the best of my knowledge] there's no mechanism preventing the 'not-so-bright' user from removing the driver, and no good will come out of it. The best way is there is real 'driver' between device and the VM device manager or some similar machanism to make the assignment manageable. Thanks, Ethan Thanks, Yuval This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V2] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午5:41，David Vrabel david.vra...@citrix.com 写道： On 11/07/14 06:52, Ethan Zhao wrote: --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref) xen_pcibk_config_free_dyn_fields(dev); xen_pcibk_config_free_dev(dev); -dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED; +pci_sriov_deassign_device(dev); pci_dev_put(dev); kfree(psdev); @@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev) dev_dbg(dev-dev, reset device\n); xen_pcibk_reset_device(dev); -dev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED; +pci_sriov_assign_device(dev); return 0; Xen's PCI passthrough works with all PCI devices not just SR-IOV ones, so the naming of the helpers isn't correct. yes, but so far, no better name for it , though the helpers work for all PCI devices. do you have any good suggestion ? Thanks, Ethan David -- Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [PATCH 2/2 V3] PCI: implement VFs assignment reference counter
Current implementation of helper function pci_vfs_assigned() is a
little complex, to get sum of VFs that assigned to VM, access low
level configuration space register and then loop in traversing
device tree.
This patch introduces an atomic reference counter for VFs those
were assigned to VM in struct pci_sriov. and simplify the code in
pci_vfs_assigned().
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset according to the suggestion from
alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 42 --
drivers/pci/pci.h |1 +
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 090f827..10efe43 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+ atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,43 +604,22 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
* Returns number of VFs belonging to this device that are assigned to a guest.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
- struct pci_dev *vfdev;
- unsigned int vfs_assigned = 0;
- unsigned short dev_id;
/* only search if we are a PF */
if (!dev-is_physfn)
return 0;
- /*
-* determine the device ID for the VFs, the vendor ID will be the
-* same as the PF so there is no need to check for that one
-*/
- pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID, dev_id);
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(dev-vendor, dev_id, NULL);
- while (vfdev) {
- /*
-* It is considered assigned if it is a virtual function with
-* our dev as the physical function and the assigned bit is set
-*/
- if (vfdev-is_virtfn (vfdev-physfn == dev)
- (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
- vfs_assigned++;
-
- vfdev = pci_get_device(dev-vendor, dev_id, vfdev);
- }
-
- return vfs_assigned;
+ if (dev-sriov)
+ return atomic_read(dev-sriov-VFs_assigned_cnt);
+ return 0;
}
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
@@ -650,6 +630,11 @@ EXPORT_SYMBOL_GPL(pci_vfs_assigned);
void pci_iov_assign_device(struct pci_dev *pdev)
{
pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_inc(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_iov_assign_device);
@@ -660,6 +645,11 @@ EXPORT_SYMBOL_GPL(pci_iov_assign_device);
void pci_iov_deassign_device(struct pci_dev *pdev)
{
pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_dec(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6bd0822..d17bda2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -235,6 +235,7 @@ struct pci_sriov {
u32 pgsz; /* page size for BAR alignment */
u8 link;/* Function Dependency Link */
u16 driver_max_VFs; /* max num VFs driver supports */
+ atomic_t VFs_assigned_cnt; /* counter of VFs assigned to VM */
struct pci_dev *dev;/* lowest numbered PF */
struct pci_dev *self; /* this PF */
struct mutex lock; /* lock for VF bus */
--
1.7.1
--
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http
[E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
- struct pci_dev *vfdev;
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
- while (vfdev) {
- /* if we don't own it we don't care */
- if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
- /* if it is assigned we cannot release it */
- if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
- return true;
- }
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
- }
+ if (pci_vfs_assigned(pdev))
+ return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_assign_device);
+
+/**
+ * pci_iov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_iov_deassign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
+
+/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
* @dev: the PCI PF device
* @numvfs: number that should be used for TotalVFs supported
diff --git a/drivers/xen/xen-pciback/pci_stub.c
b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..27e00d1 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);
- dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_iov_deassign_device(dev);
pci_dev_put(dev);
kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(dev-dev, reset device\n);
xen_pcibk_reset_device(dev);
- dev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_iov_assign_device(dev);
return 0;
config_release:
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5ece6d6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1603,6 +1603,8 @@ int pci_num_vf(struct pci_dev *dev);
int pci_vfs_assigned(struct pci_dev *dev);
int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
int pci_sriov_get_totalvfs(struct pci_dev *dev);
+void pci_iov_assign_device(struct pci_dev *dev);
+void pci_iov_deassign_device(struct pci_dev *dev);
#else
static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
{ return -ENODEV; }
@@ -1614,6 +1616,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev
*dev, u16 numvfs)
{ return 0; }
static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
{ return 0; }
+static inline void pci_iov_assign_device(struct pci_dev *dev) { }
+static inline void pci_iov_deassign_device(struct pci_dev *dev) { }
#endif
Re: [E1000-devel] [PATCH 2/2 V3] PCI: implement VFs assignment reference counter
On Fri, Jul 11, 2014 at 8:42 PM, Varka Bhadram varkabhad...@gmail.com wrote:
On 07/11/2014 06:00 PM, Ethan Zhao wrote:
Current implementation of helper function pci_vfs_assigned() is a
little complex, to get sum of VFs that assigned to VM, access low
level configuration space register and then loop in traversing
device tree.
(...)
@@ -650,6 +630,11 @@ EXPORT_SYMBOL_GPL(pci_vfs_assigned);
void pci_iov_assign_device(struct pci_dev *pdev)
{
pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
Why don't we make last two 'if' conditions into single 'if'
if (pdev-physfn pdev-physfn-sriov)
Yeah， this one looks better, that style used to tell myself, I might
forget the which side of operator first, if you sure left first,
I prefer that too :
+ atomic_inc(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_iov_assign_device);
@@ -660,6 +645,11 @@ EXPORT_SYMBOL_GPL(pci_iov_assign_device);
void pci_iov_deassign_device(struct pci_dev *pdev)
{
pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
same...
same as above.
Thanks,
Ethan
+ atomic_dec(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6bd0822..d17bda2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -235,6 +235,7 @@ struct pci_sriov {
u32 pgsz; /* page size for BAR alignment */
u8 link;/* Function Dependency Link */
u16 driver_max_VFs; /* max num VFs driver supports */
+ atomic_t VFs_assigned_cnt; /* counter of VFs assigned to VM */
struct pci_dev *dev;/* lowest numbered PF */
struct pci_dev *self; /* this PF */
struct mutex lock; /* lock for VF bus */
--
Regards,
Varka Bhadram.
--
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 2/2 V3] PCI: implement VFs assignment reference counter
On Fri, Jul 11, 2014 at 8:56 PM, Varka Bhadram varkabhad...@gmail.com wrote:
On 07/11/2014 06:21 PM, Ethan Zhao wrote:
On Fri, Jul 11, 2014 at 8:42 PM, Varka Bhadram varkabhad...@gmail.com
wrote:
On 07/11/2014 06:00 PM, Ethan Zhao wrote:
Current implementation of helper function pci_vfs_assigned() is a
little complex, to get sum of VFs that assigned to VM, access low
level configuration space register and then loop in traversing
device tree.
(...)
@@ -650,6 +630,11 @@ EXPORT_SYMBOL_GPL(pci_vfs_assigned);
void pci_iov_assign_device(struct pci_dev *pdev)
{
pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
Why don't we make last two 'if' conditions into single 'if'
if (pdev-physfn pdev-physfn-sriov)
Yeah， this one looks better, that style used to tell myself, I might
forget the which side of operator first, if you sure left first,
I prefer that too :
for operator - evaluation is from Left to Right
Definitely right !
Thanks,
Ethan
--
Regards,
Varka Bhadram.
--
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午9:13，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
Hmmm,
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
You prefer v1 again ?
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
This is real issue, the interface should work without CONFIG_PCI_IOV defined.
Should be fixed.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
you wanna do all things in one step ? Be practical. That is not my original
purpose.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
Such would go back to the v1, four patches, if one lost to be committed,
kernel could be buggy.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
-}
+if (pci_vfs_assigned(pdev))
+return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_assign_device);
+
+/**
+ * pci_iov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_iov_deassign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
+
+/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
* @dev: the PCI PF device
* @numvfs: number that should be used for TotalVFs supported
diff --git a/drivers/xen/xen-pciback/pci_stub.c
b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..27e00d1 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);
-dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+pci_iov_deassign_device(dev);
pci_dev_put(dev);
kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(dev-dev, reset device\n);
xen_pcibk_reset_device(dev
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午10:15，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote:
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
I guess it's still used, which is even worse because now we have
separate data elements, one tracking how many VFs are assigned from a PF
and another tracking each device that's assigned. What are we actually
gaining or fixing by doing this?
Of course it is used to tracking PF/VF assigned, what we gained are - we hide
the details
In assignment helpers, we wouldn't bother the users anymore and implement what
inside
The helpers, my initial purpose is to simplify the ugly pci_vfs_assigned()
with counter.
I don't want to push more change in one step, that would out of control to get
perfect result.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
-}
+if (pci_vfs_assigned(pdev))
+return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_assign_device);
+
+/**
+ * pci_iov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_iov_deassign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_deassign_device);
+
+/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
* @dev: the PCI PF device
* @numvfs: number that should be used for TotalVFs supported
diff --git a/drivers/xen/xen-pciback/pci_stub.c
b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..27e00d1 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午10:50，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:20 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午9:13，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
Hmmm,
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
You prefer v1 again ?
That's not at all what I'm suggesting. In response to v1 I suggested a
specific reordering that would allow the series to be bisectable. I'm
surprised to see v2 ignored that and combined the first 3 suggested
patches.
If what I understand right. Your points are.
1. To be bisectable.
Combine the code into 2 parches, is really bisect able, without 2, 1 works
right.
If 1 was broken into 3, any of them is lost, buggy. They must work together.
2. Define and abstract first then implement.
1 . 2 follows this point.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
This is real issue, the interface should work without CONFIG_PCI_IOV defined.
Should be fixed.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
you wanna do all things in one step ? Be practical. That is not my original
purpose.
I never suggested collapsing steps. Maybe I'd understand the original
purpose better if there was an overall description in a cover patch.
Compose a cover letter seems necessary to describe patch set if they are too
complex.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
Such would go back to the v1, four patches, if one lost to be committed,
kernel could be buggy.
There seems to be confusion between multiple patches, which is fine, and
the patch ordering from v1, which is wrong.
Do you want them work standalone, if no, the confusion is nothing.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
-}
+if (pci_vfs_assigned(pdev))
+return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午10:54，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:37 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:15，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote:
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
I guess it's still used, which is even worse because now we have
separate data elements, one tracking how many VFs are assigned from a PF
and another tracking each device that's assigned. What are we actually
gaining or fixing by doing this?
Of course it is used to tracking PF/VF assigned, what we gained are - we
hide the details
In assignment helpers, we wouldn't bother the users anymore and implement
what inside
The helpers, my initial purpose is to simplify the ugly pci_vfs_assigned()
with counter.
I don't want to push more change in one step, that would out of control to
get perfect result.
But a counter means that we're tracking the data in two separate places,
which is generally a bad idea. Cleaning up direct access to flags is
fine, but creating a separate counter doesn't really seem to add any
value.
If you are asked how many apples in your bag , you like to count them one by
one and
Give the answer ? What are you doing when you put them into the bag ?
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17
++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
-}
+if (pci_vfs_assigned(pdev))
+return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_iov_assign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_iov_assign_device);
+
+/**
+ * pci_iov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_iov_deassign_device(struct pci_dev *pdev
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午11:23，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 23:07 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:50，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:20 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午9:13，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
Hmmm,
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
You prefer v1 again ?
That's not at all what I'm suggesting. In response to v1 I suggested a
specific reordering that would allow the series to be bisectable. I'm
surprised to see v2 ignored that and combined the first 3 suggested
patches.
If what I understand right. Your points are.
1. To be bisectable.
Combine the code into 2 parches, is really bisect able, without 2, 1
works right.
If 1 was broken into 3, any of them is lost, buggy. They must work
together.
No, I never said 2 patches. I said to fix and abstract the usage, then
change the implementation. Fixing and abstracting the usage should be
multiple patches. You should not be concerned about patches in a series
getting lost, you should be concerned that each step along the way is
functional.
How about 5 patches ?
- 1 . Fix i40e with pci_vfs_assigned()
- 2. Define assign/design helper.
- 3. KVM uses helpers
- 4. Xen uses helpers
- 5. Introduce VF ref counter.
This series looks like you first suggestion.
pci_vfs_assigned
2. Define and abstract first then implement.
1 . 2 follows this point.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
This is real issue, the interface should work without CONFIG_PCI_IOV
defined.
Should be fixed.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
you wanna do all things in one step ? Be practical. That is not my
original purpose.
I never suggested collapsing steps. Maybe I'd understand the original
purpose better if there was an overall description in a cover patch.
Compose a cover letter seems necessary to describe patch set if they are too
complex.
Any series with more than a single patch should have a cover letter.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17
++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
Such would go back to the v1, four patches, if one lost to be committed,
kernel could be buggy.
There seems to be confusion between multiple patches, which is fine, and
the patch ordering from v1, which is wrong.
Do you want them work standalone, if no, the confusion is nothing.
Each step in a series should be functional. There's absolutely no
requirement that any individual patch from a series can be pulled out
separately and work on it's own. Patches build on each other.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned
*/
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月11日，下午11:25，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 23:19 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:54，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:37 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:15，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote:
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
I guess it's still used, which is even worse because now we have
separate data elements, one tracking how many VFs are assigned from a PF
and another tracking each device that's assigned. What are we actually
gaining or fixing by doing this?
Of course it is used to tracking PF/VF assigned, what we gained are - we
hide the details
In assignment helpers, we wouldn't bother the users anymore and implement
what inside
The helpers, my initial purpose is to simplify the ugly
pci_vfs_assigned() with counter.
I don't want to push more change in one step, that would out of control to
get perfect result.
But a counter means that we're tracking the data in two separate places,
which is generally a bad idea. Cleaning up direct access to flags is
fine, but creating a separate counter doesn't really seem to add any
value.
If you are asked how many apples in your bag , you like to count them one by
one and
Give the answer ? What are you doing when you put them into the bag ?
If performance mattered, then yes, keeping a count may be advantageous.
Performance does not matter in any path where this is used.
Not only performance, concurrency side-effect it caused.
And less complication, lesser buggy.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17
++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are assigned
*/
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
-}
+if (pci_vfs_assigned(pdev))
+return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..090f827 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_iov_assign_device - assign device to VM
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月12日，上午12:13，Alex Williamson alex.william...@redhat.com 写道：
On Sat, 2014-07-12 at 00:06 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午11:25，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 23:19 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:54，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:37 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:15，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote:
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to
assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps
clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
I guess it's still used, which is even worse because now we have
separate data elements, one tracking how many VFs are assigned from a PF
and another tracking each device that's assigned. What are we actually
gaining or fixing by doing this?
Of course it is used to tracking PF/VF assigned, what we gained are -
we hide the details
In assignment helpers, we wouldn't bother the users anymore and
implement what inside
The helpers, my initial purpose is to simplify the ugly
pci_vfs_assigned() with counter.
I don't want to push more change in one step, that would out of control
to get perfect result.
But a counter means that we're tracking the data in two separate places,
which is generally a bad idea. Cleaning up direct access to flags is
fine, but creating a separate counter doesn't really seem to add any
value.
If you are asked how many apples in your bag , you like to count them one
by one and
Give the answer ? What are you doing when you put them into the bag ?
If performance mattered, then yes, keeping a count may be advantageous.
Performance does not matter in any path where this is used.
Not only performance, concurrency side-effect it caused.
And less complication, lesser buggy.
I'd argue that it doesn't fix anything there, the count can still change
after it's been read but before any action has been taken based on the
count. The only thing it does is eliminate the act of re-counting.
If no counter, how to avoid traversing devices and get VFs assigned count ? To
avoid
Re-counting is enough I think.
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17
++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are
assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF ,
NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device
Re: [E1000-devel] [PATCH 1/2 V3] PCI: introduce device assignment interface and refactory related code
在 2014年7月12日，上午12:13，Alex Williamson alex.william...@redhat.com 写道：
On Sat, 2014-07-12 at 00:06 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午11:25，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 23:19 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:54，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 22:37 +0800, Ethan Zhao wrote:
在 2014年7月11日，下午10:15，Alex Williamson alex.william...@redhat.com 写道：
On Fri, 2014-07-11 at 07:13 -0600, Alex Williamson wrote:
On Fri, 2014-07-11 at 20:30 +0800, Ethan Zhao wrote:
This patch introduces two new device assignment functions
pci_iov_assign_device(),
pci_iov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to
assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v3: change the naming of device assignment helpers, because they work
for all kind of PCI device, not only SR-IOV (david.vra...@citrix.com)
v2: reorder the patchset and make it bisectable and atomic, steps
clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
- Use a cover patch to describe the series
- Version information goes here, below the ---, not above it
- I stand by the patch breakdown I suggested originally, there are too
many logical changes here in patch 1. There are easily 3 separate
patches here.
- Renaming s/sriov/iov/ doesn't address the problem David raised. The
name is still synonymous with SR-IOV and it's defined on
drivers/pci/iov.c, which is only built with CONFIG_PCI_IOV.
- PCI_DEV_FLAGS_ASSIGNED should be unused after this series, why is it
not removed?
I guess it's still used, which is even worse because now we have
separate data elements, one tracking how many VFs are assigned from a PF
and another tracking each device that's assigned. What are we actually
gaining or fixing by doing this?
Of course it is used to tracking PF/VF assigned, what we gained are -
we hide the details
In assignment helpers, we wouldn't bother the users anymore and
implement what inside
The helpers, my initial purpose is to simplify the ugly
pci_vfs_assigned() with counter.
I don't want to push more change in one step, that would out of control
to get perfect result.
But a counter means that we're tracking the data in two separate places,
which is generally a bad idea. Cleaning up direct access to flags is
fine, but creating a separate counter doesn't really seem to add any
value.
If you are asked how many apples in your bag , you like to count them one
by one and
Give the answer ? What are you doing when you put them into the bag ?
If performance mattered, then yes, keeping a count may be advantageous.
Performance does not matter in any path where this is used.
Not only performance, concurrency side-effect it caused.
And less complication, lesser buggy.
I'd argue that it doesn't fix anything there, the count can still change
after it's been read but before any action has been taken based on the
count. The only thing it does is eliminate the act of re-counting.
why do you stop me simplifying the pci_vfs_assigned() code ?
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17
++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
- This patch touch 4 separate logical code areas, who do you expect to
ack and commit it? Split it up.
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
-struct pci_dev *vfdev;
-
-/* loop through all the VFs to see if we own any that are
assigned */
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF ,
NULL);
-while (vfdev) {
-/* if we don't own it we don't care */
-if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
-/* if it is assigned we cannot release it */
-if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
-return true;
-}
-vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF
[E1000-devel] [PATCH 3/4] KVM: use PCI VFs assignment helper functions
New VFs reference counter mechanism and VFs assignment helper functions are introduced to PCI SRIOV, use them instead of manipulating device flag directly. Signed-off-by: Ethan Zhao ethan.z...@oracle.com --- virt/kvm/assigned-dev.c |2 +- virt/kvm/iommu.c|4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c index bf06577..80b477f 100644 --- a/virt/kvm/assigned-dev.c +++ b/virt/kvm/assigned-dev.c @@ -302,7 +302,7 @@ static void kvm_free_assigned_device(struct kvm *kvm, else pci_restore_state(assigned_dev-dev); - assigned_dev-dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED; + pci_sriov_deassign_device(assigned_dev-dev); pci_release_regions(assigned_dev-dev); pci_disable_device(assigned_dev-dev); diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c index 0df7d4b..6d3f5ef 100644 --- a/virt/kvm/iommu.c +++ b/virt/kvm/iommu.c @@ -194,7 +194,7 @@ int kvm_assign_device(struct kvm *kvm, goto out_unmap; } - pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED; + pci_sriov_assign_device(pdev); dev_info(pdev-dev, kvm assign device\n); @@ -220,7 +220,7 @@ int kvm_deassign_device(struct kvm *kvm, iommu_detach_device(domain, pdev-dev); - pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED; + pci_sriov_deassign_device(pdev); dev_info(pdev-dev, kvm deassign device\n); -- 1.7.1 -- Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/4] PCI: introduce VFs reference counter and simplify pci_vfs_assigned() implementation
Current implementation of helper function pci_vfs_assigned() is a little
complex, to
get sum of VFs that assigned to VM, access low level configuration space
register and
then loop in traversing device tree.
This patch introduce an atomic reference counter for VFs that assigned to VM in
struct
pci_sriov, and compose two more helper functions
pci_sriov_assign_device(),
pci_sriov_deassign_device()
to replace manipulation to device flag and the meanwhile increase and decease
the counter.
Passed building on 3.15.5
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 65 --
drivers/pci/pci.h |1 +
include/linux/pci.h |4 +++
3 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..72e267f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+ atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,45 +604,51 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
- * Returns number of VFs belonging to this device that are assigned to a guest.
+ * Returns number of VFs belonging to this device that are assigned to VM.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
- struct pci_dev *vfdev;
- unsigned int vfs_assigned = 0;
- unsigned short dev_id;
-
- /* only search if we are a PF */
if (!dev-is_physfn)
return 0;
+ if (dev-sriov)
+ return atomic_read(dev-sriov-VFs_assigned_cnt);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_vfs_assigned);
- /*
-* determine the device ID for the VFs, the vendor ID will be the
-* same as the PF so there is no need to check for that one
-*/
- pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID, dev_id);
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(dev-vendor, dev_id, NULL);
- while (vfdev) {
- /*
-* It is considered assigned if it is a virtual function with
-* our dev as the physical function and the assigned bit is set
-*/
- if (vfdev-is_virtfn (vfdev-physfn == dev)
- (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
- vfs_assigned++;
-
- vfdev = pci_get_device(dev-vendor, dev_id, vfdev);
- }
+/**
+ * pci_sriov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_sriov_assign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_inc(pdev-physfn-sriov-
+ VFs_assigned_cnt);
+}
+EXPORT_SYMBOL_GPL(pci_sriov_assign_device);
- return vfs_assigned;
+/**
+ * pci_sriov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_sriov_deassign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_dec(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
-EXPORT_SYMBOL_GPL(pci_vfs_assigned);
+EXPORT_SYMBOL_GPL(pci_sriov_deassign_device);
/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6bd0822..d17bda2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -235,6 +235,7 @@ struct pci_sriov {
u32 pgsz; /* page size for BAR alignment */
u8 link;/* Function Dependency Link */
u16 driver_max_VFs; /* max num VFs driver supports */
+ atomic_t VFs_assigned_cnt; /* counter of VFs assigned to VM */
struct pci_dev *dev;/* lowest numbered PF */
struct pci_dev *self; /* this PF */
struct mutex lock; /* lock for VF bus */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5cf6833 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1601,6 +1601,8 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn);
void pci_disable_sriov(struct pci_dev *dev);
int pci_num_vf(struct pci_dev *dev);
int pci_vfs_assigned(struct pci_dev *dev
[E1000-devel] [PATCH 4/4] i40e: use PCI VFs assignment helper function simplify i40e_vfs_are_assigned()
New VFs reference counter mechanism and VFs assignment helper functions are
introduced to
PCI SRIOV, use them instead of manipulating device flag directly.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
1 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
- struct pci_dev *vfdev;
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
- while (vfdev) {
- /* if we don't own it we don't care */
- if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
- /* if it is assigned we cannot release it */
- if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
- return true;
- }
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
- }
+ if (pci_vfs_assigned(pdev))
+ return true;
return false;
}
--
1.7.1
--
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/4] PCI: introduce VFs reference counter and simplify pci_vfs_assigned() implementation
Alex,
Thanks for your reviewing, when I create the patch order, I thought
about the question you concerned for
quit a while, make every patch be independent to each other as possible
as I could, so we can do bisect when hit
problem.
I manage to take more time to figure out better patch order.
Thanks,
Ethan
On 2014/7/11 9:48, Alex Williamson wrote:
Since there's no 0th patch, I guess I'll comment here. This series is
not bisectable, patch 1 breaks the existing implementation. I'd
suggest:
patch 1 - fix i40e
i40e only could be fixed with new interface, so it couldn't
be the first one.
patch 2 - create assign/deassign that uses dev_flags
This will be the first ?
patch 3 - convert users to new interface
Have to be the later step.
patch 4 - convert interface to use atomic_t
Could it be standalone step ?
Let me think about it.
IMHO, the assigned flag is a horrible hack and I don't know why drivers
like xenback need to use it. KVM needs to use it because it doesn't
actually have a driver to bind to when a device is assigned, it's happy
to assign devices without any driver. Thanks,
Alex
On Fri, 2014-07-11 at 08:47 +0800, Ethan Zhao wrote:
Current implementation of helper function pci_vfs_assigned() is a little
complex, to
get sum of VFs that assigned to VM, access low level configuration space
register and
then loop in traversing device tree.
This patch introduce an atomic reference counter for VFs that assigned to VM
in struct
pci_sriov, and compose two more helper functions
pci_sriov_assign_device(),
pci_sriov_deassign_device()
to replace manipulation to device flag and the meanwhile increase and
decease the counter.
Passed building on 3.15.5
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 65
--
drivers/pci/pci.h |1 +
include/linux/pci.h |4 +++
3 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..72e267f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,45 +604,51 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
- * Returns number of VFs belonging to this device that are assigned to a
guest.
+ * Returns number of VFs belonging to this device that are assigned to VM.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
-struct pci_dev *vfdev;
-unsigned int vfs_assigned = 0;
-unsigned short dev_id;
-
-/* only search if we are a PF */
if (!dev-is_physfn)
return 0;
+if (dev-sriov)
+return atomic_read(dev-sriov-VFs_assigned_cnt);
+return 0;
+}
+EXPORT_SYMBOL_GPL(pci_vfs_assigned);
-/*
- * determine the device ID for the VFs, the vendor ID will be the
- * same as the PF so there is no need to check for that one
- */
-pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID, dev_id);
-
-/* loop through all the VFs to see if we own any that are assigned */
-vfdev = pci_get_device(dev-vendor, dev_id, NULL);
-while (vfdev) {
-/*
- * It is considered assigned if it is a virtual function with
- * our dev as the physical function and the assigned bit is set
- */
-if (vfdev-is_virtfn (vfdev-physfn == dev)
-(vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
-vfs_assigned++;
-
-vfdev = pci_get_device(dev-vendor, dev_id, vfdev);
-}
+/**
+ * pci_sriov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_sriov_assign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+if (pdev-is_virtfn !pdev-is_physfn)
+if (pdev-physfn)
+if (pdev-physfn-sriov)
+atomic_inc(pdev-physfn-sriov-
+VFs_assigned_cnt);
+}
+EXPORT_SYMBOL_GPL(pci_sriov_assign_device);
-return vfs_assigned;
+/**
+ * pci_sriov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_sriov_deassign_device(struct pci_dev *pdev)
+{
+pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+if (pdev-is_virtfn !pdev-is_physfn)
+if (pdev-physfn)
+if (pdev
Re: [E1000-devel] [PATCH 1/4] PCI: introduce VFs reference counter and simplify pci_vfs_assigned() implementation
On 2014/7/11 10:22, Alex Williamson wrote:
On Fri, 2014-07-11 at 10:10 +0800, ethan zhao wrote:
Alex,
Thanks for your reviewing, when I create the patch order, I thought
about the question you concerned for
quit a while, make every patch be independent to each other as possible
as I could, so we can do bisect when hit
problem.
I manage to take more time to figure out better patch order.
Thanks,
Ethan
On 2014/7/11 9:48, Alex Williamson wrote:
Since there's no 0th patch, I guess I'll comment here. This series is
not bisectable, patch 1 breaks the existing implementation. I'd
suggest:
patch 1 - fix i40e
i40e only could be fixed with new interface, so it couldn't
be the first one.
It looks like i40e just has it's own copy of pci_vfs_assigned(), why
can't your current patch 4/4 be applied now?
Yes, i40e has its local copy of pci_vfs_assigned(),it could be
simplified .
with new interface,in another word, its a user of new interface.
Thanks,
Ethan
patch 2 - create assign/deassign that uses dev_flags
This will be the first ?
patch 3 - convert users to new interface
Have to be the later step.
patch 4 - convert interface to use atomic_t
Could it be standalone step ?
Let me think about it.
IMHO, the assigned flag is a horrible hack and I don't know why drivers
like xenback need to use it. KVM needs to use it because it doesn't
actually have a driver to bind to when a device is assigned, it's happy
to assign devices without any driver. Thanks,
Alex
On Fri, 2014-07-11 at 08:47 +0800, Ethan Zhao wrote:
Current implementation of helper function pci_vfs_assigned() is a little
complex, to
get sum of VFs that assigned to VM, access low level configuration space
register and
then loop in traversing device tree.
This patch introduce an atomic reference counter for VFs that assigned to
VM in struct
pci_sriov, and compose two more helper functions
pci_sriov_assign_device(),
pci_sriov_deassign_device()
to replace manipulation to device flag and the meanwhile increase and
decease the counter.
Passed building on 3.15.5
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 65
--
drivers/pci/pci.h |1 +
include/linux/pci.h |4 +++
3 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..72e267f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+ atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,45 +604,51 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
- * Returns number of VFs belonging to this device that are assigned to a
guest.
+ * Returns number of VFs belonging to this device that are assigned to VM.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
- struct pci_dev *vfdev;
- unsigned int vfs_assigned = 0;
- unsigned short dev_id;
-
- /* only search if we are a PF */
if (!dev-is_physfn)
return 0;
+ if (dev-sriov)
+ return atomic_read(dev-sriov-VFs_assigned_cnt);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_vfs_assigned);
- /*
- * determine the device ID for the VFs, the vendor ID will be the
- * same as the PF so there is no need to check for that one
- */
- pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID, dev_id);
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(dev-vendor, dev_id, NULL);
- while (vfdev) {
- /*
- * It is considered assigned if it is a virtual function with
- * our dev as the physical function and the assigned bit is set
- */
- if (vfdev-is_virtfn (vfdev-physfn == dev)
- (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
- vfs_assigned++;
-
- vfdev = pci_get_device(dev-vendor, dev_id, vfdev);
- }
+/**
+ * pci_sriov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_sriov_assign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_inc(pdev-physfn-sriov-
+ VFs_assigned_cnt
Re: [E1000-devel] [PATCH 1/4] PCI: introduce VFs reference counter and simplify pci_vfs_assigned() implementation
On 2014/7/11 10:33, Alex Williamson wrote:
On Fri, 2014-07-11 at 10:29 +0800, ethan zhao wrote:
On 2014/7/11 10:22, Alex Williamson wrote:
On Fri, 2014-07-11 at 10:10 +0800, ethan zhao wrote:
Alex,
Thanks for your reviewing, when I create the patch order, I thought
about the question you concerned for
quit a while, make every patch be independent to each other as possible
as I could, so we can do bisect when hit
problem.
I manage to take more time to figure out better patch order.
Thanks,
Ethan
On 2014/7/11 9:48, Alex Williamson wrote:
Since there's no 0th patch, I guess I'll comment here. This series is
not bisectable, patch 1 breaks the existing implementation. I'd
suggest:
patch 1 - fix i40e
i40e only could be fixed with new interface, so it couldn't
be the first one.
It looks like i40e just has it's own copy of pci_vfs_assigned(), why
can't your current patch 4/4 be applied now?
Yes, i40e has its local copy of pci_vfs_assigned(),it could be
simplified .
with new interface,in another word, its a user of new interface.
It's a user of the existing interface. You're missing the point,
abstract all the users of the assigned dev_flags first, then you're free
to fix the implementation of the interface in one shot without breaking
anything.
Great,you hit the center this time, agree !
patch 2 - create assign/deassign that uses dev_flags
This will be the first ?
patch 3 - convert users to new interface
Have to be the later step.
patch 4 - convert interface to use atomic_t
Could it be standalone step ?
Let me think about it.
IMHO, the assigned flag is a horrible hack and I don't know why drivers
like xenback need to use it. KVM needs to use it because it doesn't
actually have a driver to bind to when a device is assigned, it's happy
to assign devices without any driver. Thanks,
Alex
On Fri, 2014-07-11 at 08:47 +0800, Ethan Zhao wrote:
Current implementation of helper function pci_vfs_assigned() is a little
complex, to
get sum of VFs that assigned to VM, access low level configuration space
register and
then loop in traversing device tree.
This patch introduce an atomic reference counter for VFs that assigned
to VM in struct
pci_sriov, and compose two more helper functions
pci_sriov_assign_device(),
pci_sriov_deassign_device()
to replace manipulation to device flag and the meanwhile increase and
decease the counter.
Passed building on 3.15.5
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 65
--
drivers/pci/pci.h |1 +
include/linux/pci.h |4 +++
3 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..72e267f 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,45 +604,51 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
- * Returns number of VFs belonging to this device that are assigned to
a guest.
+ * Returns number of VFs belonging to this device that are assigned to
VM.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
-struct pci_dev *vfdev;
-unsigned int vfs_assigned = 0;
-unsigned short dev_id;
-
-/* only search if we are a PF */
if (!dev-is_physfn)
return 0;
+if (dev-sriov)
+return atomic_read(dev-sriov-VFs_assigned_cnt);
+return 0;
+}
+EXPORT_SYMBOL_GPL(pci_vfs_assigned);
-/*
- * determine the device ID for the VFs, the vendor ID will be
the
- * same as the PF so there is no need to check for that one
- */
-pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID,
dev_id);
-
-/* loop through all the VFs to see if we own any that are
assigned */
-vfdev = pci_get_device(dev-vendor, dev_id, NULL);
-while (vfdev) {
-/*
- * It is considered assigned if it is a virtual
function with
- * our dev as the physical function and the assigned
bit is set
- */
-if (vfdev-is_virtfn (vfdev-physfn == dev)
-(vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
-vfs_assigned
[E1000-devel] [PATCH 2/2 V2] PCI: implement VFs assignment reference counter
Current implementation of helper function pci_vfs_assigned() is a
little complex, to get sum of VFs that assigned to VM, access low
level configuration space register and then loop in traversing
device tree.
This patch introduces an atomic reference counter for VFs those
were assigned to VM in struct pci_sriov. and simplify the code in
pci_vfs_assigned().
v2: reorder the patchset according to the suggestion from
alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/pci/iov.c | 45 +
drivers/pci/pci.h |1 +
2 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index c082523..5478a0c 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -382,6 +382,7 @@ found:
iov-nres = nres;
iov-ctrl = ctrl;
iov-total_VFs = total;
+ atomic_set(iov-VFs_assigned_cnt, 0);
iov-offset = offset;
iov-stride = stride;
iov-pgsz = pgsz;
@@ -603,43 +604,21 @@ int pci_num_vf(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_num_vf);
/**
- * pci_vfs_assigned - returns number of VFs are assigned to a guest
- * @dev: the PCI device
+ * pci_vfs_assigned - returns number of VFs are assigned to VM
+ * @dev: the physical PCI device that contains the VFs.
*
- * Returns number of VFs belonging to this device that are assigned to a guest.
+ * Returns number of VFs belonging to this device that are assigned to VM.
* If device is not a physical function returns 0.
*/
int pci_vfs_assigned(struct pci_dev *dev)
{
- struct pci_dev *vfdev;
- unsigned int vfs_assigned = 0;
- unsigned short dev_id;
-
/* only search if we are a PF */
if (!dev-is_physfn)
return 0;
- /*
-* determine the device ID for the VFs, the vendor ID will be the
-* same as the PF so there is no need to check for that one
-*/
- pci_read_config_word(dev, dev-sriov-pos + PCI_SRIOV_VF_DID, dev_id);
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(dev-vendor, dev_id, NULL);
- while (vfdev) {
- /*
-* It is considered assigned if it is a virtual function with
-* our dev as the physical function and the assigned bit is set
-*/
- if (vfdev-is_virtfn (vfdev-physfn == dev)
- (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED))
- vfs_assigned++;
-
- vfdev = pci_get_device(dev-vendor, dev_id, vfdev);
- }
-
- return vfs_assigned;
+ if (dev-sriov)
+ return atomic_read(dev-sriov-VFs_assigned_cnt);
+ return 0;
}
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
@@ -650,6 +629,11 @@ EXPORT_SYMBOL_GPL(pci_vfs_assigned);
void pci_sriov_assign_device(struct pci_dev *pdev)
{
pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_inc(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_sriov_assign_device);
@@ -660,6 +644,11 @@ EXPORT_SYMBOL_GPL(pci_sriov_assign_device);
void pci_sriov_deassign_device(struct pci_dev *pdev)
{
pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ if (pdev-is_virtfn !pdev-is_physfn)
+ if (pdev-physfn)
+ if (pdev-physfn-sriov)
+ atomic_dec(pdev-physfn-sriov-
+ VFs_assigned_cnt);
}
EXPORT_SYMBOL_GPL(pci_sriov_deassign_device);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 6bd0822..d17bda2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -235,6 +235,7 @@ struct pci_sriov {
u32 pgsz; /* page size for BAR alignment */
u8 link;/* Function Dependency Link */
u16 driver_max_VFs; /* max num VFs driver supports */
+ atomic_t VFs_assigned_cnt; /* counter of VFs assigned to VM */
struct pci_dev *dev;/* lowest numbered PF */
struct pci_dev *self; /* this PF */
struct mutex lock; /* lock for VF bus */
--
1.7.1
--
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 V2] PCI: introduce device assignment interface and refactory related code
This patch introduces two new device assignment functions
pci_sriov_assign_device(),
pci_sriov_deassign_device()
along with the existed one
pci_vfs_assigned()
They construct the VFs assignment management interface, used to assign/
deassign device to VM and query the VFs reference counter. instead of
direct manipulation of device flag.
This patch refashioned the related code and make them atomic.
v2: reorder the patchset and make it bisectable and atomic, steps clear
between interface defination and implemenation according to the
suggestion from alex.william...@redhat.com
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 17 ++---
drivers/pci/iov.c | 20
drivers/xen/xen-pciback/pci_stub.c |4 ++--
include/linux/pci.h|4
virt/kvm/assigned-dev.c|2 +-
virt/kvm/iommu.c |4 ++--
6 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index 02c11a7..781040e 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -693,22 +693,9 @@ complete_reset:
static bool i40e_vfs_are_assigned(struct i40e_pf *pf)
{
struct pci_dev *pdev = pf-pdev;
- struct pci_dev *vfdev;
-
- /* loop through all the VFs to see if we own any that are assigned */
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, I40E_DEV_ID_VF , NULL);
- while (vfdev) {
- /* if we don't own it we don't care */
- if (vfdev-is_virtfn pci_physfn(vfdev) == pdev) {
- /* if it is assigned we cannot release it */
- if (vfdev-dev_flags PCI_DEV_FLAGS_ASSIGNED)
- return true;
- }
- vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
- I40E_DEV_ID_VF,
- vfdev);
- }
+ if (pci_vfs_assigned(pdev))
+ return true;
return false;
}
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index de7a747..c082523 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -644,6 +644,26 @@ int pci_vfs_assigned(struct pci_dev *dev)
EXPORT_SYMBOL_GPL(pci_vfs_assigned);
/**
+ * pci_sriov_assign_device - assign device to VM
+ * @pdev: the device to be assigned.
+ */
+void pci_sriov_assign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_assign_device);
+
+/**
+ * pci_sriov_deassign_device - deasign device from VM
+ * @pdev: the device to be deassigned.
+ */
+void pci_sriov_deassign_device(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_deassign_device);
+
+/**
* pci_sriov_set_totalvfs -- reduce the TotalVFs available
* @dev: the PCI PF device
* @numvfs: number that should be used for TotalVFs supported
diff --git a/drivers/xen/xen-pciback/pci_stub.c
b/drivers/xen/xen-pciback/pci_stub.c
index 62fcd48..1d7b747 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -133,7 +133,7 @@ static void pcistub_device_release(struct kref *kref)
xen_pcibk_config_free_dyn_fields(dev);
xen_pcibk_config_free_dev(dev);
- dev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+ pci_sriov_deassign_device(dev);
pci_dev_put(dev);
kfree(psdev);
@@ -404,7 +404,7 @@ static int pcistub_init_device(struct pci_dev *dev)
dev_dbg(dev-dev, reset device\n);
xen_pcibk_reset_device(dev);
- dev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+ pci_sriov_assign_device(dev);
return 0;
config_release:
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..ddfcceb 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1603,6 +1603,8 @@ int pci_num_vf(struct pci_dev *dev);
int pci_vfs_assigned(struct pci_dev *dev);
int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
int pci_sriov_get_totalvfs(struct pci_dev *dev);
+void pci_sriov_assign_device(struct pci_dev *dev);
+void pci_sriov_deassign_device(struct pci_dev *dev);
#else
static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
{ return -ENODEV; }
@@ -1614,6 +1616,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev
*dev, u16 numvfs)
{ return 0; }
static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
{ return 0; }
+static inline void pci_sriov_assign_device(struct pci_dev *dev) { }
+static inline void pci_sriov_deassign_device(struct pci_dev *dev) { }
#endif
#if defined(CONFIG_HOTPLUG_PCI) || defined(CONFIG_HOTPLUG_PCI_MODULE)
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm
[E1000-devel] [PATCH 2/2 Net-next v5] ixgbe: set driver_max_VFs should be done before enabling SRIOV
commit 43dc4e01 Limit number of reported VFs to device specific value
It doesn't work and always returns -EBUSY because VFs ware already enabled.
ixgbe_enable_sriov()
pci_enable_sriov()
sriov_enable()
{
... ..
iov-ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
pci_cfg_access_lock(dev);
... ...
}
pci_sriov_set_totalvfs()
{
... ...
if (dev-sriov-ctrl PCI_SRIOV_CTRL_VFE)
return -EBUSY;
...
}
So should set driver_max_VFs with pci_sriov_set_totalvfs() before
enable VFs with ixgbe_enable_sriov().
V2: revised for net-next tree.
V3:
V4: remove one signoff of two.
V5: fix style issue.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bea2cec..6e6af0d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7972,8 +7972,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
/* Mailbox */
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
-ixgbe_enable_sriov(adapter);
pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
+ixgbe_enable_sriov(adapter);
skip_sriov:
#endif
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 Net-next v5] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enabled
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
v3: revised for net-next tree.
v4: remove one signoff of two
v5: fix style issue.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |5 +
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cc06854..bea2cec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5028,7 +5028,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter
*adapter)
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB) {
-if (max_vfs 63) {
+if (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
adapter-num_vfs = 0;
e_dev_warn(max_vfs parameter out of range. Not assigning
any SR-IOV VFs\n);
} else {
@@ -7973,7 +7973,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
-pci_sriov_set_totalvfs(pdev, 63);
+pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 359f6e6..b324260 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -148,7 +148,7 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
-adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+adapter-num_vfs = min_t(unsigned int, adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -257,7 +257,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev
*dev, int num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
-if (num_vfs 63) {
+if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..8bd2919 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS - 1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS - 1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 v2] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Aaron, I will check the patch and make it pass the building, seems encoding issue. Thanks, Ethan On Wed, Jan 15, 2014 at 11:46 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: On Fri, 2013-12-27 at 01:02 -0800, Jeff Kirsher wrote: On Wed, 2013-12-25 at 00:12 +0800, Ethan Zhao wrote: Because ixgbe driver limit the max number of VF functions could be enabled to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63 in code. v2: fix a typo. Signed-off-by: Ethan Zhao ethan.ker...@gmail.com --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 + 3 files changed, 10 insertions(+), 4 deletions(-) Added to my queue, thanks Ethan! Hi Ethan, Did Jeff contact you about this failing to compile? I'm currently providing vacation covering for him and we found this was failing to compile just before he left. We captured the failure in our notes for this but there is no comment on if you were contacted or not. Regardless, when I apply this patch (with or without 2-2) we get the following error on a compilation attempt: Here's the error: Here's the error: drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ixgbe_sw_init: drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:5033: error: stray \357 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:5033: error: stray \274 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:5033: error: stray \215 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:5033: error: expected ) before numeric constant drivers/net/ethernet/intel/ixgbe/ixgbe_main.c: In function ixgbe_probe: drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7977: error: stray \357 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7977: error: stray \274 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7977: error: stray \215 in program drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:7977: error: expected ) before numeric constant make[5]: *** [drivers/net/ethernet/intel/ixgbe/ixgbe_main.o] Error 1 make[5]: *** Waiting for unfinished jobs make[4]: *** [drivers/net/ethernet/intel/ixgbe] Error 2 make[4]: *** Waiting for unfinished jobs make[3]: *** [drivers/net/ethernet/intel] Error 2 make[2]: *** [drivers/net/ethernet] Error 2 make[1]: *** [drivers/net] Error 2 make: *** [drivers] Error 2 Thanks, Aaron -- CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enabled
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
v2: fix a typo.
v3: fix a encoding issue.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0ade0cd..47e9b44 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
#ifdef CONFIG_PCI_IOV
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB)
- adapter-num_vfs = (max_vfs 63) ? 0 : max_vfs;
+ adapter-num_vfs = (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) ? 0 :
max_vfs;
#endif
/* enable itr by default in dynamic mode */
@@ -7640,7 +7640,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
- pci_sriov_set_totalvfs(pdev, 63);
+ pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 276d7b1..6925979 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -152,7 +152,8 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
- adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+ adapter-num_vfs = min_t(unsigned int,
+ adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -259,7 +260,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev *dev, int
num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
- if (num_vfs 63) {
+ if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..2593666 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS - 1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS - 1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.8.3.4 (Apple Git-47)
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 2/2] ixgbe: set driver_max_VFs should be done before enabling SRIOV
commit 43dc4e01 Limit number of reported VFs to device specific value
It doesn't work and always returns -EBUSY because VFs ware already enabled.
ixgbe_enable_sriov()
pci_enable_sriov()
sriov_enable()
{
... ..
iov-ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
pci_cfg_access_lock(dev);
... ...
}
pci_sriov_set_totalvfs()
{
... ...
if (dev-sriov-ctrl PCI_SRIOV_CTRL_VFE)
return -EBUSY;
...
}
So should set driver_max_VFs with pci_sriov_set_totalvfs() before
enable VFs with ixgbe_enable_sriov().
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 47e9b44..bfc8e94 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7639,8 +7639,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
/* Mailbox */
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
- ixgbe_enable_sriov(adapter);
pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
+ ixgbe_enable_sriov(adapter);
skip_sriov:
#endif
--
1.8.3.4 (Apple Git-47)
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
On Thu, Jan 16, 2014 at 6:00 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote: Because ixgbe driver limit the max number of VF functions could be enabled to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63 in code. v2: fix a typo. v3: fix a encoding issue. Signed-off-by: Ethan Zhao ethan.ker...@gmail.com --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 0ade0cd..47e9b44 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter) #ifdef CONFIG_PCI_IOV /* assign number of SR-IOV VFs */ if (hw-mac.type != ixgbe_mac_82598EB) - adapter-num_vfs = (max_vfs 63) ? 0 : max_vfs; Unfortunately the if statement got changed considerably with a recent commit: commit 170e85430bcbe4d18e81b5a70bb163c741381092 ixgbe: add warning when max_vfs is out of range. And the pattern no longer exists to make a match. In other words, this patch no longer applies to net-next and I have to ask you for yet another spin if you still want to squash the magic number. It's not a good news. Our distro is waiting for this patch showing up in stable. OK, info me if there is a windows for me to revise the patch. Thanks, Ethan Thanks, Aaron -- CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Aaron, Is this your net-next repo ? if so, I rebuild the patch with this repo right now . git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git Thanks, Ethan On Thu, Jan 16, 2014 at 9:54 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: On Thu, 2014-01-16 at 09:27 +0800, Ethan Zhao wrote: On Thu, Jan 16, 2014 at 6:00 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: On Wed, 2014-01-15 at 22:12 +0800, Ethan Zhao wrote: Because ixgbe driver limit the max number of VF functions could be enabled to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63 in code. v2: fix a typo. v3: fix a encoding issue. Signed-off-by: Ethan Zhao ethan.ker...@gmail.com --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++-- drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 + 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 0ade0cd..47e9b44 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter) #ifdef CONFIG_PCI_IOV /* assign number of SR-IOV VFs */ if (hw-mac.type != ixgbe_mac_82598EB) - adapter-num_vfs = (max_vfs 63) ? 0 : max_vfs; Unfortunately the if statement got changed considerably with a recent commit: commit 170e85430bcbe4d18e81b5a70bb163c741381092 ixgbe: add warning when max_vfs is out of range. And the pattern no longer exists to make a match. In other words, this patch no longer applies to net-next and I have to ask you for yet another spin if you still want to squash the magic number. It's not a good news. Our distro is waiting for this patch showing up in stable. OK, info me if there is a windows for me to revise the patch. I don't think any particular window of time is better than another. I don't see this change as needing very thorough testing so if you send in (yet) another version I'll try to get it through our internal process as rapidly as I can. Thanks, Ethan Thanks, Aaron -- CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 v3] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Aaron, Revised those patches for Dave Miller's net-next OK, passed building. resent. Thanks, Ethan On Thu, Jan 16, 2014 at 11:08 AM, Ethan Zhao ethan.ker...@gmail.com wrote: Aaron, Ok, Dave Miler's net-next tree. Thanks, Etan On Thu, Jan 16, 2014 at 10:51 AM, Brown, Aaron F aaron.f.br...@intel.com wrote: On Thu, 2014-01-16 at 09:58 +0800, Ethan Zhao wrote: Aaron, Is this your net-next repo ? if so, I rebuild the patch with this repo right now . git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next.git Thanks, Ethan Only sort of. Jeff uses it to push patches up, but I don't have an account there so am simply sending them up the old fashioned way, via email, hence it is probably not getting updated while Jeff is out. As long as your patch can apply cleanly to Dave Miller's net-next tree I should be able to pull it into our internal ones. -- CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 Net-next] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enalbed
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
v3: revised for net-next tree.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |5 +
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cc06854..bea2cec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5028,7 +5028,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter
*adapter)
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB) {
-if (max_vfs 63) {
+if (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
adapter-num_vfs = 0;
e_dev_warn(max_vfs parameter out of range. Not assigning
any SR-IOV VFs\n);
} else {
@@ -7973,7 +7973,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
-pci_sriov_set_totalvfs(pdev, 63);
+pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 359f6e6..b324260 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -148,7 +148,7 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
-adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+adapter-num_vfs = min_t(unsigned int, adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -257,7 +257,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev
*dev, int num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
-if (num_vfs 63) {
+if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..8bd2919 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS - 1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS - 1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 2/2 Net-next] ixgbe: set driver_max_VFs should be done before enabling SRIOV
commit 43dc4e01 Limit number of reported VFs to device specific value
It doesn't work and always returns -EBUSY because VFs ware already enabled.
ixgbe_enable_sriov()
pci_enable_sriov()
sriov_enable()
{
... ..
iov-ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
pci_cfg_access_lock(dev);
... ...
}
pci_sriov_set_totalvfs()
{
... ...
if (dev-sriov-ctrl PCI_SRIOV_CTRL_VFE)
return -EBUSY;
...
}
So should set driver_max_VFs with pci_sriov_set_totalvfs() before
enable VFs with ixgbe_enable_sriov().
V2: revised for net-next tree.
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bea2cec..6e6af0d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7972,8 +7972,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
/* Mailbox */
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
-ixgbe_enable_sriov(adapter);
pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
+ixgbe_enable_sriov(adapter);
skip_sriov:
#endif
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH 1/2 Net-next] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
David, Sorry about the signoffs, I am confused about how to sign the patch, If I sign it with off-work mail address, some of it is done within work-hours, maybe that why I sign if with both personal mail address and work one. If that bother you, just remove it. Thanks, Ethan On Thu, Jan 16, 2014 at 1:57 PM, David Miller da...@davemloft.net wrote: From: ethan zhao ethan.z...@oracle.com Date: Thu, 16 Jan 2014 12:25:01 +0800 Signed-off-by: Ethan Zhao ethan.z...@oracle.com Signed-off-by: Ethan Zhao ethan.ker...@gmail.com Please don't give two signoffs for yourself, it is not appropriate at all. I am very genuinely curious where you got the idea to do that, particularly as I've never seen anyone else do it before. So it's really not possible that you got the idea from someone else's actions. -- CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 net-next v4] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enalbed
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
v3: revised for net-next tree.
V4: remove one signoff of two.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h |5 +
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cc06854..bea2cec 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5028,7 +5028,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter
*adapter)
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB) {
-if (max_vfs 63) {
+if (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
adapter-num_vfs = 0;
e_dev_warn(max_vfs parameter out of range. Not assigning
any SR-IOV VFs\n);
} else {
@@ -7973,7 +7973,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
-pci_sriov_set_totalvfs(pdev, 63);
+pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 359f6e6..b324260 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -148,7 +148,7 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
-adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+adapter-num_vfs = min_t(unsigned int, adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -257,7 +257,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev
*dev, int num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
-if (num_vfs 63) {
+if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..8bd2919 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS - 1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS - 1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 2/2 Net-next v4] ixgbe: set driver_max_VFs should be done before enabling SRIOV
commit 43dc4e01 Limit number of reported VFs to device specific value
It doesn't work and always returns -EBUSY because VFs ware already enabled.
ixgbe_enable_sriov()
pci_enable_sriov()
sriov_enable()
{
... ..
iov-ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
pci_cfg_access_lock(dev);
... ...
}
pci_sriov_set_totalvfs()
{
... ...
if (dev-sriov-ctrl PCI_SRIOV_CTRL_VFE)
return -EBUSY;
...
}
So should set driver_max_VFs with pci_sriov_set_totalvfs() before
enable VFs with ixgbe_enable_sriov().
V2: revised for net-next tree.
V4: remove one signoff of two.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bea2cec..6e6af0d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7972,8 +7972,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
/* Mailbox */
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
-ixgbe_enable_sriov(adapter);
pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
+ixgbe_enable_sriov(adapter);
skip_sriov:
#endif
--
1.7.1
--
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enalbed
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0ade0cd..47e9b44 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
#ifdef CONFIG_PCI_IOV
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB)
- adapter-num_vfs = (max_vfs 63) ? 0 : max_vfs;
+ adapter-num_vfs = (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) ? 0 :
max_vfs;
#endif
/* enable itr by default in dynamic mode */
@@ -7640,7 +7640,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
- pci_sriov_set_totalvfs(pdev, 63);
+ pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 276d7b1..6925979 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -152,7 +152,8 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
- adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+ adapter-num_vfs = min_t(unsigned int,
+ adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -259,7 +260,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev *dev, int
num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
- if (num_vfs 63) {
+ if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..2593666 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS－1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS－1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.8.3.4 (Apple Git-47)
--
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 2/2] ixgbe: set driver_max_VFs should be done before enabling SRIOV
commit 43dc4e01 Limit number of reported VFs to device specific value
It doesn't work and always returns -EBUSY because VFs ware already enabled.
ixgbe_enable_sriov()
pci_enable_sriov()
sriov_enable()
{
... ..
iov-ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
pci_cfg_access_lock(dev);
... ...
}
pci_sriov_set_totalvfs()
{
... ...
if (dev-sriov-ctrl PCI_SRIOV_CTRL_VFE)
return -EBUSY;
...
}
So should set driver_max_VFs with pci_sriov_set_totalvfs() before
enable VFs with ixgbe_enable_sriov().
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 47e9b44..bfc8e94 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7639,8 +7639,8 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
/* Mailbox */
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
- ixgbe_enable_sriov(adapter);
pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
+ ixgbe_enable_sriov(adapter);
skip_sriov:
#endif
--
1.8.3.4 (Apple Git-47)
--
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
[E1000-devel] [PATCH 1/2 v2] ixgbe: define IXGBE_MAX_VFS_DRV_LIMIT macro and cleanup const 63
Because ixgbe driver limit the max number of VF functions could be enabled
to 63, so define one macro IXGBE_MAX_VFS_DRV_LIMIT and cleanup the const 63
in code.
v2: fix a typo.
Signed-off-by: Ethan Zhao ethan.ker...@gmail.com
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h | 5 +
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0ade0cd..47e9b44 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4818,7 +4818,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
#ifdef CONFIG_PCI_IOV
/* assign number of SR-IOV VFs */
if (hw-mac.type != ixgbe_mac_82598EB)
- adapter-num_vfs = (max_vfs 63) ? 0 : max_vfs;
+ adapter-num_vfs = (max_vfs IXGBE_MAX_VFS_DRV_LIMIT) ? 0 :
max_vfs;
#endif
/* enable itr by default in dynamic mode */
@@ -7640,7 +7640,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
ixgbe_init_mbx_params_pf(hw);
memcpy(hw-mbx.ops, ii-mbx_ops, sizeof(hw-mbx.ops));
ixgbe_enable_sriov(adapter);
- pci_sriov_set_totalvfs(pdev, 63);
+ pci_sriov_set_totalvfs(pdev, IXGBE_MAX_VFS_DRV_LIMIT);
skip_sriov:
#endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 276d7b1..6925979 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -152,7 +152,8 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
* physical function. If the user requests greater thn
* 63 VFs then it is an error - reset to default of zero.
*/
- adapter-num_vfs = min_t(unsigned int, adapter-num_vfs, 63);
+ adapter-num_vfs = min_t(unsigned int,
+ adapter-num_vfs,
IXGBE_MAX_VFS_DRV_LIMIT);
err = pci_enable_sriov(adapter-pdev, adapter-num_vfs);
if (err) {
@@ -259,7 +260,7 @@ static int ixgbe_pci_sriov_enable(struct pci_dev *dev, int
num_vfs)
* PF. The PCI bus driver already checks for other values out of
* range.
*/
- if (num_vfs 63) {
+ if (num_vfs IXGBE_MAX_VFS_DRV_LIMIT) {
err = -EPERM;
goto err_out;
}
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
index 4713f9f..2593666 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.h
@@ -28,6 +28,11 @@
#ifndef _IXGBE_SRIOV_H_
#define _IXGBE_SRIOV_H_
+/* ixgbe driver limit the max number of VFs could be enabled to
+ * 63 (IXGBE_MAX_VF_FUNCTIONS－1)
+ */
+#define IXGBE_MAX_VFS_DRV_LIMIT (IXGBE_MAX_VF_FUNCTIONS－1)
+
void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter);
void ixgbe_msg_task(struct ixgbe_adapter *adapter);
int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask);
--
1.8.3.4 (Apple Git-47)
--
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831iu=/4140/ostg.clktrk
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
Re: [E1000-devel] NETDEV WATCHDOG: eth0 (e1000e): transmit queue 0 timed out
Nick, You could try 7.3.21-k8-NAPI in tree or the out-of-tree version as Bjorn mentioned. To read and debug an old version driver is not a interesting thing for somebody to do. Thanks, Ethan On Tue, Dec 3, 2013 at 9:33 PM, Nick Pegg n...@nickpegg.com wrote: On Mon, Dec 2, 2013 at 10:51 PM, Ethan Zhao ethan.ker...@gmail.com wrote: Bjorn, Seems not the same bug as http://sourceforge.net/p/e1000/bugs/367/ , Nick is not running his kernel on bare metal, per the error log, he runs his kernel as HVM DomU guest or Dom0 on XEN ? so just a check of NULL will not fix that. Sorry, I neglected to say in my original email that the kernel is running as a Xen Dom0. Per Todd's request, I've opened a bug report on sourceforge and will follow up with this issue there: https://sourceforge.net/p/e1000/bugs/385/ Thanks, Nick -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH] drivers/base/core.c: always output device renaming messages.
On Sat, Oct 12, 2013 at 1:26 AM, Skidmore, Donald C donald.c.skidm...@intel.com wrote: -Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Friday, October 11, 2013 9:12 AM To: Bjorn Helgaas Cc: ethan.zhao; linux-ker...@vger.kernel.org; Skidmore, Donald C; e1000- de...@lists.sourceforge.net Subject: Re: [PATCH] drivers/base/core.c: always output device renaming messages. On Fri, Oct 11, 2013 at 10:08:09AM -0600, Bjorn Helgaas wrote: [+cc Don, e1000-devel] On Fri, Oct 11, 2013 at 9:35 AM, Greg KH gre...@linuxfoundation.org wrote: On Fri, Oct 11, 2013 at 10:58:18AM +0800, ethan.zhao wrote: From: ethan.zhao ethan.ker...@gmail.com While loading ixgbevf driver,every vf detected will be output as the same name 'eth4': ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver - version 2.8.7 Copyright (c) 2009-2012 Intel Corporation. ixgbevf :20:10.0: enabling device ( - 0002) ixgbe :20:00.0 eth0: VF Reset msg received from vf 0 ixgbevf :20:10.0: irq 199 for MSI/MSI-X ixgbevf :20:10.0: irq 200 for MSI/MSI-X ixgbevf: eth%d: ixgbevf_init_interrupt_scheme: Multiqueue Disabled: Rx Queue count = 1, Tx Queue count = 1 What is that message? That's not good, eth%d? Just a reminder that ixgbevf 2.8.7 is not the driver version that's in the kernel tree, so it's easy to spend time on mainline issue that have been fixed in the out-of-tree versions, or vice versa. Per Don, the latest ixgbevf driver is here: https://sourceforge.net/projects/e1000/files/ixgbevf%20stable/ I hate to ask why isn't this merged upstream, but given that this is an Intel network driver, I know the answer :( Thanks for pointing this out, saved me a trip through the kernel tree... That message sure does look strange eth%d? From reading your email I'm implying that it came from a 2.8.7 out or tree driver, is that correct? The reason I'm asking is I want to make sure this is not an issue in the kernel tree or the latest out of tree driver. Yes, it came from ixgbevf 2.8.7 Also for what it is worth I do have all the patches queued in Jeff K. tree that gets the kernel tree and latest out of tree driver functionally in sync. Once they pass our internal validation he should be pushing them upstream. Thanks for pointing out the error, -Don Skidmore donald.c.skidm...@intel.com -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register http://pubads.g.doubleclick.net/gampad/clk?id=60134071iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] [PATCH] drivers/base/core.c: always output device renaming messages.
On Sat, Oct 12, 2013 at 12:08 AM, Bjorn Helgaas bhelg...@google.com wrote: [+cc Don, e1000-devel] On Fri, Oct 11, 2013 at 9:35 AM, Greg KH gre...@linuxfoundation.org wrote: On Fri, Oct 11, 2013 at 10:58:18AM +0800, ethan.zhao wrote: From: ethan.zhao ethan.ker...@gmail.com While loading ixgbevf driver,every vf detected will be output as the same name 'eth4': ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver - version 2.8.7 Copyright (c) 2009-2012 Intel Corporation. ixgbevf :20:10.0: enabling device ( - 0002) ixgbe :20:00.0 eth0: VF Reset msg received from vf 0 ixgbevf :20:10.0: irq 199 for MSI/MSI-X ixgbevf :20:10.0: irq 200 for MSI/MSI-X ixgbevf: eth%d: ixgbevf_init_interrupt_scheme: Multiqueue Disabled: Rx Queue count = 1, Tx Queue count = 1 What is that message? That's not good, eth%d? Just a reminder that ixgbevf 2.8.7 is not the driver version that's in the kernel tree, so it's easy to spend time on mainline issue that have been fixed in the out-of-tree versions, or vice versa. Thanks, Yes, the driver version of ixgbevf in mainline is far later from the ones in sourceforge. the eth%d issue maybe was fixed in recent 2.11.3 . Per Don, the latest ixgbevf driver is here: https://sourceforge.net/projects/e1000/files/ixgbevf%20stable/ Bjorn -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register http://pubads.g.doubleclick.net/gampad/clk?id=60134071iu=/4140/ostg.clktrk ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang
Joe, Possibly your customer is running a kernel without source code on a platform whose vendor wouldn't like to fix BIOS issue( Is that a HP/Dell server ?). Anyway, to see if is a payload issue or, you could change the payload size with setpci tool to those devices and set the link retrain bit to trigger the link retraining to debug the issue and identity the root cause. I thinks it is much easier than modify the BIOS or eeprom of NIC. e.g. set device control register to 0f 00 (128 bytes payload size) # setpci -v -s 00:02.0 98.w=000f set device link control register to 60h (retrain the link) # setpci -v -s 00:02.0 a0.b=60 Hope it works, Just my 2 cents. ethan.z...@oracle.com On Wed, Nov 28, 2012 at 11:53 PM, Fujinaka, Todd todd.fujin...@intel.com wrote: The only EEPROM I know about or can speak to is the one attached to the 82571 and it doesn't set the MaxPayloadSize. That's done by the BIOS. Todd Fujinaka Technical Marketing Engineer LAN Access Division (LAD) Intel Corporation todd.fujin...@intel.com (503) 712-4565 -Original Message- From: Joe Jin [mailto:joe@oracle.com] Sent: Wednesday, November 28, 2012 12:31 AM To: Ben Hutchings Cc: Fujinaka, Todd; Mary Mcgrath; net...@vger.kernel.org; e1000-de...@lists.sf.net; linux-ker...@vger.kernel.org; linux-pci Subject: Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang On 11/28/12 02:10, Ben Hutchings wrote: On Tue, 2012-11-27 at 17:32 +, Fujinaka, Todd wrote: Forgive me if I'm being too repetitious as I think some of this has been mentioned in the past. We (and by we I mean the Ethernet part and driver) can only change the advertised availability of a larger MaxPayloadSize. The size is negotiated by both sides of the link when the link is established. The driver should not change the size of the link as it would be poking at registers outside of its scope and is controlled by the upstream bridge (not us). [...] MaxPayloadSize (MPS) is not negotiated between devices but is programmed by the system firmware (at least for devices present at boot - the kernel may be responsible in case of hotplug). You can use the kernel parameter 'pci=pcie_bus_perf' (or one of several others) to set a policy that overrides this, but no policy will allow setting MPS above the device's MaxPayloadSizeSupported (MPSS). Ben, Unfortunately I'm using 3.0.x kernel and this is not included in the kernel. So I'm trying to use ethtool modify it from eeprom to see if help or no. Todd, I'll review all MaxPayload for all devices, but need to say if it mismatch, customer could not modify it from BIOS for there was not entry at there, to test it, we have to find how to verify if this is the root cause, so still need to find the offset in eeprom. Thanks in advance, Joe -- Keep yourself connected to Go Parallel: VERIFY Test and improve your parallel project with help from experts and peers. http://goparallel.sourceforge.net ___ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel#174; Ethernet, visit http://communities.intel.com/community/wired
[E1000-devel] [next-net PATCH] drivers/net/ethernet/intel/e1000e: fix unregistered net_device ethX name output by e1000e
commit ca3ccc6835943287b6f69e973c126a02bc4de409
Author: ethan.zhao ethan.ker...@gmail.com
Date: Wed Jun 6 07:32:11 2012 -0700
modified: drivers/net/ethernet/intel/e1000e/param.c
While e1000e_check_options() is called, netdev is not registered, so the
e1000e driver will print out confused ethernet interface name
(unregistered net_device) :
e1000e :04:00.0:(unregistered net_device): Interrupt
Throttling Rate (ints/sec) set to dynamic conservative mode
So change e_info() back to dev_printk() by simply redefine the
e_info macro used by
e1000e_check_options() and e1000_validate_option
after applied this patch, we got:
e1000e :04:00.0: Interrupt Throttling Rate (ints/sec) set to
dynamic conservative mode
e1000e :04:00.0: irq 95 for MSI/MSI-X
diff --git a/drivers/net/ethernet/intel/e1000e/param.c
b/drivers/net/ethernet/intel/e1000e/param.c
index 55cc156..0f4f9db 100644
--- a/drivers/net/ethernet/intel/e1000e/param.c
+++ b/drivers/net/ethernet/intel/e1000e/param.c
@@ -186,6 +186,17 @@ struct e1000_option {
} arg;
};
+
+/**
+ * For netdev is not registered here, will get (unregistered
net_device) such confused
+ * name, so change back to dev_printk() lazy redfine the e_info()
+ */
+#ifdef e_info
+#undef e_info
+#define e_info(format, arg...) \
+ dev_printk(KERN_INFO,adapter-pdev-dev, format, ## arg)
+#endif
+
static int __devinit e1000_validate_option(unsigned int *value,
const struct e1000_option *opt,
struct e1000_adapter *adapter)
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel#174; Ethernet, visit
http://communities.intel.com/community/wired
