* re: mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
@ 2012-06-09 9:09 Dan Carpenter
2012-06-09 11:29 ` Jack Morgenstein
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-06-09 9:09 UTC (permalink / raw)
To: jackm; +Cc: netdev
Hello Jack Morgenstein,
The patch ab9c17a009ee: "mlx4_core: Modify driver initialization flow
to accommodate SRIOV for Ethernet" from Dec 13, 2011, leads to the
following static checker warning:
drivers/net/ethernet/mellanox/mlx4/main.c:424 mlx4_slave_cap()
warn: suspicious bitop condition
423 /*fail if the hca has an unknown capability */
424 if ((hca_param.global_caps | HCA_GLOBAL_CAP_MASK) !=
425 HCA_GLOBAL_CAP_MASK) {
426 mlx4_err(dev, "Unknown hca global capabilities\n");
427 return -ENOSYS;
428 }
The test sort of makes sense but HCA_GLOBAL_CAP_MASK is zero so we could
as well say:
if (hca_param.global_caps) { ...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
2012-06-09 9:09 mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet Dan Carpenter
@ 2012-06-09 11:29 ` Jack Morgenstein
2012-06-10 1:03 ` Ben Hutchings
0 siblings, 1 reply; 4+ messages in thread
From: Jack Morgenstein @ 2012-06-09 11:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, dotanb, Or Gerlitz
On Saturday 09 June 2012 12:09, Dan Carpenter wrote:
> 423 /*fail if the hca has an unknown capability */
> 424 if ((hca_param.global_caps | HCA_GLOBAL_CAP_MASK) !=
> 425 HCA_GLOBAL_CAP_MASK) {
> 426 mlx4_err(dev, "Unknown hca global capabilities\n");
> 427 return -ENOSYS;
> 428 }
>
> The test sort of makes sense but HCA_GLOBAL_CAP_MASK is zero so we could
> as well say:
>
> if (hca_param.global_caps) { ...
>
The parameter global_caps is to guarantee that the guest OS is capable
of supporting the driver which is running on the master (hypervisor).
If a new feature impacts the guests, we will define a bit in the
global_caps bitmask and the master driver will set this bit and
the global_caps bitmask with the newly set bit will be passed
to the slaves.
The HCA_GLOBAL_CAP_MASK on the slave OS represents the set
of such features which the slave knows how to support.
If a new feature appears which is not set in the slave's
HCA_GLOBAL_CAP_MASK, this test detects the discrepancy and
the slave driver aborts.
This global_caps parameter passed from the master to the slave
is currently zero, because all current master capabilities
are supported on the slaves.
In the (probably near) future, when new capabilities are added,
Slave O/S's will also need to be updated. If the master O/S driver is
updated, but an older slave O/S driver isstill installed, the test
will fail and the driver on the slave will abort.
-Jack
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
2012-06-09 11:29 ` Jack Morgenstein
@ 2012-06-10 1:03 ` Ben Hutchings
2012-06-17 15:18 ` Jack Morgenstein
0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2012-06-10 1:03 UTC (permalink / raw)
To: Jack Morgenstein; +Cc: Dan Carpenter, netdev, dotanb, Or Gerlitz
On Sat, 2012-06-09 at 14:29 +0300, Jack Morgenstein wrote:
> On Saturday 09 June 2012 12:09, Dan Carpenter wrote:
> > 423 /*fail if the hca has an unknown capability */
> > 424 if ((hca_param.global_caps | HCA_GLOBAL_CAP_MASK) !=
> > 425 HCA_GLOBAL_CAP_MASK) {
> > 426 mlx4_err(dev, "Unknown hca global capabilities\n");
> > 427 return -ENOSYS;
> > 428 }
> >
> > The test sort of makes sense but HCA_GLOBAL_CAP_MASK is zero so we could
> > as well say:
> >
> > if (hca_param.global_caps) { ...
> >
> The parameter global_caps is to guarantee that the guest OS is capable
> of supporting the driver which is running on the master (hypervisor).
>
> If a new feature impacts the guests, we will define a bit in the
> global_caps bitmask and the master driver will set this bit and
> the global_caps bitmask with the newly set bit will be passed
> to the slaves.
>
> The HCA_GLOBAL_CAP_MASK on the slave OS represents the set
> of such features which the slave knows how to support.
> If a new feature appears which is not set in the slave's
> HCA_GLOBAL_CAP_MASK, this test detects the discrepancy and
> the slave driver aborts.
[...]
It would be more idiomatic to write the condition as:
if (hca_param.global_caps & ~(u64)HCA_GLOBAL_CAP_MASK)
(although that cast probably belongs in the definition of
HCA_GLOBAL_CAP_MASK).
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet
2012-06-10 1:03 ` Ben Hutchings
@ 2012-06-17 15:18 ` Jack Morgenstein
0 siblings, 0 replies; 4+ messages in thread
From: Jack Morgenstein @ 2012-06-17 15:18 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Dan Carpenter, netdev, dotanb, Or Gerlitz, yevgenyp
On Sunday 10 June 2012 04:03, Ben Hutchings wrote:
> It would be more idiomatic to write the condition as:
>
> if (hca_param.global_caps & ~(u64)HCA_GLOBAL_CAP_MASK)
>
> (although that cast probably belongs in the definition of
> HCA_GLOBAL_CAP_MASK).
>
You are correct!
We need to submit a patch to fix this.
-Jack
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-06-17 15:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-09 9:09 mlx4_core: Modify driver initialization flow to accommodate SRIOV for Ethernet Dan Carpenter
2012-06-09 11:29 ` Jack Morgenstein
2012-06-10 1:03 ` Ben Hutchings
2012-06-17 15:18 ` Jack Morgenstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox