date:20180129

[PATCH] Make grub-install check for errors from efibootmgr

2018-01-29 Thread Steve McIntyre

Code is currently ignoring errors from efibootmgr, giving users
clearly bogus output like:

Setting up grub-efi-amd64 (2.02~beta3-4) ...
Installing for x86_64-efi platform.
Could not delete variable: No space left on device
Could not prepare Boot variable: No space left on device
Installation finished. No error reported.

and then potentially unbootable systems. If efibootmgr fails,
grub-install should know that and report it!

We've been using this patch in Debian now for some time, with no ill
effects.

Signed-off-by: Steve McIntyre <93...@debian.org>
---
 grub-core/osdep/unix/platform.c | 25 +++--
 include/grub/util/install.h |  2 +-
 util/grub-install.c | 18 +-
 3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfcaca..b3a617e44 100644
--- a/grub-core/osdep/unix/platform.c
+++ b/grub-core/osdep/unix/platform.c
@@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
   dev);
 }
 
-static void
+static int
 grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
 {
   int fd;
   pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, 
);
   char *line = NULL;
   size_t len = 0;
+  int ret;
 
   if (!pid)
 {
   grub_util_warn (_("Unable to open stream from %s: %s"),
  "efibootmgr", strerror (errno));
-  return;
+  return errno;
 }
 
   FILE *fp = fdopen (fd, "r");
@@ -98,14 +99,13 @@ grub_install_remove_efi_entries_by_distributor (const char 
*efi_distributor)
 {
   grub_util_warn (_("Unable to open stream from %s: %s"),
  "efibootmgr", strerror (errno));
-  return;
+  return errno;
 }
 
   line = xmalloc (80);
   len = 80;
   while (1)
 {
-  int ret;
   char *bootnum;
   ret = getline (, , fp);
   if (ret == -1)
@@ -119,23 +119,25 @@ grub_install_remove_efi_entries_by_distributor (const 
char *efi_distributor)
   bootnum = line + sizeof ("Boot") - 1;
   bootnum[4] = '\0';
   if (!verbosity)
-   grub_util_exec ((const char * []){ "efibootmgr", "-q",
+   ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
  "-b", bootnum,  "-B", NULL });
   else
-   grub_util_exec ((const char * []){ "efibootmgr",
+   ret = grub_util_exec ((const char * []){ "efibootmgr",
  "-b", bootnum, "-B", NULL });
 }
 
   free (line);
+  return ret;
 }
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
   const char *efifile_path,
   const char *efi_distributor)
 {
   const char * efidir_disk;
   int efidir_part;
+  int ret;
   efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
   efidir_part = efidir_grub_dev->disk->partition ? 
efidir_grub_dev->disk->partition->number + 1 : 1;
 
@@ -151,23 +153,26 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
   grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
 #endif
   /* Delete old entries from the same distributor.  */
-  grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  if (ret)
+return ret;
 
   char *efidir_part_str = xasprintf ("%d", efidir_part);
 
   if (!verbosity)
-grub_util_exec ((const char * []){ "efibootmgr", "-q",
+ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
  "-c", "-d", efidir_disk,
  "-p", efidir_part_str, "-w",
  "-L", efi_distributor, "-l", 
  efifile_path, NULL });
   else
-grub_util_exec ((const char * []){ "efibootmgr",
+ret = grub_util_exec ((const char * []){ "efibootmgr",
  "-c", "-d", efidir_disk,
  "-p", efidir_part_str, "-w",
  "-L", efi_distributor, "-l", 
  efifile_path, NULL });
   free (efidir_part_str);
+  return ret;
 }
 
 void
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 5910b0c09..0dba8b67f 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
 const char *
 grub_install_get_default_x86_platform (void);
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
   const char *efifile_path,
   const char *efi_distributor);
diff --git a/util/grub-install.c b/util/grub-install.c
index 5e4cdfd2b..690f180c5 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1848,9 +1848,13 @@ main (int argc, char *argv[])
  if (!removable && update_nvram)
{
  /* Try to make this image bootable using the EFI Boot Manager, if 
available.  */
- grub_install_register_efi (efidir_grub_dev,
-

Re: A thread on grub-bug could need attention

2018-01-29 Thread Thomas Schmitt

Hi,

Daniel Kiper wrote:
> > 2) Create EFI System Partition (code EF00) using gdisk. 256.0 MiB will
> > suffice.
> > 3) Format EFI System Partition
> >   # mkdosfs /dev/sdb1
> >
> >4) Create GRUB2 EFI bootable image
> >   # mount /dev/sdb1 /mnt
> >   # mkdir /mnt/efi/boot
> >   # grub-mkstandalone -O x86_64-efi -o /mnt/efi/boot/bootx64.efi

Michel Bouissou wrote:
> I tested the USB stick on my son's machine, and I do get a grub prompt.

Does this mean that the decisive trick is to use GPT with EFI partition
instead of a MBR partition of type 0xEF ?

Or is there something missing in the FAT filesystem images of e.g.
Debian installation ISOs like
  
https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-9.3.0-amd64-netinst.iso
 
?

Michel: Does a simple ISO "output.iso" work, when made by

  mkdir ./minimal
  echo dummy >./minimal/dummy
  grub-mkrescue -o output.iso ./minimal

and then copied onto the plain USB stick device (e.g. /dev/sdc) ?
grub-mkrescue produces valid GPT in ISO 9660.


Have a nice day :)

Thomas


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: A thread on grub-bug could need attention

2018-01-29 Thread Michel Bouissou

Hi again,

Le 29/01/2018 à 16:29, Michel Bouissou a écrit :
> I created the USB stick per your instructions. I will try to give it a
> shot tonight on my son's machine.

I tested the USB stick on my son's machine, and I do get a grub prompt.

From there, if I type "help", I get a list of grub shell commands.

And if I type "reboot", the machine reboots.

So tha'ts a progress.

Thanks for the suggestion.

ॐ

-- 
Michel Bouissou  OpenPGP ID 0xEB04D09C

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Make grub-install check for errors from efibootmgr

2018-01-29 Thread Daniel Kiper

On Mon, Jan 29, 2018 at 02:04:54PM +, Steve McIntyre wrote:
> Code is currently ignoring errors from efibootmgr, giving users
> clearly bogus output like:
>
> Setting up grub-efi-amd64 (2.02~beta3-4) ...
> Installing for x86_64-efi platform.
> Could not delete variable: No space left on device
> Could not prepare Boot variable: No space left on device
> Installation finished. No error reported.
>
> and then potentially unbootable systems. If efibootmgr fails,
> grub-install should know that and report it!
>
> We've been using this patch in Debian now for some time, with no ill
> effects.
>
> Signed-off-by: Steve McIntyre <93...@debian.org>
> ---
>  grub-core/osdep/unix/platform.c | 24 +++-
>  include/grub/util/install.h |  2 +-
>  util/grub-install.c | 14 +++---
>  3 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
> index a3fcfcaca..fdd57a027 100644
> --- a/grub-core/osdep/unix/platform.c
> +++ b/grub-core/osdep/unix/platform.c
> @@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
>  dev);
>  }
>
> -static void
> +static int
>  grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
>  {
>int fd;
>pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, 
> );
>char *line = NULL;
>size_t len = 0;
> +  int error = 0;

s/error/ret/ or s/error/rc/ or s/error/rv/ here and below please...
In that order of preference...

>if (!pid)
>  {
>grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
> -  return;
> +  return errno;
>  }
>
>FILE *fp = fdopen (fd, "r");
> @@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char 
> *efi_distributor)
>  {
>grub_util_warn (_("Unable to open stream from %s: %s"),
> "efibootmgr", strerror (errno));
> -  return;
> +  return errno;
>  }
>
>line = xmalloc (80);
> @@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const 
> char *efi_distributor)
>bootnum = line + sizeof ("Boot") - 1;
>bootnum[4] = '\0';
>if (!verbosity)
> - grub_util_exec ((const char * []){ "efibootmgr", "-q",
> + error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-b", bootnum,  "-B", NULL });
>else
> - grub_util_exec ((const char * []){ "efibootmgr",
> + error = grub_util_exec ((const char * []){ "efibootmgr",
> "-b", bootnum, "-B", NULL });
>  }
>
>free (line);
> +  return error;
>  }
>
> -void
> +int
>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>  const char *efifile_path,
>  const char *efi_distributor)
>  {
>const char * efidir_disk;
>int efidir_part;
> +  int error = 0;
>efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
>efidir_part = efidir_grub_dev->disk->partition ? 
> efidir_grub_dev->disk->partition->number + 1 : 1;
>
> @@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t 
> efidir_grub_dev,
>grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>  #endif
>/* Delete old entries from the same distributor.  */
> -  grub_install_remove_efi_entries_by_distributor (efi_distributor);
> +  error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
> +  if (error)
> +return error;
>
>char *efidir_part_str = xasprintf ("%d", efidir_part);
>
>if (!verbosity)
> -grub_util_exec ((const char * []){ "efibootmgr", "-q",
> +error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l",
> efifile_path, NULL });
>else
> -grub_util_exec ((const char * []){ "efibootmgr",
> +error = grub_util_exec ((const char * []){ "efibootmgr",
> "-c", "-d", efidir_disk,
> "-p", efidir_part_str, "-w",
> "-L", efi_distributor, "-l",
> efifile_path, NULL });
>free (efidir_part_str);
> +  return error;
>  }
>
>  void
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 5910b0c09..0dba8b67f 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
>  const char *
>  grub_install_get_default_x86_platform (void);
>
> -void
> +int
>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>  const char *efifile_path,
>  const char *efi_distributor);
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 5e4cdfd2b..e783824ba 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1848,9 +1848,13 @@ main (int argc,

Re: [PATCH] Make grub-install check for errors from efibootmgr

2018-01-29 Thread Steve McIntyre

On Mon, Jan 29, 2018 at 06:57:51PM +0100, Daniel Kiper wrote:
>On Mon, Jan 29, 2018 at 02:04:54PM +, Steve McIntyre wrote:
>>
>> diff --git a/grub-core/osdep/unix/platform.c 
>> b/grub-core/osdep/unix/platform.c
>> index a3fcfcaca..fdd57a027 100644
>> --- a/grub-core/osdep/unix/platform.c
>> +++ b/grub-core/osdep/unix/platform.c
>> @@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
>> dev);
>>  }
>>
>> -static void
>> +static int
>>  grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
>>  {
>>int fd;
>>pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, 
>> );
>>char *line = NULL;
>>size_t len = 0;
>> +  int error = 0;
>
>s/error/ret/ or s/error/rc/ or s/error/rv/ here and below please...
>In that order of preference...

ACK.

...

>> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>> index 5910b0c09..0dba8b67f 100644
>> --- a/include/grub/util/install.h
>> +++ b/include/grub/util/install.h
>> @@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
>>  const char *
>>  grub_install_get_default_x86_platform (void);
>>
>> -void
>> +int
>>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>> const char *efifile_path,
>> const char *efi_distributor);
>> diff --git a/util/grub-install.c b/util/grub-install.c
>> index 5e4cdfd2b..e783824ba 100644
>> --- a/util/grub-install.c
>> +++ b/util/grub-install.c
>> @@ -1848,9 +1848,13 @@ main (int argc, char *argv[])
>>if (!removable && update_nvram)
>>  {
>>/* Try to make this image bootable using the EFI Boot Manager, if 
>> available.  */
>> -  grub_install_register_efi (efidir_grub_dev,
>> +  int error = 0;
>
>I think that you do not need this initialization.

ACK. Updated patch coming shortly.

-- 
Steve McIntyre, Cambridge, UK.st...@einval.com
The two hard things in computing:
 * naming things
 * cache invalidation
 * off-by-one errors  -- Stig Sandbeck Mathisen


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] build: Use pkg-config to find Freetype

2018-01-29 Thread Daniel Kiper

On Thu, Jan 25, 2018 at 12:23:58PM +, Colin Watson wrote:
> pkg-config is apparently preferred over freetype-config these days (see
> the BUGS section of freetype-config(1)).  pkg-config support was added
> to Freetype in version 2.1.5, which was released in 2003, so it should

OK but please add somewhere into INSTALL that starting from this patch
Freetype 2.1.5 and newer are only supported versions.

> comfortably be available everywhere by now.
>
> Fixes Debian bug #887721.
>
> Reported-by: Hugh McMaster 
> Signed-off-by: Colin Watson 
> ---
>  INSTALL   |  9 ---
>  Makefile.am   |  6 ++---
>  Makefile.util.def |  4 +--
>  configure.ac  | 74 
> +++
>  4 files changed, 41 insertions(+), 52 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index f3c20edc8..899b9cac5 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -38,6 +38,7 @@ configuring the GRUB.
>  * GNU binutils 2.9.1.0.23 or later
>  * Flex 2.5.35 or later
>  * Other standard GNU/Unix tools
> +* pkg-config

Please put this after Flex.

>  * a libc with large file support (e.g. glibc 2.1 or later)
>
>  On GNU/Linux, you also need:
> @@ -158,8 +159,8 @@ For this example the configure line might look like (more 
> details below)
>  (some options are optional and included here for completeness but some rarely
>  used options are omitted):
>
> -./configure BUILD_CC=gcc BUILD_FREETYPE=freetype-config 
> --host=amd64-linux-gnu
> -CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" 
> FREETYPE=amd64-linux-gnu-freetype-config
> +./configure BUILD_CC=gcc BUILD_PKG_CONFIG=pkg-config --host=amd64-linux-gnu
> +CC=amd64-linux-gnu-gcc CFLAGS="-g -O2" PKG_CONFIG=amd64-linux-gnu-pkg-config
>  --target=arm --with-platform=uboot TARGET_CC=arm-elf-gcc
>  TARGET_CFLAGS="-Os -march=armv6" TARGET_CCASFLAGS="-march=armv6"
>  TARGET_OBJCOPY="arm-elf-objcopy" TARGET_STRIP="arm-elf-strip"
> @@ -176,7 +177,7 @@ corresponding platform are not needed for the platform in 
> question.
>  2. BUILD_CFLAGS= for C options for build.
>  3. BUILD_CPPFLAGS= for C preprocessor options for build.
>  4. BUILD_LDFLAGS= for linker options for build.
> -5. BUILD_FREETYPE= for freetype-config for build (optional).
> +5. BUILD_PKG_CONFIG= for pkg-config for build (optional).
>
>- For host
>  1. --host= to autoconf name of host.
> @@ -184,7 +185,7 @@ corresponding platform are not needed for the platform in 
> question.
>  3. HOST_CFLAGS= for C options for host.
>  4. HOST_CPPFLAGS= for C preprocessor options for host.
>  5. HOST_LDFLAGS= for linker options for host.
> -6. FREETYPE= for freetype-config for host (optional).
> +6. PKG_CONFIG= for pkg-config for host (optional).
>  7. Libdevmapper if any must be in standard linker folders (-ldevmapper) 
> (optional).
>  8. Libfuse if any must be in standard linker folders (-lfuse) (optional).
>  9. Libzfs if any must be in standard linker folders (-lzfs) (optional).
> diff --git a/Makefile.am b/Makefile.am
> index 7795baeb6..da4e65bcc 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -71,7 +71,7 @@ endif
>  starfield_theme_files = $(srcdir)/themes/starfield/blob_w.png 
> $(srcdir)/themes/starfield/boot_menu_c.png 
> $(srcdir)/themes/starfield/boot_menu_e.png 
> $(srcdir)/themes/starfield/boot_menu_ne.png 
> $(srcdir)/themes/starfield/boot_menu_n.png 
> $(srcdir)/themes/starfield/boot_menu_nw.png 
> $(srcdir)/themes/starfield/boot_menu_se.png 
> $(srcdir)/themes/starfield/boot_menu_s.png 
> $(srcdir)/themes/starfield/boot_menu_sw.png 
> $(srcdir)/themes/starfield/boot_menu_w.png 
> $(srcdir)/themes/starfield/slider_c.png 
> $(srcdir)/themes/starfield/slider_n.png 
> $(srcdir)/themes/starfield/slider_s.png 
> $(srcdir)/themes/starfield/starfield.png 
> $(srcdir)/themes/starfield/terminal_box_c.png 
> $(srcdir)/themes/starfield/terminal_box_e.png 
> $(srcdir)/themes/starfield/terminal_box_ne.png 
> $(srcdir)/themes/starfield/terminal_box_n.png 
> $(srcdir)/themes/starfield/terminal_box_nw.png 
> $(srcdir)/themes/starfield/terminal_box_se.png 
> $(srcdir)/themes/starfield/terminal_box_s.png 
> $(srcdir)/themes/starfield/terminal_box_sw.png 
> $(srcdir)/themes/starfield/terminal_box_w.png 
> $(srcdir)/themes/starfield/theme.txt $(srcdir)/themes/starfield/README 
> $(srcdir)/themes/starfield/COPYING.CC-BY-SA-3.0
>
>  build-grub-mkfont$(BUILD_EXEEXT): util/grub-mkfont.c grub-core/unidata.c 
> grub-core/kern/emu/misc.c util/misc.c
> - $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 -DGRUB_BUILD_PROGRAM_NAME=\"build-grub-mkfont\" $^ 
> $(build_freetype_cflags) $(build_freetype_libs)
> + $(BUILD_CC) -o $@ -I$(top_srcdir)/include $(BUILD_CFLAGS) 
> $(BUILD_CPPFLAGS) $(BUILD_LDFLAGS) -DGRUB_MKFONT=1 -DGRUB_BUILD=1 
> -DGRUB_UTIL=1 -DGRUB_BUILD_PROGRAM_NAME=\"build-grub-mkfont\" $^ 
>

Re: [PATCH] chainloader: Fix wrong break condition (must be AND not OR)

2018-01-29 Thread Daniel Kiper

On Sun, Jan 21, 2018 at 04:02:10PM +0100, C. Masloch wrote:
> The definition of bpb's num_total_sectors_16 and num_total_sectors_32
> is that either the 16-bit field is non-zero and is used (in which case
> eg mkfs.fat sets the 32-bit field to zero), or it is zero and the
> 32-bit field is used. Therefore, a BPB is invalid only if *both*
> fields are zero; having one field as zero and the other as non-zero is
> the case to be expected.

Could you provide reference to the spec which says that?

> This affects all users of grub_chainloader_patch_bpb which are in
> chainloader.c, freedos.c, and ntldr.c

I am happy that you fix that issue but
  https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system#BPB331_OFS_15h
shows that life is more complicated.

Could you take that into account?

> Tested with lDebug booted in qemu via grub2's
> FreeDOS direct loading support, refer to
> https://bitbucket.org/ecm/ldosboot + https://bitbucket.org/ecm/ldebug

Could you put your SOB here?

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Make pmtimer tsc calibration not take 51 seconds to fail.

2018-01-29 Thread Daniel Kiper

On Fri, Jan 19, 2018 at 04:10:01PM -0500, Peter Jones wrote:
> On Thu, Jan 18, 2018 at 11:52:53PM +0100, Daniel Kiper wrote:
> > On Tue, Jan 16, 2018 at 01:16:17PM -0500, Peter Jones wrote:
> > > On my laptop running at 2.4GHz, if I run a VM where tsc calibration
> > > using pmtimer will fail presuming a broken pmtimer, it takes ~51 seconds
> > > to do so (as measured with the stopwatch on my phone), with a tsc delta
> > > of 0x1cd1c85300, or around 125 billion cycles.
> > >
> > > If instead of trying to wait for 5-200ms to show up on the pmtimer, we try
> > > to wait for 5-200us, it decides it's broken in ~0x7998f9e TSCs, aka ~2
> > > million cycles, or more or less instantly.
> > >
> > > Additionally, this reading the pmtimer was returning 0x anyway,
> > > and that's obviously an invalid return.  I've added a check for that and
> > > 0 so we don't bother waiting for the test if what we're seeing is dead
> > > pins with no response at all.
> > >
> > > Signed-off-by: Peter Jones 
> > > ---
> > >  grub-core/kern/i386/tsc_pmtimer.c | 43 
> > > ++-
> > >  1 file changed, 33 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/grub-core/kern/i386/tsc_pmtimer.c 
> > > b/grub-core/kern/i386/tsc_pmtimer.c
> > > index c9c36169978..609402b8376 100644
> > > --- a/grub-core/kern/i386/tsc_pmtimer.c
> > > +++ b/grub-core/kern/i386/tsc_pmtimer.c
> > > @@ -38,30 +38,53 @@ grub_pmtimer_wait_count_tsc (grub_port_t pmtimer,
> > >grub_uint64_t start_tsc;
> > >grub_uint64_t end_tsc;
> > >int num_iter = 0;
> > > +  int bad_reads = 0;
> > >
> > > -  start = grub_inl (pmtimer) & 0xff;
> > > +  start = grub_inl (pmtimer) & 0x3fff;
> >
> > I am not sure why you are changing this to 0x3fff...
>
> Upon re-reading, I am not either.  (As you see I wrote this patch 2+
> months ago while working on something else, so the memory has already
> faded.)

Yeah, I know how it works...

> Obviously I need to make a version 2 of this patch to fix some issues.

Great!

> > >last = start;
> > >end = start + num_pm_ticks;
> > >start_tsc = grub_get_tsc ();
> > >while (1)
> > >  {
> > > -  cur = grub_inl (pmtimer) & 0xff;
> >
> > What about 24-bit timers? I would leave this here...
>
> I was blisfully unaware of 24-bit timers.  I'll put that back and add a
> comment for future readers of the code, and check for 0xff below.

Thanks!

> > > +  cur = grub_inl (pmtimer);
> > > +
> > > +  /* If we get 10 reads in a row that are obviously dead pins, 
> > > there's no
> > > +  reason to do this thousands of times.
> > > +   */
> > > +  if (cur == 0x || cur == 0)
> >
> > ...and here I would check for 0xff and 0.
>
> Sure.
>
> >
> > > + {
> > > +   bad_reads++;
> > > +   grub_dprintf ("pmtimer", "cur: 0x%08x bad_reads: %d\n", cur, 
> > > bad_reads);
> > > +
> > > +   if (bad_reads == 10)
> > > + return 0;
> > > + }
> > > +  else if (bad_reads)
> > > + bad_reads = 0;
> >
> > Do we really need to reset this?
>
> I mean, it's technically correct, but I don't think you're going to see
> 0 and 0xff as valid values often enough to actually care.
>
> > > +  cur &= 0x3fff;
> > > +
> > >if (cur < last)
> > > - cur |= 0x100;
> > > + cur |= 0x4000;
> > >num_iter++;
>
> You didn't spot this, but it's wrong for exactly the same reason the
> initialization of start is.  I'm pretty sure when I did this part, I was
> letting the comment below tell me what was going on, and completely
> missed that the comment doesn't match the conditional at all.
>
> > >if (cur >= end)
> > >   {
> > > end_tsc = grub_get_tsc ();
> > > +   grub_dprintf ("pmtimer", "tsc delta is 0x%016lx\n",
> > > + end_tsc - start_tsc);
> > > return end_tsc - start_tsc;
> > >   }
> > > -  /* Check for broken PM timer.
> > > -  5000 TSCs is between 5 ms (10GHz) and 200 ms (250 MHz)
> > > -  if after this time we still don't have 1 ms on pmtimer, then
> > > -  pmtimer is broken.
> > > +  /* Check for broken PM timer.  5000 TSCs is between 5us (10GHz) and
> >  ^^^ 500ns?
> >
> > > +  200us (250 MHz).  If after this time we still don't have 1us on
> >  ^ 20us?
>
> You know, I stared at this for a surprising amount of time when writing
> it trying to figure out why these numbers seemed weird.  While I have
> the diff in front of me rather than the code as it is now, I see that
> it's because I was starting with the comment that was previously there,
> which had 50M instead of 5M for the TSC count.  So I wound up dividing
> these by 10k instead of 1k.  Woops.  Will fix in v2.
>
> >
> > > +  pmtimer, then pmtimer is broken.
> > > */
> > > -  if ((num_iter & 0xff) == 0 && grub_get_tsc () - start_tsc > 
> > > 500) {
> > > - return 0;
> > > -  }
> > > +  end_tsc = grub_get_tsc();
> > > +  if

Re: A thread on grub-bug could need attention

2018-01-29 Thread Michel Bouissou

Hello,

Le 29/01/2018 à 13:49, Daniel Kiper a écrit :
> OK, I have looked through the email exchange. I have not seen any info
> about the UEFI/firmware update. Have you done that? If you have not
> please do.
I have installed the latest HP UEFI/firmware from HP on this machine the
day I got it (last december)

Version is 01.11 (Machine came with 01.09 AFAIR)

>  If it does not help please create bootable USB stick in
> following way.

I created the USB stick per your instructions. I will try to give it a
shot tonight on my son's machine.

For your reference, the machine from which I created the stick is a
Manjaro and grub package version is 2.02.0-4.

After the grub-mkstandalone phase, the /mnt/efi/boot contains :
-rwxr-xr-x 1 root root 12068352 29 janv. 16:19 bootx64.efi

Thanks for your help.

Kind regards.

-- 

ॐ

Michel Bouissou  OpenPGP ID 0xEB04D09C



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Make grub-install check for errors from efibootmgr

2018-01-29 Thread Steve McIntyre

Code is currently ignoring errors from efibootmgr, giving users
clearly bogus output like:

Setting up grub-efi-amd64 (2.02~beta3-4) ...
Installing for x86_64-efi platform.
Could not delete variable: No space left on device
Could not prepare Boot variable: No space left on device
Installation finished. No error reported.

and then potentially unbootable systems. If efibootmgr fails,
grub-install should know that and report it!

We've been using this patch in Debian now for some time, with no ill
effects.

Signed-off-by: Steve McIntyre <93...@debian.org>
---
 grub-core/osdep/unix/platform.c | 24 +++-
 include/grub/util/install.h |  2 +-
 util/grub-install.c | 14 +++---
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfcaca..fdd57a027 100644
--- a/grub-core/osdep/unix/platform.c
+++ b/grub-core/osdep/unix/platform.c
@@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
   dev);
 }
 
-static void
+static int
 grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
 {
   int fd;
   pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, 
);
   char *line = NULL;
   size_t len = 0;
+  int error = 0;
 
   if (!pid)
 {
   grub_util_warn (_("Unable to open stream from %s: %s"),
  "efibootmgr", strerror (errno));
-  return;
+  return errno;
 }
 
   FILE *fp = fdopen (fd, "r");
@@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char 
*efi_distributor)
 {
   grub_util_warn (_("Unable to open stream from %s: %s"),
  "efibootmgr", strerror (errno));
-  return;
+  return errno;
 }
 
   line = xmalloc (80);
@@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const 
char *efi_distributor)
   bootnum = line + sizeof ("Boot") - 1;
   bootnum[4] = '\0';
   if (!verbosity)
-   grub_util_exec ((const char * []){ "efibootmgr", "-q",
+   error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
  "-b", bootnum,  "-B", NULL });
   else
-   grub_util_exec ((const char * []){ "efibootmgr",
+   error = grub_util_exec ((const char * []){ "efibootmgr",
  "-b", bootnum, "-B", NULL });
 }
 
   free (line);
+  return error;
 }
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
   const char *efifile_path,
   const char *efi_distributor)
 {
   const char * efidir_disk;
   int efidir_part;
+  int error = 0;
   efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
   efidir_part = efidir_grub_dev->disk->partition ? 
efidir_grub_dev->disk->partition->number + 1 : 1;
 
@@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
   grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
 #endif
   /* Delete old entries from the same distributor.  */
-  grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  if (error)
+return error;
 
   char *efidir_part_str = xasprintf ("%d", efidir_part);
 
   if (!verbosity)
-grub_util_exec ((const char * []){ "efibootmgr", "-q",
+error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
  "-c", "-d", efidir_disk,
  "-p", efidir_part_str, "-w",
  "-L", efi_distributor, "-l", 
  efifile_path, NULL });
   else
-grub_util_exec ((const char * []){ "efibootmgr",
+error = grub_util_exec ((const char * []){ "efibootmgr",
  "-c", "-d", efidir_disk,
  "-p", efidir_part_str, "-w",
  "-L", efi_distributor, "-l", 
  efifile_path, NULL });
   free (efidir_part_str);
+  return error;
 }
 
 void
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 5910b0c09..0dba8b67f 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
 const char *
 grub_install_get_default_x86_platform (void);
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
   const char *efifile_path,
   const char *efi_distributor);
diff --git a/util/grub-install.c b/util/grub-install.c
index 5e4cdfd2b..e783824ba 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1848,9 +1848,13 @@ main (int argc, char *argv[])
  if (!removable && update_nvram)
{
  /* Try to make this image bootable using the EFI Boot Manager, if 
available.  */
- grub_install_register_efi (efidir_grub_dev,
+ int error = 0;
+ error = grub_install_register_efi (efidir_grub_dev,

Re: [PATCH 0/8] xen: add pvh guest support

2018-01-29 Thread Juergen Gross

On 29/01/18 13:15, Daniel Kiper wrote:
> On Thu, Dec 14, 2017 at 12:44:36PM +0100, Juergen Gross wrote:
>> On 14/12/17 12:32, Daniel Kiper wrote:
>>> On Thu, Dec 14, 2017 at 12:26:02PM +0100, Juergen Gross wrote:
 On 14/12/17 12:19, Daniel Kiper wrote:
> On Fri, Dec 01, 2017 at 12:12:50PM +0100, Daniel Kiper wrote:
>> On Fri, Dec 01, 2017 at 06:37:37AM +0100, Juergen Gross wrote:
>>> On 30/11/17 22:03, Daniel Kiper wrote:
 On Wed, Nov 29, 2017 at 02:46:42PM +0100, Juergen Gross wrote:
> This patch series adds support for booting Linux as PVH guest.
>
> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh
> platform grub is booted as a standalone image directly by Xen.
>
> For booting Linux kernel it is using the standard linux kernel
> loader. The only modification of the linux loader is to pass the
> ACPI RSDP address via boot parameters to the kernel, as that table
> might not be located at the usual physical address just below 1MB.
>
> As the related Linux kernel patches are not yet accepted please
> wait for this to happen before applying the series.

 So, may I review the patches or should I hold on? And could you
 provide a link to the "Linux kernel patches" mentioned above?
>>>
>>> Please review!
>>
>> Will do in a week or so.
>
> Swamped by some urgent internal work. I will try to review this before
> Xmas but if it do not happen then I will do it in the first halt of
> January. Sorry for delays.

 Thanks.

 BTW: the Linux kernel patches have been accepted by the x86 maintainers.
>>>
>>> Perfect! Could you send me their commit ids?
>>
>> Right now they are in the tip tree in the x86/boot branch:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/boot
>>
>> Commit-Ids are:
>> 2f74cbf947f45fa082dda8eac1a1f1299a372f49
>> 0c89cf36424f7c1177de8a5712514d7cc2eb369f
>> 88750a6c33f813b815516990f01fb5ee488c477e
>> 930ba49b2ce7b09a5eddc21385fd944ba6b4e829
> 
> My schedule was completely destroyed by many unexpected work and family
> issues. It looks that I am recovering slowly. I am going to take a look
> at the patches in a week or two. Should I review these ones or are you
> going to post something new?

There is only one issue which has to be fixed: passing on the RSDP
address to the kernel will require some more work, as extending struct
linux_kernel_header won't be so easy (multiple distros seem to have
shipped a grub2 with some patches clobbering the data directly after
that struct in the kernel). So patch 2 will be a little bit more
complicated due to the need to pass the interface version known by
grub2 back to the kernel.

I'm not sure when I'll be able to do that work, as Meltdown has led
to quite some work on my side. :-(

For all other patches I'm not aware of any modifications needed.


Juergen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: A thread on grub-bug could need attention

2018-01-29 Thread Daniel Kiper

On Mon, Jan 08, 2018 at 01:48:42PM +0100, Daniel Kiper wrote:
> On Wed, Dec 20, 2017 at 12:35:20PM +0100, Thomas Schmitt wrote:
> > Hi,
> >
> > i am unhappy about the state of the thread
> >
> >   http://lists.gnu.org/archive/html/bug-grub/2017-12/msg00010.html
> >   "HP ProBook x360 11 G1 EE incompatible with grub"
> >
> > which is about an EFI implementation which SYSLINUX for EFI can operate
> > from USB stick whereas GRUB from USB stick ends up with black screen.
> >
> > An allergy of the firmware towards ISO 9660 hybrid images on USB stick
> > can surely be ruled out by
> >   http://lists.gnu.org/archive/html/bug-grub/2017-12/msg00022.html
> >   (The only known production ISO with SYSLINUX for EFI does work)
> >   http://lists.gnu.org/archive/html/bug-grub/2017-12/msg00024.html
> >   (A normal distro installation works after replacing GRUB by SYSLINUX)
> >
> > The bug reporter, Michel Bouissou, seems well apt enough to do experiments.
> > But i am not apt enough to propose any beyond ISO 9660.
> > Could people from this list please give Michel some hints about how to
> > retrieve more information about the problem between GRUB and this
> > peculiar EFI ?
>
> Thank you for a notice. Sorry for late reply. I am just back after a bit
> longer vacation. Now I am recovering and trying to get at speed. I will
> take closer look at the thread next week.

OK, I have looked through the email exchange. I have not seen any info
about the UEFI/firmware update. Have you done that? If you have not
please do. If it does not help please create bootable USB stick in
following way. I assume that USB stick is /dev/sdb device. If not please
change device path below accordingly. Additionally, if you use Debian,
IIRC you do, please install at least following extra packages: dosfstools,
gdisk, grub-common and grub-efi-amd64-bin. I assume that other distros
has similar ones.

1) Zap GPT and MBR - DANGEROUS - it removes all GPT and MBR partitions
   # sgdisk -Z /dev/sdb

2) Create EFI System Partition (code EF00) using gdisk. 256.0 MiB will suffice.

3) Format EFI System Partition
   # mkdosfs /dev/sdb1

4) Create GRUB2 EFI bootable image
   # mount /dev/sdb1 /mnt
   # mkdir /mnt/efi/boot
   # grub-mkstandalone -O x86_64-efi -o /mnt/efi/boot/bootx64.efi
   # umount /dev/sdb1

Reboot HP machine from this USB stick. You should see normal GRUB2 prompt.
If it does not happen then I will think about next steps.

If you have any questions please drop me a line.

Additionally, I will be at FOSDEM this weekend. So, if you wish and you
will be there then you can bring the machine and I can take a look.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] ahci: Improve error handling

2018-01-29 Thread Daniel Kiper

On Tue, Jan 23, 2018 at 01:00:56PM +0100, Daniel Kiper wrote:
> On Fri, Jan 19, 2018 at 02:13:29PM +0100, Stefan Fritsch wrote:
> > From: Stefan Fritsch 
> >
> > Check the error bits in the interrupt status register. According to the
> > AHCI 1.2 spec, "Interrupt sources that are disabled (?0?) are still
> > reflected in the status registers.", so this should work even though
> > grub uses polling
> >
> > This fixes the following problem on a Fujitsu E744 laptop:
> >
> > Sometimes there is a very long delay (up to several minutes) when
> > booting from hard disk. It seems accessing the DVD drive (which has no
> > disk inserted) sometimes fails with some errors, which leads to each
> > access being stalled until the 20s timeout triggers. This seems to
> > happen when grub is trying to read filesystem/partition data.
> >
> > The problem is that the command_issue bit that is checked in the loop is
> > only reset if the "HBA receives a FIS which clears the BSY, DRQ, and ERR
> > bits for the command", but the ERR bit is never cleared. Therefore
> > command_issue is never reset and grub waits for the timeout.
> >
> > The relevant bit in our case is the Task File Error Status (TFES), which
> > is equivalent to the ERR bit 0 in tfd. But this patch also checks
> > the other error bits except for the "Interface non-fatal error status"
> > bit.
> >
> > Signed-off-by: Stefan Fritsch 
>
> Reviewed-by: Daniel Kiper 
>
> If there are no objections I will apply this by the end of this week.

