Skip to contentSkip to conversation

Add testpci command

PATCH In review Jonathan McDowell 4 participants 16 messages Sep 28, 2012 – Apr 5, 2013 Kbuild

lore
Share & tools
lore
#0 lore raw

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?
#1 lore raw

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"
| 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 -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
#2 lore raw

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"
| 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 +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 for 

required");

commands/testpci.c +3 -0
+
+  if ((grub_strlen(args[0]) != 9) || args[0][4] != ':')
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Device ID must be 

xxxx: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

#3 lore raw
In reply to Jonathan McDowell

В Чт., 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

#4 lore raw
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
#5 lore raw
In reply to Jonathan McDowell
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
#6 lore raw

Re: [PATCH] Add testpci command (v2)

In reply to 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"
| fi

in 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
#7 lore raw

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"
| fi

in 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"
| fi

which 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 :(

#8 lore raw

Re: [PATCH] Add testpci command (v3)

In reply to Jonathan McDowell
Quoted 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"
| fi

in 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"
| fi

which 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.
#9 lore raw

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
#10 lore raw

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_INT means that the argument is decimal integer but it's not the case here.

Please follow indent style.

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.

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
#11 lore raw

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_INT means that the argument is decimal integer but it's not the case here.

Please follow indent style.

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.

Ok, I've updated against latest bzr and made the changes as above. Here's v4.

-----
=== 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

#12 lore raw

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
#13 lore raw

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.

#14 lore raw

Re: [PATCH] Add testpci command (v5)

In reply to Jonathan McDowell
Quoted 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_INT means 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_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.

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
#15 lore raw

Re: [PATCH] Add testpci command (v4)

In reply to Jonathan McDowell
Quoted 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_INT means that the argument is decimal integer but it's not the case here.
Please follow indent style.
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.

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 +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