All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add fwconfig command
@ 2017-01-23 23:43 Matthew Garrett
  2017-01-24 14:52 ` Konrad Rzeszutek Wilk
  2017-01-25 18:05 ` Andrei Borzenkov
  0 siblings, 2 replies; 10+ messages in thread
From: Matthew Garrett @ 2017-01-23 23:43 UTC (permalink / raw)
  To: grub-devel; +Cc: Matthew Garrett

Add a command to read values from the qemu fwcfg store. This allows data
to be passed from the qemu command line to grub.

Example use:

echo '(hd0,1)' >rootdev
qemu -fw_cfg opt/rootdev,file=rootdev

fwconfig opt/rootdev root
---
 docs/grub.texi                |   6 +++
 grub-core/Makefile.core.def   |   6 +++
 grub-core/commands/fwconfig.c | 121 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 grub-core/commands/fwconfig.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 4469638..4f8a378 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3818,6 +3818,7 @@ you forget a command, you can run the command @command{help}
 * eval::                        Evaluate agruments as GRUB commands
 * export::                      Export an environment variable
 * false::                       Do nothing, unsuccessfully
+* fwconfig::                    Retrieves a value from the qemu fwcfg store
 * getenv::                      Retrieve an EFI firmware variable
 * gettext::                     Translate a string
 * gptsync::                     Fill an MBR based on GPT entries
@@ -4259,6 +4260,11 @@ Do nothing, unsuccessfully.  This is mainly useful in control constructs
 such as @code{if} and @code{while} (@pxref{Shell-like scripting}).
 @end deffn
 
+@node fwconfig
+@subsection fwconig
+@deffn Command fwconfig fwpath envvar
+Retrieves @var{fwpath} from the qemu fwcfg store and stores it in @var{envvar}
+
 @node getenv
 @subsection getenv
 
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index db77a7f..f6b6f38 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2362,3 +2362,9 @@ module = {
   common = loader/i386/xen_file64.c;
   extra_dist = loader/i386/xen_fileXX.c;
 };
