Add testpci command
PATCH 1/1 Add testpci command
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at /dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.
I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
| if testpci 8086:1237; then | set default="2" | fi
in my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
-----
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6 -0
@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };
=== added file 'grub-core/commands/testpci.c'
commands/testpci.c +75 -0
75 lines not on this page · raw message
-----
J.
Signature · 1 line
I'm out of bed and dressed. What more do you want?
Why don't you just put a file on that virtual disk somewhere, call it
_I_AM_A_VIRTUAL_MACHINE_, and just test for its existence using the comprehensive filesystem support of GRUB 2 ?
--S
Quoting Jonathan McDowell, who wrote the following on Thu, 27 Sep 2012:
Quoted 130 lines
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
| if testpci 8086:1237; then | set default="2" | fiin my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
Signed-off-by Jonathan McDowell <[email protected]>-----
=== modified file 'grub-core/Makefile.core.def'Makefile.core.def +6
-0@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };=== added file '
grub-core/commands/testpci.c'commands/testpci.c +75
-1@@ -0,0 +1,75 @@ +/* testpci.c - Test for PCI device existence. */ +/* +* GRUB -- GRand Unified Bootloader +* Copyright (C) 2012 Free Software Foundation, Inc. +* +* GRUB is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* GRUB is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with GRUB. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> + +GRUB_MOD_LICENSE ("GPLv3+" ); + +static int pcifound; +static grub_pci_id_t pcisearch; + +static int NESTED_FUNC_ATTR +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), +grub_pci_id_t pciid) +{ +if (pciid == pcisearch) { + pcifound =1 ; +return 1 ; + } + +return 0 ; +} + +static grub_err_t +grub_cmd_testpci (grub_command_t ctxt __attribute__ ((unused)), +int argc, +char **args) +{ + pcifound =0 ; + +if (argc !=1 ) +return grub_error (GRUB_ERR_BAD_ARGUMENT ,"Device to search for required" ); + +if ((grub_strlen (args[0 ]) !=9 ) || args[0 ][4 ] !=':' ) +return grub_error (GRUB_ERR_BAD_ARGUMENT ,"Device ID must be xxxx:yyyy" ); + + pcisearch =grub_strtoul (args[0 ],0 ,16 ) + + (grub_strtoul (&args[0 ][5 ],0 ,16 ) <<16 ); + +grub_pci_iterate (grub_testpci_iter); + +return pcifound ?GRUB_ERR_NONE :grub_error (GRUB_ERR_TEST_FAILURE ,"false" ); +} + +static grub_command_t cmd; + +GRUB_MOD_INIT (testpci) +{ + cmd =grub_register_command ("testpci" , grub_cmd_testpci,"xxxx:yyyy" , +N_ ("Test for PCI device existence." )); +} + +GRUB_MOD_FINI (testpci) +{ +grub_unregister_command (cmd); +} -----J.
Signature · 6 lines
I'm out of bed and dressed. What more do you want? _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
Bah. Ignore that. I see your problem now.
--S
Quoting Seth Goldberg, who wrote the following on Thu, 27 Sep 2012:
Quoted 147 lines
Why don't you just put a file on that virtual disk somewhere, call it
_I_AM_A_VIRTUAL_MACHINE_, and just test for its existence using the comprehensive filesystem support of GRUB 2 ?--S
Quoting Jonathan McDowell, who wrote the following on Thu, 27 Sep 2012:
Quoted 123 lines
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
| if testpci 8086:1237; then | set default="2" | fiin my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
Signed-off-by Jonathan McDowell <[email protected]>-----
=== modified file 'grub-core/Makefile.core.def'Makefile.core.def +6
-0@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };=== added file '
grub-core/commands/testpci.c'commands/testpci.c +51
-0@@ -0,0 +1,75 @@ +/* testpci.c - Test for PCI device existence. */ +/* +* GRUB -- GRand Unified Bootloader +* Copyright (C) 2012 Free Software Foundation, Inc. +* +* GRUB is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* GRUB is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with GRUB. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> + +GRUB_MOD_LICENSE ("GPLv3+" ); + +static int pcifound; +static grub_pci_id_t pcisearch; + +static int NESTED_FUNC_ATTR +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), +grub_pci_id_t pciid) +{ +if (pciid == pcisearch) { + pcifound =1 ; +return 1 ; + } + +return 0 ; +} + +static grub_err_t +grub_cmd_testpci (grub_command_t ctxt __attribute__ ((unused)), +int argc, +char **args) +{ + pcifound =0 ; + +if (argc !=1 ) + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Device to search forrequired");
commands/testpci.c +3
-0+ +if ((grub_strlen (args[0 ]) !=9 ) || args[0 ][4 ] !=':' ) + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Device ID must bexxxx:yyyy");
commands/testpci.c +7
-0+ + pcisearch =grub_strtoul (args[0 ],0 ,16 ) + + (grub_strtoul (&args[0 ][5 ],0 ,16 ) <<16 ); + +grub_pci_iterate (grub_testpci_iter); + + return pcifound ? GRUB_ERR_NONE : grub_error (GRUB_ERR_TEST_FAILURE,"false");
commands/testpci.c +14
-1+} + +static grub_command_t cmd; + +GRUB_MOD_INIT (testpci) +{ + cmd =grub_register_command ("testpci" , grub_cmd_testpci,"xxxx:yyyy" , +N_ ("Test for PCI device existence." )); +} + +GRUB_MOD_FINI (testpci) +{ +grub_unregister_command (cmd); +} -----Quoted 10 lines
J.
Signature · 6 lines
I'm out of bed and dressed. What more do you want? _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
В Чт., 27/09/2012 в 14:42 -0700, Jonathan McDowell пишет:
Quoted 1 line
| if testpci 8086:1237; then
What about making it "testpci --vendor XXX --product YYY" to leave room for possible extensions in the future?
-andrey
Quoted 6 lines · Andrey Borzenkov
В Чт., 27/09/2012 в 14:42 -0700, Jonathan McDowell пишет:
Quoted 1 line
| if testpci 8086:1237; then
What about making it "testpci --vendor XXX --product YYY" to leave room for possible extensions in the future?
This is a good idea. It allows to easily have e.g. class matching.
Quoted 8 lines
-andrey
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 10 lines · Jonathan McDowell
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on
This problem may be better solved by smbios tables or specific checker for qemu since this same ID is actually a real chipset AFAIR.
Quoted 5 lines
that. I have:
| if testpci 8086:1237; then | set default="2" | fi
I'd prefer the syntax proposed by Andrey.
Quoted 45 lines
=== added file '
grub-core/commands/testpci.c'commands/testpci.c +41
-0@@ -0,0 +1,75 @@ +/* testpci.c - Test for PCI device existence. */ +/* +* GRUB -- GRand Unified Bootloader +* Copyright (C) 2012 Free Software Foundation, Inc. +* +* GRUB is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* GRUB is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with GRUB. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> + +GRUB_MOD_LICENSE ("GPLv3+" ); + +static int pcifound; +static grub_pci_id_t pcisearch; + +static int NESTED_FUNC_ATTR +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), +grub_pci_id_t pciid) +{ +if (pciid == pcisearch) { + pcifound =1 ; +return 1 ; + } + +return 0 ; +}
commands/testpci.c line 51
Quoted 10 lines
commands/testpci.c +10
-0+ +static grub_err_t +grub_cmd_testpci (grub_command_t ctxt __attribute__ ((unused)), +int argc, +char **args) +{ + pcifound =0 ; + +if (argc !=1 ) +return grub_error (GRUB_ERR_BAD_ARGUMENT ,"Device to search for required" );
Please use the common message "obe argument expected" and add N_(...) so it gets translated
commands/testpci.c line 58
Quoted 7 lines
commands/testpci.c +7
-0+ +if ((grub_strlen (args[0 ]) !=9 ) || args[0 ][4 ] !=':' ) +return grub_error (GRUB_ERR_BAD_ARGUMENT ,"Device ID must be xxxx:yyyy" ); + + pcisearch =grub_strtoul (args[0 ],0 ,16 ) + + (grub_strtoul (&args[0 ][5 ],0 ,16 ) <<16 ); +
This is slightly wrong. But I skip this part since with the other parameter proposition this code part will disappear anyway.
commands/testpci.c line 68
Quoted 10 lines
commands/testpci.c +10
-0+grub_pci_iterate (grub_testpci_iter); + +return pcifound ?GRUB_ERR_NONE :grub_error (GRUB_ERR_TEST_FAILURE ,"false" ); +} + +static grub_command_t cmd; + +GRUB_MOD_INIT (testpci) +{ + cmd = grub_register_command ("testpci", grub_cmd_testpci, "xxxx:yyyy",
It should be N_("VENDOR:DEVICE"), not undescriptive xxxx
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
Re: [PATCH] Add testpci command (v2)
In reply to Jonathan McDowellQuoted 12 lines · Jonathan McDowell
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
Quoted 6 lines
| if testpci 8086:1237; then | set default="2" | fiin my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
So, a v2, changing to use --vendor/--product and also adding --subvendor/--subproduct as it was pointed out to me that the subsystem information can be used to confirm the hypervisor presence. I now have:
| if testpci --vendor=0x8086 --subvendor=0x1af4 --subproduct=0x1100; then | set default="2" | fi
which looks for an Intel device that has a Red Hat / KVM subsystem ID.
-----
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6 -0
@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };
=== added file 'grub-core/commands/testpci.c'
commands/testpci.c +114 -0
114 lines not on this page · raw message
-----
J.
Signature · 3 lines
Web [ "Don't pull the trigger. That would kinda give away the ] site: http:// [ game." ] Made by www.earth.li/~noodles/ [ ] HuggieTag 0.0.24
Re: [PATCH] Add testpci command (v2)
В Ср., 10/10/2012 в 17:21 -0700, Jonathan McDowell пишет:
Quoted 24 lines · Jonathan McDowell
Quoted 12 lines
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
Quoted 6 lines
| if testpci 8086:1237; then | set default="2" | fiin my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
So, a v2, changing to use --vendor/--product and also adding --subvendor/--subproduct as it was pointed out to me that the subsystem information can be used to confirm the hypervisor presence. I now have:
Quoted 6 lines
| if testpci --vendor=0x8086 --subvendor=0x1af4 --subproduct=0x1100; then | set default="2" | fiwhich looks for an Intel device that has a Red Hat / KVM subsystem ID.
Could you also add minimal documentation for it? Otherwise nobody will even know that command exists :(
Re: [PATCH] Add testpci command (v3)
In reply to Jonathan McDowellQuoted 24 lines · Jonathan McDowell
Quoted 12 lines · Jonathan McDowell
I have a machine with both Linux and Windows installed on the hard drive. Linux runs on the bare metal and I occasionally run the Windows install in a VM using KVM pointed at
/dev/sda. However if I'm not quick enough, or Windows decides to reboot when I'm not around to notice, the grub running under KVM will decide to boot Linux and much confusion ensues.I couldn't find an easy way to test what the running environment was, so I knocked up a simple testpci command that allows me to test for the existence of a PCI device and change the default boot option based on that. I have:
Quoted 6 lines
| if testpci 8086:1237; then | set default="2" | fiin my grub.cfg to correctly select the Windows partition if the qemu provided host bridge is present.
So, a v2, changing to use --vendor/--product and also adding --subvendor/--subproduct as it was pointed out to me that the subsystem information can be used to confirm the hypervisor presence. I now have:
Quoted 5 lines
| if testpci --vendor=0x8086 --subvendor=0x1af4 --subproduct=0x1100; then | set default="2" | fiwhich looks for an Intel device that has a Red Hat / KVM subsystem ID.
And here's v3, which adds some minimal documentation about the new command.
-----
=== modified file 'docs/grub.texi'
grub.texi +13 -0
@@ -3302,6 +3302,7 @@ * search:: Search devices by file, label, or UUID * sendkey:: Emulate keystrokes * set:: Set an environment variable +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size
@@ -4068,6 +4069,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. +@end deffn + + @node true @subsection true
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6 -0
@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };
=== added file 'grub-core/commands/testpci.c'
commands/testpci.c +114 -0
114 lines not on this page · raw message
-----
J.
Signature · 1 line
You drive a hard bargain for a seagull.
Re: [PATCH] Add testpci command (v3)
Quoted 78 lines · Jonathan McDowell
-----
=== modified file 'docs/grub.texi'grub.texi +13
-0@@ -3302,6 +3302,7 @@ * search:: Search devices by file, label, or UUID * sendkey:: Emulate keystrokes * set:: Set an environment variable +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size@@ -4068,6 +4069,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. +@end deffn + + @node true @subsection true=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6
-0@@ -837,6 +837,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };=== added file '
grub-core/commands/testpci.c'commands/testpci.c +25
-0@@ -0,0 +1,114 @@ +/* testpci.c - Test for PCI device existence. */ +/* +* GRUB -- GRand Unified Bootloader +* Copyright (C) 2012 Free Software Foundation, Inc. +* +* GRUB is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* GRUB is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with GRUB. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> +
commands/testpci.c line 35
Quoted 10 lines
commands/testpci.c +10
-0+GRUB_MOD_LICENSE ("GPLv3+" ); + +static const struct grub_arg_option options[] = + { + {"vendor" ,'v' ,0 ,N_ ("PCI vendor ID to look for" ),0 ,ARG_TYPE_INT }, + {"product" ,'p' ,0 ,N_ ("PCI product ID to look for" ),0 ,ARG_TYPE_INT }, + {"subvendor" ,'s' ,0 ,N_ ("PCI subsystem vendor ID to look for" ),0 ,ARG_TYPE_INT }, + {"subproduct" ,'t' ,0 ,N_ ("PCI subsystem product ID to look for" ),0 ,ARG_TYPE_INT }, + {0 ,0 ,0 ,0 ,0 ,0 } + };
ARG_TYPE_INT means that the argument is decimal integer but it's not the case here.
Quoted 15 lines
commands/testpci.c +15
-0+ +static int pcifound; +static grub_uint32_t pcimask, pcisubmask; +static grub_pci_id_t pcisearch; +static grub_pci_id_t pcisubsearch; + +static int NESTED_FUNC_ATTR +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), +grub_pci_id_t pciid) +{ +grub_pci_address_t addr; +grub_uint32_t subsystem; + +if ((pciid & pcimask) != pcisearch) { +return 0 ;
commands/testpci.c line 60
Quoted 10 lines
commands/testpci.c +10
-0+ } + +if (pcisubmask !=0 ) { + addr =grub_pci_make_address (dev,GRUB_PCI_REG_SUBVENDOR ); + subsystem =grub_pci_read (addr); +if ((subsystem & pcisubmask) != pcisubsearch) { +return 0 ; + } + } +
Please follow indent style.
Quoted 23 lines
commands/testpci.c +23
-0+ pcifound =1 ; +return 1 ; +} + +static grub_err_t +grub_cmd_testpci (grub_extcmd_context_t ctxt, +int argc __attribute__ ((unused)), +char **args __attribute__ ((unused))) +{ +struct grub_arg_list *state = ctxt->state; + + pcifound = pcimask = pcisubmask = pcisearch = pcisubsearch =0 ; + +if (state[0 ].set) { + pcisearch |=grub_strtoull (state[0 ].arg,0 ,16 ); + pcimask |=0xFFFF ; + } + +if (state[1 ].set) { + pcisearch |= (grub_strtoull (state[1 ].arg,0 ,16 ) <<16 ); + pcimask |=0xFFFF0000 ; + } +
commands/testpci.c line 93
Quoted 10 lines
commands/testpci.c +10
-0+if (state[2 ].set) { + pcisubsearch |=grub_strtoull (state[2 ].arg,0 ,16 ); + pcisubmask |=0xFFFF ; + } + +if (state[3 ].set) { + pcisubsearch |= (grub_strtoull (state[3 ].arg,0 ,16 ) <<16 ); + pcisubmask |=0xFFFF0000 ; + } +
You need to check grub_errno to catch any error in number parsing. Also you need to update a little bit because of recent callback changes. Other than this the patch is fine.
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
Re: [PATCH] Add testpci command (v4)
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko
On 13.10.2012 00:18, Jonathan McDowell wrote:
(some code)
Quoted 8 lines
ARG_TYPE_INTmeans that the argument is decimal integer but it's not the case here.Please follow indent style.
You need to check
grub_errnoto catch any error in number parsing. Also you need to update a little bit because of recent callback changes. Other than this the patch is fine.
Ok, I've updated against latest bzr and made the changes as above. Here's v4.
-----
=== modified file 'docs/grub.texi'
grub.texi +13 -0
@@ -3366,6 +3366,7 @@ * sendkey:: Emulate keystrokes * set:: Set an environment variable * source:: Read a configuration file in same context +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size
@@ -4147,6 +4148,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. +@end deffn + + @node true @subsection true
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6 -0
@@ -862,6 +862,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };
=== added file 'grub-core/commands/testpci.c'
commands/testpci.c +128 -0
128 lines not on this page · raw message
-----
J.
Signature · 2 lines
"Basically, if you're allowed to own it, you're probably allowed to burn it." -- Stephen Gower, ox.general
Re: [PATCH] Add testpci command (v4)
Does not "testpci" without parameters always return TRUE?
Quoted 231 lines · Jonathan McDowell
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko
On 13.10.2012 00:18, Jonathan McDowell wrote:
(some code)
Quoted 8 lines
ARG_TYPE_INTmeans that the argument is decimal integer but it's not the case here.Please follow indent style.
You need to check
grub_errnoto catch any error in number parsing. Also you need to update a little bit because of recent callback changes. Other than this the patch is fine.Ok, I've updated against latest bzr and made the changes as above. Here's v4.
Signed-off-by Jonathan McDowell <[email protected]>-----
=== modified file 'docs/grub.texi'grub.texi 2013-01-23 01:35:32 +0000 renamed from grub.texi 2013-01-21 16:53:41 +0000 +13
-0@@ -3366,6 +3366,7 @@ * sendkey:: Emulate keystrokes * set:: Set an environment variable * source:: Read a configuration file in same context +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size@@ -4147,6 +4148,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. +@end deffn + + @node true @subsection true=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def 2013-01-23 01:34:44 +0000 renamed from Makefile.core.def 2013-01-16 19:44:11 +0000 +6
-0@@ -862,6 +862,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };=== added file '
grub-core/commands/testpci.c'commands/testpci.c 2013-01-23 22:21:11 +0000 renamed from commands/testpci.c 1970-01-01 00:00:00 +0000 +128
-1@@ -0,0 +1,128 @@ +/* testpci.c - Test for PCI device existence. */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2013 Free Software Foundation, Inc. + * + * GRUB is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * GRUB is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> + +GRUB_MOD_LICENSE ("GPLv3+"); + +static const struct grub_arg_option options[] = + { + {"vendor", 'v' , 0, N_("PCI vendor ID to look for"), 0, ARG_TYPE_STRING}, + {"product", 'p' , 0, N_("PCI product ID to look for"), 0, ARG_TYPE_STRING}, + {"subvendor", 's' , 0, N_("PCI subsystem vendor ID to look for"), 0, ARG_TYPE_STRING}, + {"subproduct", 't' , 0, N_("PCI subsystem product ID to look for"), 0, ARG_TYPE_STRING}, + {0, 0, 0, 0, 0, 0} + }; + +static grub_uint32_t pcimask, pcisubmask; +static grub_pci_id_t pcisearch; +static grub_pci_id_t pcisubsearch; + +static int +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), + grub_pci_id_t pciid, + void *data) +{ + grub_pci_address_t addr; + grub_uint32_t subsystem; + int *found = data; + + if ((pciid & pcimask) != pcisearch) { + return 0; + } + + if (pcisubmask != 0) { + addr = grub_pci_make_address (dev, GRUB_PCI_REG_SUBVENDOR); + subsystem = grub_pci_read (addr); + if ((subsystem & pcisubmask) != pcisubsearch) { + return 0; + } + } + + *found = 1; + return 1; +} + +static grub_err_t +grub_cmd_testpci (grub_extcmd_context_t ctxt, + int argc __attribute__ ((unused)), + char **args __attribute__ ((unused))) +{ + struct grub_arg_list *state = ctxt->state; + int found = 0; + + pcimask = pcisubmask = pcisearch = pcisubsearch = 0; + + if (state[0].set) { + pcisearch |= grub_strtoull (state[0].arg, 0, 16); + if (grub_errno != GRUB_ERR_NONE) { + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI vendor ID")); + } + pcimask |= 0xFFFF; + } + + if (state[1].set) { + pcisearch |= (grub_strtoull (state[1].arg, 0, 16) << 16); + if (grub_errno != GRUB_ERR_NONE) { + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI product ID")); + } + pcimask |= 0xFFFF0000; + } + + if (state[2].set) { + pcisubsearch |= grub_strtoull (state[2].arg, 0, 16); + if (grub_errno != GRUB_ERR_NONE) { + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI subsystem vendor ID")); + } + pcisubmask |= 0xFFFF; + } + + if (state[3].set) { + pcisubsearch |= (grub_strtoull (state[3].arg, 0, 16) << 16); + if (grub_errno != GRUB_ERR_NONE) { + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI subsystem product ID")); + } + pcisubmask |= 0xFFFF0000; + } + + grub_pci_iterate (grub_testpci_iter, &found); + + return found ? GRUB_ERR_NONE : grub_error (GRUB_ERR_TEST_FAILURE, "false"); +} + +static grub_extcmd_t cmd; + +GRUB_MOD_INIT(testpci) +{ + cmd = grub_register_extcmd ("testpci", grub_cmd_testpci, 0, + N_("[--vendor=VENDORID] [--product=PRODUCTID] " + "[--subvendor=VENDORID] " + "[--subproduct=PRODUCTID]"), + N_("Test for PCI device existence."), + options); +} + +GRUB_MOD_FINI(testpci) +{ + grub_unregister_extcmd (cmd); +} -----J.
--
"Basically, if you're allowed to own it, you're probably allowed to burn it." -- Stephen Gower, ox.general-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)iQIcBAEBCAAGBQJRAhRJAAoJEP8WL8XPP7rRtKsP/jubezexP4PNc6VDQSYyYjD+ GUDQ6W84rFJu3UhoWUFBBBCHHyw9d/bBzzi/52kVmskrMw/KxP8pEe+fz70//4rq rlIlgCWvgvL3c32cHQrBlBIk1eiNKby9Go1j/u+kqa+bDE03XHyY6yy6pWKQ7wuO 1Kx0EPYSarLSE5Ds1q8vdoljdQgBBq/B/aZb579HHC5a4W635ZtkSid+RPNuSylO kxjdTdHoJzw73FW9XUCd1DjBSXMhSp5QCbDfYeCtbhpIv8mis6mgWrcp1+EwgSWL Viv52Xm4OIjwXK8r3uKmf2jHp5PO4iJmmYP3cER0RPIHORKjGM1nszqtgSBj1gK/ upgWPD/D1COu0r1Bgs9wlNXq/MESzwQaAqp+I1wpZUSlGWV7kuymIros3/Y16wgV n1glHfh4SlAlORo0VzhDv5r4hwZAjgWtlhT2pFR3ikWml5TzJkX1QHAlQxoeNL7Z fUiWJ4lgdfi6uqrMGzJ4OGYuMBWOGhUMaxkBwS4vRgEen5BB3/dLxaYKJ8Be1d6U K49dAYnf2dccpKRerECkTYCkrMS+FJPgYIamfHFVwd4me1zj9WyXwUbaH/ROYHF6 TAkxl4s5W8lj7M15v76qA1o+yhY2cDoTJq+kPD+0HksDaxRqjoWMmhEUdHJtx8K5 zdtgmocVpi54pwhPBxn8
=wf4i
-----END PGP SIGNATURE-----_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Add testpci command (v4)
Quoted 2 lines · Andrey Borzenkov
Does not "testpci" without parameters always return TRUE?
No, it tests for presence of PCI bus. This is actually a potentially useful functionality.
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
Re: [PATCH] Add testpci command (v4)
Quoted 7 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 2 lines · Andrey Borzenkov
Does not "testpci" without parameters always return TRUE?
No, it tests for presence of PCI bus. This is actually a potentially useful functionality.
Ah, OK. In this case documentation should mention it. Otherwise it is not clear what it does.
Re: [PATCH] Add testpci command (v5)
In reply to Jonathan McDowellQuoted 7 lines · Jonathan McDowell
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko
On 13.10.2012 00:18, Jonathan McDowell wrote:
(some code)
Quoted 2 lines
ARG_TYPE_INTmeans that the argument is decimal integer but it's not the case here.
Quoted 8 lines
Quoted 5 lines
Please follow indent style.
You need to check
grub_errnoto catch any error in number parsing. Also you need to update a little bit because of recent callback changes. Other than this the patch is fine.Ok, I've updated against latest bzr and made the changes as above. Here's v4.
And adding the mention of returning true if any PCI device is found when no options specified, here's v5.
-----
=== modified file 'docs/grub.texi'
grub.texi +13 -0
@@ -3366,6 +3366,7 @@ * sendkey:: Emulate keystrokes * set:: Set an environment variable * source:: Read a configuration file in same context +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size
@@ -4147,6 +4148,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. If no options are specified returns true if any PCI device is found. +@end deffn + + @node true @subsection true
=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6 -0
@@ -862,6 +862,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };
=== added file 'grub-core/commands/testpci.c'
commands/testpci.c +128 -0
128 lines not on this page · raw message
-----
J.
Signature · 3 lines
Web [ The less time planning, the more time programming. ] site: http:// [ ] Made by www.earth.li/~noodles/ [ ] HuggieTag 0.0.24
Re: [PATCH] Add testpci command (v4)
In reply to Jonathan McDowellQuoted 98 lines · Jonathan McDowell
Quoted 1 line · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 1 line · Jonathan McDowell
(some code)
Quoted 6 lines
Quoted 6 lines
ARG_TYPE_INTmeans that the argument is decimal integer but it's not the case here.
Please follow indent style.
You need to checkgrub_errnoto catch any error in number parsing. Also you need to update a little bit because of recent callback changes. Other than this the patch is fine.Ok, I've updated against latest bzr and made the changes as above. Here's v4.
Signed-off-by Jonathan McDowell <[email protected]>-----
=== modified file 'docs/grub.texi'grub.texi +13
-0@@ -3366,6 +3366,7 @@ * sendkey:: Emulate keystrokes * set:: Set an environment variable * source:: Read a configuration file in same context +* testpci:: Test for the existence of a PCI device * true:: Do nothing, successfully * unset:: Unset an environment variable * uppermem:: Set the upper memory size@@ -4147,6 +4148,18 @@ @end deffn +@node testpci +@subsection testpci + +@deffn Command testpci [@option{--vendor=VENDORID}] @ + [@option{--product=PRODUCTID}] [@option{--subvendor VENDORID}] @ + [@option{--subproduct PRODUCTID}] +Test for the existence of a given PCI device. Returns true if a device +matching the specified vendor/product and/or subvendor/subproduct information +is found. +@end deffn + + @node true @subsection true=== modified file 'grub-core/Makefile.core.def'
Makefile.core.def +6
-0@@ -862,6 +862,12 @@ }; module = { + name = testpci; + common = commands/testpci.c; + enable = pci; +}; + +module = { name = true; common = commands/true.c; };=== added file '
grub-core/commands/testpci.c'commands/testpci.c +30
-0@@ -0,0 +1,128 @@ +/* testpci.c - Test for PCI device existence. */ +/* +* GRUB -- GRand Unified Bootloader +* Copyright (C) 2013 Free Software Foundation, Inc. +* +* GRUB is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 3 of the License, or +* (at your option) any later version. +* +* GRUB is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with GRUB. If not, see <http://www.gnu.org/licenses/>. +*/ + +#include <grub/pci.h> +#include <grub/dl.h> +#include <grub/extcmd.h> +#include <grub/misc.h> +#include <grub/i18n.h> + +GRUB_MOD_LICENSE ("GPLv3+" ); + +static const struct grub_arg_option options[] = + { + {"vendor" ,'v' ,0 ,N_ ("PCI vendor ID to look for" ),0 ,ARG_TYPE_STRING },
commands/testpci.c line 40
Quoted 10 lines
commands/testpci.c +10
-0+ {"product", 'p' , 0, N_("PCI product ID to look for"), 0, ARG_TYPE_STRING}, + {"subvendor", 's' , 0, N_("PCI subsystem vendor ID to look for"), 0, ARG_TYPE_STRING}, + {"subproduct", 't' , 0, N_("PCI subsystem product ID to look for"), 0, ARG_TYPE_STRING}, + {0, 0, 0, 0, 0, 0} + }; + +static grub_uint32_t pcimask, pcisubmask; +static grub_pci_id_t pcisearch; +static grub_pci_id_t pcisubsearch; +
This should be inside a structure passed through the iterator, not a static variable.
Quoted 29 lines
commands/testpci.c +29
-0+static int +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)), +grub_pci_id_t pciid, +void *data) +{ +grub_pci_address_t addr; +grub_uint32_t subsystem; +int *found = data; + +if ((pciid & pcimask) != pcisearch) { +return 0 ; + } + +if (pcisubmask !=0 ) { + addr =grub_pci_make_address (dev,GRUB_PCI_REG_SUBVENDOR ); + subsystem =grub_pci_read (addr); +if ((subsystem & pcisubmask) != pcisubsearch) { +return 0 ; + } + } + + *found =1 ; +return 1 ; +} + +static grub_err_t +grub_cmd_testpci (grub_extcmd_context_t ctxt, +int argc __attribute__ ((unused)), +char **args __attribute__ ((unused)))
commands/testpci.c line 79
Quoted 10 lines
commands/testpci.c +10
-0+{ +struct grub_arg_list *state = ctxt->state; +int found =0 ; + + pcimask = pcisubmask = pcisearch = pcisubsearch =0 ; + +if (state[0 ].set) { + pcisearch |=grub_strtoull (state[0 ].arg,0 ,16 ); +if (grub_errno !=GRUB_ERR_NONE ) { +return grub_error (GRUB_ERR_BAD_ARGUMENT ,N_ ("Couldn't parse PCI vendor ID" ));
Lowercase c
commands/testpci.c line 87
Quoted 8 lines
commands/testpci.c +8
-0+ } + pcimask |=0xFFFF ; + } + +if (state[1 ].set) { + pcisearch |= (grub_strtoull (state[1 ].arg,0 ,16 ) <<16 ); +if (grub_errno !=GRUB_ERR_NONE ) { +return grub_error (GRUB_ERR_BAD_ARGUMENT ,N_ ("Couldn't parse PCI product ID" ));
ditto
commands/testpci.c line 95
Quoted 8 lines
commands/testpci.c +8
-0+ } + pcimask |=0xFFFF0000 ; + } + +if (state[2 ].set) { + pcisubsearch |=grub_strtoull (state[2 ].arg,0 ,16 ); +if (grub_errno !=GRUB_ERR_NONE ) { +return grub_error (GRUB_ERR_BAD_ARGUMENT ,N_ ("Couldn't parse PCI subsystem vendor ID" ));
ditto
commands/testpci.c line 103
Quoted 8 lines
commands/testpci.c +8
-0+ } + pcisubmask |=0xFFFF ; + } + +if (state[3 ].set) { + pcisubsearch |= (grub_strtoull (state[3 ].arg,0 ,16 ) <<16 ); +if (grub_errno !=GRUB_ERR_NONE ) { +return grub_error (GRUB_ERR_BAD_ARGUMENT ,N_ ("Couldn't parse PCI subsystem product ID" ));
ditto.
Quoted 40 lines
commands/testpci.c +25
-1+ } + pcisubmask |=0xFFFF0000 ; + } + +grub_pci_iterate (grub_testpci_iter, &found); + +return found ?GRUB_ERR_NONE :grub_error (GRUB_ERR_TEST_FAILURE ,"false" ); +} + +static grub_extcmd_t cmd; + +GRUB_MOD_INIT (testpci) +{ + cmd =grub_register_extcmd ("testpci" , grub_cmd_testpci,0 , +N_ ("[--vendor=VENDORID] [--product=PRODUCTID] " +"[--subvendor=VENDORID] " +"[--subproduct=PRODUCTID]" ), +N_ ("Test for PCI device existence." ), + options); +} + +GRUB_MOD_FINI (testpci) +{ +grub_unregister_extcmd (cmd); +} -----J.
-- "Basically, if you're allowed to own it, you're probably allowed to burn it." -- Stephen Gower, ox.general
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel