On Oct 14, 3:05pm, Paul Goyette wrote:
}
} Module Name: src
} Committed By: pgoyette
} Date: Mon May 24 20:29:41 UTC 2010
}
} Modified Files:
} src/distrib/sets/lists/modules: mi
} src/sys/dev/pci: files.pci pci.c pci_subr.c pcivar.h
} src/sys/modules: Makefile
} Added Files:
} src/sys/dev/pci: pci_verbose.c
} src/sys/modules/pciverbose: Makefile
}
} Log Message:
} Extract the vendor/product tables and related access routines into a
} separate kernel module. Update pci bus attach routine to load the
} module (if available) when we're about to start scanning the bus, and
} unload the module after the scan is finished.
There is no for it unload the autoloaded module.
} XXX Cardbus (and possibly other) drivers should also be modified to
} XXX load the module before scanning/attaching devices.
Yep. :->
} Modified files:
}
} Index: src/sys/dev/pci/pci.c
} diff -u src/sys/dev/pci/pci.c:1.127 src/sys/dev/pci/pci.c:1.128
} --- src/sys/dev/pci/pci.c:1.127 Wed Feb 24 22:38:01 2010
} +++ src/sys/dev/pci/pci.c Mon May 24 20:29:41 2010
} @@ -1,4 +1,4 @@
} -/* $NetBSD: pci.c,v 1.127 2010/02/24 22:38:01 dyoung Exp $ */
} +/* $NetBSD: pci.c,v 1.128 2010/05/24 20:29:41 pgoyette Exp $ */
}
} /*
} * Copyright (c) 1995, 1996, 1997, 1998
} @@ -36,12 +36,13 @@
} */
}
} #include <sys/cdefs.h>
} -__KERNEL_RCSID(0, "$NetBSD: pci.c,v 1.127 2010/02/24 22:38:01 dyoung Exp $");
} +__KERNEL_RCSID(0, "$NetBSD: pci.c,v 1.128 2010/05/24 20:29:41 pgoyette Exp
$");
}
} #include "opt_pci.h"
}
} #include <sys/param.h>
} #include <sys/malloc.h>
} +#include <sys/module.h>
} #include <sys/systm.h>
} #include <sys/device.h>
}
} @@ -106,7 +107,12 @@
} KASSERT(ifattr && !strcmp(ifattr, "pci"));
} KASSERT(locators);
}
} + pci_verbose_ctl(true); /* Try to load the pciverbose module */
} +
Does this only cover boot time, or will it also cover bus rescans
(i.e. drvctl -r)?
} pci_enumerate_bus(sc, locators, NULL, NULL);
} +
} + pci_verbose_ctl(false); /* Now try to unload it */
Don't forcibly unload the module. No reason for this module to be
any different then any other module that is autoloaded.
} +
} return 0;
} }
}
}
} Index: src/sys/dev/pci/pci_subr.c
} diff -u src/sys/dev/pci/pci_subr.c:1.79 src/sys/dev/pci/pci_subr.c:1.80
} --- src/sys/dev/pci/pci_subr.c:1.79 Thu Mar 4 22:55:20 2010
} +++ src/sys/dev/pci/pci_subr.c Mon May 24 20:29:41 2010
} [...]
} -const char *
} -pci_findproduct(pcireg_t id_reg)
} +#if defined(_KERNEL)
} +/*
} + * Routine to load/unload the pciverbose kernel module as needed
} + */
} +void pci_verbose_ctl(bool load)
} {
} -#ifdef PCIVERBOSE
} - static char buf[256];
} - pci_vendor_id_t vendor = PCI_VENDOR(id_reg);
} - pci_product_id_t product = PCI_PRODUCT(id_reg);
} - size_t n;
} -
} - for (n = 0; n < __arraycount(pci_products); n++) {
} - if (pci_products[n] == vendor && pci_products[n+1] == product)
} - return pci_untokenstring(&pci_products[n+2], buf,
} - sizeof(buf));
} -
} - /* Skip Tokens */
} - n += 2;
} - while (pci_products[n] != 0 && n < __arraycount(pci_products))
} - n++;
} + static int loaded = 0;
} +
} + if (load) {
} + if (loaded++ == 0)
} + module_load("pciverbose", MODCTL_LOAD_FORCE, NULL,
} + MODULE_CLASS_MISC);
As discussed previously, module_load() is for the use of
modctl(2); it should not be used when automatically loading modules.
By doing this, you're ignoring the value of the kern.module.autoload
sysctl variable. In other words, you're overriding the administrator
and that is a very bad thing. This must be changed to a call to
module_autoload().
} + return;
} }
} -#endif
} - return (NULL);
} + if (--loaded == 0)
} + module_unload("pciverbose");
There is no need to forcibly unload the module. Let the MODULAR
subsystem and/or administrator handle this.
} }
} +#endif
}
} Index: src/sys/dev/pci/pci_verbose.c
} diff -u /dev/null src/sys/dev/pci/pci_verbose.c:1.1
} --- /dev/null Mon May 24 20:29:41 2010
} +++ src/sys/dev/pci/pci_verbose.c Mon May 24 20:29:40 2010
} @@ -0,0 +1,155 @@
} +/* $NetBSD: pci_verbose.c,v 1.1 2010/05/24 20:29:40 pgoyette Exp $ */
} +
} +/*
} + * Copyright (c) 1997 Zubin D. Dittia. All rights reserved.
} + * Copyright (c) 1995, 1996, 1998, 2000
} + * Christopher G. Demetriou. All rights reserved.
} + * Copyright (c) 1994 Charles M. Hannum. All rights reserved.
} + *
} + * Redistribution and use in source and binary forms, with or without
} + * modification, are permitted provided that the following conditions
} + * are met:
} + * 1. Redistributions of source code must retain the above copyright
} + * notice, this list of conditions and the following disclaimer.
} + * 2. Redistributions in binary form must reproduce the above copyright
} + * notice, this list of conditions and the following disclaimer in the
} + * documentation and/or other materials provided with the distribution.
} + * 3. All advertising materials mentioning features or use of this software
} + * must display the following acknowledgement:
} + * This product includes software developed by Charles M. Hannum.
} + * 4. The name of the author may not be used to endorse or promote products
} + * derived from this software without specific prior written permission.
} + *
} + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
} + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
} + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
} + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
} + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
} + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
} + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
} + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
} + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
} + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
} + */
} +
} +/*
} + * PCI autoconfiguration support functions.
} + *
} + * Note: This file is also built into a userland library (libpci).
} + * Pay attention to this when you make modifications.
} + */
} +
} +#include <sys/cdefs.h>
} +__KERNEL_RCSID(0, "$NetBSD: pci_verbose.c,v 1.1 2010/05/24 20:29:40 pgoyette
Exp $");
} +
} +#ifdef _KERNEL_OPT
} +#include "opt_pci.h"
} +#endif
} +
} +#include <sys/param.h>
} +
} +#ifdef _KERNEL
} +#include <sys/module.h>
} +#else
} +#include <pci.h>
} +#endif
} +
} +#include <dev/pci/pcireg.h>
} +#ifdef _KERNEL
} +#include <dev/pci/pcivar.h>
} +#endif
} +
} +#include <dev/pci/pcidevs.h>
} +
} +/*
} + * Descriptions of of known vendors and devices ("products").
} + */
} +
} +#include <dev/pci/pcidevs_data.h>
} +
} +#ifndef _KERNEL
} +#include <string.h>
} +#endif
} +
} +#ifdef _KERNEL
} +static int pciverbose_modcmd(modcmd_t, void *);
} +
} +MODULE(MODULE_CLASS_MISC, pciverbose, NULL);
} +
} +static int
} +pciverbose_modcmd(modcmd_t cmd, void *arg)
} +{
} + aprint_normal("%s: cmd %d\n", __func__, cmd); /* XXX */
} + switch (cmd) {
} + case MODULE_CMD_INIT:
} + pci_findvendor_vec = pci_findvendor;
} + pci_findproduct_vec = pci_findproduct;
} + pci_unmatched = "unmatched ";
} + return 0;
} + case MODULE_CMD_FINI:
} + pci_findvendor_vec = pci_null;
} + pci_findproduct_vec = pci_null;
} + pci_unmatched = "";
} + return 0;
I would much prefer to see the module save the previous value of
*_vec and restore it when unloading. Most likely pci_null is the
correct value, but I don't think modules should be making assumptions
like this.
} + default:
} + return ENOTTY;
} + }
} +}
} +#endif /* KERNEL */
} +
}
}-- End of excerpt from Paul Goyette