+
+module = {
+  name = fwconfig;
+  common = commands/fwconfig.c;
+  enable = x86;
+};
diff --git a/grub-core/commands/fwconfig.c b/grub-core/commands/fwconfig.c
new file mode 100644
index 0000000..289d167
--- /dev/null
+++ b/grub-core/commands/fwconfig.c
@@ -0,0 +1,121 @@
+/* fwconfig.c - command to read config from qemu fwconfig  */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015  CoreOS, 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/dl.h>
+#include <grub/misc.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/cpu/io.h>
+#include <grub/i18n.h>
+#include <grub/mm.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define SELECTOR 0x510
+#define DATA 0x511
+
+#define SIGNATURE_INDEX 0x00
+#define DIRECTORY_INDEX 0x19
+
+static grub_extcmd_t cmd_read_fwconfig;
+
+struct grub_qemu_fwcfgfile {
+  grub_uint32_t size;
+  grub_uint16_t select;
+  grub_uint16_t reserved;
+  char name[56];
+};
+
+static const struct grub_arg_option options[] =
+  {
+    {0, 'v', 0, N_("Save read value into variable VARNAME."),
+     N_("VARNAME"), ARG_TYPE_STRING},
+    {0, 0, 0, 0, 0, 0}
+  };
+
+static grub_err_t
+grub_cmd_fwconfig (grub_extcmd_context_t ctxt, int argc, char **argv)
+{
+  grub_uint32_t i, j, value = 0;
+  struct grub_qemu_fwcfgfile file;
+  char fwsig[4], signature[4] = { 'Q', 'E', 'M', 'U' };
+
+  if (argc != 2)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  /* Verify that we have meaningful hardware here */
+  grub_outw(SIGNATURE_INDEX, SELECTOR);
+  for (i=0; i<sizeof(fwsig); i++)
+      fwsig[i] = grub_inb(DATA);
+
+  if (grub_memcmp(fwsig, signature, sizeof(signature)) != 0)
+    return grub_error (GRUB_ERR_BAD_DEVICE, N_("invalid fwconfig hardware signature: got 0x%x%x%x%x"), fwsig[0], fwsig[1], fwsig[2], fwsig[3]);
+
+  /* Find out how many file entries we have */
+  grub_outw(DIRECTORY_INDEX, SELECTOR);
+  value = grub_inb(DATA) | grub_inb(DATA) << 8 | grub_inb(DATA) << 16 | grub_inb(DATA) << 24;
+  value = grub_be_to_cpu32(value);
+  /* Read the file description for each file */
+  for (i=0; i<value; i++)
+    {
+      for (j=0; j<sizeof(file); j++)
+	{
+	  ((char *)&file)[j] = grub_inb(DATA);
+	}
+      /* Check whether it matches what we're looking for, and if so read the file */
+      if (grub_strncmp(file.name, argv[0], sizeof(file.name)) == 0)
+	{
+	  grub_uint32_t filesize = grub_be_to_cpu32(file.size);
+	  grub_uint16_t location = grub_be_to_cpu16(file.select);
+	  char *data = grub_malloc(filesize+1);
+
+	  if (!data)
+	      return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("can't allocate buffer for data"));
+
+	  grub_outw(location, SELECTOR);
+	  for (j=0; j<filesize; j++)
+	    {
+	      data[j] = grub_inb(DATA);
+	    }
+
+	  data[filesize] = '\0';
+
+	  grub_env_set (argv[1], data);
+
+	  grub_free(data);
+	  return 0;
+	}
+    }
+
+  return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't find entry %s"), argv[0]);
+}
+
+GRUB_MOD_INIT(fwconfig)
+{
+  cmd_read_fwconfig =
+    grub_register_extcmd ("fwconfig", grub_cmd_fwconfig, 0,
+			  N_("PATH VAR"),
+			  N_("Set VAR to the contents of fwconfig PATH"),
+			  options);
+}
+
+GRUB_MOD_FINI(fwconfig)
+{
+  grub_unregister_extcmd (cmd_read_fwconfig);
+}
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-23 23:43 [PATCH] Add fwconfig command Matthew Garrett
@ 2017-01-24 14:52 ` Konrad Rzeszutek Wilk
  2017-01-24 16:36   ` Colin Watson
  2017-01-25 18:05 ` Andrei Borzenkov
  1 sibling, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-24 14:52 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Matthew Garrett

On Mon, Jan 23, 2017 at 03:43:32PM -0800, Matthew Garrett wrote:
> Add a command to read values from the qemu fwcfg store. This allows data
> to be passed from the qemu command line to grub.
> 
> Example use:
> 
> echo '(hd0,1)' >rootdev
> qemu -fw_cfg opt/rootdev,file=rootdev
> 
> fwconfig opt/rootdev root
> ---
>  docs/grub.texi                |   6 +++
>  grub-core/Makefile.core.def   |   6 +++
>  grub-core/commands/fwconfig.c | 121 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 grub-core/commands/fwconfig.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 4469638..4f8a378 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3818,6 +3818,7 @@ you forget a command, you can run the command @command{help}
>  * eval::                        Evaluate agruments as GRUB commands
>  * export::                      Export an environment variable
>  * false::                       Do nothing, unsuccessfully
> +* fwconfig::                    Retrieves a value from the qemu fwcfg store
>  * getenv::                      Retrieve an EFI firmware variable
>  * gettext::                     Translate a string
>  * gptsync::                     Fill an MBR based on GPT entries
> @@ -4259,6 +4260,11 @@ Do nothing, unsuccessfully.  This is mainly useful in control constructs
>  such as @code{if} and @code{while} (@pxref{Shell-like scripting}).
>  @end deffn
>  
> +@node fwconfig
> +@subsection fwconig
> +@deffn Command fwconfig fwpath envvar
> +Retrieves @var{fwpath} from the qemu fwcfg store and stores it in @var{envvar}
> +
>  @node getenv
>  @subsection getenv
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index db77a7f..f6b6f38 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2362,3 +2362,9 @@ module = {
>    common = loader/i386/xen_file64.c;
>    extra_dist = loader/i386/xen_fileXX.c;
>  };
> +
> +module = {
> +  name = fwconfig;
> +  common = commands/fwconfig.c;
> +  enable = x86;
> +};
> diff --git a/grub-core/commands/fwconfig.c b/grub-core/commands/fwconfig.c
> new file mode 100644
> index 0000000..289d167
> --- /dev/null
> +++ b/grub-core/commands/fwconfig.c
> @@ -0,0 +1,121 @@
> +/* fwconfig.c - command to read config from qemu fwconfig  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015  CoreOS, Inc.


Hmm, 

See https://www.gnu.org/licenses/why-assign.html


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

So what is your option here (see the 'at your option'). 

> + *
> + *  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/>.
> + */


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24 14:52 ` Konrad Rzeszutek Wilk
@ 2017-01-24 16:36   ` Colin Watson
  2017-01-24 16:40     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Colin Watson @ 2017-01-24 16:36 UTC (permalink / raw)
  To: grub-devel

On Tue, Jan 24, 2017 at 09:52:35AM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 23, 2017 at 03:43:32PM -0800, Matthew Garrett wrote:
> > + *  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.
> 
> So what is your option here (see the 'at your option'). 

This language indicates that it's at the option of the person
redistributing or modifying it whether they do so under the terms of
version 3 or of some later version.  Matthew is not required to pick
one.

Also:

  <cjwatson@niejwein ~/src/gnu/grub2/git/grub (master=)>$ git grep 'at your option' | wc -l
  1412

-- 
Colin Watson                                       [cjwatson@ubuntu.com]


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24 16:36   ` Colin Watson
@ 2017-01-24 16:40     ` Konrad Rzeszutek Wilk
  2017-01-24 17:36       ` Colin Watson
  0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-01-24 16:40 UTC (permalink / raw)
  To: The development of GNU GRUB

On Tue, Jan 24, 2017 at 04:36:03PM +0000, Colin Watson wrote:
> On Tue, Jan 24, 2017 at 09:52:35AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Mon, Jan 23, 2017 at 03:43:32PM -0800, Matthew Garrett wrote:
> > > + *  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.
> > 
> > So what is your option here (see the 'at your option'). 
> 
> This language indicates that it's at the option of the person
> redistributing or modifying it whether they do so under the terms of
> version 3 or of some later version.  Matthew is not required to pick
> one.

I mean I am reading this and it sounds like that
too but then this is an legal aggreement and those seem to
operate on some weird rules.

Does FSF have an unqualified answer to this?

> 
> Also:
> 
>   <cjwatson@niejwein ~/src/gnu/grub2/git/grub (master=)>$ git grep 'at your option' | wc -l
>   1412

Which could mean that nobody reads that stuff and just copies
and pastes.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24 16:40     ` Konrad Rzeszutek Wilk
@ 2017-01-24 17:36       ` Colin Watson
  2017-01-24 18:18         ` Thomas Schmitt
  0 siblings, 1 reply; 10+ messages in thread
From: Colin Watson @ 2017-01-24 17:36 UTC (permalink / raw)
  To: grub-devel

On Tue, Jan 24, 2017 at 11:40:57AM -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 24, 2017 at 04:36:03PM +0000, Colin Watson wrote:
> > This language indicates that it's at the option of the person
> > redistributing or modifying it whether they do so under the terms of
> > version 3 or of some later version.  Matthew is not required to pick
> > one.
> 
> I mean I am reading this and it sounds like that
> too but then this is an legal aggreement and those seem to
> operate on some weird rules.
> 
> Does FSF have an unqualified answer to this?

The text Matthew uses is straight from
https://www.gnu.org/licenses/gpl.html#howto, which seems pretty
unqualified to me.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24 17:36       ` Colin Watson
@ 2017-01-24 18:18         ` Thomas Schmitt
  2017-01-24 18:48           ` Lennart Sorensen
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Schmitt @ 2017-01-24 18:18 UTC (permalink / raw)
  To: grub-devel

Hi,

Colin Watson wrote:
> https://www.gnu.org/licenses/gpl.html#howto

One should emphasize that the FSF when designing new license versions only
considers compatibility to older FSF licenses if they bear the "or later"
clause.

E.g. it is not possible to combine GPL version 2 software with LGPL version 3
software if the GPL does not offer the opportunity to get upgraded to GPL
version 3. That's because LGPL 3 forbids some patent evilness whereas GPL 2
forbids to add forbiddances.
  https://www.gnu.org/licenses/license-list.en.html#LGPLv3

According to Richard Stallman, the design flaw is not in LGPL 3 but in issueing
GPL 2 without "or later" clause. (Company lawyers may well contradict.)


Have a nice day :)

Thomas



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24 18:18         ` Thomas Schmitt
@ 2017-01-24 18:48           ` Lennart Sorensen
  0 siblings, 0 replies; 10+ messages in thread
From: Lennart Sorensen @ 2017-01-24 18:48 UTC (permalink / raw)
  To: The development of GNU GRUB

On Tue, Jan 24, 2017 at 07:18:13PM +0100, Thomas Schmitt wrote:
> One should emphasize that the FSF when designing new license versions only
> considers compatibility to older FSF licenses if they bear the "or later"
> clause.
> 
> E.g. it is not possible to combine GPL version 2 software with LGPL version 3
> software if the GPL does not offer the opportunity to get upgraded to GPL
> version 3. That's because LGPL 3 forbids some patent evilness whereas GPL 2
> forbids to add forbiddances.
>   https://www.gnu.org/licenses/license-list.en.html#LGPLv3
> 
> According to Richard Stallman, the design flaw is not in LGPL 3 but in issueing
> GPL 2 without "or later" clause. (Company lawyers may well contradict.)

Yes the nice GPL feature where you can't add anything to it, unless you
are the FSF, then you can.

Some people don't like handing future control to someone else like that.

Most users of the GPL seem OK with it though.

-- 
Len Sorensen


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-23 23:43 [PATCH] Add fwconfig command Matthew Garrett
  2017-01-24 14:52 ` Konrad Rzeszutek Wilk
@ 2017-01-25 18:05 ` Andrei Borzenkov
  1 sibling, 0 replies; 10+ messages in thread
From: Andrei Borzenkov @ 2017-01-25 18:05 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Matthew Garrett

24.01.2017 02:43, Matthew Garrett пишет:
> Add a command to read values from the qemu fwcfg store. This allows data
> to be passed from the qemu command line to grub.
> 
> Example use:
> 
> echo '(hd0,1)' >rootdev
> qemu -fw_cfg opt/rootdev,file=rootdev
> 
> fwconfig opt/rootdev root

The name sounds way too generic. Unless we have plans to unify such
interface on multiple platforms, I would expect something like fw_cfg
(to match QEMU documentation) or even qemu_fw_cfg to make it pretty obvious.

> ---
>  docs/grub.texi                |   6 +++
>  grub-core/Makefile.core.def   |   6 +++
>  grub-core/commands/fwconfig.c | 121 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 grub-core/commands/fwconfig.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 4469638..4f8a378 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3818,6 +3818,7 @@ you forget a command, you can run the command @command{help}
>  * eval::                        Evaluate agruments as GRUB commands
>  * export::                      Export an environment variable
>  * false::                       Do nothing, unsuccessfully
> +* fwconfig::                    Retrieves a value from the qemu fwcfg store
>  * getenv::                      Retrieve an EFI firmware variable
>  * gettext::                     Translate a string
>  * gptsync::                     Fill an MBR based on GPT entries
> @@ -4259,6 +4260,11 @@ Do nothing, unsuccessfully.  This is mainly useful in control constructs
>  such as @code{if} and @code{while} (@pxref{Shell-like scripting}).
>  @end deffn
>  
> +@node fwconfig
> +@subsection fwconig
> +@deffn Command fwconfig fwpath envvar
> +Retrieves @var{fwpath} from the qemu fwcfg store and stores it in @var{envvar}
> +

Your patch adds "-v" option that is not documented here

>  @node getenv
>  @subsection getenv
>  
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index db77a7f..f6b6f38 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -2362,3 +2362,9 @@ module = {
>    common = loader/i386/xen_file64.c;
>    extra_dist = loader/i386/xen_fileXX.c;
>  };
> +
> +module = {
> +  name = fwconfig;
> +  common = commands/fwconfig.c;
> +  enable = x86;

QEMU supports fw_cfg at least on ARM (according to documentation), but I
guess this can be done later if needed.

> +};
> diff --git a/grub-core/commands/fwconfig.c b/grub-core/commands/fwconfig.c
> new file mode 100644
> index 0000000..289d167
> --- /dev/null
> +++ b/grub-core/commands/fwconfig.c
> @@ -0,0 +1,121 @@
> +/* fwconfig.c - command to read config from qemu fwconfig  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2015  CoreOS, Inc.
> + *

I agree with other comments that at least does not align with copyrights
of other sources.

> + *  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/dl.h>
> +#include <grub/misc.h>
> +#include <grub/extcmd.h>
> +#include <grub/env.h>
> +#include <grub/cpu/io.h>
> +#include <grub/i18n.h>
> +#include <grub/mm.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#define SELECTOR 0x510
> +#define DATA 0x511
> +
> +#define SIGNATURE_INDEX 0x00
> +#define DIRECTORY_INDEX 0x19
> +

QEMU also provdes MMIO interface, but again, can be added later. I
wonder if we could use ACPI interface to discover them here?

> +static grub_extcmd_t cmd_read_fwconfig;
> +
> +struct grub_qemu_fwcfgfile {
> +  grub_uint32_t size;
> +  grub_uint16_t select;
> +  grub_uint16_t reserved;
> +  char name[56];
> +};
> +
> +static const struct grub_arg_option options[] =
> +  {
> +    {0, 'v', 0, N_("Save read value into variable VARNAME."),
> +     N_("VARNAME"), ARG_TYPE_STRING},

This option is not used anywhere. Also long options, please!

> +    {0, 0, 0, 0, 0, 0}
> +  };
> +
> +static grub_err_t
> +grub_cmd_fwconfig (grub_extcmd_context_t ctxt, int argc, char **argv)
> +{
> +  grub_uint32_t i, j, value = 0;
> +  struct grub_qemu_fwcfgfile file;
> +  char fwsig[4], signature[4] = { 'Q', 'E', 'M', 'U' };
> +
> +  if (argc != 2)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
> +

I would really prefer using options over positional parameters to make
it more extensible. One obvious extension would be --list to see
available files and --test to be able to verify in script whether file
exists.

> +  /* Verify that we have meaningful hardware here */
> +  grub_outw(SIGNATURE_INDEX, SELECTOR);
> +  for (i=0; i<sizeof(fwsig); i++)
> +      fwsig[i] = grub_inb(DATA);
> +
> +  if (grub_memcmp(fwsig, signature, sizeof(signature)) != 0)
> +    return grub_error (GRUB_ERR_BAD_DEVICE, N_("invalid fwconfig hardware signature: got 0x%x%x%x%x"), fwsig[0], fwsig[1], fwsig[2], fwsig[3]);
> +
> +  /* Find out how many file entries we have */
> +  grub_outw(DIRECTORY_INDEX, SELECTOR);

QEMU documentation says that IOport selector register is little endian,
so this probably needs to be grub_cpu_to_le16_compile_time
(DIRECTORY_INDEX).

> +  value = grub_inb(DATA) | grub_inb(DATA) << 8 | grub_inb(DATA) << 16 | grub_inb(DATA) << 24;
> +  value = grub_be_to_cpu32(value);
> +  /* Read the file description for each file */
> +  for (i=0; i<value; i++)
> +    {
> +      for (j=0; j<sizeof(file); j++)
> +	{
> +	  ((char *)&file)[j] = grub_inb(DATA);
> +	}
> +      /* Check whether it matches what we're looking for, and if so read the file */
> +      if (grub_strncmp(file.name, argv[0], sizeof(file.name)) == 0)
> +	{
> +	  grub_uint32_t filesize = grub_be_to_cpu32(file.size);
> +	  grub_uint16_t location = grub_be_to_cpu16(file.select);
> +	  char *data = grub_malloc(filesize+1);
> +
> +	  if (!data)
> +	      return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("can't allocate buffer for data"));
> +
> +	  grub_outw(location, SELECTOR);

grub_cpu_to_le16 again.

> +	  for (j=0; j<filesize; j++)
> +	    {
> +	      data[j] = grub_inb(DATA);
> +	    }
> +
> +	  data[filesize] = '\0';
> +
> +	  grub_env_set (argv[1], data);
> +
> +	  grub_free(data);
> +	  return 0;
> +	}
> +    }
> +
> +  return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't find entry %s"), argv[0]);
> +}
> +
> +GRUB_MOD_INIT(fwconfig)
> +{
> +  cmd_read_fwconfig =
> +    grub_register_extcmd ("fwconfig", grub_cmd_fwconfig, 0,
> +			  N_("PATH VAR"),
> +			  N_("Set VAR to the contents of fwconfig PATH"),
> +			  options);
> +}
> +
> +GRUB_MOD_FINI(fwconfig)
> +{
> +  grub_unregister_extcmd (cmd_read_fwconfig);
> +}
> 




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add fwconfig command
  2017-01-24  0:32 Matthew Garrett
@ 2017-01-24 19:49 ` Daniel Kiper
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kiper @ 2017-01-24 19:49 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Matthew Garrett

On Mon, Jan 23, 2017 at 04:32:26PM -0800, Matthew Garrett wrote:
> Add a command to read values from the qemu fwcfg store. This allows data
> to be passed from the qemu command line to grub.
>
> Example use:
>
> echo '(hd0,1)' >rootdev
> qemu -fw_cfg opt/rootdev,file=rootdev
>
> fwconfig opt/rootdev root

I think that this should be made clear that fwconfig command should be
executed in GRUB not in shell. At least I understand it in that way.

[...]

> +static grub_err_t
> +grub_cmd_fwconfig (grub_extcmd_context_t ctxt, int argc, char **argv)
> +{
> +  grub_uint32_t i, j, value = 0;
> +  struct grub_qemu_fwcfgfile file;
> +  char fwsig[4], signature[4] = { 'Q', 'E', 'M', 'U' };
> +
> +  if (argc != 2)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
> +
> +  /* Verify that we have meaningful hardware here */
> +  grub_outw(SIGNATURE_INDEX, SELECTOR);
> +  for (i=0; i<sizeof(fwsig); i++)
> +      fwsig[i] = grub_inb(DATA);
> +
> +  if (grub_memcmp(fwsig, signature, sizeof(signature)) != 0)
> +    return grub_error (GRUB_ERR_BAD_DEVICE, N_("invalid fwconfig hardware signature: got 0x%x%x%x%x"), fwsig[0], fwsig[1], fwsig[2], fwsig[3]);

Too long line. Could you wrap it?

> +
> +  /* Find out how many file entries we have */
> +  grub_outw(DIRECTORY_INDEX, SELECTOR);
> +  value = grub_inb(DATA) | grub_inb(DATA) << 8 | grub_inb(DATA) << 16 | grub_inb(DATA) << 24;

Ditto.

> +  value = grub_be_to_cpu32(value);
> +  /* Read the file description for each file */
> +  for (i=0; i<value; i++)
> +    {
> +      for (j=0; j<sizeof(file); j++)
> +	{
> +	  ((char *)&file)[j] = grub_inb(DATA);
> +	}
> +      /* Check whether it matches what we're looking for, and if so read the file */

Ditto.

> +      if (grub_strncmp(file.name, argv[0], sizeof(file.name)) == 0)
> +	{
> +	  grub_uint32_t filesize = grub_be_to_cpu32(file.size);
> +	  grub_uint16_t location = grub_be_to_cpu16(file.select);
> +	  char *data = grub_malloc(filesize+1);
> +
> +	  if (!data)
> +	      return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("can't allocate buffer for data"));

Ditto.

> +
> +	  grub_outw(location, SELECTOR);
> +	  for (j=0; j<filesize; j++)
> +	    {
> +	      data[j] = grub_inb(DATA);
> +	    }
> +
> +	  data[filesize] = '\0';
> +
> +	  grub_env_set (argv[1], data);
> +
> +	  grub_free(data);
> +	  return 0;
> +	}
> +    }
> +
> +  return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't find entry %s"), argv[0]);

Ditto.

Additionally, could you be more in line with GRUB2 coding style?
I know that it is not strictly defined but if you look at a few
files you will catch what I mean.

Though in general LGTM but this is not 2.02 material.
I am adding this to my radar for next release.

Daniel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Add fwconfig command
@ 2017-01-24  0:32 Matthew Garrett
  2017-01-24 19:49 ` Daniel Kiper
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Garrett @ 2017-01-24  0:32 UTC (permalink / raw)
  To: grub-devel; +Cc: Matthew Garrett

