[PATCH 1/1] Fix PCIe LER when GRUB2 accesses non-enabled MMIO data from VGA

2018-03-28 Thread mike.travis
A GPU inserted into a PCIe I/O slot disappears during system startup.
The problem centers around GRUB and a specific VGA init function in
efi_uga.c.  This causes an LER (link error recorvery) because the MMIO
memory has not been enabled before attempting access.

The fix is to add the same coding used in other VGA drivers, specifically
to add a check to insure that it is indeed a VGA controller.  And then
enable the MMIO address space with the specific bits.

Signed-off-by: Mike Travis 
Reviewed-by: Michael Chang 
Reviewed-by: Daniel Kiper 
---
v1:change class to subclass, remove parens around "enable mem" code
---
 grub-2.02/grub-core/video/efi_uga.c |   15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

Index: grub-2.02/grub-core/video/efi_uga.c
===
--- grub-2.02.orig/grub-core/video/efi_uga.c
+++ grub-2.02/grub-core/video/efi_uga.c
@@ -95,9 +95,18 @@ find_card (grub_pci_device_t dev, grub_p
 {
   struct find_framebuf_ctx *ctx = data;
   grub_pci_address_t addr;
+  grub_pci_address_t rcaddr;
+  grub_uint32_t subclass;
 
   addr = grub_pci_make_address (dev, GRUB_PCI_REG_CLASS);
-  if (grub_pci_read (addr) >> 24 == 0x3)
+  subclass = (grub_pci_read (addr) >> 16) & 0x;
+  if (subclass != GRUB_PCI_CLASS_SUBCLASS_VGA)
+return 0;
+
+  /* Enable MEM address spaces */
+  rcaddr = grub_pci_make_address (dev, GRUB_PCI_REG_COMMAND);
+  grub_pci_write_word (rcaddr, grub_pci_read_word (rcaddr) | 
GRUB_PCI_COMMAND_MEM_ENABLED);
+
 {
   int i;
 

-- 


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


Re: [PATCH 2/4] Accept Both ESC and F8 as user interrupt keys

2018-03-28 Thread Hans de Goede

Hi,

On 28-03-18 17:11, Lennart Sorensen wrote:

On Wed, Mar 28, 2018 at 05:06:53PM +0200, Hans de Goede wrote:

Hmm, well people will still be able to use ESC to get the grub boot
menu, rather then the firmware boot-menu on those.

The problem is that our current check for ESC only approach is problematic
because it conflicts with the enter firmware-setup key on almost all Bay Trail,
Cherry Trail and Apollo Lake devices. You need to try hard to find a device
in one of those 3 categories which does not use ESC for this. Note I'm
aware some devices exist, but using ESC for this is really really common
among these devices.

I'm open to other suggestions, but I think we really need to add another
key to avoid the pressing ESC at boot already has another meaning problem
and F8 seems like an ok choice.

Either way thank you for the input on this.


I don't disagree about having another key, I just disagree that F8 is
a good choice.

Looking at https://kb.wisc.edu/page.php?id=58779#here it appears no one
uses F4 for anything.  That would seem like a better choice than F8
at least.  F6 also seems to be free but F4 seems nicer on desktop
keyboards.


Looking at that table I have to agree that F4 seems like the best second
key to use. Actually looking at that table ESC seems like a poor choice,
but I guess it makes sense for serial-terminals as well as from a general
UI pov (and we're stuck with it now anyways).

I'm going to wait for Daniel's review of the other patches before I do
a v2 of this patch-set. I will switch to F4 for v2.

Regards,

Hans


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


[GRUB PARTUUID PATCH V8 3/4] Add PARTUUID detection support to grub-probe

2018-03-28 Thread Nicholas Vinson
Add PARTUUID detection support grub-probe for MBR and GPT partition
schemes.

Signed-off-by: Nicholas Vinson 
---
 util/grub-probe.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 21cb80fbe..8a013548a 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -62,6 +63,7 @@ enum {
   PRINT_DRIVE,
   PRINT_DEVICE,
   PRINT_PARTMAP,
+  PRINT_PARTUUID,
   PRINT_ABSTRACTION,
   PRINT_CRYPTODISK_UUID,
   PRINT_HINT_STR,
@@ -85,6 +87,7 @@ static const char *targets[] =
 [PRINT_DRIVE]  = "drive",
 [PRINT_DEVICE] = "device",
 [PRINT_PARTMAP]= "partmap",
+[PRINT_PARTUUID]   = "partuuid",
 [PRINT_ABSTRACTION]= "abstraction",
 [PRINT_CRYPTODISK_UUID]= "cryptodisk_uuid",
 [PRINT_HINT_STR]   = "hints_string",
@@ -181,6 +184,43 @@ probe_partmap (grub_disk_t disk, char delim)
 }
 }
 
+static void
+probe_partuuid (grub_disk_t disk, char delim)
+{
+  grub_partition_t p = disk->partition;
+
+  /*
+   * Nested partitions not supported for now.
+   * Non-nested partitions must have disk->partition->parent == NULL
+   */
+  if (p && p->parent == NULL)
+{
+  disk->partition = p->parent;
+  if (strcmp(p->partmap->name, "msdos") == 0)
+   {
+   /*
+* The partition GUID for MSDOS is the partition number (starting
+* with 1) prepended with the NT disk signature.
+*/
+   grub_uint32_t nt_disk_sig;
+
+   if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
+   sizeof(nt_disk_sig), &nt_disk_sig) == 0)
+ grub_printf ("%08x-%02x",
+  grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
+   }
+  else if (strcmp(p->partmap->name, "gpt") == 0)
+   {
+ struct grub_gpt_partentry gptdata;
+
+ if (grub_disk_read (disk, p->offset, p->index,
+ sizeof(gptdata), &gptdata) == 0)
+   print_gpt_guid(gptdata.guid);
+   }
+  disk->partition = p;
+}
+}
+
 static void
 probe_cryptodisk_uuid (grub_disk_t disk, char delim)
 {
@@ -635,6 +675,12 @@ probe (const char *path, char **device_names, char delim)
/* Check if dev->disk itself is contained in a partmap.  */
probe_partmap (dev->disk, delim);
 
+  else if (print == PRINT_PARTUUID)
+   {
+ probe_partuuid (dev->disk, delim);
+ putchar (delim);
+   }
+
   else if (print == PRINT_MSDOS_PARTTYPE)
{
  if (dev->disk->partition
-- 
2.16.3


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


[GRUB PARTUUID PATCH V8 2/4] Update grub_gpt_partentry

2018-03-28 Thread Nicholas Vinson
Rename grub_gpt_part_type to grub_gpt_part_guid and update
grub_gpt_partentry to use this type for both the partition type GUID
string and the partition GUID string entries.  This change ensures that
the two GUID fields are handled more consistently and helps to simplify
the changes needed to add Linux partition GUID support.

Signed-off-by: Nicholas Vinson 
Reviewed-by: Daniel Kiper 

---
 grub-core/disk/ldm.c | 2 +-
 grub-core/partmap/gpt.c  | 4 ++--
 include/grub/gpt_partition.h | 8 
 util/grub-install.c  | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 0f978ad05..2a22d2d6c 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -135,7 +135,7 @@ msdos_has_ldm_partition (grub_disk_t dsk)
   return has_ldm;
 }
 
-static const grub_gpt_part_type_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM;
+static const grub_gpt_part_guid_t ldm_type = GRUB_GPT_PARTITION_TYPE_LDM;
 
 /* Helper for gpt_ldm_sector.  */
 static int
diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
index 83bcba779..103f6796f 100644
--- a/grub-core/partmap/gpt.c
+++ b/grub-core/partmap/gpt.c
@@ -33,10 +33,10 @@ static grub_uint8_t grub_gpt_magic[8] =
 0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54
   };
 
-static const grub_gpt_part_type_t grub_gpt_partition_type_empty = 
GRUB_GPT_PARTITION_TYPE_EMPTY;
+static const grub_gpt_part_guid_t grub_gpt_partition_type_empty = 
GRUB_GPT_PARTITION_TYPE_EMPTY;
 
 #ifdef GRUB_UTIL
-static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot = 
GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
+static const grub_gpt_part_guid_t grub_gpt_partition_type_bios_boot = 
GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
 #endif
 
 /* 512 << 7 = 65536 byte sectors.  */
diff --git a/include/grub/gpt_partition.h b/include/grub/gpt_partition.h
index 1b32f6725..354fe2246 100644
--- a/include/grub/gpt_partition.h
+++ b/include/grub/gpt_partition.h
@@ -22,14 +22,14 @@
 #include 
 #include 
 
-struct grub_gpt_part_type
+struct grub_gpt_part_guid
 {
   grub_uint32_t data1;
   grub_uint16_t data2;
   grub_uint16_t data3;
   grub_uint8_t data4[8];
 } __attribute__ ((aligned(8)));
-typedef struct grub_gpt_part_type grub_gpt_part_type_t;
+typedef struct grub_gpt_part_guid grub_gpt_part_guid_t;
 
 #define GRUB_GPT_PARTITION_TYPE_EMPTY \
   { 0x0, 0x0, 0x0, \
@@ -70,8 +70,8 @@ struct grub_gpt_header
 
 struct grub_gpt_partentry
 {
-  grub_gpt_part_type_t type;
-  grub_uint8_t guid[16];
+  grub_gpt_part_guid_t type;
+  grub_gpt_part_guid_t guid;
   grub_uint64_t start;
   grub_uint64_t end;
   grub_uint64_t attrib;
diff --git a/util/grub-install.c b/util/grub-install.c
index 690f180c5..78d0138cb 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -714,7 +714,7 @@ is_prep_partition (grub_device_t dev)
   if (grub_disk_read (dev->disk, p->offset, p->index,
  sizeof (gptdata), &gptdata) == 0)
{
- const grub_gpt_part_type_t template = {
+ const grub_gpt_part_guid_t template = {
grub_cpu_to_le32_compile_time (0x9e1a2d38),
grub_cpu_to_le16_compile_time (0xc612),
grub_cpu_to_le16_compile_time (0x4316),
-- 
2.16.3


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


[GRUB PARTUUID PATCH V8 4/4] Update grub script template files

2018-03-28 Thread Nicholas Vinson
Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
partuuid target.  Update grub.texi documenation.

Signed-off-by: Nicholas Vinson 
---
 docs/grub.texi  |  9 +
 util/grub-mkconfig.in   |  3 +++
 util/grub.d/10_linux.in | 12 +---
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 65b4bbeda..019ce5d03 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1424,6 +1424,15 @@ the Linux kernel, using a @samp{root=UUID=...} kernel 
parameter.  This is
 usually more reliable, but in some cases it may not be appropriate.  To
 disable the use of UUIDs, set this option to @samp{true}.
 
+@item GRUB_ENABLE_LINUX_PARTUUID
+Setting this option to @samp{true} changes the behavior of
+@command{grub-mkconfig} so that it identifies the device containing the root
+filesystem by the partition UUID via the @samp{root=PARTUUID=...} kernel
+parameter.  This is desirable for Linux systems where identifying the root
+filesystem by its filesystem UUID or device name is inappropriate.  However,
+this option is only supported in Linux kernel versions 2.6.37 (3.10 for systems
+using the MSDOS partition scheme) or newer.
+
 @item GRUB_DISABLE_RECOVERY
 If this option is set to @samp{true}, disable the generation of recovery
 mode menu entries.
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 35ef583b0..9a1f92bdf 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -134,6 +134,7 @@ fi
 # Device containing our userland.  Typically used for root= parameter.
 GRUB_DEVICE="`${grub_probe} --target=device /`"
 GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE} --target=fs_uuid 2> 
/dev/null`" || true
+GRUB_DEVICE_PARTUUID="`${grub_probe} --device ${GRUB_DEVICE} --target=partuuid 
2> /dev/null`" || true
 
 # Device containing our /boot partition.  Usually the same as GRUB_DEVICE.
 GRUB_DEVICE_BOOT="`${grub_probe} --target=device /boot`"
@@ -188,6 +189,7 @@ if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then 
GRUB_ACTUAL_DEFAULT="`"${grub
 # override them.
 export GRUB_DEVICE \
   GRUB_DEVICE_UUID \
+  GRUB_DEVICE_PARTUUID \
   GRUB_DEVICE_BOOT \
   GRUB_DEVICE_BOOT_UUID \
   GRUB_FS \
@@ -223,6 +225,7 @@ export GRUB_DEFAULT \
   GRUB_TERMINAL_OUTPUT \
   GRUB_SERIAL_COMMAND \
   GRUB_DISABLE_LINUX_UUID \
+  GRUB_ENABLE_LINUX_PARTUUID \
   GRUB_DISABLE_RECOVERY \
   GRUB_VIDEO_BACKEND \
   GRUB_GFXMODE \
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index faedf74e1..53de33bea 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -45,12 +45,18 @@ esac
 
 # btrfs may reside on multiple devices. We cannot pass them as value of root= 
parameter
 # and mounting btrfs requires user space scanning, so force UUID in this case.
-if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = 
"xtrue" ] \
-|| ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
+if [ "x${GRUB_DEVICE_UUID}" = "x" ] && [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
+|| [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
+|| ( ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
+&& ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}" ) \
 || ( test -e "${GRUB_DEVICE}" && uses_abstraction "${GRUB_DEVICE}" lvm ); 
then
   LINUX_ROOT_DEVICE=${GRUB_DEVICE}
-else
+elif [ "x${GRUB_ENABLE_LINUX_PARTUUID}" != "xtrue" ] \
+|| [ "x${GRUB_DEVICE_PARTUUID}" = "x" ] \
+|| ! test -e "/dev/disk/by-partuuid/${GRUB_DEVICE_PARTUUID}"; then
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+else
+  LINUX_ROOT_DEVICE=PARTUUID=${GRUB_DEVICE_PARTUUID}
 fi
 
 case x"$GRUB_FS" in
-- 
2.16.3


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


[GRUB PARTUUID PATCH V8 0/4] Add PARTUUID detection support

2018-03-28 Thread Nicholas Vinson
Changes from Patch v7:
- Changed checks in probe_partuuid() to use the variable 'p' instead
  of disk->partition
- Moved 'disk->partition = p;' assignment to inside the
'p & p->parent == NULL' statement
- Moved 'disk->partition = p->parent;' assignment to before partmap
  name checks.
- Fixed formatting issues.
- Copied Daniel Kiper's reviewed-by line to unaltered patches.

Changes from Patch v6:
- Corrected spelling and grammatical errors in description text for
  GRUB_ENABLE_LINUX_PARTUUID
- Moved disk->partition save & restore logic to beginning and end of
  probe_partuuid()
- Fixed formatting errors in probe_partuuid

Changes from Patch v5:
- Added sign-off by lines
- Fixed formatting errors found by Daniel Kiper

Changes from Patch v4:
- Updated grub.texi to reflect new behavior for
  GRUB_ENABLE_LINUX_PARTUUID

- Updated 10_linux.in logic to favor the PARTUUID when
  GRUB_ENABLE_LINUX_PARTUUID is enabled and GRUB_DISABLE_LINUX_UUID
  is disabled.

Changes from Patch v3:
- Removed flex-2.6.3 compatibility patch

- Removed Steve Kenton's patch

Changes from Patch v2:
- Added flex-2.6.3 compatibility patch

- Fixed a GPT partition read error

- Added Steve Kenton's patch

- Changed struct grub_part_gpt_type name to struct
  grub_part_gpt_part_guid

- Changed grub_part_gpt_type_t typedef name to grub_part_gpt_guid_t

- Added sprint_gpt_guid to Steve Kenton's patch

- Updated v1 and Steve Kenton's patch to use similar methods when
  reading partition GUIDs.

Changes from Patch v1:
- Added GRUB_ENABLE_LINUX_PARTUUID variable description to grub.texi

- Removed added gpt_part_guid copy logic from
  grub_gpt_partition_map_iterate()

- Removed added NT disk signature copy logic from
  grub_partition_msdos_iterate()

- Removed modifications to partition number increment logic

- Removed added guid union definition.

- Added GRUB_ENABLE_LINUX_PARTUUID to grub-mkconfig.in export list

- Moved PRINT_GPT_PARTTYPE printing logic to print_gpt_guid()
  function in grub-probe.c

- Updated PRINT_GPT_PARTTYPE case to call print_gpt_guid() function
  in grub-probe.c.

- Created probe_partuuid() function in grub-probe.c

- Updated print == PRINT_PARTUUID check logic in probe() to call
  probe_partuuid().

- Updated UUID logic in 10_linux.in to enable root=PARTUUID feature
  only if GRUB_DISABLE_LINUX_UUID is not set to true,
  and GRUB_DEVICE_PARTUUID is not empty, GRUB_ENABLE_LINUX_PARTUUID
  is set to true.

Hello,

This is a request to add PARTUUID detection support grub-probe for MBR
and GPT partition schemes.  The Linux kernel supports mounting the root
filesystem by Linux device name or by the Partition [GU]UID.  GRUB's
mkconfig, however, currently only supports specifying the rootfs in the
kernel command-line by Linux device name unless an initramfs is also
present.  When an initramfs is present GRUB's mkconfig will set the
kernel's root parameter value to either the Linux device name or to the
filesystem [GU]UID.

Therefore, the only way to protect a Linux system from failing to boot
when its Linux storage device names change is to either manually edit
grub.cfg or /etc/default/grub and append root=PARTUUID=xxx to the
command-line or create an initramfs that understands how to mount
devices by filesystem [G]UID and let grub-mkconfig pass the filesystem
[GU]UID to the initramfs.

The goal of this patch set is to enable root=PARTUUID=xxx support in
grub-mkconfig, so that users don't have to manually edit
/etc/default/grub or grub.cfg, or create an initramfs for the sole
purpose of having a robust bootloader configuration for Linux.

Thanks,
Nicholas Vinson

Nicholas Vinson (4):
  Centralize guid prints
  Update grub_gpt_partentry
  Add PARTUUID detection support to grub-probe
  Update grub script template files

 docs/grub.texi   |  9 ++
 grub-core/disk/ldm.c |  2 +-
 grub-core/partmap/gpt.c  |  4 +--
 include/grub/gpt_partition.h |  8 ++---
 util/grub-install.c  |  2 +-
 util/grub-mkconfig.in|  3 ++
 util/grub-probe.c| 76 +++-
 util/grub.d/10_linux.in  | 12 +--
 8 files changed, 90 insertions(+), 26 deletions(-)

-- 
2.16.3


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


[GRUB PARTUUID PATCH V8 1/4] Centralize guid prints

2018-03-28 Thread Nicholas Vinson
Define print_gpt_guid(), so there is a central function for printing
GUID strings.  This change is a precursor for later patches which rely
on this logic.

Signed-off-by: Nicholas Vinson 
Reviewed-by: Daniel Kiper 

---
 util/grub-probe.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index 8ac527d2f..21cb80fbe 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -129,6 +129,20 @@ get_targets_string (void)
   return str;
 }
 
+static int
+print_gpt_guid (grub_gpt_part_guid_t guid)
+{
+  guid.data1 = grub_le_to_cpu32 (guid.data1);
+  guid.data2 = grub_le_to_cpu16 (guid.data2);
+  guid.data3 = grub_le_to_cpu16 (guid.data3);
+
+  return grub_printf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
+ guid.data1, guid.data2, guid.data3, guid.data4[0],
+ guid.data4[1], guid.data4[2], guid.data4[3],
+ guid.data4[4], guid.data4[5], guid.data4[6],
+ guid.data4[7]);
+}
+
 static void
 do_print (const char *x, void *data)
 {
@@ -641,21 +655,7 @@ probe (const char *path, char **device_names, char delim)
 
   if (grub_disk_read (dev->disk, p->offset, p->index,
   sizeof (gptdata), &gptdata) == 0)
-{
-  grub_gpt_part_type_t gpttype;
-  gpttype.data1 = grub_le_to_cpu32 (gptdata.type.data1);
-  gpttype.data2 = grub_le_to_cpu16 (gptdata.type.data2);
-  gpttype.data3 = grub_le_to_cpu16 (gptdata.type.data3);
-  grub_memcpy (gpttype.data4, gptdata.type.data4, 8);
-
-  grub_printf 
("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-   gpttype.data1, gpttype.data2,
-   gpttype.data3, gpttype.data4[0], 
-   gpttype.data4[1], gpttype.data4[2],
-   gpttype.data4[3], gpttype.data4[4],
-   gpttype.data4[5], gpttype.data4[6],
-   gpttype.data4[7]);
-}
+   print_gpt_guid(gptdata.type);
   dev->disk->partition = p;
 }
   putchar (delim);
-- 
2.16.3


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


Re: [PATCH 0/4] Make hidden menu really hidden

2018-03-28 Thread Daniel Kiper
Hi Hans,

On Wed, Mar 28, 2018 at 04:50:24PM +0200, Hans de Goede wrote:
> Hi All,
>
> Let me start with a quick self-intro:
>
> I'm a FOSS enthusiast / developer working for Red Hat, my latest project
> at Red Hat is chasing what has over the years become the magical unicorn
> of desktop Linux distros: a smooth graphical boot where the machine goes
> from the BIOS screen to the graphical login manager in one smooth flow,
> without any (80x25) text mode messages or black-screens in between.
>
> These 4 patches modify grub so that when timeout_style=hidden is used,
> and the user does not press a key to show the menu the EFI display is never
> switched to text mode and the vendor logo stays in place on the monitor.
>
> The first patch adds a new "version" command, this is not really related
> to the other 3 patches, but when I started working on grub I wanted to
> verify I was running my own build and I was surprised there was no such
> command, so I added one.
>
> The second patch is also only somewhat related, if the menu-timeout is
> short, one way to still make sure you get the grub menu is to start
> pressing the ESC key before grub is loaded, but on some systems ESC is
> the hotkey to enter the firmware setup, so this commit also adds support
> for pressing F8 to ge the menu. F8 is used by the Windows boot menu, so
> almost all x86 firmwares don't use this key for their own purposes. This
> will also make it easier for users coming from Windows to get the menu
> (if they know that F8 is used for the boot menu Windows).
>
> The third patch makes changes which a lot of distros have been carrying
> as distro patches since 2013 at least, these changes make grub be quiet
> during boot, except for the menu. These changes were not universally liked
> in the past, because they may make debugging boot problems harder in some
> cases, so they have never been merged.
>
> This version of these changes makes the quiet behavior configurable
> through a ./configure option which defaults to the old verbose behavior,
> which should hopefully make everyone happy. This patch is a mix of Fedora
> and Ubuntu patches for this, picking the best of both.
>
> The fourth patch modifies the EFI terminal code to not switch the EFI
> display to textmode until the first text is output. Note that for this
> patch to actually make a difference, no text must be output, so we need
> the "quiet" patch to be enabled and timeout_style=hidden. If any error
> (or other output happens) grub will immediately switch to text mode and
> show the message to the user.

I will take a look at the patches in a week or so. Please be patient.

Daniel

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


Re: [PATCH 2/4] Accept Both ESC and F8 as user interrupt keys

2018-03-28 Thread Lennart Sorensen
On Wed, Mar 28, 2018 at 05:06:53PM +0200, Hans de Goede wrote:
> Hmm, well people will still be able to use ESC to get the grub boot
> menu, rather then the firmware boot-menu on those.
> 
> The problem is that our current check for ESC only approach is problematic
> because it conflicts with the enter firmware-setup key on almost all Bay 
> Trail,
> Cherry Trail and Apollo Lake devices. You need to try hard to find a device
> in one of those 3 categories which does not use ESC for this. Note I'm
> aware some devices exist, but using ESC for this is really really common
> among these devices.
> 
> I'm open to other suggestions, but I think we really need to add another
> key to avoid the pressing ESC at boot already has another meaning problem
> and F8 seems like an ok choice.
> 
> Either way thank you for the input on this.

I don't disagree about having another key, I just disagree that F8 is
a good choice.

Looking at https://kb.wisc.edu/page.php?id=58779#here it appears no one
uses F4 for anything.  That would seem like a better choice than F8
at least.  F6 also seems to be free but F4 seems nicer on desktop
keyboards.

-- 
Len Sorensen

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


Re: [PATCH 2/4] Accept Both ESC and F8 as user interrupt keys

2018-03-28 Thread Hans de Goede

Hi,

On 28-03-18 16:56, Lennart Sorensen wrote:

On Wed, Mar 28, 2018 at 04:50:26PM +0200, Hans de Goede wrote:

On most Bay Trail, Cherry Trail and Apollo Lake devices the ESC key is
the hotkey to enter the BIOS/EFI setup screen.

This makes it hard for the user to show the grub-menu when it is hidden
and a short timeout is used, because pressing ESC too early leads to the
user entering the BIOS/EFI setup screen.

F8 is (almost?) always free (on X86/PC platforms) as Windows uses this for
its boot menu, so also accept F8 as interrupt/show-menu key. As an added
advantage this is also more discoverable / easier for users coming from
Windows.


I have seen F8 and F12 used as boot menu keys on many systems.  I would
not consider it usually free.  Maybe it has become less common after
windows started using it, but it isn't that unusual.  ASUS seems to be
a major user of F8 for boot menu.  Not exactly an uncommon system board.
Some Lenovo desktops also use F8 for the boot menu.


Hmm, well people will still be able to use ESC to get the grub boot
menu, rather then the firmware boot-menu on those.

The problem is that our current check for ESC only approach is problematic
because it conflicts with the enter firmware-setup key on almost all Bay Trail,
Cherry Trail and Apollo Lake devices. You need to try hard to find a device
in one of those 3 categories which does not use ESC for this. Note I'm
aware some devices exist, but using ESC for this is really really common
among these devices.

I'm open to other suggestions, but I think we really need to add another
key to avoid the pressing ESC at boot already has another meaning problem
and F8 seems like an ok choice.

Either way thank you for the input on this.

Regards,

Hans

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


Re: [PATCH 2/4] Accept Both ESC and F8 as user interrupt keys

2018-03-28 Thread Lennart Sorensen
On Wed, Mar 28, 2018 at 04:50:26PM +0200, Hans de Goede wrote:
> On most Bay Trail, Cherry Trail and Apollo Lake devices the ESC key is
> the hotkey to enter the BIOS/EFI setup screen.
> 
> This makes it hard for the user to show the grub-menu when it is hidden
> and a short timeout is used, because pressing ESC too early leads to the
> user entering the BIOS/EFI setup screen.
> 
> F8 is (almost?) always free (on X86/PC platforms) as Windows uses this for
> its boot menu, so also accept F8 as interrupt/show-menu key. As an added
> advantage this is also more discoverable / easier for users coming from
> Windows.

I have seen F8 and F12 used as boot menu keys on many systems.  I would
not consider it usually free.  Maybe it has become less common after
windows started using it, but it isn't that unusual.  ASUS seems to be
a major user of F8 for boot menu.  Not exactly an uncommon system board.
Some Lenovo desktops also use F8 for the boot menu.

-- 
Len Sorensen

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


[PATCH 3/4] Optionally print less messages at boot

2018-03-28 Thread Hans de Goede
The patch optionally makes grub not show any text (be fully quiet) when
timeout_style=hidden is set and the user does not interrupt the boot.

Combined with a later patch in this series which makes grub not touch
the EFI console unless it actually has some text to print, this will keep
the vendor logo which EFI put on the display in place until the kernel
touches the display. Leading to a more smooth / seamless boot experience.

At least Fedora/RHEL/CentOS and Ubuntu have been carrying patches for this
for a long time now (since 2013). There have been several attempts to
upstream these patches in the past already, which have been rejected
because not everyone likes the quiet behavior.

This patch makes the quiet behavior optional and defaults to off, so
unless grub is compiled with the new --enable-quiet-boot configure option
this patch changes nothing.

This patch is a mix of the Fedora patches for this and:
https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/tree/debian/patches/maybe_quiet.patch
https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/tree/debian/patches/gettext_quiet.patch
Specifically the configure.ac changes for making this optional come from
the Ubuntu patches, as Fedora's patches where simply unconditionally
removing all the unwanted grub_printf calls.

Cc: Colin Watson 
Signed-off-by: Hans de Goede 
---
 config.h.in   |  2 ++
 configure.ac  | 16 
 grub-core/boot/i386/pc/boot.S |  2 ++
 grub-core/boot/i386/pc/diskboot.S | 12 +---
 grub-core/gettext/gettext.c   |  7 +++
 grub-core/kern/main.c |  2 ++
 grub-core/normal/menu.c   |  2 ++
 grub-core/normal/menu_entry.c |  2 ++
 util/grub.d/10_linux.in   | 19 ++-
 9 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/config.h.in b/config.h.in
index 9e8f9911b..d2c4ce8e5 100644
--- a/config.h.in
+++ b/config.h.in
@@ -12,6 +12,8 @@
 /* Define to 1 to enable disk cache statistics.  */
 #define DISK_CACHE_STATS @DISK_CACHE_STATS@
 #define BOOT_TIME_STATS @BOOT_TIME_STATS@
+/* Define to 1 to make GRUB quieter at boot time.  */
+#define QUIET_BOOT @QUIET_BOOT@
 
 /* We don't need those.  */
 #define MINILZO_CFG_SKIP_LZO_PTR 1
diff --git a/configure.ac b/configure.ac
index c7888e40f..a544080d7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1834,6 +1834,17 @@ fi
 AC_SUBST([LIBZFS])
 AC_SUBST([LIBNVPAIR])
 
+AC_ARG_ENABLE([quiet-boot],
+  [AS_HELP_STRING([--enable-quiet-boot],
+  [emit fewer messages at boot time 
(default=no)])],
+  [], [enable_quiet_boot=no])
+if test x"$enable_quiet_boot" = xyes ; then
+  QUIET_BOOT=1
+else
+  QUIET_BOOT=0
+fi
+AC_SUBST([QUIET_BOOT])
+
 LIBS=""
 
 AC_SUBST([FONT_SOURCE])
@@ -2086,5 +2097,10 @@ echo "Without liblzma (no support for XZ-compressed mips 
images) ($liblzma_excus
 else
 echo "With liblzma from $LIBLZMA (support for XZ-compressed mips images)"
 fi
+if [ x"$enable_quiet_boot" = xyes ]; then
+echo With quiet boot: Yes
+else
+echo With quiet boot: No
+fi
 echo "***"
 ]
diff --git a/grub-core/boot/i386/pc/boot.S b/grub-core/boot/i386/pc/boot.S
index 2bd0b2d28..a0b023589 100644
--- a/grub-core/boot/i386/pc/boot.S
+++ b/grub-core/boot/i386/pc/boot.S
@@ -249,8 +249,10 @@ real_start:
/* save drive reference first thing! */
pushw   %dx
 
+#if !QUIET_BOOT
/* print a notification message on the screen */
MSG(notification_string)
+#endif
 
/* set %si to the disk address packet */
movw$disk_address_packet, %si
diff --git a/grub-core/boot/i386/pc/diskboot.S 
b/grub-core/boot/i386/pc/diskboot.S
index 1ee4cf5b2..9fb44acbb 100644
--- a/grub-core/boot/i386/pc/diskboot.S
+++ b/grub-core/boot/i386/pc/diskboot.S
@@ -23,7 +23,13 @@
  *  defines for the code go here
  */
 
+#if !QUIET_BOOT
 #define MSG(x) movw $x, %si; call LOCAL(message)
+#else
+#define MSG(x)
+#endif
+
+#define ERR(x) movw $x, %si; call LOCAL(message)
 
.file   "diskboot.S"
 
@@ -305,17 +311,17 @@ LOCAL(bootit):
  * BIOS Geometry translation error (past the end of the disk geometry!).
  */
 LOCAL(geometry_error):
-   MSG(geometry_error_string)
+   ERR(geometry_error_string)
jmp LOCAL(general_error)
 
 /*
  * Read error on the disk.
  */
 LOCAL(read_error):
-   MSG(read_error_string)
+   ERR(read_error_string)
 
 LOCAL(general_error):
-   MSG(general_error_string)
+   ERR(general_error_string)
 
 /* go here when you need to stop the machine hard after an error condition */
 LOCAL(stop):   jmp LOCAL(stop)
diff --git a/grub-core/gettext/gettext.c b/grub-core/gettext/gettext.c
index 4880cefe3..f0e7a24ed 100644
--- a/grub-core/gettext/gettext.c
+++ b/grub-core/gettext/gettext.c
@@ -427,6 +427,13 @@ grub_gettext_init_ext (struct grub_gettext_context *ctx,
   if (locale[0] == 'e' && locale[1] == 'n'
   && (locale[2

[PATCH 4/4] EFI: Do not set text-mode until we actually need it

2018-03-28 Thread Hans de Goede
If we're running with a hidden menu we may never need text mode, so do not
change the video-mode to text until we actually need it.

Signed-off-by: Hans de Goede 
---
 grub-core/term/efi/console.c | 72 +++-
 1 file changed, 47 insertions(+), 25 deletions(-)

diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index 02f64ea74..d9fd7cf48 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -24,6 +24,11 @@
 #include 
 #include 
 
+static grub_err_t grub_prepare_for_text_output(struct grub_term_output *term);
+
+static int text_mode_available = -1;
+static int text_colorstate = -1;
+
 static grub_uint32_t
 map_char (grub_uint32_t c)
 {
@@ -66,14 +71,14 @@ map_char (grub_uint32_t c)
 }
 
 static void
-grub_console_putchar (struct grub_term_output *term __attribute__ ((unused)),
+grub_console_putchar (struct grub_term_output *term,
  const struct grub_unicode_glyph *c)
 {
   grub_efi_char16_t str[2 + 30];
   grub_efi_simple_text_output_interface_t *o;
   unsigned i, j;
 
-  if (grub_efi_is_finished)
+  if (grub_prepare_for_text_output (term))
 return;
 
   o = grub_efi_system_table->con_out;
@@ -220,14 +225,15 @@ grub_console_getkey (struct grub_term_input *term)
 }
 
 static struct grub_term_coordinate
-grub_console_getwh (struct grub_term_output *term __attribute__ ((unused)))
+grub_console_getwh (struct grub_term_output *term)
 {
   grub_efi_simple_text_output_interface_t *o;
   grub_efi_uintn_t columns, rows;
 
   o = grub_efi_system_table->con_out;
-  if (grub_efi_is_finished || efi_call_4 (o->query_mode, o, o->mode->mode,
- &columns, &rows) != GRUB_EFI_SUCCESS)
+  if (grub_prepare_for_text_output (term) != GRUB_ERR_NONE ||
+  efi_call_4 (o->query_mode, o, o->mode->mode,
+ &columns, &rows) != GRUB_EFI_SUCCESS)
 {
   /* Why does this fail?  */
   columns = 80;
@@ -238,11 +244,11 @@ grub_console_getwh (struct grub_term_output *term 
__attribute__ ((unused)))
 }
 
 static struct grub_term_coordinate
-grub_console_getxy (struct grub_term_output *term __attribute__ ((unused)))
+grub_console_getxy (struct grub_term_output *term)
 {
   grub_efi_simple_text_output_interface_t *o;
 
-  if (grub_efi_is_finished)
+  if (grub_efi_is_finished || text_mode_available != 1)
 return (struct grub_term_coordinate) { 0, 0 };
 
   o = grub_efi_system_table->con_out;
@@ -250,12 +256,12 @@ grub_console_getxy (struct grub_term_output *term 
__attribute__ ((unused)))
 }
 
 static void
-grub_console_gotoxy (struct grub_term_output *term __attribute__ ((unused)),
+grub_console_gotoxy (struct grub_term_output *term,
 struct grub_term_coordinate pos)
 {
   grub_efi_simple_text_output_interface_t *o;
 
-  if (grub_efi_is_finished)
+  if (grub_prepare_for_text_output (term))
 return;
 
   o = grub_efi_system_table->con_out;
@@ -268,7 +274,7 @@ grub_console_cls (struct grub_term_output *term 
__attribute__ ((unused)))
   grub_efi_simple_text_output_interface_t *o;
   grub_efi_int32_t orig_attr;
 
-  if (grub_efi_is_finished)
+  if (grub_efi_is_finished || text_mode_available != 1)
 return;
 
   o = grub_efi_system_table->con_out;
@@ -279,8 +285,7 @@ grub_console_cls (struct grub_term_output *term 
__attribute__ ((unused)))
 }
 
 static void
-grub_console_setcolorstate (struct grub_term_output *term
-   __attribute__ ((unused)),
+grub_console_setcolorstate (struct grub_term_output *term,
grub_term_color_state state)
 {
   grub_efi_simple_text_output_interface_t *o;
@@ -288,6 +293,12 @@ grub_console_setcolorstate (struct grub_term_output *term
   if (grub_efi_is_finished)
 return;
 
+  if (text_mode_available != 1) {
+/* Avoid "color_normal" environment writes causing a switch to textmode */
+text_colorstate = state;
+return;
+  }
+
   o = grub_efi_system_table->con_out;
 
   switch (state) {
@@ -307,12 +318,12 @@ grub_console_setcolorstate (struct grub_term_output *term
 }
 
 static void
-grub_console_setcursor (struct grub_term_output *term __attribute__ ((unused)),
+grub_console_setcursor (struct grub_term_output *term,
int on)
 {
   grub_efi_simple_text_output_interface_t *o;
 
-  if (grub_efi_is_finished)
+  if (grub_efi_is_finished || text_mode_available != 1)
 return;
 
   o = grub_efi_system_table->con_out;
@@ -320,18 +331,38 @@ grub_console_setcursor (struct grub_term_output *term 
__attribute__ ((unused)),
 }
 
 static grub_err_t
-grub_efi_console_output_init (struct grub_term_output *term)
+grub_prepare_for_text_output(struct grub_term_output *term)
 {
-  grub_efi_set_text_mode (1);
+  if (grub_efi_is_finished)
+return GRUB_ERR_BAD_DEVICE;
+
+  if (text_mode_available != -1)
+return text_mode_available ? 0 : GRUB_ERR_BAD_DEVICE;
+
+  if (! grub_efi_set_text_mode (1))
+{
+  /* This really should never happen */
+

[PATCH 2/4] Accept Both ESC and F8 as user interrupt keys

2018-03-28 Thread Hans de Goede
On most Bay Trail, Cherry Trail and Apollo Lake devices the ESC key is
the hotkey to enter the BIOS/EFI setup screen.

This makes it hard for the user to show the grub-menu when it is hidden
and a short timeout is used, because pressing ESC too early leads to the
user entering the BIOS/EFI setup screen.

F8 is (almost?) always free (on X86/PC platforms) as Windows uses this for
its boot menu, so also accept F8 as interrupt/show-menu key. As an added
advantage this is also more discoverable / easier for users coming from
Windows.

Signed-off-by: Hans de Goede 
---
 grub-core/commands/sleep.c | 8 ++--
 grub-core/normal/menu.c| 7 ---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/grub-core/commands/sleep.c b/grub-core/commands/sleep.c
index e77e7900f..9f16956e0 100644
--- a/grub-core/commands/sleep.c
+++ b/grub-core/commands/sleep.c
@@ -51,12 +51,16 @@ static int
 grub_interruptible_millisleep (grub_uint32_t ms)
 {
   grub_uint64_t start;
+  int key;
 
   start = grub_get_time_ms ();
 
-  while (grub_get_time_ms () - start < ms)
-if (grub_getkey_noblock () == GRUB_TERM_ESC)
+  while (grub_get_time_ms () - start < ms) {
+key = grub_getkey_noblock ();
+/* ESC sometimes is the BIOS setup hotkey, also allow F8 as intr. */
+if (key == GRUB_TERM_ESC || key == GRUB_TERM_KEY_F8)
   return 1;
+  }
 
   return 0;
 }
diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
index e7a83c2d6..d813fade1 100644
--- a/grub-core/normal/menu.c
+++ b/grub-core/normal/menu.c
@@ -610,8 +610,8 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
  print_countdown (pos, timeout);
}
 
-  /* Enter interruptible sleep until Escape or a menu hotkey is pressed,
- or the timeout expires.  */
+  /* Sleep until a menu hotkey is pressed, we are interrupted by an ESC/F8
+ keypress, or the timeout expires. */
   saved_time = grub_get_time_ms ();
   while (1)
{
@@ -624,7 +624,8 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
  if (entry >= 0)
break;
}
- if (key == GRUB_TERM_ESC)
+ /* ESC sometimes is the BIOS setup hotkey, also allow F8 as intr. */
+ if (key == GRUB_TERM_ESC || key == GRUB_TERM_KEY_F8)
{
  timeout = -1;
  break;
-- 
2.17.0.rc1


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


[PATCH 1/4] Add new "version" command

2018-03-28 Thread Hans de Goede
Add a new "version" command which prints the grub PACKAGE_STRING +
build time and date. This is useful to check if the expected version is
running, for e.g. trouble-shooting purposes.

Signed-off-by: Hans de Goede 
---
 grub-core/kern/corecmd.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/grub-core/kern/corecmd.c b/grub-core/kern/corecmd.c
index d9412a316..43273b901 100644
--- a/grub-core/kern/corecmd.c
+++ b/grub-core/kern/corecmd.c
@@ -170,6 +170,15 @@ grub_core_cmd_ls (struct grub_command *cmd __attribute__ 
((unused)),
   return grub_errno;
 }
 
+/* version */
+static grub_err_t
+grub_core_cmd_version (struct grub_command *cmd __attribute__ ((unused)),
+  int argc, char *argv[])
+{
+  grub_printf ("%s, build %s %s\n", PACKAGE_STRING, __DATE__, __TIME__);
+  return 0;
+}
+
 void
 grub_register_core_commands (void)
 {
@@ -186,4 +195,6 @@ grub_register_core_commands (void)
 N_("[ARG]"), N_("List devices or files."));
   grub_register_command ("insmod", grub_core_cmd_insmod,
 N_("MODULE"), N_("Insert a module."));
+  grub_register_command ("version", grub_core_cmd_version, 0,
+N_("Print grub version and build time."));
 }
-- 
2.17.0.rc1


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


[PATCH 0/4] Make hidden menu really hidden

2018-03-28 Thread Hans de Goede
Hi All,

Let me start with a quick self-intro:

I'm a FOSS enthusiast / developer working for Red Hat, my latest project
at Red Hat is chasing what has over the years become the magical unicorn
of desktop Linux distros: a smooth graphical boot where the machine goes
from the BIOS screen to the graphical login manager in one smooth flow,
without any (80x25) text mode messages or black-screens in between.

These 4 patches modify grub so that when timeout_style=hidden is used,
and the user does not press a key to show the menu the EFI display is never
switched to text mode and the vendor logo stays in place on the monitor.

The first patch adds a new "version" command, this is not really related
to the other 3 patches, but when I started working on grub I wanted to
verify I was running my own build and I was surprised there was no such
command, so I added one.

The second patch is also only somewhat related, if the menu-timeout is
short, one way to still make sure you get the grub menu is to start
pressing the ESC key before grub is loaded, but on some systems ESC is
the hotkey to enter the firmware setup, so this commit also adds support
for pressing F8 to ge the menu. F8 is used by the Windows boot menu, so
almost all x86 firmwares don't use this key for their own purposes. This
will also make it easier for users coming from Windows to get the menu
(if they know that F8 is used for the boot menu Windows).

The third patch makes changes which a lot of distros have been carrying
as distro patches since 2013 at least, these changes make grub be quiet
during boot, except for the menu. These changes were not universally liked
in the past, because they may make debugging boot problems harder in some
cases, so they have never been merged.

This version of these changes makes the quiet behavior configurable
through a ./configure option which defaults to the old verbose behavior,
which should hopefully make everyone happy. This patch is a mix of Fedora
and Ubuntu patches for this, picking the best of both.

The fourth patch modifies the EFI terminal code to not switch the EFI
display to textmode until the first text is output. Note that for this
patch to actually make a difference, no text must be output, so we need
the "quiet" patch to be enabled and timeout_style=hidden. If any error
(or other output happens) grub will immediately switch to text mode and
show the message to the user.

Regards,

Hans


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


stale comment in diskboot.S ?

2018-03-28 Thread Cao jin
Hi,

I was reading the code, and I want to confirm is following comment is stale?

/*
 * _start is loaded at 0x2000 and is jumped to with
 * CS:IP 0:0x2000 in kernel.
 */

As I understand, it is loaded at 0x8000 and is jumped to with 0:0x8000.
-- 
Sincerely,
Cao jin



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


Re: [PATCH] F2FS support

2018-03-28 Thread Daniel Kiper
On Thu, Mar 22, 2018 at 04:47:47PM +, Pete Batard wrote:
> From 40030514e682191281e8ddba8d1e0835e6b685dc Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim 
> Date: Thu, 4 May 2017 19:12:00 +0100
> Subject: [PATCH] F2FS support
>
> "F2FS (Flash-Friendly File System) is flash-friendly file system which was 
> merged
> into Linux kernel v3.8 in 2013.
>
> The motive for F2FS was to build a file system that from the start, takes into
> account the characteristics of NAND flash memory-based storage devices (such 
> as
> solid-state disks, eMMC, and SD cards).
>
> F2FS was designed on a basis of a log-structured file system approach, which
> remedies some known issues of the older log structured file systems, such as
> the snowball effect of wandering trees and high cleaning overhead. In 
> addition,
> since a NAND-based storage device shows different characteristics according to
> its internal geometry or flash memory management scheme (such as the Flash
> Translation Layer or FTL), it supports various parameters not only for
> configuring on-disk layout, but also for selecting allocation and cleaning
> algorithm.", quote by https://en.wikipedia.org/wiki/F2FS.
>
> The source codes for F2FS are available from:
>
> http://git.kernel.org/cgit/linux/kernel/git/jaegeuk/f2fs.git
> http://git.kernel.org/cgit/linux/kernel/git/jaegeuk/f2fs-tools.git
>
> Update:
>  - This patch has been integrated in OpenMandriva Lx 3.
>https://www.openmandriva.org/
>
> Acked-by: Andrei Borzenkov 

Please drop this Acked-by. I will ask you to do some changes, mostly
nitpicks, and this means that it is no longer valid.

> Signed-off-by: Jaegeuk Kim 

Lack of your SOB.

[...]

> diff --git a/docs/grub.texi b/docs/grub.texi
> index 65b4bbe..5afdc5a 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -360,7 +360,8 @@ blocklist notation. The currently supported filesystem 
> types are @dfn{Amiga
>  Fast FileSystem (AFFS)}, @dfn{AtheOS fs}, @dfn{BeFS},
>  @dfn{BtrFS} (including raid0, raid1, raid10, gzip and lzo),
>  @dfn{cpio} (little- and big-endian bin, odc and newc variants),
> -@dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32}, @dfn{exFAT}, 
> @dfn{HFS},
> +@dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32}, @dfn{exFAT},
> +@dfn{f2fs}, @dfn{HFS},
>  @dfn{HFS+}, @dfn{ISO9660} (including Joliet, Rock-ridge and multi-chunk 
> files),

I would like to see this in one line:

@dfn{exFAT}, @dfn{f2fs}, @dfn{HFS}, @dfn{HFS+},

Hmmm... s/f2fs/F2FS/?

>  @dfn{JFS}, @dfn{Minix fs} (versions 1, 2 and 3), @dfn{nilfs2},
>  @dfn{NTFS} (including compression), @dfn{ReiserFS}, @dfn{ROMFS},
> @@ -5375,7 +5376,7 @@ NTFS, JFS, UDF, HFS+, exFAT, long filenames in FAT, 
> Joliet part of
>  ISO9660 are treated as UTF-16 as per specification. AFS and BFS are read
>  as UTF-8, again according to specification. BtrFS, cpio, tar, squash4, minix,
>  minix2, minix3, ROMFS, ReiserFS, XFS, ext2, ext3, ext4, FAT (short names),
> -RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are assumed
> +f2fs, RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are assumed

s/f2fs/F2FS/?

>  to be UTF-8. This might be false on systems configured with legacy charset
>  but as long as the charset used is superset of ASCII you should be able to
>  access ASCII-named files. And it's recommended to configure your system to 
> use
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 2c1d62c..fc4767f 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1315,6 +1315,11 @@ module = {
>  };
>
>  module = {
> +  name = f2fs;
> +  common = fs/f2fs.c;
> +};
> +
> +module = {
>name = fshelp;
>common = fs/fshelp.c;
>  };
> diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c
> new file mode 100644
> index 000..7fb256f
> --- /dev/null
> +++ b/grub-core/fs/f2fs.c
> @@ -0,0 +1,1289 @@
> +/*
> + *  f2fs.c - Flash-Friendly File System
> + *
> + *  Written by Jaegeuk Kim 
> + *
> + *  Copyright (C) 2015  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 .
> + */

Lack of empty line.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +/* F2FS Magic Number */
> +#define F2FS_SUPER_MAGIC 0xF2F52010
> +#define CHECKSUM_OFFSET  4092/* 

Re: [GRUB PARTUUID PATCH V7 3/4] Add PARTUUID detection support to grub-probe

2018-03-28 Thread Daniel Kiper
On Tue, Mar 27, 2018 at 09:30:10PM -0700, Nick Vinson wrote:
> On 03/27/2018 01:52 PM, Daniel Kiper wrote:
> > On Mon, Mar 26, 2018 at 11:07:58PM -0700, Nicholas Vinson wrote:
> >> Add PARTUUID detection support grub-probe for MBR and GPT partition
> >> schemes.
> >>
> >> Signed-off-by: Nicholas Vinson 
> >> ---
> >>  util/grub-probe.c | 48 
> >>  1 file changed, 48 insertions(+)
> >>
> >> diff --git a/util/grub-probe.c b/util/grub-probe.c
> >> index 21cb80fbe..48ef1e2ec 100644
> >> --- a/util/grub-probe.c
> >> +++ b/util/grub-probe.c
> >> @@ -28,6 +28,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -62,6 +63,7 @@ enum {
> >>PRINT_DRIVE,
> >>PRINT_DEVICE,
> >>PRINT_PARTMAP,
> >> +  PRINT_PARTUUID,
> >>PRINT_ABSTRACTION,
> >>PRINT_CRYPTODISK_UUID,
> >>PRINT_HINT_STR,
> >> @@ -85,6 +87,7 @@ static const char *targets[] =
> >>  [PRINT_DRIVE]  = "drive",
> >>  [PRINT_DEVICE] = "device",
> >>  [PRINT_PARTMAP]= "partmap",
> >> +[PRINT_PARTUUID]   = "partuuid",
> >>  [PRINT_ABSTRACTION]= "abstraction",
> >>  [PRINT_CRYPTODISK_UUID]= "cryptodisk_uuid",
> >>  [PRINT_HINT_STR]   = "hints_string",
> >> @@ -181,6 +184,45 @@ probe_partmap (grub_disk_t disk, char delim)
> >>  }
> >>  }
> >>
> >> +static void
> >> +probe_partuuid (grub_disk_t disk, char delim)
> >> +{
> >> +  grub_partition_t p = disk->partition;
> >
> > Lack of empty line.
>
> added an empty line.
>
> >
> >> +  /*
> >> +   * Nested partitions not supported for now.
> >> +   * Non-nested partitions must have disk->partition->parent == NULL
> >> +   */
> >> +  if (disk->partition && disk->partition->parent == NULL)
> >> +{
> >> +  if (strcmp(disk->partition->partmap->name, "msdos") == 0)
> >> +  {
> >> +  /*
> >> +   * The partition GUID for MSDOS is the partition number (starting
> >> +   * with 1) prepended with the NT disk signature.
> >> +   */
> >> +  grub_uint32_t nt_disk_sig;
> >> +  disk->partition = p->parent;
> >> +
> >> +  if (grub_disk_read (disk, 0, GRUB_BOOT_MACHINE_WINDOWS_NT_MAGIC,
> >> +  sizeof(nt_disk_sig), &nt_disk_sig) == 0)
> >> +
> >
> > Redundant empty line.
>
> removed the empty line.
> >
> >> +  grub_printf ("%08x-%02x",
> >> +   grub_le_to_cpu32(nt_disk_sig), 1 + p->number);
> >> +  }
> >> +  else if (strcmp(disk->partition->partmap->name, "gpt") == 0)
> >> +  {
> >> +struct grub_gpt_partentry gptdata;
> >> +
> >> +disk->partition = p->parent;
> >> +
> >> +if (grub_disk_read (disk, p->offset, p->index,
> >> +sizeof(gptdata), &gptdata) == 0)
> >> +  print_gpt_guid(gptdata.guid);
> >> +  }
> >
> > Why "disk->partition = p;" is not here?
> > Because compiler complains if it is here?
>
> I misread my own diff and thought you had asked for me to put it below
> instead of here.  Either location works, so it's not too critical where
> it's put.

Great! So, please put it there where I have asked for earlier.

> > Anyway, if I know the reason for above I can fix
> > earlier ntipicks myself before commiting this patch.>
> > Well, "disk->partition = p->parent;" can be before
> > "if (strcmp(disk->partition->partmap->name, "msdos") == 0)".
> > Am I right?
>
> Yes, but it'll require some changes to the checks to make such a change
> work.  For example, "disk->partition->partmap->name" would have to be
> changed to "p->partmap->name"

I am OK with that change.

> I'll send a second email with an updated patch for this commit.  If you
> would like me to regenerate the entire patch set, please let me know.

Entire patchset please. If you do not change anything in other patches
you can add my RB to them.

Daniel

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