Applied!

Thanks,

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] sparc64: fix OF path names for sun4v systems

2018-01-29 Thread John Paul Adrian Glaubitz


Hi!

On 12/19/2017 10:36 PM, Daniel Kiper wrote:

I think that change was suggested by me as the code wouldn't build with
-Werror without the initialization. -Werror is the default when building
GRUB, if I remember correctly.


Eric, if it is true please leave the initialization otherwise drop it.


Any chance we can continue with this patch series?

Thanks,

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/8] xen: add pvh guest support

2018-01-29 Thread Daniel Kiper

On Thu, Dec 14, 2017 at 12:44:36PM +0100, Juergen Gross wrote:
> On 14/12/17 12:32, Daniel Kiper wrote:
> > On Thu, Dec 14, 2017 at 12:26:02PM +0100, Juergen Gross wrote:
> >> On 14/12/17 12:19, Daniel Kiper wrote:
> >>> On Fri, Dec 01, 2017 at 12:12:50PM +0100, Daniel Kiper wrote:
>  On Fri, Dec 01, 2017 at 06:37:37AM +0100, Juergen Gross wrote:
> > On 30/11/17 22:03, Daniel Kiper wrote:
> >> On Wed, Nov 29, 2017 at 02:46:42PM +0100, Juergen Gross wrote:
> >>> This patch series adds support for booting Linux as PVH guest.
> >>>
> >>> Similar to i386/xen and x86_64/xen platforms the new i386/xenpvh
> >>> platform grub is booted as a standalone image directly by Xen.
> >>>
> >>> For booting Linux kernel it is using the standard linux kernel
> >>> loader. The only modification of the linux loader is to pass the
> >>> ACPI RSDP address via boot parameters to the kernel, as that table
> >>> might not be located at the usual physical address just below 1MB.
> >>>
> >>> As the related Linux kernel patches are not yet accepted please
> >>> wait for this to happen before applying the series.
> >>
> >> So, may I review the patches or should I hold on? And could you
> >> provide a link to the "Linux kernel patches" mentioned above?
> >
> > Please review!
> 
>  Will do in a week or so.
> >>>
> >>> Swamped by some urgent internal work. I will try to review this before
> >>> Xmas but if it do not happen then I will do it in the first halt of
> >>> January. Sorry for delays.
> >>
> >> Thanks.
> >>
> >> BTW: the Linux kernel patches have been accepted by the x86 maintainers.
> >
> > Perfect! Could you send me their commit ids?
>
> Right now they are in the tip tree in the x86/boot branch:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/boot
>
> Commit-Ids are:
> 2f74cbf947f45fa082dda8eac1a1f1299a372f49
> 0c89cf36424f7c1177de8a5712514d7cc2eb369f
> 88750a6c33f813b815516990f01fb5ee488c477e
> 930ba49b2ce7b09a5eddc21385fd944ba6b4e829