Add a command to read values from the qemu fwcfg store. This allows data
to be passed from the qemu command line to grub.

Example use:

echo '(hd0,1)' >rootdev
qemu -fw_cfg opt/rootdev,file=rootdev

fwconfig opt/rootdev root
---
 docs/grub.texi                |   6 +++
 grub-core/Makefile.core.def   |   6 +++
 grub-core/commands/fwconfig.c | 121 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+)
 create mode 100644 grub-core/commands/fwconfig.c

diff --git a/docs/grub.texi b/docs/grub.texi
index 4469638..4f8a378 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3818,6 +3818,7 @@ you forget a command, you can run the command @command{help}
 * eval::                        Evaluate agruments as GRUB commands
 * export::                      Export an environment variable
 * false::                       Do nothing, unsuccessfully
+* fwconfig::                    Retrieves a value from the qemu fwcfg store
 * getenv::                      Retrieve an EFI firmware variable
 * gettext::                     Translate a string
 * gptsync::                     Fill an MBR based on GPT entries
@@ -4259,6 +4260,11 @@ Do nothing, unsuccessfully.  This is mainly useful in control constructs
 such as @code{if} and @code{while} (@pxref{Shell-like scripting}).
 @end deffn
 
+@node fwconfig
+@subsection fwconig
+@deffn Command fwconfig fwpath envvar
+Retrieves @var{fwpath} from the qemu fwcfg store and stores it in @var{envvar}
+
 @node getenv
 @subsection getenv
 
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index db77a7f..f6b6f38 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2362,3 +2362,9 @@ module = {
   common = loader/i386/xen_file64.c;
   extra_dist = loader/i386/xen_fileXX.c;
 };
