module.viomap support for ppc64
Subsystems (3)
David,
these 2 patches add support for a modules.viomap to depmod.
It needs also a kernel change.
Current MODULE_DEVICE_TABLE(vio, $table); defines 2 char pointers. I'm not sure if depmod can really handle it. Where do they point to in the module binary? I could find an answer, so far. I just declared an array. According to Segher, the "name" and "compatible" properties have an undefined size. So we have to pick a value and stick with it. I used 32, for alignment reasons.
What additional padding is required in VIO_DEVICE_SIZE64?
The depmod change produces a file like that:/lib/modules/2.6.5-0-pseries64/modules.viomap
# vio module name compatible ibmveth network IBM,l-lan ibmvscsic scsi-3 IBM,v-scsi ibmvscsis v-scsi-host IBM,v-scsi-host hvcs serial-server hvterm2
linux-2.6.5/drivers/char/hvcs.c +1 -1
@@ -416,7 +416,7 @@ int khvcsd(void *unused)static struct vio_device_id hvcs_driver_table[]__devinitdata = { {"serial-server" ,"hvterm2" }, - {0 ,} + {"" ,"" } };MODULE_DEVICE_TABLE (vio, hvcs_driver_table);
linux-2.6.5/drivers/net/ibmveth.c +1 -1
@@ -1123,7 +1123,7 @@ static void ibmveth_proc_unregister_drivstatic struct vio_device_id ibmveth_device_table[]__devinitdata = { {"network" ,"IBM,l-lan" }, - {0 ,} + {"" ,"" } };MODULE_DEVICE_TABLE (vio, ibmveth_device_table);
linux-2.6.5/drivers/scsi/ibmvscsi/ibmvscsis.c +1 -1
@@ -2754,7 +2754,7 @@ static int ibmvscsis_remove(struct vio_dstatic struct vio_device_id ibmvscsis_device_table[]__devinitdata = { {"v-scsi-host" ,"IBM,v-scsi-host" }, - {0 ,} + {"" ,"" } };MODULE_DEVICE_TABLE (vio, ibmvscsis_device_table);
linux-2.6.5/drivers/scsi/ibmvscsi/rpa_vscsi.c +1 -1
@@ -270,7 +270,7 @@ static struct vio_device_id rpa_device_t {"scsi-3", "IBM,v-scsi"}, /* Note: This entry can go away whenall the firmware is up to date */ {"vscsi", "IBM,v-scsi"}, - {0,} + {"", ""} };/**
linux-2.6.5/include/asm-ppc64/vio.h +3 -2
@@ -85,9 +85,10 @@ static inline int vio_set_dma_mask(strucextern struct bus_type vio_bus_type; +#define VIO_DEVTABLE_PROPERTY_LENGTH 32struct vio_device_id { -char *type; -char *compat; +char type[VIO_DEVTABLE_PROPERTY_LENGTH ]; +char compat[VIO_DEVTABLE_PROPERTY_LENGTH ]; };struct vio_driver {
depmod.c +1 -0
@@ -595,6 +595,7 @@ static struct depfile depfiles[] = { { "modules.usbmap", output_usb_table }, { "modules.ccwmap", output_ccw_table }, { "modules.ieee1394map", output_ieee1394_table }, + { "modules.viomap", output_vio_table }, { "modules.isapnpmap", output_isapnp_table }, { "modules.inputmap", output_input_table }, { "modules.alias", output_aliases },
depmod.h +2 -0
@@ -36,6 +36,8 @@ struct modulevoid *pci_table;unsigned int usb_size;void *usb_table; +unsigned int vio_size; +void *vio_table;unsigned int ieee1394_size;void *ieee1394_table;unsigned int ccw_size;
moduleops_core.c +4 -0
@@ -176,6 +176,10 @@ static void PERBIT(fetch_tables)(struct module->ccw_table =PERBIT (deref_sym)(module->data,"__mod_ccw_device_table" ); + module->vio_size =PERBIT (VIO_DEVICE_SIZE ); + module->vio_table =PERBIT (deref_sym)(module->data, +"__mod_vio_device_table" ); + module->ieee1394_size =PERBIT (IEEE1394_DEVICE_SIZE ); module->ieee1394_table =PERBIT (deref_sym)(module->data,"__mod_ieee1394_device_table" );
tables.c +24 -0
@@ -127,7 +127,31 @@ void output_ieee1394_table(struct moduleoutput_ieee1394_entry (fw, shortname, out); } } +static void output_vio_entry (struct vio_device_id *fw,char *name,FILE *out) +{ +fprintf (out,"%-20s %-32s %s\n" , + name, fw->name, fw->compat); +} +void output_vio_table (struct module *modules,FILE *out) +{ +struct module *i; + +fprintf (out,"%-20s %-32s compatible\n" ,"# vio module" ,"name" ); + +for (i = modules; i; i = i->next) { +struct vio_device_id *fw; +char shortname[strlen (i->pathname) +1 ]; + +if (!i->vio_table) +continue ; + +make_shortname (shortname, i->pathname); +for (fw = i->vio_table; *fw->name; + fw = (void *) fw + i->vio_size) +output_vio_entry (fw, shortname, out); + } +}/* We set driver_data to zero */ static void output_ccw_entry (struct ccw_device_id *ccw,char *name,FILE *out)
tables.h +10 -0
@@ -35,6 +35,15 @@ struct usb_device_id {#define USB_DEVICE_SIZE32 (5 * 2 + 6 * 1 + 4)#define USB_DEVICE_SIZE64 (5 * 2 + 6 * 1 + 8) +#define VIO_DEVTABLE_PROPERTY_LENGTH 32 +struct vio_device_id { +char name[VIO_DEVTABLE_PROPERTY_LENGTH ]; +char compat[VIO_DEVTABLE_PROPERTY_LENGTH ]; +}; + +#define VIO_DEVICE_SIZE32 (2 * VIO_DEVTABLE_PROPERTY_LENGTH + 4) +#define VIO_DEVICE_SIZE64 (2 * VIO_DEVTABLE_PROPERTY_LENGTH + 8) +struct ieee1394_device_id {unsigned int match_flags;unsigned int vendor_id;
@@ -119,6 +128,7 @@ struct input_device_id_32 {/* Functions provided by tables.c */ struct module ;void output_usb_table (struct module *modules,FILE *out); +void output_vio_table (struct module *modules,FILE *out);void output_ieee1394_table (struct module *modules,FILE *out);void output_pci_table (struct module *modules,FILE *out);void output_ccw_table (struct module *modules,FILE *out);
Signature · 3 lines
USB is for mice, FireWire is for men! sUse lINUX ag, nÜRNBERG
Quoted 3 lines · Olaf Hering
Current
MODULE_DEVICE_TABLE(vio, $table); defines 2 char pointers. I'm not sure if depmod can really handle it. Where do they point to in the module binary? I could find an answer, so far. I just declared an array.
Olaf explained on irc that output_vio_entry() below was finding NULL for the name and compat pointers. Perhaps some additional relocation needs to take place before those can be used?
Quoted 49 lines
diff -p -purN
module-init-tools-3.0-pre10.orig/moduleops_core.cmodule-init-tools-3.0-pre10/moduleops_core.cmoduleops_core.c +4
-0@@ -176,6 +176,10 @@ static void PERBIT(fetch_tables)(struct module->ccw_table =PERBIT (deref_sym)(module->data,"__mod_ccw_device_table" ); + module->vio_size =PERBIT (VIO_DEVICE_SIZE ); + module->vio_table =PERBIT (deref_sym)(module->data, +"__mod_vio_device_table" ); + module->ieee1394_size =PERBIT (IEEE1394_DEVICE_SIZE ); module->ieee1394_table =PERBIT (deref_sym)(module->data,"__mod_ieee1394_device_table" );diff -p -purN
module-init-tools-3.0-pre10.orig/tables.cmodule-init-tools-3.0-pre10/tables.ctables.c +24
-0@@ -127,7 +127,31 @@ void output_ieee1394_table(struct moduleoutput_ieee1394_entry (fw, shortname, out); } } +static void output_vio_entry (struct vio_device_id *fw,char *name,FILE *out) +{ +fprintf (out,"%-20s %-32s %s\n" , + name, fw->name, fw->compat); +} +void output_vio_table (struct module *modules,FILE *out) +{ +struct module *i; + +fprintf (out,"%-20s %-32s compatible\n" ,"# vio module" ,"name" ); + +for (i = modules; i; i = i->next) { +struct vio_device_id *fw; +char shortname[strlen (i->pathname) +1 ]; + +if (!i->vio_table) +continue ; + +make_shortname (shortname, i->pathname); +for (fw = i->vio_table; *fw->name; + fw = (void *) fw + i->vio_size) +output_vio_entry (fw, shortname, out); + } +}/* We set driver_data to zero */ static void output_ccw_entry (struct ccw_device_id *ccw,char *name,FILE *out)
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
Quoted 8 lines · Hollis Blanchard
Quoted 3 lines · Olaf Hering
Current
MODULE_DEVICE_TABLE(vio, $table); defines 2 char pointers. I'm not sure if depmod can really handle it. Where do they point to in the module binary? I could find an answer, so far. I just declared an array.Olaf explained on irc that
output_vio_entry()below was finding NULL for the name and compat pointers. Perhaps some additional relocation needs to take place before those can be used?
1) Please use char arrays of some fixed size.
2) Please modify scripts/mod/file2alias.c in the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.
3) I will still accept patches to module-init-tools if required for 2.4 compatibility, but they will be going away at some point!
Hope that clarifies!
Rusty.
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
Quoted 4 lines · Rusty Russell
2) Please modify
scripts/mod/file2alias.cin the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.
I found no user of the modules.alias file. Hotplug still uses the map files. Parsing one big file will not improve performance, but thats a different story.
A hack for 2.6.8-rc4 is below. Can I read the alias file via while read a b c ; do : done < modules.alias ?
Is b supposed to contain not spaces? What special delimiter chars are allowed? The 'name' and 'compat' property can contain almost any char. I used '^' for the time being.
Quoted 2 lines
3) I will still accept patches to module-init-tools if required for 2.4 compatibility, but they will be going away at some point!
Noone cares about that old junk.
drivers/char/hvcs.c +1 -1
@@ -502,7 +502,7 @@ static int khvcsd(void *unused)static struct vio_device_id hvcs_driver_table[]__devinitdata = { {"serial-server" ,"hvterm2" }, - {0 , } + {"" ,"" } };MODULE_DEVICE_TABLE (vio, hvcs_driver_table);
drivers/net/ibmveth.c +1 -1
@@ -1119,7 +1119,7 @@ static void ibmveth_proc_unregister_drivstatic struct vio_device_id ibmveth_device_table[]__devinitdata = { {"network" ,"IBM,l-lan" }, - {0 ,} + {"" ,"" } };MODULE_DEVICE_TABLE (vio, ibmveth_device_table);
include/asm-ppc64/vio.h +3 -2
@@ -86,9 +86,10 @@ static inline int vio_set_dma_mask(strucextern struct bus_type vio_bus_type; +#define VIO_DEVTABLE_PROPERTY_LENGTH 32struct vio_device_id { -char *type; -char *compat; +char type[VIO_DEVTABLE_PROPERTY_LENGTH ]; +char compat[VIO_DEVTABLE_PROPERTY_LENGTH ]; };struct vio_driver {
include/linux/mod_devicetable.h +5 -0
@@ -164,5 +164,10 @@ struct pnp_card_device_id { } devs[PNP_MAX_DEVICES]; }; +#define VIO_DEVTABLE_PROPERTY_LENGTH 32 +struct VIO_device_id { +char name[VIO_DEVTABLE_PROPERTY_LENGTH ]; +char compat[VIO_DEVTABLE_PROPERTY_LENGTH ]; +};#endif /* LINUX_MOD_DEVICETABLE_H */
scripts/mod/file2alias.c +10 -0
@@ -198,6 +198,13 @@ static int do_pnp_card_entry(const char }return 1 ; } +/* looks like: "vio:cCdD..." */ +static int do_vio_entry (const char *filename, +struct VIO_device_id *id,char *alias) +{ +sprintf (alias,"vio:%s^%s" , id->name, id->compat); +return 1 ; +}/* Ignore any prefix, eg. v850 prepends _ */ static inline int sym_is (const char *symbol,const char *name)
@@ -271,6 +278,9 @@ void handle_moddevtable(struct module *melse if (sym_is (symname,"__mod_pnp_card_device_table" ))do_table (symval, sym->st_size,sizeof (struct pnp_card_device_id ), do_pnp_card_entry, mod); +else if (sym_is (symname,"__mod_vio_device_table" )) +do_table (symval, sym->st_size,sizeof (struct VIO_device_id ), + do_vio_entry, mod); }/* Now add out buffered information to the generated C source */
Signature · 3 lines
USB is for mice, FireWire is for men! sUse lINUX ag, nÜRNBERG
Quoted 10 lines · Olaf Hering
Quoted 4 lines · Rusty Russell
2) Please modify
scripts/mod/file2alias.cin the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.I found no user of the modules.alias file. Hotplug still uses the map files. Parsing one big file will not improve performance, but thats a different story.
You don't use the modules.alias file. You simply "modprobe vio:xyz^abc" and modprobe reads modules.alias if necessary (the user can also insert aliases in the modprobe.conf file, for example). Note that fnmatch is used, so you can actually use ? and * in your generated aliases.
Quoted 5 lines
A hack for 2.6.8-rc4 is below. Can I read the alias file via while read a b c ; do : done < modules.alias ?
Is b supposed to contain not spaces? What special delimiter chars are allowed? The 'name' and 'compat' property can contain almost any char. I used '^' for the time being.
Spaces are probably a bad idea, yes. ^ is a little odd, but probably not a bad choice. You could even use a full: "vio:name:%s:compat:%s" if you wanted to.
Quoted 4 lines
Quoted 2 lines
3) I will still accept patches to module-init-tools if required for 2.4 compatibility, but they will be going away at some point!
Noone cares about that old junk.
Shh... Marcelo might get offended 8)
Patch looks fine...
Rusty.
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
Quoted 23 lines · Rusty Russell
Quoted 10 lines · Olaf Hering
Quoted 4 lines · Rusty Russell
2) Please modify
scripts/mod/file2alias.cin the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.I found no user of the modules.alias file. Hotplug still uses the map files. Parsing one big file will not improve performance, but thats a different story.
You don't use the modules.alias file. You simply "modprobe vio:xyz^abc" and modprobe reads modules.alias if necessary (the user can also insert aliases in the modprobe.conf file, for example). Note that fnmatch is used, so you can actually use ? and * in your generated aliases.
Quoted 5 lines
A hack for 2.6.8-rc4 is below. Can I read the alias file via while read a b c ; do : done < modules.alias ?
Is b supposed to contain not spaces? What special delimiter chars are allowed? The 'name' and 'compat' property can contain almost any char. I used '^' for the time being.
Quoted 8 lines
Spaces are probably a bad idea, yes. ^ is a little odd, but probably not a bad choice. You could even use a full: "vio:name:%s:compat:%s" if you wanted to.
Quoted 4 lines
Quoted 2 lines
3) I will still accept patches to module-init-tools if required for 2.4 compatibility, but they will be going away at some point!
Noone cares about that old junk.
I do care!
Quoted 1 line
Shh... Marcelo might get offended 8)
Yes, I, I, rrrr :)
Quoted 8 lines · Rusty Russell
Quoted 7 lines · Olaf Hering
Quoted 4 lines · Rusty Russell
2) Please modify
scripts/mod/file2alias.cin the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.
Quoted 8 lines
Quoted 3 lines
I found no user of the modules.alias file. Hotplug still uses the map files. Parsing one big file will not improve performance, but thats a different story.
You don't use the modules.alias file. You simply "modprobe vio:xyz^abc" and modprobe reads modules.alias if necessary (the user can also insert aliases in the modprobe.conf file, for example). Note that fnmatch is used, so you can actually use ? and * in your generated aliases.
But that complicates the parser. Current the hotplug agent script reads the simple modules.foomap and generates a list of possible drivers. Then it looks into the blacklist file to see if one of the possible modules should not be loaded, and skips this module.
Is there such functionality for the modules.alias file in module-init-tools? I played around with modprobe -n, but could not figure it out. Unfortunately, some hardware has more than one driver. bcm5700/tg3, eepro100/e100 and maybe more.
#!/bin/bash
cd /sys/bus/pci/devices || exit 1
for i in *
do
test -d $i || continue
read pci_class < $i/class read pci_vendor < $i/vendor read pci_device < $i/device
read pci_subsystem_vendor < $i/subsystem_vendor
read pci_subsystem_device < $i/subsystem_device
pci_baseclass=$((($pci_class >> 16) & 0xff))
pci_subclass=$((($pci_class >> 8) & 0xff))
pci_interface=$(($pci_class & 0xff))
pci_alias=printf 'pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02X' ${pci_vendor} ${pci_device} ${pci_subsystem_vendor} ${pci_subsystem_device} ${pci_baseclass} ${pci_subclass} ${pci_interface} echo $i # $pci_alias/sbin/modprobe -nvva $pci_alias
done
Signature · 3 lines
USB is for mice, FireWire is for men! sUse lINUX ag, nÜRNBERG
Quoted 19 lines · Olaf Hering
Quoted 8 lines · Rusty Russell
Quoted 7 lines · Olaf Hering
Quoted 4 lines · Rusty Russell
2) Please modify
scripts/mod/file2alias.cin the kernel source, not the module tools. The modules.XXXmap files are deprecated: device tables are supposed to be converted to aliases in the build process, and that is how userspace tools like hotplug are to find them.Quoted 8 lines
Quoted 3 lines
I found no user of the modules.alias file. Hotplug still uses the map files. Parsing one big file will not improve performance, but thats a different story.
You don't use the modules.alias file. You simply "modprobe vio:xyz^abc" and modprobe reads modules.alias if necessary (the user can also insert aliases in the modprobe.conf file, for example). Note that fnmatch is used, so you can actually use ? and * in your generated aliases.
Quoted 4 lines
But that complicates the parser. Current the hotplug agent script reads the simple modules.foomap and generates a list of possible drivers. Then it looks into the blacklist file to see if one of the possible modules should not be loaded, and skips this module.
Complicates? Leave it to modprobe. Don't read any files. Really.
Current implementation of aliases is to load one at random: multiple alias resolution is undefined because noone knew what we should do (load them all? Load until one succeeds?). But note that that the base config file overrides anything extracted from the modules themselves, so users/distributions can always specify an exact match.
Quoted 4 lines
Is there such functionality for the modules.alias file in module-init-tools? I played around with modprobe -n, but could not figure it out. Unfortunately, some hardware has more than one driver. bcm5700/tg3, eepro100/e100 and maybe more.
OK, I think the difference here is that I feel modprobe should resolve it. What's the right answer? Do we need a new "unalias" config cmd which does the blacklist, or is the current positive method better? How do you currently decide?
Thanks!
Rusty.
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
Quoted 5 lines · Rusty Russell
Current implementation of aliases is to load one at random: multiple alias resolution is undefined because noone knew what we should do (load them all? Load until one succeeds?). But note that that the base config file overrides anything extracted from the modules themselves, so users/distributions can always specify an exact match.
How is the blacklist stuff supposed to work then? It must be possible to map an alias entry to a list of modules, and check if any of them is blacklisted. Then just 'for i in $DRIVERS ; do modprobe $i ; done'.
Quoted 9 lines
Quoted 4 lines
Is there such functionality for the modules.alias file in module-init-tools? I played around with modprobe -n, but could not figure it out. Unfortunately, some hardware has more than one driver. bcm5700/tg3, eepro100/e100 and maybe more.
OK, I think the difference here is that I feel modprobe should resolve it. What's the right answer? Do we need a new "unalias" config cmd which does the blacklist, or is the current positive method better? How do you currently decide?
modprobe or some other tool can certainly resolve it, but something has to make a decision based on a blacklist if a certain module can be loaded.
Look at /etc/hotplug/pci.agent how it currently done, it parses the modules.pcimap, fills $DRIVERS and passes that variable to the generic modprobe function from hotplug.functions. It reads /etc/hotplug/blacklist to decide if any of the modules listed in DRIVERS must be skipped.
Signature · 3 lines
USB is for mice, FireWire is for men! sUse lINUX ag, nÜRNBERG
Quoted 11 lines · Olaf Hering
Quoted 5 lines · Rusty Russell
Current implementation of aliases is to load one at random: multiple alias resolution is undefined because noone knew what we should do (load them all? Load until one succeeds?). But note that that the base config file overrides anything extracted from the modules themselves, so users/distributions can always specify an exact match.
How is the blacklist stuff supposed to work then? It must be possible to map an alias entry to a list of modules, and check if any of them is blacklisted. Then just 'for i in $DRIVERS ; do modprobe $i ; done'.
Olaf, I'm trying to understand the problem. We currently have a system where modprobe resolves aliases, and that's good because it allows users to override those aliases. We could have a "modprobe --resolve-aliases" function, but that breaks down if the user puts in an install command, which they should be able to do.
Looking through SLES and my Debian system, I see:
1) Modules which never want autoloading (eg. evbug)
2) Modules which defer to other modules (eg. de4x5)
3) Modules for which you want to suppress autoloading altogether because they cause problems, or are handled by other subsystems.
For (1), the information should be contained in the module itself. This will involve a kernel patch.
For (2), the information should be placed in /etc/modprobe.d/ overriding those aliases the preferred way.
For (3), modprobe cannot have this in a config file, because what needs to be blacklisted depends on who is invoking modprobe. For example, there's a comment that ISDN modules on SuSE are not handled by hotplug, but /etc/init.d/isdn.
So, a solution would be:
a) A "MODULE_NO_TABLES" macro which suppresses ID table alias generation,
b) A -X argument to modprobe which says "don't load these modules", and
c) Change modprobe to load all the aliases which match, not just one? I hadn't done this because I wasn't aware of anything which needed it: is there something?
I think we're getting there: sorry if I seem a bit obtuse... Rusty.
Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell