* [PATCH v2 0/7] support >512b sector disks with old/buggy firmware
@ 2020-05-24 12:25 Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in) Mihai Moldovan
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
This is a patch series enabling successful boots on machines with
old/buggy system firmware that always assumes that hardware is using
512-bytes sectors, but more modern disks (e.g., 4Kn disks).
I will not get into the details in that cover letter, since the commits
themselves are supposed to be verbosely explaining what has been done
and why exactly, including proper comments within the code, but I will
give a short summary of the user-visible features:
- it works for me (yay!)
- only x86 is supported, the SPARC core.img writing code is so
different and weird that was I not able to merge the changes to
both platforms (and OFW probably does not suffer from these
problems anyway)
- as a drive-by fix, stop allocating RS codes space on SPARC which
cannot use it
- adds disk sector size autodetection in the biosdisk module
- adds a new, documented command line option --emu-512b that spaces
out core.img in such a way that these buggy machines can read it
correctly and enables the former mentioned disk sector size
autodetection
- as another drive-by fix, make the diskfilter debug output more
usable.
This stuff is not perfect and it likely also cannot be:
- the disk sector size autodetection relies on content being
*different* in the first two sectors of a disk, so it will fail to
detect the sector size if, for example, the disk is empty
- the last few sectors on a disk will not be readable if the system
firmware is not very lenient.
Lastly, most of this stuff is opt-in. Users will need to explicitly
pass the new --emu-512b option to grub-install to get the new behavior.
This will also automatically set the biosdisk_autodetect_sector_size
environment variable in core.img's embedded config file. The
environment variable seems to stick after loading the actual (menu)
config file.
V2:
- unbreak SPARC due to a very, very, very stupid typo
- fix a few typos in GPT code comments.
Mihai Moldovan (7):
biosdisk: autodetect hardware sector size (opt-in)
biosdisk: restore LBA mode after read/write failures
setup: add support for native sector addressing w/ 512-bytes lengths
grub-install: hook up --emu-512b to sector size autodetection in
biosdisk
docs/grub: document --emu-512b install option
diskfilter: write out currently scanned partition
gpt: respect native sector size if set/detected
docs/grub.texi | 15 +
grub-core/disk/diskfilter.c | 54 ++-
grub-core/disk/i386/pc/biosdisk.c | 642 +++++++++++++++++++++++++++++-
grub-core/disk/ldm.c | 10 +-
grub-core/fs/btrfs.c | 9 +-
grub-core/fs/zfs/zfs.c | 9 +-
grub-core/partmap/gpt.c | 79 +++-
grub-core/partmap/msdos.c | 30 +-
include/grub/emu/hostdisk.h | 3 +-
include/grub/fs.h | 3 +-
include/grub/i386/pc/biosdisk.h | 3 +
include/grub/partition.h | 3 +-
include/grub/util/install.h | 4 +-
util/grub-install.c | 44 +-
util/grub-setup.c | 10 +-
util/setup.c | 349 ++++++++++++++--
16 files changed, 1204 insertions(+), 63 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in)
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 2/7] biosdisk: restore LBA mode after read/write failures Mihai Moldovan
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
Some buggy (usually older) system firmware always reports a sector size
of 512 bytes - the default one for a very long time.
Newer disk drives diverge from that with longer sector sizes (typically
4096 bytes) and sometimes provide an 512-bytes-emulation layer. If the
latter is not provided, grub will get into trouble.
With 4Kn drives and buggy/older firmware a combination of things can
happen:
- the system firmware reads exactly 512 bytes (only) from a sector
and places it into memory, leaving the rest unaddress- and readable
- the system firmware reads 512 bytes of the requested sector and
additionally length * 512 bytes and places it into memory
- the system firmware *thinks* it reads 512 bytes of the requested
sector (only), but actually fetches more (usually the native sector
size) and places all of that into memory
From these behaviors, the first one is at least safe, but very
frustrating, the second one is tolerable and the third one is downright
harmful because it's essentially leading to undetected buffer
overflows.
Therefore, we actually need to determine *two* sizes:
- the read size (i.e., how much of a physical sector will be put into
memory by the system firmware) and
- the actual disk sector size.
Determining the read size is relatively easy and goes like that:
- poison scratch memory area with a known pattern
- tell the system firmware to read exactly one sector
- scan the scratch memory area for the first occurrence of the
poisoning pattern.
The read size hence is the difference between the first poisoning
pattern occurrence and the start of the scratch memory area.
However, determining the physical sector size is a lot trickier and
requires a pretty nasty heuristic:
- read one read size block from the *second* physical hardware sector
into a buffer
- read (MAX_HW_SECTOR_SIZE + read_size) / read_size blocks from the
first physical hardware sector into a different buffer
- scan the latter buffer until finding data from the former buffer.
Crucially, MAX_HW_SECTOR_SIZE must be small enough to not encompass the
whole usable scratch area (i.e., minus DAP in any case) and allow at
least one more read size block to be put into the buffer. That means
that the maximum supported hardware sector size is one order of
magnitude lower (pow-2-based) than the maximum supported read size.
Also, this heuristic is *dangerous* because it assumes and implies
that:
- the first and second physical sectors on the drive have diverging
content
- the first physical sector does not contain the first read size
block of the second physical sector.
This assumption usually holds for GPT-partitioned disks since GPT
starts at sector 2/LBA 1, but can easily fail for MBR-partitioned disks
with a standard post-MBR gap, especially if the disk has been wiped at
some point in time and nothing wrote to the post-MBR gap.
It will also trivially fail for completely blank disks.
This said, I see no other way to (more reliably) detect the physical
sector size.
If the sector size is detected correctly, almost everything should
work. There is one pretty big caveat: if the read block and sector
sizes do not match, then the last
pow (2, sector_bits - read_block_bits) sectors will not be fully
accessible, because:
- the amount of sectors to be read must be given in as read size
blocks
- start sector + amount of (virtual, read-block-sized) sectors would
overflow the total sectors count as returned by the drive
- system firmware often checks this size constraints and errors out.
The point here is that the system firmware *might* check for this
overflow, but needs not.
So, for affected sectors, we will:
- cancel any write requests since it's just too dangerous
- try to opportunistically read.
If reading succeeded, we then need to make sure that the returned data
actually matches what we expect, i.e., identify truncated reads.
To do this, we employ the same scratch region poisoning scheme like we
do when determining the read block size.
Partial reads (and other errors) will be discarded.
This usually means not being able to access the last sectors of a disk,
but there's no way to work around that issue.
Disclaimer: this code is opt-in. It will not be triggered by default.
Users must explicitly set the environment variable
biosdisk_autodetect_sector_size to 1 (preferably) or any
value different from zero.
---
grub-core/disk/i386/pc/biosdisk.c | 626 +++++++++++++++++++++++++++++-
include/grub/i386/pc/biosdisk.h | 3 +
2 files changed, 620 insertions(+), 9 deletions(-)
diff --git a/grub-core/disk/i386/pc/biosdisk.c b/grub-core/disk/i386/pc/biosdisk.c
index 8ca250c77..044bc48f3 100644
--- a/grub-core/disk/i386/pc/biosdisk.c
+++ b/grub-core/disk/i386/pc/biosdisk.c
@@ -28,11 +28,36 @@
#include <grub/err.h>
#include <grub/term.h>
#include <grub/i18n.h>
+#include <grub/env.h>
GRUB_MOD_LICENSE ("GPLv3+");
static int cd_drive = 0;
+/* Determined by fair dice roll. */
+static const grub_uint64_t grub_biosdisk_poison_pattern[64] =
+ {
+ 0xF660FE98E09DF125, 0xD123C6FEDC70D7F8, 0x5B66A37CD24ADA47, 0xB83E4A9DB0E12A04,
+ 0x132DA014C28284AB, 0xF5C68B442B2AA5BD, 0xE21F60B0BFEFC1CF, 0x5F976D7F7F5BD4DC,
+ 0xFCB4F76ACD915CE8, 0x5C6E6DE8437CE13B, 0xF790D7BA4F629C2D, 0x4EAB723F8B90F387,
+ 0x6270892631C70107, 0xB4332B5DFB0A3E6D, 0x09FD5C6AA025EAB0, 0xF39DCF68620BF1BE,
+ 0x3B966D9C50A3A407, 0x3DA77872C8288688, 0xFBDA19DFB95940DD, 0x4A136CFE2B3DD4A5,
+ 0xF14B447C6CCB7EED, 0x92F2D74BD4CA99C0, 0x3CFF39AC8CE04880, 0x14212C27A1106A50,
+ 0x033EC0A7122519C8, 0xD0F7F9FA4A269434, 0x21A55CD807FF3F5E, 0xE6A66A84650AD10B,
+ 0x68E98E5DF15D9A6F, 0xD7DF78B2A3DEC128, 0xE364966B5EAAAEA7, 0x96F19C0FF3121EEE,
+ 0x78FD8D9EDDC9C358, 0xF9981071A4E0DA61, 0xCB5FB448E13EF785, 0x872A0CD898AB4F17,
+ 0x64574AF0E2CB75C4, 0xED72A3009C00746F, 0x43C9BBBA9283F3DB, 0x3B5891BC9B0DE3F0,
+ 0xD5F16F1BCDCD88B4, 0xED4D5C20229BB63E, 0x14ECB7F68803AF5E, 0x8240D2555E729FA6,
+ 0xECE77CA33E78F78D, 0x93772E94777226E0, 0xCC6C8BB2DDC8323B, 0x5E13D0C413C5A7D3,
+ 0x9799FD9136523F24, 0x02C942D7FFBD9BDC, 0xCEE49D0F6B7FFDB0, 0x12C785E7DC01A725,
+ 0x58E4F2D4353EDF6A, 0xB186F0BA1D8E9027, 0xD8C728D71BBCDD47, 0xA187BE57A2E98EF6,
+ 0x4D6568EC45CE249B, 0x5256CE99E4F812E2, 0x41CE1E1197D5987E, 0x3A1076EFDE1D63EF,
+ 0xE085586A0609FF7A, 0x58F5F23A289A1ABC, 0xF68B104B1D440019, 0x13D1B4D7FDE31685
+ };
+
static int grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap);
+static grub_err_t grub_biosdisk_read (grub_disk_t disk,
+ grub_disk_addr_t sector,
+ grub_size_t size, char *buf);
static int grub_biosdisk_get_num_floppies (void)
{
@@ -329,6 +354,93 @@ grub_biosdisk_iterate (grub_disk_dev_iterate_hook_t hook, void *hook_data,
return 0;
}
+static void
+grub_biosdisk_poison_scratch (const unsigned char dap_stop)
+{
+ /* Make sure that the poison pattern is a 512-bytes block. */
+ COMPILE_TIME_ASSERT (GRUB_DISK_SECTOR_SIZE == sizeof (grub_biosdisk_poison_pattern));
+
+ grub_size_t i = 0;
+ grub_size_t poison_pattern_size = sizeof (grub_biosdisk_poison_pattern);
+ grub_uint64_t *scratch_p = (grub_uint64_t *)
+ GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+ /* By default, poison the whole region. */
+ grub_size_t scratch_size = GRUB_MEMORY_MACHINE_SCRATCH_SIZE;
+
+ /*
+ * But if dap_stop has been provided, limit the poisoned area to everything
+ * before the DAP location.
+ */
+ if (dap_stop)
+ {
+ scratch_size = GRUB_BIOSDISK_DAP_OFFSET;
+ }
+
+ /* The size is actually the count of poisoning pattern blocks. */
+ scratch_size = grub_divmod64 (scratch_size, poison_pattern_size, 0);
+
+ /* Actually poison region. */
+ for (i = 0; i < scratch_size; ++i)
+ {
+ grub_memmove (scratch_p + (i * ARRAY_SIZE (grub_biosdisk_poison_pattern)),
+ grub_biosdisk_poison_pattern, poison_pattern_size);
+ }
+}
+
+static grub_err_t
+grub_biosdisk_find_poison_pattern (const unsigned char dap_stop,
+ unsigned char *found,
+ grub_size_t *block_offset)
+{
+ grub_err_t ret = GRUB_ERR_BAD_ARGUMENT;
+
+ if ((!found) || (!block_offset))
+ {
+ return ret;
+ }
+
+ ret = GRUB_ERR_NONE;
+
+ /* Make sure that the poison pattern is a 512-bytes block. */
+ COMPILE_TIME_ASSERT (GRUB_DISK_SECTOR_SIZE == sizeof (grub_biosdisk_poison_pattern));
+
+ grub_size_t i = 0;
+ grub_size_t poison_pattern_size = sizeof (grub_biosdisk_poison_pattern);
+ grub_uint64_t *scratch_p = (grub_uint64_t *)
+ GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+ /* By default, scan the whole region. */
+ grub_size_t scratch_size = GRUB_MEMORY_MACHINE_SCRATCH_SIZE;
+
+ /*
+ * But if dap_stop has been provided, limit the scanned area to everything
+ * before the DAP location.
+ */
+ if (dap_stop)
+ {
+ scratch_size = GRUB_BIOSDISK_DAP_OFFSET;
+ }
+
+ /* The size is actually the count of poisoning pattern blocks. */
+ scratch_size = grub_divmod64 (scratch_size, poison_pattern_size, 0);
+
+ /* Actually scan the region. */
+ *found = 0;
+ for (i = 0; i < scratch_size; ++i)
+ {
+ if (0 == grub_memcmp (scratch_p + (i * ARRAY_SIZE (grub_biosdisk_poison_pattern)),
+ grub_biosdisk_poison_pattern, poison_pattern_size))
+ {
+ *found = 1;
+ *block_offset = i;
+ break;
+ }
+ }
+
+ return ret;
+}
+
static grub_err_t
grub_biosdisk_open (const char *name, grub_disk_t disk)
{
@@ -360,6 +472,27 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
{
/* HDD */
int version;
+ unsigned int detected_log_read_size = 0;
+ unsigned int detected_read_size = 0;
+ unsigned int detected_sector_size = 0;
+ unsigned int detected_log_sector_size = 0;
+ grub_size_t i = 0;
+ grub_size_t max_reads = 0;
+ unsigned char found = 0;
+ int autodetect = 0;
+ const char *autodetect_str = grub_env_get ("biosdisk_autodetect_sector_size");
+
+ char *buf_lhs = grub_malloc (GRUB_BIOSDISK_DAP_OFFSET); /* Max supported buffer size. */
+ if (!buf_lhs)
+ {
+ return grub_errno;
+ }
+
+ char *buf_rhs = grub_malloc (GRUB_BIOSDISK_DAP_OFFSET >> 1); /* Max supported sector size. */
+ if (!buf_rhs)
+ {
+ return grub_errno;
+ }
disk->log_sector_size = 9;
@@ -395,6 +528,287 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
}
}
}
+
+ if (autodetect_str)
+ {
+ grub_errno = 0;
+ autodetect = !!(grub_strtoul (autodetect_str, 0, 0));
+
+ if (grub_errno)
+ {
+ /*
+ * Env variable set, but not recognized as a number.
+ * Assume that the user intended to enable this feature.
+ */
+ autodetect = 1;
+ }
+ }
+
+ /* Initialize read block size to queried/defaulted sector size. */
+ data->log_read_size = disk->log_sector_size;
+
+ if (autodetect)
+ {
+ /*
+ * We now either have a sector size as returned by the firmware
+ * or hardcoded/initialized it to 512 bytes.
+ *
+ * Unfortunately, some old/buggy firmware versions always return
+ * a sector size of 512 bytes, regardless of the actual disk hardware
+ * sector size.
+ *
+ * Using a smaller sector size than actually supported by the
+ * hardware will lead to nasty memory corruptions, because:
+ * - we have a hardcoded scratch memory region used for read and
+ * write operations
+ * - this memory region is bounded
+ * - a DAP struct, containing read/write parameters for the
+ * firmware, is put at an offset of 32 KByte within the scratch
+ * region
+ *
+ * This will generally work fine if the sector size returned by the
+ * firmware matches the hardware sector size (and we make sure that
+ * (SAFE_SECTORS * SECTOR_SIZE) does not overflow the scratch memory
+ * region or reaches into the DAP) or the hardware sector size is
+ * smaller than the sector size reported by the firmware, but is very
+ * dangerous if the hardware sector size is bigger than the sector
+ * size returned by the firmware.
+ *
+ * Additionally, sometimes read and sector sizes are decoupled.
+ * For example, there is firmware that wrongfully reports a fixed
+ * sector size of 512 bytes AND writes at most 512 bytes to the
+ * buffer for a one-sector read. In such a case, the read length must
+ * be specified as a read-block multiple, while sector addressing is
+ * hardware-sector-size based (because the firmware just passes the
+ * start sector value right to the actual disk).
+ *
+ * To counter this, try to detect the actual read block size and
+ * hardware sector size by:
+ * - poisoning the whole scratch memory area with a specific value
+ * - set safe sector size to 1
+ * - read a block off the disk drive (and make sure this succeeds)
+ * - determine the read block size by scanning for the poisoning
+ * value
+ * - fill two buffers with reads of LBAs 0 and 1 (taking read
+ * block size into account)
+ * - determine the hardware sector size by searching for the first
+ * matching block (hoping that both sectors contain different
+ * data)
+ */
+
+ /* Poison region. */
+ grub_biosdisk_poison_scratch (0);
+
+ /*
+ * Set safe sector size to one sector for upcoming read operations.
+ */
+ data->sectors = 1;
+
+ /* Set disk->data temporarily to our data object. */
+ disk->data = data;
+
+ /* Read one block. In case of failure, error out. */
+ found = 0;
+ if (GRUB_ERR_NONE == grub_biosdisk_read (disk, 0, 1, buf_rhs))
+ {
+ /* Compare the data with our poison pattern. */
+ if (grub_biosdisk_find_poison_pattern (1, &found, &i))
+ {
+ return grub_error (GRUB_ERR_BUG,
+ "%s: unable to search for poisoning pattern "
+ "in sector size autodetection - bad RAM?",
+ disk->name);
+ }
+ }
+ else
+ {
+ disk->data = NULL;
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+ grub_free (data);
+ return grub_error (GRUB_ERR_BAD_DEVICE,
+ "%s: read failure while determining INT13H read size",
+ disk->name);
+ }
+
+ detected_read_size = sizeof (grub_biosdisk_poison_pattern) * i;
+
+ if (!found || (detected_read_size < 512) || (detected_read_size > 16384))
+ {
+ disk->data = NULL;
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+ grub_free (data);
+ if (!found)
+ {
+ return grub_error (GRUB_ERR_BAD_DEVICE, "%s: no INT13H read size "
+ "detected",
+ disk->name);
+ }
+ else
+ {
+ return grub_error (GRUB_ERR_BAD_DEVICE, "%s: INT13H read size has "
+ "unsupported length %u",
+ disk->name, detected_read_size);
+ }
+ }
+
+ /* Convert detected read size to log2 value ... */
+ for (detected_log_read_size = 0;
+ (1U << detected_log_read_size) < detected_read_size;
+ ++detected_log_read_size);
+
+ /* ... and check that it's a sane power-of-two read block size. */
+ if ((1U << detected_log_read_size) == detected_read_size)
+ {
+ data->log_read_size = detected_log_read_size;
+ }
+ else
+ {
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+ grub_free (data);
+ return grub_error (GRUB_ERR_BAD_DEVICE,
+ "%s: unable to determine INT13H read size",
+ disk->name);
+ }
+
+ /*
+ * Determining the hardware sector size is also tricky.
+ * Buggy firmware versions read less data than the hardware sector
+ * size (typically 512 bytes), so we will have to tell the firmware
+ * to read (MAX_HW_SECTOR_SIZE + read_size) / read_size blocks to
+ * fetch enough data to detect sector sizes up to the maximum
+ * supported.
+ *
+ * Hence:
+ * - read read_size bytes from second HW sector into buf_rhs
+ * - set safe sector count to
+ * (MAX_HW_SECTOR_SIZE + read_size) / read_size and read the same
+ * amount of blocks from first HW sector into buf_lhs
+ * - scan for read_size-sized data in buf_rhs in buf_lhs
+ *
+ * Assuming that the actual hardware sector size is less than or
+ * equal to the maximum supported sector size, buf_lhs should always
+ * contain the first block of the second HW sector somewhere. The
+ * difference between buf_lhs' start and the second HW sector block
+ * must thus be the HW sector size.
+ *
+ * NOTE: this heuristic is inherently unsafe because it relies on
+ * disk data. It should work fine with GPT-partitioned disks,
+ * since the GPT always starts at the second HW sector, but can
+ * otherwise return wrong results. For instance, the HW sector
+ * size would be determined as 512 bytes for a zero-filled disk
+ * with MBR data in the first 512 bytes of the first HW sector,
+ * even though the actual HW sector size might differ.
+ */
+
+ /* Fill buffers with data from hard disk. */
+ i = grub_divmod64 (GRUB_BIOSDISK_DAP_OFFSET >> 1, detected_read_size, 0);
+
+ /*
+ * Setting disk->log_sector_size to the read size will correctly fill
+ * the provided buffer without us needing to manually copy data from
+ * the memory scratch area.
+ */
+ disk->log_sector_size = detected_log_read_size;
+
+ if (GRUB_ERR_NONE != grub_biosdisk_read (disk, 1, 1, buf_rhs))
+ {
+ disk->data = NULL;
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+ grub_free (data);
+ return grub_error (GRUB_ERR_BAD_DEVICE,
+ "%s: read failure for sector 1 while "
+ "determining sector size",
+ disk->name);
+ }
+
+ /* Set data->sectors to (MAX_HW_SECTOR_SIZE + read_size) / read_size. */
+ data->sectors = i + 1;
+
+ if (GRUB_ERR_NONE != grub_biosdisk_read (disk, 0, i + 1, buf_lhs))
+ {
+ disk->data = NULL;
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+ grub_free (data);
+ return grub_error (GRUB_ERR_BAD_DEVICE,
+ "%s: read failure for sector 0 while "
+ "determining sector size",
+ disk->name);
+ }
+
+ /*
+ * No more reads necessary, so reset disk->data.
+ * Will be set correctly at a later time.
+ */
+ disk->data = NULL;
+
+ /*
+ * Try to find the first matching read block in both buf_lhs and
+ * buf_rhs.
+ * Assuming that both sectors contain different data, stumbling
+ * across the same block value means that we have crossed sector
+ * boundaries in the first buffer.
+ */
+ max_reads = (2 * i);
+ found = 0;
+ for (i = 1; i < max_reads; ++i)
+ {
+ if (0 == grub_memcmp (buf_lhs + (i * detected_read_size), buf_rhs,
+ detected_read_size))
+ {
+ found = 1;
+ break;
+ }
+ }
+
+ detected_sector_size = i * detected_read_size;
+
+ /* Free buffers, we don't need them any longer. */
+ grub_free (buf_lhs);
+ grub_free (buf_rhs);
+
+ if (!found || (detected_sector_size < 512)
+ || (detected_sector_size > 16348)
+ || (detected_sector_size < detected_read_size))
+ {
+ grub_free (data);
+ if (!found)
+ {
+ return grub_error (GRUB_ERR_BAD_DEVICE, "%s: no sector size "
+ "detected",
+ disk->name);
+ }
+ else
+ {
+ return grub_error (GRUB_ERR_BAD_DEVICE, "%s: sector size has "
+ "unsupported length %u",
+ disk->name, detected_sector_size);
+ }
+ }
+
+ /* Convert detected sector size to log2 value ... */
+ for (detected_log_sector_size = 0;
+ (1U << detected_log_sector_size) < detected_sector_size;
+ ++detected_log_sector_size);
+
+ /* ... and check that it's a sane power-of-two sector size. */
+ if ((1U << detected_log_sector_size) == detected_sector_size)
+ {
+ disk->log_sector_size = detected_log_sector_size;
+ }
+ else
+ {
+ return grub_error (GRUB_ERR_BAD_DEVICE,
+ "%s: unable to determine sector size, "
+ "detected value %u is not a power-of-two "
+ "value",
+ disk->name, detected_sector_size);
+ }
+ }
}
if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
@@ -470,13 +884,64 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
N_("attempt to read or write outside of disk `%s'"),
disk->name);
+ /*
+ * If the native sector size doesn't match the read-block size, we're working
+ * with old, buggy system firmware and newer drives.
+ *
+ * We want to handle operations on sectors at the end in a special way, so
+ * check if this operation would be affected.
+ */
+ unsigned char past_end = 0;
+ if ((disk->log_sector_size != data->log_read_size)
+ && ((sector + size) > disk->total_sectors))
+ {
+ past_end = 1;
+
+ if (cmd)
+ {
+ /*
+ * Write operations are especially scary.
+ *
+ * If the system firmware writes only some of the data (e.g., because
+ * it copies only part of the buffer), we'll end up with
+ * partially updated sectors.
+ *
+ * Needless to say, we should definitely avoid this.
+ */
+ return grub_error (GRUB_ERR_WRITE_ERROR, N_ ("refusing to write partial "
+ "data to disk sector %llu on `%s'"),
+ (unsigned long long) sector,
+ disk->name);
+ }
+ else
+ {
+ /*
+ * Read operations aren't much better.
+ *
+ * Reads could either succeed and return the correct amount of data
+ * (if the system firmware is lenient, doesn't check bounds, fetches
+ * data into a buffer big enough and hands over the data correctly
+ * sized), XOR suceed and return partial data, XOR error out.
+ *
+ * We will go ahead and try to read the data opportunistically,
+ * but will also have to differentiate partial read sectors from
+ * fully read sectors.
+ *
+ * To do this, we'll use about the same poisoning technique we're
+ * also using for determining the read block size in
+ * grub_biosdisk_open.
+ */
+ grub_biosdisk_poison_scratch (1);
+ }
+ }
+
if (data->flags & GRUB_BIOSDISK_FLAG_LBA)
{
struct grub_biosdisk_dap *dap;
+ /* Put the DAP at a 32 KByte offset from scratch memory start. */
dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
- + (data->sectors
- << disk->log_sector_size));
+ + GRUB_BIOSDISK_DAP_OFFSET);
dap->length = sizeof (*dap);
dap->reserved = 0;
dap->blocks = size;
@@ -503,10 +968,101 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
else
if (grub_biosdisk_rw_int13_extensions (cmd + 0x42, data->drive, dap))
{
- /* Fall back to the CHS mode. */
+ if (past_end)
+ {
+ /*
+ * Reading "past the end" failed, which likely means that the
+ * system firmware checks the length and start sector against
+ * the total sectors.
+ *
+ * It doesn't make sense to retry.
+ */
+ return grub_error (GRUB_ERR_READ_ERROR, N_ ("opportunistically tried to read "
+ "end sectors from %llu on `%s', "
+ "but system firmware does not seem "
+ "to support this"),
+ (unsigned long long) sector,
+ disk->name);
+ }
+
+ /*
+ * If the operation failed and we didn't try to read the end
+ * sectors, fall back to the CHS mode ...
+ */
+ grub_err_t chs_read = GRUB_ERR_NONE;
data->flags &= ~GRUB_BIOSDISK_FLAG_LBA;
disk->total_sectors = data->cylinders * data->heads * data->sectors;
- return grub_biosdisk_rw (cmd, disk, sector, size, segment);
+
+ chs_read = grub_biosdisk_rw (cmd, disk, sector, size, segment);
+
+ /* Pass CHS operation result through. */
+ return chs_read;
+ }
+ else if (past_end)
+ {
+ /*
+ * An opportunistic past-the-end read operation completed
+ * successfully, which means that the system firmware didn't check
+ * for an out-of-device-bounds read.
+ *
+ * In turn, we'll have to check if the read data is actually
+ * complete.
+ */
+ grub_size_t real_size = size >> (disk->log_sector_size - data->log_read_size);
+ grub_size_t i = 0;
+ unsigned char found = 0;
+ if (grub_biosdisk_find_poison_pattern (1, &found, &i))
+ {
+ return grub_error (GRUB_ERR_BUG,
+ N_ ("%s: unable to search for poisoning pattern "
+ "in past-the-end read operation - bad RAM?"),
+ disk->name);
+ }
+
+ if (found)
+ {
+ /* Found pattern at a later position than expected. */
+ if (i > real_size)
+ {
+ return grub_error (GRUB_ERR_BUG,
+ N_ ("%s: poisoning pattern found at block offset %llu, "
+ "but expected at (at most) %llu, this should not happen"),
+ disk->name, i, real_size);
+ }
+
+ /* Read partial data. */
+ if (i < real_size)
+ {
+ return grub_error (GRUB_ERR_READ_ERROR, N_ ("opportunistically tried to read "
+ "end sectors from %llu on `%s', "
+ "but system firmware does not seem "
+ "to support this"),
+ (unsigned long long) sector,
+ disk->name);
+ }
+ }
+ else
+ {
+ /* Read past the original DAP. */
+ if ((real_size << disk->log_sector_size) > GRUB_BIOSDISK_DAP_OFFSET)
+ {
+ return grub_error (GRUB_ERR_BUG,
+ N_ ("%s: attempted to read more data (%llu bytes) than would "
+ "fit into the buffer (%llu bytes), this should not happen"),
+ disk->name, real_size << disk->log_sector_size,
+ GRUB_BIOSDISK_DAP_OFFSET);
+ }
+
+ /* System firmware wrote more data to buffer than expected?! */
+ if ((real_size << disk->log_sector_size) < GRUB_BIOSDISK_DAP_OFFSET)
+ {
+ return grub_error (GRUB_ERR_BUG,
+ N_ ("%s: unable to find poisoning pattern after returned "
+ "data, did the system firmware return more data than "
+ "expected?"),
+ disk->name);
+ }
+ }
}
}
else
@@ -563,7 +1119,24 @@ get_safe_sectors (grub_disk_t disk, grub_disk_addr_t sector)
grub_size_t size;
grub_uint64_t offset;
struct grub_biosdisk_data *data = disk->data;
- grub_uint32_t sectors = data->sectors;
+
+ /*
+ * Calculate usable area, but leave a safety margin of one sector.
+ * This can leave about half of the scratch region (currently) unused, but
+ * data safety is arguably more important than a few additional read
+ * operations.
+ */
+ grub_uint32_t sectors = (GRUB_BIOSDISK_DAP_OFFSET >> disk->log_sector_size)
+ - 1;
+
+ /*
+ * For CHS access, we want to read at most one head at a time,
+ * so limit the safe sector count if necessary.
+ */
+ if (data->sectors < sectors)
+ {
+ sectors = data->sectors;
+ }
/* OFFSET = SECTOR % SECTORS */
grub_divmod64 (sector, sectors, &offset);
@@ -577,16 +1150,48 @@ static grub_err_t
grub_biosdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
grub_size_t size, char *buf)
{
+ struct grub_biosdisk_data *data = disk->data;
+
while (size)
{
- grub_size_t len;
+ grub_size_t len, raw_len;
len = get_safe_sectors (disk, sector);
if (len > size)
len = size;
- if (grub_biosdisk_rw (GRUB_BIOSDISK_READ, disk, sector, len,
+ /*
+ * Usually, we want to pass the amount of sectors to read or write to
+ * grub_biosdisk_rw.
+ *
+ * Unfortunately, for (old) buggy BIOS versions, we will have to pass
+ * a read-block sized value to actually get the desired data.
+ *
+ * Look up the autodetection code and comments in grub_biosdisk_open for
+ * a description of this stuff.
+ *
+ * Now, there is one caveat to all of this: system firmware *thinks*
+ * that we're passing a sector-based length value to it and will
+ * check sector + length to make sure it stays below (or equals) the
+ * total sector value as returned by the hardware.
+ *
+ * This is *bad*.
+ * Why?
+ *
+ * For instance, if we want to read the last sector, we'll only be able
+ * to pass 1 as the length and hence will get only one read-sized block
+ * (so, e.g., only the first 512 bytes of the sector). All the other data
+ * will be inaccessible.
+ *
+ * For the second-to-last sector, it would be at most two read-sized
+ * blocks etc.
+ *
+ * But even worse, we can't do anything to work around that issue.
+ */
+ raw_len = len << (disk->log_sector_size - data->log_read_size);
+
+ if (grub_biosdisk_rw (GRUB_BIOSDISK_READ, disk, sector, raw_len,
GRUB_MEMORY_MACHINE_SCRATCH_SEG))
return grub_errno;
@@ -612,16 +1217,19 @@ grub_biosdisk_write (grub_disk_t disk, grub_disk_addr_t sector,
while (size)
{
- grub_size_t len;
+ grub_size_t len, raw_len;
len = get_safe_sectors (disk, sector);
if (len > size)
len = size;
+ /* Please check grub_biosdisk_read for an explanation of this magic. */
+ raw_len = len << (disk->log_sector_size - data->log_read_size);
+
grub_memcpy ((void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR, buf,
len << disk->log_sector_size);
- if (grub_biosdisk_rw (GRUB_BIOSDISK_WRITE, disk, sector, len,
+ if (grub_biosdisk_rw (GRUB_BIOSDISK_WRITE, disk, sector, raw_len,
GRUB_MEMORY_MACHINE_SCRATCH_SEG))
return grub_errno;
diff --git a/include/grub/i386/pc/biosdisk.h b/include/grub/i386/pc/biosdisk.h
index 3d8071684..ca01cb7ae 100644
--- a/include/grub/i386/pc/biosdisk.h
+++ b/include/grub/i386/pc/biosdisk.h
@@ -33,6 +33,8 @@
#define GRUB_BIOSDISK_CDTYPE_MASK 0xF
+#define GRUB_BIOSDISK_DAP_OFFSET (1 << 15)
+
struct grub_biosdisk_data
{
int drive;
@@ -40,6 +42,7 @@ struct grub_biosdisk_data
unsigned long heads;
unsigned long sectors;
unsigned long flags;
+ unsigned int log_read_size;
};
/* Drive Parameters. */
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/7] biosdisk: restore LBA mode after read/write failures
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in) Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 3/7] setup: add support for native sector addressing w/ 512-bytes lengths Mihai Moldovan
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
Falling back to the old-style CHS mode after read failures is a valid
error recovery technique, but read failures can just happen. One
prominent example of that is trying to read the (somewhat) inaccessible
end sectors on newer disks with old, buggy system firmware.
Restore LBA access mode after an old-style CHS mode read/write try
again. This might lead to additional read/write operations (in case they
fail again), but also fixes issues like the biosdisk constantly being
unable to read data off sectors that are located behind the (shy of)
8-GB-mark.
---
grub-core/disk/i386/pc/biosdisk.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/grub-core/disk/i386/pc/biosdisk.c b/grub-core/disk/i386/pc/biosdisk.c
index 044bc48f3..3d55e68f8 100644
--- a/grub-core/disk/i386/pc/biosdisk.c
+++ b/grub-core/disk/i386/pc/biosdisk.c
@@ -990,11 +990,27 @@ grub_biosdisk_rw (int cmd, grub_disk_t disk,
* sectors, fall back to the CHS mode ...
*/
grub_err_t chs_read = GRUB_ERR_NONE;
+
+ /* Save old data first. */
+ grub_uint64_t old_total_sectors = disk->total_sectors;
+
+ /*
+ * Switch into CHS mode and call the old-style reading code path.
+ */
data->flags &= ~GRUB_BIOSDISK_FLAG_LBA;
disk->total_sectors = data->cylinders * data->heads * data->sectors;
-
chs_read = grub_biosdisk_rw (cmd, disk, sector, size, segment);
+ /*
+ * Whatever happened, restore LBA access.
+ *
+ * We don't want the biosdisk instance to permanently degrade into
+ * an old-style CHS access mode after a read failure. Read failures
+ * can happen, especially for the end sectors.
+ */
+ data->flags |= GRUB_BIOSDISK_FLAG_LBA;
+ disk->total_sectors = old_total_sectors;
+
/* Pass CHS operation result through. */
return chs_read;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/7] setup: add support for native sector addressing w/ 512-bytes lengths
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in) Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 2/7] biosdisk: restore LBA mode after read/write failures Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 4/7] grub-install: hook up --emu-512b to sector size autodetection in biosdisk Mihai Moldovan
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
In order to successfully boot with non-512-bytes sector disks and
buggy/older system firmware, core.img must be written in a special way.
This means that:
- each part of core.img that needs to be directly addressable MUST
start on a hardware sector
- the addressing must be native-sector-size-based
- lengths are still based on 512-bytes blocks.
We can get such a layout by padding data out a bit, like this:
- write the first 512-bytes block to the start of a native sector
(this already is implicitly true through the embedding routine)
- potentially leave a gap up until the next native sector (i.e.,
zero-fill)
- write the next 512-bytes block
- write the next 126 512-bytes blocks, since data is read-in in 127
512-bytes blocks at a time, inherited from buggy "Phoenix BIOS"
system firmware that was not able to read more data at a time
- potentially leave a gap until the next native sector
- repeat writing blocks as specified above until reaching core.img's
end
- DON'T add useless padding at the end.
This is all optional and comes with a new switch to grub-install:
--emu-512b
It also:
- only works when embedding core.img (sorry, no blocklists support)
- requires MBR- or GPT-embedding (sorry, no BtrFS- or zfs-embedding
support)
- will only work for x86 BIOS targets (because the SPARC code is so
different in certain aspects that I cannot meaningfully generalize
it).
Additionally, this commit fixes a small SPARC issue as a drive-by: SPARC
does not support Reed-Solomon Codes and the code for generating them was
properly #ifdef'd out, but the maximum needed sectors was still
uselessly doubled to accomodate RS. From now on, only x86 BIOS targets
will reserve space for RS codes (unless disabled, of course).
---
grub-core/disk/ldm.c | 10 +-
grub-core/fs/btrfs.c | 9 +-
grub-core/fs/zfs/zfs.c | 9 +-
grub-core/partmap/gpt.c | 44 ++++-
grub-core/partmap/msdos.c | 30 +++-
include/grub/emu/hostdisk.h | 3 +-
include/grub/fs.h | 3 +-
include/grub/partition.h | 3 +-
include/grub/util/install.h | 4 +-
util/grub-install.c | 18 +-
util/grub-setup.c | 10 +-
util/setup.c | 349 +++++++++++++++++++++++++++++++++---
12 files changed, 449 insertions(+), 43 deletions(-)
diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
index 2a22d2d6c..47fda7a39 100644
--- a/grub-core/disk/ldm.c
+++ b/grub-core/disk/ldm.c
@@ -954,7 +954,8 @@ grub_err_t
grub_util_ldm_embed (struct grub_disk *disk, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors)
+ grub_disk_addr_t **sectors,
+ int emu_512b)
{
struct grub_diskfilter_pv *pv = NULL;
struct grub_diskfilter_vg *vg;
@@ -964,6 +965,13 @@ grub_util_ldm_embed (struct grub_disk *disk, unsigned int *nsectors,
if (embed_type != GRUB_EMBED_PCBIOS)
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"LDM currently supports only PC-BIOS embedding");
+
+ if (emu_512b)
+ {
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ "512 bytes sector length emulation is not implemented for LDM-embedding");
+ }
+
if (disk->partition)
return grub_error (GRUB_ERR_BUG, "disk isn't LDM");
pv = grub_diskfilter_get_pv_from_disk (disk, &vg);
diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 63f9657a6..3949ea1a0 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -2151,7 +2151,8 @@ grub_btrfs_embed (grub_device_t device __attribute__ ((unused)),
unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors)
+ grub_disk_addr_t **sectors,
+ int emu_512b)
{
unsigned i;
@@ -2159,6 +2160,12 @@ grub_btrfs_embed (grub_device_t device __attribute__ ((unused)),
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"BtrFS currently supports only PC-BIOS embedding");
+ if (emu_512b)
+ {
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ "512 bytes sector length emulation is not implemented for BtrFS-embedding");
+ }
+
if (64 * 2 - 1 < *nsectors)
return grub_error (GRUB_ERR_OUT_OF_RANGE,
N_("your core.img is unusually large. "
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index b5e10fd0b..3b4c65f56 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -4323,7 +4323,8 @@ grub_zfs_embed (grub_device_t device __attribute__ ((unused)),
unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors)
+ grub_disk_addr_t **sectors,
+ int emu_512b)
{
unsigned i;
@@ -4331,6 +4332,12 @@ grub_zfs_embed (grub_device_t device __attribute__ ((unused)),
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"ZFS currently supports only PC-BIOS embedding");
+ if (emu_512b)
+ {
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ "512 bytes sector length emulation is not implemented for zfs-embedding");
+ }
+
if ((VDEV_BOOT_SIZE >> GRUB_DISK_SECTOR_BITS) < *nsectors)
return grub_error (GRUB_ERR_OUT_OF_RANGE,
N_("your core.img is unusually large. "
diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
index 103f6796f..4b968529a 100644
--- a/grub-core/partmap/gpt.c
+++ b/grub-core/partmap/gpt.c
@@ -169,8 +169,13 @@ static grub_err_t
gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors)
+ grub_disk_addr_t **sectors,
+ int emu_512b)
{
+ struct gpt_partition_map_embed_ctx orig_ctx = {
+ .start = 0,
+ .len = 0
+ };
struct gpt_partition_map_embed_ctx ctx = {
.start = 0,
.len = 0
@@ -186,6 +191,43 @@ gpt_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
if (err)
return err;
+ orig_ctx = ctx;
+ if (emu_512b)
+ {
+ unsigned int emu_sec_factor_bits = disk->log_sector_size
+ - GRUB_DISK_SECTOR_BITS;
+ /*
+ * Make sure the start is HW-sector-aligned and modify the partition size
+ * accordingly.
+ */
+
+ /*
+ * Align and truncate to physical sector (always less or equal to current
+ * value.
+ */
+ ctx.start = ctx.start >> emu_sec_factor_bits;
+ ctx.start = ctx.start << emu_sec_factor_bits;
+
+ if (ctx.start != orig_ctx.start)
+ {
+ /*
+ * ctx.start is unaligned, so add a physical sector factor and align
+ * and truncate again.
+ */
+ ctx.start = orig_ctx.start + ((1U) << emu_sec_factor_bits);
+
+ ctx.start = ctx.start >> emu_sec_factor_bits;
+ ctx.start = ctx.start << emu_sec_factor_bits;
+
+ ctx.len -= (ctx.start - orig_ctx.start);
+ }
+ }
+
+ /*
+ * N.B.: ctx can either be an aligned version or just its original value from
+ * this point on, depending on whether emu_512b was requested or not.
+ */
+
if (ctx.len == 0)
return grub_error (GRUB_ERR_FILE_NOT_FOUND,
N_("this GPT partition label contains no BIOS Boot Partition;"
diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
index 7b8e45076..314bd8afd 100644
--- a/grub-core/partmap/msdos.c
+++ b/grub-core/partmap/msdos.c
@@ -236,7 +236,8 @@ static grub_err_t
pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors)
+ grub_disk_addr_t **sectors,
+ int emu_512b)
{
grub_disk_addr_t end = ~0ULL;
struct grub_msdos_partition_mbr mbr;
@@ -246,6 +247,13 @@ pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
grub_disk_addr_t lastaddr = 1;
grub_disk_addr_t ext_offset = 0;
grub_disk_addr_t offset = 0;
+ unsigned int emu_sec_factor = (1U) << (disk->log_sector_size
+ - GRUB_DISK_SECTOR_BITS);
+
+ if (!emu_512b)
+ {
+ emu_sec_factor = 1;
+ }
if (embed_type != GRUB_EMBED_PCBIOS)
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
@@ -326,22 +334,23 @@ pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
break;
}
- if (end >= *nsectors + 1)
+ if (end >= *nsectors + emu_sec_factor)
{
unsigned i, j;
char *embed_signature_check;
unsigned int orig_nsectors, avail_nsectors;
orig_nsectors = *nsectors;
- *nsectors = end - 1;
+ *nsectors = end - emu_sec_factor;
avail_nsectors = *nsectors;
+
if (*nsectors > max_nsectors)
*nsectors = max_nsectors;
*sectors = grub_malloc (*nsectors * sizeof (**sectors));
if (!*sectors)
return grub_errno;
for (i = 0; i < *nsectors; i++)
- (*sectors)[i] = 1 + i;
+ (*sectors)[i] = emu_sec_factor + i;
/* Check for software that is already using parts of the embedding
* area.
@@ -362,6 +371,15 @@ pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
continue;
grub_util_warn (_(message_warn[embed_signatures[j].type]),
(*sectors)[i], embed_signatures[j].name);
+
+ if (emu_512b)
+ {
+ return grub_error (GRUB_ERR_OUT_OF_RANGE,
+ N_("leaving gaps for other software in the "
+ "embedding area is not supported in 512 "
+ "bytes emulation mode"));
+ }
+
avail_nsectors--;
if (avail_nsectors < *nsectors)
*nsectors = avail_nsectors;
@@ -390,12 +408,12 @@ pc_partition_map_embed (struct grub_disk *disk, unsigned int *nsectors,
return GRUB_ERR_NONE;
}
- if (end <= 1)
+ if (end <= emu_sec_factor)
return grub_error (GRUB_ERR_FILE_NOT_FOUND,
N_("this msdos-style partition label has no "
"post-MBR gap; embedding won't be possible"));
- if (*nsectors > 62)
+ if (*nsectors > (63 - emu_sec_factor))
return grub_error (GRUB_ERR_OUT_OF_RANGE,
N_("your core.img is unusually large. "
"It won't fit in the embedding area"));
diff --git a/include/grub/emu/hostdisk.h b/include/grub/emu/hostdisk.h
index e006f0b38..e70d474e8 100644
--- a/include/grub/emu/hostdisk.h
+++ b/include/grub/emu/hostdisk.h
@@ -51,7 +51,8 @@ grub_err_t
grub_util_ldm_embed (struct grub_disk *disk, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors);
+ grub_disk_addr_t **sectors,
+ int emu_512b);
#endif
const char *
grub_hostdisk_os_dev_to_grub_drive (const char *os_dev, int add);
diff --git a/include/grub/fs.h b/include/grub/fs.h
index 302e48d4b..a3f72b6fa 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -88,7 +88,8 @@ struct grub_fs
grub_err_t (*fs_embed) (grub_device_t device, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors);
+ grub_disk_addr_t **sectors,
+ int emu_512b);
/* Whether this filesystem reserves first sector for DOS-style boot. */
int reserved_first_sector;
diff --git a/include/grub/partition.h b/include/grub/partition.h
index 7adb7ec6e..2a6f5e6d1 100644
--- a/include/grub/partition.h
+++ b/include/grub/partition.h
@@ -55,7 +55,8 @@ struct grub_partition_map
grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,
unsigned int max_nsectors,
grub_embed_type_t embed_type,
- grub_disk_addr_t **sectors);
+ grub_disk_addr_t **sectors,
+ int emu_512b);
#endif
};
typedef struct grub_partition_map *grub_partition_map_t;
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 2631b1074..daaef1725 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -193,13 +193,13 @@ grub_util_bios_setup (const char *dir,
const char *boot_file, const char *core_file,
const char *dest, int force,
int fs_probe, int allow_floppy,
- int add_rs_codes);
+ int add_rs_codes, int emu_512b);
void
grub_util_sparc_setup (const char *dir,
const char *boot_file, const char *core_file,
const char *dest, int force,
int fs_probe, int allow_floppy,
- int add_rs_codes);
+ int add_rs_codes, int emu_512b);
char *
grub_install_get_image_targets_string (void);
diff --git a/util/grub-install.c b/util/grub-install.c
index 8970b73aa..781ad3fc0 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -79,6 +79,7 @@ static char *label_color;
static char *label_bgcolor;
static char *product_version;
static int add_rs_codes = 1;
+static int emu_512b = 0;
enum
{
@@ -105,6 +106,7 @@ enum
OPTION_DISK_MODULE,
OPTION_NO_BOOTSECTOR,
OPTION_NO_RS_CODES,
+ OPTION_EMU_512B,
OPTION_MACPPC_DIRECTORY,
OPTION_LABEL_FONT,
OPTION_LABEL_COLOR,
@@ -224,6 +226,10 @@ argp_parser (int key, char *arg, struct argp_state *state)
add_rs_codes = 0;
return 0;
+ case OPTION_EMU_512B:
+ emu_512b = 1;
+ return 0;
+
case OPTION_DEBUG:
verbosity++;
return 0;
@@ -285,6 +291,10 @@ static struct argp_option options[] = {
{"no-rs-codes", OPTION_NO_RS_CODES, 0, 0,
N_("Do not apply any reed-solomon codes when embedding core.img. "
"This option is only available on x86 BIOS targets."), 0},
+ {"emu-512b", OPTION_EMU_512B, 0, 0,
+ N_("Use native-sector-size addressing, but emulate 512-bytes sector lengths when embedding the core image. "
+ "Workaround for broken firmware. "
+ "This option is only available on x86 BIOS targets."), 0},
{"debug", OPTION_DEBUG, 0, OPTION_HIDDEN, 0, 2},
{"no-floppy", OPTION_NO_FLOPPY, 0, OPTION_HIDDEN, 0, 2},
@@ -1704,7 +1714,7 @@ main (int argc, char *argv[])
"boot.img");
grub_install_copy_file (boot_img_src, boot_img, 1);
- grub_util_info ("%sgrub-bios-setup %s %s %s %s %s --directory='%s' --device-map='%s' '%s'",
+ grub_util_info ("%sgrub-bios-setup %s %s %s %s %s %s --directory='%s' --device-map='%s' '%s'",
/* TRANSLATORS: This is a prefix in the log to indicate that usually
a command would be executed but due to an option was skipped. */
install_bootsector ? "" : _("NOT RUNNING: "),
@@ -1713,6 +1723,7 @@ main (int argc, char *argv[])
force ? "--force " : "",
!fs_probe ? "--skip-fs-probe" : "",
!add_rs_codes ? "--no-rs-codes" : "",
+ emu_512b ? "--emu-512b" : "",
platdir,
device_map,
install_device);
@@ -1721,7 +1732,8 @@ main (int argc, char *argv[])
if (install_bootsector)
grub_util_bios_setup (platdir, "boot.img", "core.img",
install_drive, force,
- fs_probe, allow_floppy, add_rs_codes);
+ fs_probe, allow_floppy, add_rs_codes,
+ emu_512b);
break;
}
case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
@@ -1748,7 +1760,7 @@ main (int argc, char *argv[])
grub_util_sparc_setup (platdir, "boot.img", "core.img",
install_drive, force,
fs_probe, allow_floppy,
- 0 /* unused */ );
+ 0 /* unused */, 0 /* unused */);
break;
}
diff --git a/util/grub-setup.c b/util/grub-setup.c
index 42b98ad3c..8297436df 100644
--- a/util/grub-setup.c
+++ b/util/grub-setup.c
@@ -95,6 +95,9 @@ static struct argp_option options[] = {
{"no-rs-codes", NO_RS_CODES_KEY, 0, 0,
N_("Do not apply any reed-solomon codes when embedding core.img. "
"This option is only available on x86 BIOS targets."), 0},
+ {"emu-512b", 'e', 0, 0,
+ N_("Use native-sector-size addressing, but emulate 512 bytes sector lengths when embedding the core image. "
+ "Workaround for broken firmware."), 0},
{ 0, 0, 0, 0, 0, 0 }
};
@@ -135,6 +138,7 @@ struct arguments
int allow_floppy;
char *device;
int add_rs_codes;
+ int emu_512b;
};
static error_t
@@ -194,6 +198,10 @@ argp_parser (int key, char *arg, struct argp_state *state)
arguments->add_rs_codes = 0;
break;
+ case 'e':
+ arguments->emu_512b = 1;
+ break;
+
case ARGP_KEY_ARG:
if (state->arg_num == 0)
arguments->device = xstrdup(arg);
@@ -315,7 +323,7 @@ main (int argc, char *argv[])
arguments.core_file ? : DEFAULT_CORE_FILE,
dest_dev, arguments.force,
arguments.fs_probe, arguments.allow_floppy,
- arguments.add_rs_codes);
+ arguments.add_rs_codes, arguments.emu_512b);
/* Free resources. */
grub_fini_all ();
diff --git a/util/setup.c b/util/setup.c
index 3be88aae1..0574a826a 100644
--- a/util/setup.c
+++ b/util/setup.c
@@ -254,7 +254,8 @@ SETUP (const char *dir,
const char *boot_file, const char *core_file,
const char *dest, int force,
int fs_probe, int allow_floppy,
- int add_rs_codes __attribute__ ((unused))) /* unused on sparc64 */
+ int add_rs_codes __attribute__ ((unused)), /* unused on sparc64 */
+ int emu_512b)
{
char *core_path;
char *boot_img, *core_img, *boot_path;
@@ -267,6 +268,14 @@ SETUP (const char *dir,
bl.first_sector = (grub_disk_addr_t) -1;
+#ifdef GRUB_SETUP_SPARC64
+ if (emu_512b)
+ {
+ grub_util_error ("%s", _("512 bytes sector length emulation mode"
+ "is currently not available on SPARC platforms"));
+ }
+#endif
+
#ifdef GRUB_SETUP_BIOS
bl.current_segment =
GRUB_BOOT_I386_PC_KERNEL_SEG + (GRUB_DISK_SECTOR_SIZE >> 4);
@@ -408,6 +417,25 @@ SETUP (const char *dir,
int i;
grub_fs_t fs;
unsigned int nsec, maxsec;
+ unsigned int emu_sec_factor = (1U) << (dest_dev->disk->log_sector_size
+ - GRUB_DISK_SECTOR_BITS);
+ grub_uint64_t fill = 0;
+
+ /*
+ * If the block doesn't end on physical sector size, calculate emulated
+ * sector count to fill up to physical sector size.
+ */
+ fill = emu_sec_factor - (0x7f % emu_sec_factor);
+
+ if (emu_512b)
+ {
+ /* Sanity checks. */
+ if ((int)(emu_sec_factor) < 1)
+ {
+ grub_util_error ("%s", _("invalid hardware sector size; "
+ "embedding won't be possible"));
+ }
+ }
grub_partition_iterate (dest_dev->disk, identify_partmap, &ctx);
@@ -501,19 +529,7 @@ SETUP (const char *dir,
goto unable_to_embed;
}
- nsec = core_sectors;
-
- if (add_rs_codes)
- maxsec = 2 * core_sectors;
- else
- maxsec = core_sectors;
-
-#ifdef GRUB_SETUP_BIOS
- if (maxsec > ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR)
- >> GRUB_DISK_SECTOR_BITS))
- maxsec = ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR)
- >> GRUB_DISK_SECTOR_BITS);
-#endif
+ nsec = maxsec = core_sectors;
#ifdef GRUB_SETUP_SPARC64
/*
@@ -525,15 +541,66 @@ SETUP (const char *dir,
maxsec += 2;
#endif
+ if (emu_512b)
+ {
+ grub_int64_t read_blocks_quot = (nsec - 1) / 0x7f;
+ grub_int64_t read_blocks_rem = (nsec - 1) % 0x7f;
+
+ /*
+ * We want to fill/pad each complete 127-sectors-block, but not the
+ * last one.
+ *
+ * To do so, we calculate the number of blocks (complete or incomplete)
+ * and subtract one.
+ */
+ grub_int64_t emu_blocks_fill = read_blocks_quot + (!!read_blocks_rem)
+ - 1;
+
+ if (emu_blocks_fill >= 0)
+ {
+ nsec = nsec
+ /*
+ * Padding to fill first HW sector - keep
+ * boot.img/diskboot.img isolated!
+ *
+ * N.B.: calculation is safe as long as the grub-internal
+ * sector size is smaller than or equal to the hardware
+ * sector size.
+ * We checked for this previously.
+ */
+ + (emu_sec_factor - 1)
+ /*
+ * Pad each completed(!) 127-sectors-block to a full
+ * hardware sector, so that each block starts on a proper
+ * hardware sector.
+ */
+ + ((unsigned int)(emu_blocks_fill) * fill);
+ maxsec = nsec;
+ }
+ }
+
+#ifdef GRUB_SETUP_BIOS
+ /*
+ * SPARC currently does not support RS codes.
+ */
+ if (add_rs_codes)
+ maxsec *= 2;
+
+ if (maxsec > ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR)
+ >> GRUB_DISK_SECTOR_BITS))
+ maxsec = ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR)
+ >> GRUB_DISK_SECTOR_BITS);
+#endif
+
if (is_ldm)
err = grub_util_ldm_embed (dest_dev->disk, &nsec, maxsec,
- GRUB_EMBED_PCBIOS, §ors);
+ GRUB_EMBED_PCBIOS, §ors, emu_512b);
else if (ctx.dest_partmap)
err = ctx.dest_partmap->embed (dest_dev->disk, &nsec, maxsec,
- GRUB_EMBED_PCBIOS, §ors);
+ GRUB_EMBED_PCBIOS, §ors, emu_512b);
else
err = fs->fs_embed (dest_dev, &nsec, maxsec,
- GRUB_EMBED_PCBIOS, §ors);
+ GRUB_EMBED_PCBIOS, §ors, emu_512b);
if (!err && nsec < core_sectors)
{
err = grub_error (GRUB_ERR_OUT_OF_RANGE,
@@ -563,9 +630,150 @@ SETUP (const char *dir,
}
bl.block = bl.first_block;
- for (i = 0; i < nsec; i++)
- save_blocklists (sectors[i] + grub_partition_get_start (ctx.container),
- 0, GRUB_DISK_SECTOR_SIZE, &bl);
+ {
+ grub_disk_addr_t last_hw_sector = 0;
+ unsigned int block_pos = 0;
+ int first_sec = 0;
+ int block_end = 0;
+
+ for (i = 0; i < nsec; i++)
+ {
+ grub_disk_addr_t sector = sectors[i]
+ + grub_partition_get_start (ctx.container);
+
+ if (emu_512b)
+ {
+ /*
+ * When booting off a disk, grub reads in core.img in chunks of
+ * 127 blocks, which tradionally are 512 bytes long. The block
+ * length is inherited from old, buggy "Phoenix BIOS" system
+ * firmare that didn't support reading more data at a time.
+ *
+ * The nifty (or scary, depending on your point of view)
+ * consequence of that is that only each 127th sector is
+ * directly addressed, while the others sectors are read-in
+ * via a length parameter based on 512-bytes blocks.
+ *
+ * For buggy firmware that is not aware of anything but
+ * 512-bytes-sector-hardware, but always assumes (and returns)
+ * 512-bytes blocks, sector *addressing* is based on the native
+ * hardware sector size (because this parameter is just passed
+ * to the drive directly), while the length is 512-bytes based.
+ *
+ * In 512-bytes-emulation mode, we hence have to modify the
+ * (internal) blocklist to use native sector adressing, like
+ * this:
+ * - put the first 512-bytes block at the start of a physical
+ * sector (implicitly true through the embedding routine),
+ * since is the code reading in additional data
+ * - put the second 512-bytes block at the start of the next
+ * physical sector (i.e., leaving a gap if necessary)
+ * - put the next 126 512-bytes blocks right after the
+ * previous (i.e., no gap)
+ * - put the next 512-bytes block at the start of the next
+ * possible physical sector (i.e., leaving a gap)
+ * - put the next 126 ...
+ *
+ * This eventually leads to:
+ * - correct native-sector-addressing sector addresses at the
+ * start of each block that NEEDS to be directly addressed
+ * - fake, incrementing sector addresses for the next 126
+ * 512-bytes blocks (which shouldn't hurt since they are
+ * not used in any meaningful way)
+ * - implicitly generating a new blocklist after 127
+ * 512-bytes blocks because the next native sector address
+ * will be smaller than the previously written fake sector
+ * address.
+ *
+ * Even more eventually, this scheme should lead to grub
+ * correctly reading in all data of its core.img file and
+ * successful booting.
+ */
+ if (0 == i)
+ {
+ first_sec = 1;
+ last_hw_sector = sector / emu_sec_factor;
+ }
+ else
+ {
+ /* Sanity check. */
+ grub_disk_addr_t prev_sector = sectors[i - 1]
+ + grub_partition_get_start (ctx.container);
+
+ if (1 != (sector - prev_sector))
+ {
+ grub_util_error ("%s", _("embedding non-contiguous "
+ "ranges is not supported in "
+ "512-bytes sector emulation "
+ "mode"));
+ }
+
+ /* Must be somewhere within a block. */
+ ++block_pos;
+
+ if (block_end)
+ {
+ /*
+ * Previously been at a block end, recalculate HW sector.
+ * This will automatically create a new blocklist if necessary.
+ */
+ last_hw_sector = sector / emu_sec_factor;
+
+ /* Reset block end marker. */
+ block_end = 0;
+ }
+ else
+ {
+ ++last_hw_sector;
+ }
+
+ if (0x7f == block_pos)
+ {
+ /* Last emulated sector in this block. */
+ block_end = 1;
+ }
+ }
+ }
+ else
+ {
+ last_hw_sector = sector;
+ }
+
+ save_blocklists (last_hw_sector, 0, GRUB_DISK_SECTOR_SIZE, &bl);
+
+ if (emu_512b)
+ {
+ /*
+ * N.B.: advancing i here past nsec is safe as long as we don't
+ * use it - in the worst case, the loop will terminate
+ * right away.
+ */
+ if (first_sec)
+ {
+ /*
+ * Skip over the next logical sectors to reach the next HW
+ * sector.
+ *
+ * N.B.: decrease by one due to the implicit increment at
+ * the end of the loop!
+ */
+ i += (emu_sec_factor - 1);
+
+ /* Reset first sector counter to not jump again next time. */
+ first_sec = 0;
+ }
+
+ if (block_end)
+ {
+ /* Likewise, if necessary, but with a different offset. */
+ i += fill;
+
+ /* Reset position counter within a block. */
+ block_pos = 0;
+ }
+ }
+ }
+ }
/* Make sure that the last blocklist is a terminator. */
if (bl.block == bl.first_block)
@@ -641,10 +849,97 @@ SETUP (const char *dir,
}
/* Write the core image onto the disk. */
- for (i = 0; i < nsec; i++)
- grub_disk_write (dest_dev->disk, sectors[i], 0,
- GRUB_DISK_SECTOR_SIZE,
- core_img + i * GRUB_DISK_SECTOR_SIZE);
+ char *null_sector = xmalloc (GRUB_DISK_SECTOR_SIZE);
+ memset (null_sector, 0, GRUB_DISK_SECTOR_SIZE);
+
+ {
+ int fill_null = 0;
+ unsigned int block_pos = 0;
+ int first_hw_sec = 0;
+ int block_end = 0;
+ char *core_img_location = core_img;
+
+ for (i = 0; i < nsec; i++)
+ {
+ grub_disk_addr_t sector = sectors[i];
+ char *read_location = core_img_location;
+
+ /*
+ * For a description of what we're doing here in 512-bytes-emulation
+ * mode, refer to the blocklist writing section.
+ *
+ * The magic in this part is that we have to add some padding to
+ * regions that will NOT contain data from core.img.
+ * For simplicity's sake, we'll pad those regions with zero-filled
+ * 512-bytes blocks.
+ */
+ if (emu_512b)
+ {
+ /* Set flag when processing first HW sector. */
+ if (0 == i)
+ {
+ first_hw_sec = 1;
+ }
+ else
+ {
+ /* Reset, completely handled first HW sector. */
+ if (emu_sec_factor == i)
+ {
+ first_hw_sec = 0;
+ fill_null = 0;
+ }
+
+ /* Reset, handled full block, up to HW sector end. */
+ if ((0x7f + fill) == block_pos)
+ {
+ block_end = 0;
+ fill_null = 0;
+ block_pos = 0;
+ }
+
+ ++block_pos;
+
+ if (0x7f == block_pos)
+ {
+ /* Last emulated sector in this block. */
+ block_end = 1;
+ }
+ }
+ }
+
+ if (fill_null)
+ {
+ read_location = null_sector;
+ }
+
+ grub_util_info ("writing to (virtual) disk sector %" PRIuGRUB_UINT64_T, sectors[i]);
+ grub_disk_write (dest_dev->disk,
+ sector, 0,
+ GRUB_DISK_SECTOR_SIZE,
+ read_location);
+
+ /* Advance core_img_location if we actually used data from it. */
+ if (!fill_null)
+ {
+ core_img_location += GRUB_DISK_SECTOR_SIZE;
+ }
+
+ if (emu_512b)
+ {
+ /*
+ * Note that fill_null will be reset once reaching the HW
+ * sector end in the first part of the iteration loop.
+ */
+ if ((first_hw_sec) || (block_end))
+ {
+ /* Zero-fill rest of HW sector. */
+ fill_null = 1;
+ }
+ }
+ }
+ }
+
+ grub_free (null_sector);
#endif
#ifdef GRUB_SETUP_SPARC64
@@ -678,6 +973,12 @@ unable_to_embed:
grub_util_error ("%s", _("embedding is not possible, but this is required for "
"RAID and LVM install"));
+ if (emu_512b)
+ {
+ grub_util_error ("%s", _("512-bytess-emulation only available when "
+ "embedding"));
+ }
+
{
grub_fs_t fs;
fs = grub_fs_probe (root_dev);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/7] grub-install: hook up --emu-512b to sector size autodetection in biosdisk
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
` (2 preceding siblings ...)
2020-05-24 12:25 ` [PATCH v2 3/7] setup: add support for native sector addressing w/ 512-bytes lengths Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 5/7] docs/grub: document --emu-512b install option Mihai Moldovan
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
Chances are that if you need the
native-sector-addressing-with-512-bytes-lengths feature, you will also
need grub to autodetect the native sector size later on.
---
util/grub-install.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/util/grub-install.c b/util/grub-install.c
index 781ad3fc0..7aec83229 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1362,6 +1362,32 @@ main (int argc, char *argv[])
fprintf (load_cfg_f, "set debug='%s'\n",
debug_image);
}
+
+ if (emu_512b)
+ {
+ switch (platform)
+ {
+ case GRUB_INSTALL_PLATFORM_I386_PC:
+ if (!load_cfg_f)
+ {
+ load_cfg_f = grub_util_fopen (load_cfg, "wb");
+ }
+ have_load_cfg = 1;
+ /*
+ * Exporting this variable would be nice, but the export command
+ * is not part of the rescue shell, so that won't work.
+ */
+ fprintf (load_cfg_f,
+ "set biosdisk_autodetect_sector_size='1'\n");
+ break;
+ default:
+ grub_util_error ("%s", _("native-sector-addressing with "
+ "512-bytes length emulation is not "
+ "supported on your platform"));
+ break;
+ }
+ }
+
char *prefix_drive = NULL;
char *install_drive = NULL;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 5/7] docs/grub: document --emu-512b install option
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
` (3 preceding siblings ...)
2020-05-24 12:25 ` [PATCH v2 4/7] grub-install: hook up --emu-512b to sector size autodetection in biosdisk Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 6/7] diskfilter: write out currently scanned partition Mihai Moldovan
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
---
docs/grub.texi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/docs/grub.texi b/docs/grub.texi
index 1ce9993a5..f071487d6 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6433,6 +6433,21 @@ validate the contents of the bootloader embedding area, or in more
modern systems with GPT-style partition tables (@pxref{BIOS
installation}) where GRUB does not reside in any unpartitioned space
outside of the MBR. Disable the Reed-Solomon codes with this option.
+
+@item --emu-512b
+By default on x86 BIOS systems, @command{grub-install} assumes a 512
+bytes sector size when generating and writing the core image. This
+option makes grub use the disk's native sector size for addressing the
+core image's blocks and a 512 bytes read block size, as often found
+with old/buggy system firmware. To that effect, the core image is
+spread out so that each read block starts on a proper hardware sector.
+Additionally, it enables read block and sector size autodetection in
+the @samp{biosdisk} access module by setting the
+@samp{biosdisk_autodetect_sector_size} variable to @samp{1}. You should
+@emph{only} use this option if you @emph{know} that your system
+firmware is buggy and can't handle disks with sector sizes bigger than
+512 bytes correctly and your machine is trying to boot off such a disk.
+It @emph{will} otherwise break normal operation.
@end table
@node Invoking grub-mkconfig
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 6/7] diskfilter: write out currently scanned partition
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
` (4 preceding siblings ...)
2020-05-24 12:25 ` [PATCH v2 5/7] docs/grub: document --emu-512b install option Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 7/7] gpt: respect native sector size if set/detected Mihai Moldovan
2020-05-27 7:29 ` [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
Knowing the disk is fine, but also knowing the partition number is even
better.
Also, add additional non-util debug messages while scanning for explicit
diskfilter types. It can't hurt to get the information in both modes.
---
grub-core/disk/diskfilter.c | 54 +++++++++++++++++++++++++++++++++----
1 file changed, 49 insertions(+), 5 deletions(-)
diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 67bf37a9c..ae7f3e4d2 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -133,10 +133,46 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
struct grub_diskfilter_pv_id id;
grub_diskfilter_t diskfilter;
+ /*
+ * name + ", partition " (12) + partition number as number + \0
+ *
+ * We'll assume a maximum limit of 99999 (or 100000, if zero-indexed)
+ * partitions here, which should be plenty.
+ *
+ * MBR is limited to 3 + 128 (logical) partitions, GPT by default reserves
+ * enough space for 128 partitions, but can (theoretically) be extended
+ * beyond that limit.
+ *
+ * People have tried to go up to 65536, which is/was a limit in gdisk, but
+ * also found out that a lot of tools can't cope with more than 8192
+ * partitions.
+ *
+ * The pactical benefit is doubtful, but we still can spare a few additional
+ * bytes in the string to be compatible with huge amounts of partition
+ * entries.
+ */
+ const int full_name_max_length = grub_strlen (name) + 18;
+ char *full_name = grub_zalloc (full_name_max_length);
+ if (!full_name)
+ {
+ return 0;
+ }
+
+ /* Add disk name. */
+ grub_snprintf (full_name, full_name_max_length, "%s", name);
+
+ if (p)
+ {
+ /* Add partition description and number. */
+ grub_snprintf (full_name + grub_strlen (name),
+ full_name_max_length - grub_strlen (name),
+ ", partition %d", p->number);
+ }
+
grub_dprintf ("diskfilter", "Scanning for DISKFILTER devices on disk %s\n",
- name);
+ full_name);
#ifdef GRUB_UTIL
- grub_util_info ("Scanning for DISKFILTER devices on disk %s", name);
+ grub_util_info ("Scanning for DISKFILTER devices on disk %s", full_name);
#endif
disk->partition = p;
@@ -149,14 +185,19 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
&& m->disk->dev->id == disk->dev->id
&& m->part_start == grub_partition_get_start (disk->partition)
&& m->part_size == grub_disk_get_size (disk))
- return 0;
+ {
+ grub_free (full_name);
+ return 0;
+ }
}
for (diskfilter = grub_diskfilter_list; diskfilter; diskfilter = diskfilter->next)
{
+ grub_dprintf ("diskfilter", "Scanning for %s devices on disk %s\n",
+ diskfilter->name, full_name);
#ifdef GRUB_UTIL
- grub_util_info ("Scanning for %s devices on disk %s",
- diskfilter->name, name);
+ grub_util_info ("Scanning for %s devices on disk %s",
+ diskfilter->name, full_name);
#endif
id.uuid = 0;
id.uuidlen = 0;
@@ -166,6 +207,7 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
{
if (id.uuidlen)
grub_free (id.uuid);
+ grub_free (full_name);
return 0;
}
if (arr && id.uuidlen)
@@ -179,6 +221,8 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
grub_errno = GRUB_ERR_NONE;
}
+ grub_free (full_name);
+
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 7/7] gpt: respect native sector size if set/detected
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
` (5 preceding siblings ...)
2020-05-24 12:25 ` [PATCH v2 6/7] diskfilter: write out currently scanned partition Mihai Moldovan
@ 2020-05-24 12:25 ` Mihai Moldovan
2020-05-27 7:29 ` [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-24 12:25 UTC (permalink / raw
To: The development of GNU GRUB
The GPT parsing code includes some autodetection magic for non-default
sector sizes.
This is working fine if the underlaying disk abstraction uses the
default sector size of 512 bytes, but will return wrong information (and
read invalid data!) if it is not.
Fix this by skipping the sector size autodetection if the disk's sector
size is not the default one and trust it to be correct for subsequent
sector calculations.
---
grub-core/partmap/gpt.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c
index 4b968529a..38721275f 100644
--- a/grub-core/partmap/gpt.c
+++ b/grub-core/partmap/gpt.c
@@ -58,7 +58,7 @@ grub_gpt_partition_map_iterate (grub_disk_t disk,
grub_uint64_t entries;
unsigned int i;
int last_offset = 0;
- int sector_log = 0;
+ unsigned int sector_log = 0;
/* Read the protective MBR. */
if (grub_disk_read (disk, 0, 0, sizeof (mbr), &mbr))
@@ -76,16 +76,39 @@ grub_gpt_partition_map_iterate (grub_disk_t disk,
return grub_error (GRUB_ERR_BAD_PART_TABLE, "no GPT partition map found");
/* Read the GPT header. */
- for (sector_log = 0; sector_log < MAX_SECTOR_LOG; sector_log++)
+ if (disk->log_sector_size > GRUB_DISK_SECTOR_BITS)
{
+ /*
+ * If the disk sector size has been set to a value different from the
+ * default/internal one, it means that *something* autodetected the
+ * native sector size, we don't have to and also shouldn't try to use a
+ * sector size heuristic.
+ */
+ sector_log = disk->log_sector_size - GRUB_DISK_SECTOR_BITS;
+
if (grub_disk_read (disk, 1 << sector_log, 0, sizeof (gpt), &gpt))
return grub_errno;
- if (grub_memcmp (gpt.magic, grub_gpt_magic, sizeof (grub_gpt_magic)) == 0)
- break;
+ if (grub_memcmp (gpt.magic, grub_gpt_magic, sizeof (grub_gpt_magic)) != 0)
+ return grub_error (GRUB_ERR_BAD_PART_TABLE, "no valid GPT header");
+ }
+ else
+ {
+ /*
+ * Otherwise, the native sector size might be wrong, so try to
+ * autodetect.
+ */
+ for (sector_log = 0; sector_log < MAX_SECTOR_LOG; sector_log++)
+ {
+ if (grub_disk_read (disk, 1 << sector_log, 0, sizeof (gpt), &gpt))
+ return grub_errno;
+
+ if (grub_memcmp (gpt.magic, grub_gpt_magic, sizeof (grub_gpt_magic)) == 0)
+ break;
+ }
+ if (sector_log == MAX_SECTOR_LOG)
+ return grub_error (GRUB_ERR_BAD_PART_TABLE, "no valid GPT header");
}
- if (sector_log == MAX_SECTOR_LOG)
- return grub_error (GRUB_ERR_BAD_PART_TABLE, "no valid GPT header");
grub_dprintf ("gpt", "Read a valid GPT header\n");
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/7] support >512b sector disks with old/buggy firmware
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
` (6 preceding siblings ...)
2020-05-24 12:25 ` [PATCH v2 7/7] gpt: respect native sector size if set/detected Mihai Moldovan
@ 2020-05-27 7:29 ` Mihai Moldovan
7 siblings, 0 replies; 9+ messages in thread
From: Mihai Moldovan @ 2020-05-27 7:29 UTC (permalink / raw
To: The development of GNU GRUB
[-- Attachment #1.1: Type: text/plain, Size: 867 bytes --]
* On 5/24/20 2:25 PM, Mihai Moldovan wrote:
> This is a patch series enabling successful boots on machines with
> old/buggy system firmware that always assumes that hardware is using
> 512-bytes sectors, but more modern disks (e.g., 4Kn disks).
> [...]
I know that these commits are missing a SOB line.
Projects handle that differently; usually SOB is ignored or frowned upon. I
wasn't sure whether the GNU GRUB project requires it and a quick check on the
commit log revealed both commits with and without SOB, so I didn't add them.
Reading further messages on the list, it appears that the project does make use
of sign-offs, so I'll add them either a.) after a successful review prior a
merge or b.) on change requests that also require a revbump.
No reason to spam the list with an identical V3 patch set just for SOBs. :)
Mihai
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-27 7:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-24 12:25 [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 1/7] biosdisk: autodetect hardware sector size (opt-in) Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 2/7] biosdisk: restore LBA mode after read/write failures Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 3/7] setup: add support for native sector addressing w/ 512-bytes lengths Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 4/7] grub-install: hook up --emu-512b to sector size autodetection in biosdisk Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 5/7] docs/grub: document --emu-512b install option Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 6/7] diskfilter: write out currently scanned partition Mihai Moldovan
2020-05-24 12:25 ` [PATCH v2 7/7] gpt: respect native sector size if set/detected Mihai Moldovan
2020-05-27 7:29 ` [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Mihai Moldovan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.