[PATCH] Fix CPUID documentation and return values

[PATCH] Fix CPUID documentation and return values

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

Hello. CPUID command currently doesn't return a real grub_err_t and
errors aren't set which doesn't fit in grub2's design.
Addirionally "help cpuid" doesn't give really useful info. So for
future expandability and less confusion I propose the attached patch
partially based on original patch by Robert Millan

-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

[-- Attachment #2: cpuid.diff --]
[-- Type: text/plain, Size: 2061 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 752bde8..ab758eb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2009-07-27  Vladimir Serbinenko  <phcoder@gmail.com>
+2009-07-27  Robert Millan  <rmh.grub@aybabtu.com>
+
+	Fix cpuid command.
+
+	* commands/i386/cpuid.c (options): New variable.
+	(grub_cmd_cpuid): Return real error.
+	(GRUB_MOD_INIT(cpuid)): Declare options.
+
 2009-07-25  Felix Zielcke  <fzielcke@z-51.de>
 
 	* kern/file.c (grub_file_open): Revert to previous check with
diff --git a/commands/i386/cpuid.c b/commands/i386/cpuid.c
index b80b14c..71f33ef 100644
--- a/commands/i386/cpuid.c
+++ b/commands/i386/cpuid.c
@@ -23,25 +23,33 @@
 #include <grub/mm.h>
 #include <grub/env.h>
 #include <grub/command.h>
+#include <grub/extcmd.h>
 
 #define cpuid(num,a,b,c,d) \
   asm volatile ("xchgl %%ebx, %1; cpuid; xchgl %%ebx, %1" \
 		: "=a" (a), "=r" (b), "=c" (c), "=d" (d)  \
 		: "0" (num))
 
+static const struct grub_arg_option options[] =
+  {
+    {"long-mode", 'l', 0, "check for long mode flag (default)", 0, 0},
+    {0, 0, 0, 0, 0, 0}
+  };
+
 #define bit_LM (1 << 29)
 
 static unsigned char has_longmode = 0;
 
 static grub_err_t
-grub_cmd_cpuid (struct grub_command *cmd __attribute__ ((unused)),
-	       int argc __attribute__ ((unused)),
-	       char **args __attribute__ ((unused)))
+grub_cmd_cpuid (grub_extcmd_t cmd __attribute__ ((unused)),
+		int argc __attribute__ ((unused)),
+		char **args __attribute__ ((unused)))
 {
-  return !has_longmode;
+  return has_longmode ? GRUB_ERR_NONE
+    : grub_error (GRUB_ERR_TEST_FAILURE, "false");
 }
 
-static grub_command_t cmd;
+static grub_extcmd_t cmd;
 
 GRUB_MOD_INIT(cpuid)
 {
@@ -78,11 +86,11 @@ GRUB_MOD_INIT(cpuid)
 done:
 #endif
 
-  cmd = grub_register_command ("cpuid", grub_cmd_cpuid,
-			       0, "Check for CPU features");
+  cmd = grub_register_extcmd ("cpuid", grub_cmd_cpuid, GRUB_COMMAND_FLAG_BOTH,
+			      "cpuid [-l]", "Check for CPU features", options);
 }
 
 GRUB_MOD_FINI(cpuid)
 {
-  grub_unregister_command (cmd);
+  grub_unregister_extcmd (cmd);
 }

Re: [PATCH] Fix CPUID documentation and return values

[Robert Millan]

On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Hello. CPUID command currently doesn't return a real grub_err_t and
> errors aren't set which doesn't fit in grub2's design.
> Addirionally "help cpuid" doesn't give really useful info. So for
> future expandability and less confusion I propose the attached patch
> partially based on original patch by Robert Millan

Hi,

I don't remember about this patch.  I assume I wrote it in 2007 when
merging cpuid support for first time.  Any idea why it wasn't it committed
back then?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] Fix CPUID documentation and return values

[Vladimir 'phcoder' Serbinenko]

On Tue, Jul 28, 2009 at 8:15 PM, Robert Millan<rmh@aybabtu.com> wrote:
> On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> Hello. CPUID command currently doesn't return a real grub_err_t and
>> errors aren't set which doesn't fit in grub2's design.
>> Addirionally "help cpuid" doesn't give really useful info. So for
>> future expandability and less confusion I propose the attached patch
>> partially based on original patch by Robert Millan
>
> Hi,
>
> I don't remember about this patch.  I assume I wrote it in 2007 when
> merging cpuid support for first time.
Yes, it is.
>  Any idea why it wasn't it committed
> back then?
>
None. Yoshinori K Okuji approved it here:
http://lists.gnu.org/archive/html/grub-devel/2007-05/msg00148.html
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] Fix CPUID documentation and return values

[Robert Millan]

On Tue, Jul 28, 2009 at 11:05:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> On Tue, Jul 28, 2009 at 8:15 PM, Robert Millan<rmh@aybabtu.com> wrote:
> > On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> >> Hello. CPUID command currently doesn't return a real grub_err_t and
> >> errors aren't set which doesn't fit in grub2's design.
> >> Addirionally "help cpuid" doesn't give really useful info. So for
> >> future expandability and less confusion I propose the attached patch
> >> partially based on original patch by Robert Millan
> >
> > Hi,
> >
> > I don't remember about this patch.  I assume I wrote it in 2007 when
> > merging cpuid support for first time.
> Yes, it is.
> >  Any idea why it wasn't it committed
> > back then?
> >
> None. Yoshinori K Okuji approved it here:
> http://lists.gnu.org/archive/html/grub-devel/2007-05/msg00148.html

I vaguely recall having disabled it because there was some issue with
scripting.  Assuming it's working properly now, I have no objection with
it.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] Fix CPUID documentation and return values

[Felix Zielcke]

Am Freitag, den 31.07.2009, 18:07 +0200 schrieb Robert Millan:
> On Tue, Jul 28, 2009 at 11:05:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > On Tue, Jul 28, 2009 at 8:15 PM, Robert Millan<rmh@aybabtu.com> wrote:
> > > On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > >> Hello. CPUID command currently doesn't return a real grub_err_t and
> > >> errors aren't set which doesn't fit in grub2's design.
> > >> Addirionally "help cpuid" doesn't give really useful info. So for
> > >> future expandability and less confusion I propose the attached patch
> > >> partially based on original patch by Robert Millan
> > >
> > > Hi,
> > >
> > > I don't remember about this patch.  I assume I wrote it in 2007 when
> > > merging cpuid support for first time.
> > Yes, it is.
> > >  Any idea why it wasn't it committed
> > > back then?
> > >
> > None. Yoshinori K Okuji approved it here:
> > http://lists.gnu.org/archive/html/grub-devel/2007-05/msg00148.html
> 
> I vaguely recall having disabled it because there was some issue with
> scripting.  Assuming it's working properly now, I have no objection with
> it.
> 

There was one.
if [ 0=1 ] ; then echo t ; else echo f ; fi
did print t but then also `error: false'
This is now fixed.

-- 
Felix Zielcke
Proud Debian Maintainer

Re: [PATCH] Fix CPUID documentation and return values

[Robert Millan]

On Fri, Jul 31, 2009 at 06:19:11PM +0200, Felix Zielcke wrote:
> Am Freitag, den 31.07.2009, 18:07 +0200 schrieb Robert Millan:
> > On Tue, Jul 28, 2009 at 11:05:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > > On Tue, Jul 28, 2009 at 8:15 PM, Robert Millan<rmh@aybabtu.com> wrote:
> > > > On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > > >> Hello. CPUID command currently doesn't return a real grub_err_t and
> > > >> errors aren't set which doesn't fit in grub2's design.
> > > >> Addirionally "help cpuid" doesn't give really useful info. So for
> > > >> future expandability and less confusion I propose the attached patch
> > > >> partially based on original patch by Robert Millan
> > > >
> > > > Hi,
> > > >
> > > > I don't remember about this patch.  I assume I wrote it in 2007 when
> > > > merging cpuid support for first time.
> > > Yes, it is.
> > > >  Any idea why it wasn't it committed
> > > > back then?
> > > >
> > > None. Yoshinori K Okuji approved it here:
> > > http://lists.gnu.org/archive/html/grub-devel/2007-05/msg00148.html
> > 
> > I vaguely recall having disabled it because there was some issue with
> > scripting.  Assuming it's working properly now, I have no objection with
> > it.
> > 
> 
> There was one.
> if [ 0=1 ] ; then echo t ; else echo f ; fi
> did print t but then also `error: false'
> This is now fixed.

But the cpuid CLI worked differently, IIRC

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] Fix CPUID documentation and return values

[Felix Zielcke]

Am Freitag, den 31.07.2009, 18:26 +0200 schrieb Robert Millan:
> On Fri, Jul 31, 2009 at 06:19:11PM +0200, Felix Zielcke wrote:
> > Am Freitag, den 31.07.2009, 18:07 +0200 schrieb Robert Millan:
> > > On Tue, Jul 28, 2009 at 11:05:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > > > On Tue, Jul 28, 2009 at 8:15 PM, Robert Millan<rmh@aybabtu.com> wrote:
> > > > > On Tue, Jul 28, 2009 at 01:03:51AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > > > >> Hello. CPUID command currently doesn't return a real grub_err_t and
> > > > >> errors aren't set which doesn't fit in grub2's design.
> > > > >> Addirionally "help cpuid" doesn't give really useful info. So for
> > > > >> future expandability and less confusion I propose the attached patch
> > > > >> partially based on original patch by Robert Millan
> > > > >
> > > > > Hi,
> > > > >
> > > > > I don't remember about this patch.  I assume I wrote it in 2007 when
> > > > > merging cpuid support for first time.
> > > > Yes, it is.
> > > > >  Any idea why it wasn't it committed
> > > > > back then?
> > > > >
> > > > None. Yoshinori K Okuji approved it here:
> > > > http://lists.gnu.org/archive/html/grub-devel/2007-05/msg00148.html
> > > 
> > > I vaguely recall having disabled it because there was some issue with
> > > scripting.  Assuming it's working properly now, I have no objection with
> > > it.
> > > 
> > 
> > There was one.
> > if [ 0=1 ] ; then echo t ; else echo f ; fi
> > did print t but then also `error: false'
> > This is now fixed.
> 
> But the cpuid CLI worked differently, IIRC
> 

Ah in the thread you said that
`if cpuid --long-mode; then ...; else ...; fi'
doestn't work.

But I just tested this with 
`if cpuid --long-mode; then echo true; else echo false; fi'
in qemu and in 32bit I get false and in 64bit I get true.



-- 
Felix Zielcke
Proud Debian Maintainer

Re: [PATCH] Fix CPUID documentation and return values

[Robert Millan]

On Fri, Jul 31, 2009 at 06:41:44PM +0200, Felix Zielcke wrote:
> 
> Ah in the thread you said that
> `if cpuid --long-mode; then ...; else ...; fi'
> doestn't work.
> 
> But I just tested this with 
> `if cpuid --long-mode; then echo true; else echo false; fi'
> in qemu and in 32bit I get false and in 64bit I get true.

Should be fine then.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."