Hi,
Attached is a patch against FreeBSD HEAD (svn 222248, aka. FreeBSD
9-CURRENT) which solves the Intel IXGBE driver problem reported below.
Changes:
* IXGBE's probe() function now return BUS_PROBE_DEFAULT as it should.
* IXGBE's attach() now inquire resource_disabled() to enable hints to
explicitly disable supported adapters.
* device_if.m's wording has been updated. (This file's descriptions
of DEVICE_* functions seem redundant as it is also written in man 9
DEVICE_*.9)
* IXGBE's module build Makefile now use name if_ixgbe instead of
old-style ixgbe. Man pages and forth's loader.conf already reflect this.
The wording in device_if.m is likely to provide you with a good laugh,
so I suggest rewording it further :) It is an attempt at ensuring future
driver developers does -not- return zero in their probe() function,
unless absolutely required. I am however not sure if we need this file's
descriptions any longer, as the man pages seem far better?
The attached file also patch nicely against RELENG_8, which is still open.
I assume it is enough to submit this patch to the freebsd-hackers
mailinglist?
Philip Soeberg ////
[email protected] (@ @) soeberg.net
------------------------------oOO--(_)--OOo------------------
On 23-05-2011 16:32, John Baldwin wrote:
On Monday, May 23, 2011 10:13:41 am Philip Soeberg wrote:
Hi fellow FreeBSD hackers,
I've just completed designing a new driver for the Intels IXGBE suite of
network adapters, but is building my driver as a kernel module to be
loaded after system boot.
The current sys/dev/ixgbe/ixgbe.c driver which attach to Intels adapters
return a zero in it's probe() function (which equals to
BUS_PROBE_SPECIFIC).. This has the distinct disadvantage that I cannot,
through my module, call a device_detach() on the devices I support, and
afterward expect being probed for them. A BUS_PROBE_SPECIFIC, according
to wording in sys/sys/bus.h, inform the OS that "Only I can use this
device".
I assume this (transcanding from FreeBSD 7.0-STABLE through to FreeBSD
9-CURRENT) is in error? I would expect sys/dev/ixgbe/ixgbe.c's probe()
function to return BUS_PROBE_DEFAULT, which is the "Base OS default
driver"..
Yes, that is true.
If this is true, then we should probably also update
sys/kern/device_if.m's description of the probe() method as to reflect
the BUS_PROBE_* return values in a clearer way than is currently described.
Do you want me to provide a patch? (it's really a one liner for ixgbe.c
and a couple of alterations to the device_if.m, if need be)
device_if.m was probably just never updated from when BUS_PROBE_* were added.
Updating it would be a good thing.
I would also expect the ixgbe.c driver to do a quick resource_disabled()
in it's attach() function, so that we can disable specific adapters
through kenv hint.ix.0.disabled=1..
That is not universally supported (i.e. it's not a part of new-bus
specifically). For buses that support hinted devices, they do all generally
support being able to disable a hinted device, but disabling bus-enumerated
devices is not generally supported.
Given that I can't use device_detach() on a device hogged by the IXGBE
driver, can any one of you help me with a way around this problem? I
can't use the hints, and I can't detach() the device.. how can I get my
kernel module to attach the device?
I think ixgbe has to be fixed to use BUS_PROBE_DEFAULT. Very few drivers
should use '0' for their probe return value.
Index: sys/kern/device_if.m
===================================================================
--- sys/kern/device_if.m (revision 222248)
+++ sys/kern/device_if.m (working copy)
@@ -97,10 +97,18 @@
* used to generate an informative message when DEVICE_ATTACH()
* is called.
*
+ * Driver election is determined by the negative return value of
+ * the probe function on a highest-value-best-match type effort.
+ * Drivers included in the kernel normally return -20 (via
+ * convenience define BUS_PROBE_DEFAULT) which allows for a more
+ * specific driver to be installed at a later time.
+ *
* As a special case, if a driver returns zero, the driver election
* is cut short and that driver will attach to the device
- * immediately.
+ * immediately. A return value of zero is rarely used.
*
+ * Please see man page for DEVICE_PROBE() for more information.
+ *
* For example, a probe method for a pci device driver might look
* like this:
*
@@ -110,7 +118,7 @@
* if (pci_get_vendor(dev) == FOOVENDOR &&
* pci_get_device(dev) == FOODEVICE) {
* device_set_desc(dev, "Foo device");
- * return (0);
+ * return (BUS_PROBE_DEFAULT);
* }
* return (ENXIO);
* }
@@ -128,7 +136,7 @@
* @retval 0 if the driver strongly matches this device
* @retval negative if the driver can match this device - the
* least negative value is used to select the
- * driver
+ * driver. Typical return value is BUS_PROBE_DEFAULT.
* @retval ENXIO if the driver does not match the device
* @retval positive if some kind of error was detected during
* the probe, a regular unix error code should
Index: sys/modules/ixgbe/Makefile
===================================================================
--- sys/modules/ixgbe/Makefile (revision 222248)
+++ sys/modules/ixgbe/Makefile (working copy)
@@ -1,6 +1,6 @@
#$FreeBSD$
.PATH: ${.CURDIR}/../../dev/ixgbe
-KMOD = ixgbe
+KMOD = if_ixgbe
SRCS = device_if.h bus_if.h pci_if.h
SRCS += ixgbe.c ixv.c
# Shared source
Index: sys/dev/ixgbe/ixgbe.c
===================================================================
--- sys/dev/ixgbe/ixgbe.c (revision 222248)
+++ sys/dev/ixgbe/ixgbe.c (working copy)
@@ -318,7 +318,7 @@
* ixgbe_probe determines if the driver should be loaded on
* adapter based on PCI vendor/device id of the adapter.
*
- * return 0 on success, positive on failure
+ * return BUS_PROBE_DEFAULT on success, ENXIO on failure
*********************************************************************/
static int
@@ -357,7 +357,7 @@
ixgbe_driver_version);
device_set_desc_copy(dev, adapter_name);
++ixgbe_total_ports;
- return (0);
+ return (BUS_PROBE_DEFAULT);
}
ent++;
}
@@ -383,6 +383,13 @@
u16 csum;
u32 ctrl_ext;
+ /* If device unit is disabled through hints, e.g.
+ (hint.ix.<unitnumber>.disabled=1), don't attach it */
+ if (resource_disabled(device_get_name(dev), device_get_unit(dev))) {
+ device_printf(dev, "device is disabled\n");
+ return (ENXIO);
+ }
+
INIT_DEBUGOUT("ixgbe_attach: begin");
/* Allocate, clear, and link in our adapter structure */
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "[email protected]"