My schedule was completely destroyed by many unexpected work and family
issues. It looks that I am recovering slowly. I am going to take a look
at the patches in a week or two. Should I review these ones or are you
going to post something new?

Sorry for delays.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Keep the native terminal active when enabling gfxterm

2018-01-29 Thread Daniel Kiper

On Fri, Jan 19, 2018 at 12:05:10AM +0100, Daniel Kiper wrote:
> On Thu, Jan 18, 2018 at 11:57:26AM -0700, dann frazier wrote:
> > grub-mkconfig will set GRUB_TERMINAL_OUTPUT to "gfxterm" unless the user
> > has overridden it. On EFI systems, this will stop output from going to the
> > default "console" terminal. When the EFI fw console is configured to output 
> > to
> > both serial and video, this will cause GRUB to only display on video - while
> > continuing to accept input from both video and serial.
> >
> > Instead of switching from "console" to "gfxterm", let's output to both.
>
> Reviewed-by: Daniel Kiper 
>
> If there are no objections I will apply this next week.

Applied!

Thanks,

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Make grub-install check for errors from efibootmgr

Re: A thread on grub-bug could need attention

Re: A thread on grub-bug could need attention

Re: [PATCH] Make grub-install check for errors from efibootmgr

Re: [PATCH] Make grub-install check for errors from efibootmgr

Re: [PATCH] build: Use pkg-config to find Freetype

Re: [PATCH] chainloader: Fix wrong break condition (must be AND not OR)

Re: [PATCH] Make pmtimer tsc calibration not take 51 seconds to fail.

Re: A thread on grub-bug could need attention

[PATCH] Make grub-install check for errors from efibootmgr

Re: [PATCH 0/8] xen: add pvh guest support

Re: A thread on grub-bug could need attention

Re: [PATCH] ahci: Improve error handling

Re: [PATCH] sparc64: fix OF path names for sun4v systems

Re: [PATCH 0/8] xen: add pvh guest support

Re: [PATCH] Keep the native terminal active when enabling gfxterm

16 matches

Site Navigation

Mail list logo

Footer information