+
+module = {
+  name = fwconfig;
+  common = commands/fwconfig.c;
+  enable = x86;
+};
diff --git a/grub-core/commands/fwconfig.c b/grub-core/commands/fwconfig.c
new file mode 100644
index 0000000..289d167
--- /dev/null
+++ b/grub-core/commands/fwconfig.c
@@ -0,0 +1,121 @@
+/* fwconfig.c - command to read config from qemu fwconfig  */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2015  CoreOS, 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/dl.h>
+#include <grub/misc.h>
+#include <grub/extcmd.h>
+#include <grub/env.h>
+#include <grub/cpu/io.h>
+#include <grub/i18n.h>
+#include <grub/mm.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define SELECTOR 0x510
+#define DATA 0x511
+
+#define SIGNATURE_INDEX 0x00
+#define DIRECTORY_INDEX 0x19
+
+static grub_extcmd_t cmd_read_fwconfig;
+
+struct grub_qemu_fwcfgfile {
+  grub_uint32_t size;
+  grub_uint16_t select;
+  grub_uint16_t reserved;
+  char name[56];
+};
+
+static const struct grub_arg_option options[] =
+  {
+    {0, 'v', 0, N_("Save read value into variable VARNAME."),
+     N_("VARNAME"), ARG_TYPE_STRING},
+    {0, 0, 0, 0, 0, 0}
+  };
+
+static grub_err_t
+grub_cmd_fwconfig (grub_extcmd_context_t ctxt, int argc, char **argv)
+{
+  grub_uint32_t i, j, value = 0;
+  struct grub_qemu_fwcfgfile file;
+  char fwsig[4], signature[4] = { 'Q', 'E', 'M', 'U' };
+
+  if (argc != 2)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
+
+  /* Verify that we have meaningful hardware here */
+  grub_outw(SIGNATURE_INDEX, SELECTOR);
+  for (i=0; i<sizeof(fwsig); i++)
+      fwsig[i] = grub_inb(DATA);
+
+  if (grub_memcmp(fwsig, signature, sizeof(signature)) != 0)
+    return grub_error (GRUB_ERR_BAD_DEVICE, N_("invalid fwconfig hardware signature: got 0x%x%x%x%x"), fwsig[0], fwsig[1], fwsig[2], fwsig[3]);
+
+  /* Find out how many file entries we have */
+  grub_outw(DIRECTORY_INDEX, SELECTOR);
+  value = grub_inb(DATA) | grub_inb(DATA) << 8 | grub_inb(DATA) << 16 | grub_inb(DATA) << 24;
+  value = grub_be_to_cpu32(value);
+  /* Read the file description for each file */
+  for (i=0; i<value; i++)
+    {
+      for (j=0; j<sizeof(file); j++)
+	{
+	  ((char *)&file)[j] = grub_inb(DATA);
+	}
+      /* Check whether it matches what we're looking for, and if so read the file */
+      if (grub_strncmp(file.name, argv[0], sizeof(file.name)) == 0)
+	{
+	  grub_uint32_t filesize = grub_be_to_cpu32(file.size);
+	  grub_uint16_t location = grub_be_to_cpu16(file.select);
+	  char *data = grub_malloc(filesize+1);
+
+	  if (!data)
+	      return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("can't allocate buffer for data"));
+
+	  grub_outw(location, SELECTOR);
+	  for (j=0; j<filesize; j++)
+	    {
+	      data[j] = grub_inb(DATA);
+	    }
+
+	  data[filesize] = '\0';
+
+	  grub_env_set (argv[1], data);
+
+	  grub_free(data);
+	  return 0;
+	}
+    }
+
+  return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't find entry %s"), argv[0]);
+}
+
+GRUB_MOD_INIT(fwconfig)
+{
+  cmd_read_fwconfig =
+    grub_register_extcmd ("fwconfig", grub_cmd_fwconfig, 0,
+			  N_("PATH VAR"),
+			  N_("Set VAR to the contents of fwconfig PATH"),
+			  options);
+}
+
+GRUB_MOD_FINI(fwconfig)
+{
+  grub_unregister_extcmd (cmd_read_fwconfig);
+}
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-01-25 18:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 23:43 [PATCH] Add fwconfig command Matthew Garrett
2017-01-24 14:52 ` Konrad Rzeszutek Wilk
2017-01-24 16:36   ` Colin Watson
2017-01-24 16:40     ` Konrad Rzeszutek Wilk
2017-01-24 17:36       ` Colin Watson
2017-01-24 18:18         ` Thomas Schmitt
2017-01-24 18:48           ` Lennart Sorensen
2017-01-25 18:05 ` Andrei Borzenkov
2017-01-24  0:32 Matthew Garrett
2017-01-24 19:49 ` Daniel Kiper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.