Bug 183234 - newfs_msdos(8): [patch] can't boot BeagleBone Black when newfs_msdos is trimming to a multiple of sectors per track
Summary: newfs_msdos(8): [patch] can't boot BeagleBone Black when newfs_msdos is trimm...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-23 11:20 UTC by guyyur
Modified: 2019-01-15 16:13 UTC (History)
5 users (show)

See Also:


Attachments
remove_delta.patch (812 bytes, patch)
2013-10-23 11:20 UTC, guyyur
no flags Details | Diff
huge_sec_conversion.patch (502 bytes, patch)
2013-10-23 11:20 UTC, guyyur
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description guyyur 2013-10-23 11:20:00 UTC
I am booting the BeagleBone Black from a 4 GB microSD card with a
31 MB FAT16 slice aligned on 1 MB and a FreeBSD slice.
The BeagleBone Black firmware ignores the FAT16 filesystem and
doesn't even try to load the MLO file ('CCCC' on ttyU0).
If the FAT16 bpbSectors field is adjusted to be the full sector count
of the slice the boot succeeds.


# gpart show
=>     63  7744449  mmcsd0  MBR  (3.7G)
       63     1985          - free -  (993K)
     2048    63488       1  !4  [active]  (31M)
    65536  7677952       2  freebsd  (3.7G)
  7743488     1024          - free -  (512K)

=>      0  7677952  mmcsd0s2  BSD  (3.7G)
        0  7677952         1  freebsd-ufs  (3.7G)

=>      0  7677952  ufsid/52617ff06c9a983b  BSD  (3.7G)
        0  7677952                       1  freebsd-ufs  (3.7G)


# newfs_msdos -F 16 -c 8 -r 2 -o 2048 -m 0xF8 -L BOOT /dev/md0s1
newfs_msdos: trim 47 sectors to adjust to a multiple of 63
/dev/md0s1: 63344 sectors in 7918 FAT16 clusters (4096 bytes/cluster)
BytesPerSec=512 SecPerClust=8 ResSectors=2 FATs=2 RootDirEnts=512 Sectors=63441 Media=0xf8 FATsecs=31 SecPerTrack=63 Heads=255 HiddenSecs=2048


Didn't work with "newfs_msdos -F 16 -c 8 -L BOOT /dev/md0s1" either.
I added -r, -o and -m based on Windows format which booted succesfully.
The boot only succeeded after changing bpbSectors from 63441 to 63488
in the FAT16 Boot Record.

Fix: You can specify -h, -u, -S, -s and -o flags to bypass the delta check
in newfs_msdos.

1a. NetBSD removed the check in newfs_msdos.c cvs rev 1.40
    http://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/newfs_msdos/newfs_msdos.c.diff?r1=1.39&r2=1.40
    Attached patch (remove_delta.patch)

1b. If the trimming is still needed for older BIOSes,
    maybe an option can be added to skip the delta check.
    Attached patch (add_opt_G.patch) that adds a "-G" option.

2. I noticed the code only converts from bpbHugeSectors to bpbSectors if
   the sum of the hidden and huge sectors is less than or equal MAXU16.
   When formatting in Windows bpbSectors is still used for 63488 sectors
   and 2048 hidden (sum > MAXU16).
   The hidden sectors count is the number of sectors before the
   FAT16 Boot Record so it shouldn't affect the sector count.
   Attached patch (huge_sec_conversion.patch) to only check for
   bpb.bpbHugeSectors <= MAXU16 when converting to bpbSectors.


Index: sbin/newfs_msdos/newfs_msdos.8
===================================================================
--- sbin/newfs_msdos/newfs_msdos.8	(revision 256722)
+++ sbin/newfs_msdos/newfs_msdos.8	(working copy)
@@ -38,6 +38,7 @@
 .Op Fl B Ar boot
 .Op Fl C Ar create-size
 .Op Fl F Ar FAT-type
+.Op Fl G
 .Op Fl I Ar VolumeId
 .Op Fl L Ar label
 .Op Fl O Ar OEM
@@ -103,6 +104,8 @@
 smaller than the size specified as parameter.
 .It Fl F Ar FAT-type
 FAT type (one of 12, 16, or 32).
+.It Fl G
+Ignore geometry in sector count calculation.
 .It Fl I Ar VolumeID
 Volume ID, a 32 bit number in decimal or hexadecimal (0x...) format.
 .It Fl L Ar label
Index: sbin/newfs_msdos/newfs_msdos.c
===================================================================
--- sbin/newfs_msdos/newfs_msdos.c	(revision 256722)
+++ sbin/newfs_msdos/newfs_msdos.c	(working copy)
@@ -236,11 +236,11 @@
 int
 main(int argc, char *argv[])
 {
-    static const char opts[] = "@:NB:C:F:I:L:O:S:a:b:c:e:f:h:i:k:m:n:o:r:s:u:";
+    static const char opts[] = "@:NB:C:F:GI:L:O:S:a:b:c:e:f:h:i:k:m:n:o:r:s:u:";
     const char *opt_B = NULL, *opt_L = NULL, *opt_O = NULL, *opt_f = NULL;
-    u_int opt_F = 0, opt_I = 0, opt_S = 0, opt_a = 0, opt_b = 0, opt_c = 0;
-    u_int opt_e = 0, opt_h = 0, opt_i = 0, opt_k = 0, opt_m = 0, opt_n = 0;
-    u_int opt_o = 0, opt_r = 0, opt_s = 0, opt_u = 0;
+    u_int opt_F = 0, opt_G = 0, opt_I = 0, opt_S = 0, opt_a = 0, opt_b = 0;
+    u_int opt_c = 0, opt_e = 0, opt_h = 0, opt_i = 0, opt_k = 0, opt_m = 0;
+    u_int opt_n = 0, opt_o = 0, opt_r = 0, opt_s = 0, opt_u = 0;
     int opt_N = 0;
     int Iflag = 0, mflag = 0, oflag = 0;
     char buf[MAXPATHLEN];
@@ -283,6 +283,9 @@
 		errx(1, "%s: bad FAT type", optarg);
 	    opt_F = atoi(optarg);
 	    break;
+	case 'G':
+	    opt_G = 1;
+	    break;
 	case 'I':
 	    opt_I = argto4(optarg, 0, "volume ID");
 	    Iflag = 1;
@@ -406,7 +409,7 @@
 	getdiskinfo(fd, fname, dtype, oflag, &bpb);
 	bpb.bpbHugeSectors -= (opt_ofs / bpb.bpbBytesPerSec);
 	delta = bpb.bpbHugeSectors % bpb.bpbSecPerTrack;
-	if (delta != 0) {
+	if (delta != 0 && !opt_G) {
 	    warnx("trim %d sectors to adjust to a multiple of %d",
 		(int)delta, bpb.bpbSecPerTrack);
 	    bpb.bpbHugeSectors -= delta;
@@ -1033,6 +1036,7 @@
 	    "\t-B get bootstrap from file\n"
 	    "\t-C create image file with specified size\n"
 	    "\t-F FAT type (12, 16, or 32)\n"
+	    "\t-G ignore geometry in sector count calculation\n"
 	    "\t-I volume ID\n"
 	    "\t-L volume label\n"
 	    "\t-N don't create file system: just print out parameters\n"
--- add_opt_G.patch ends here ---
How-To-Repeat: Create a FAT16 slices and a FreeBSD slice on a microSD card.
The FAT16 slice should be aligned on 1 MB and have a size that is
not divisible by the sectors per track of the media.
Use newfs_msdos to create the filesystem.
Try to boot the BeagleBone Black from the microSD card.
Comment 1 Andrey Fesenko 2015-07-02 07:16:59 UTC
Thanks this patch work for me, my error messages without this https://lists.freebsd.org/pipermail/freebsd-arm/2015-June/011568.html
Comment 2 Warner Losh freebsd_committer freebsd_triage 2015-08-02 16:13:58 UTC
I think this is an excellent change.

The lies and deceit believed as gospel truth have lead to situations where there's no way to have the right geometry for all users. Having a way to say "I know that the geometry reported is bogus, please do it this other way" is critical.

Unless there's a good objection, I plan to commit this soon.
Comment 3 Andrey Fesenko 2015-08-24 15:09:52 UTC
(In reply to Warner Losh from comment #2)
Hello, whether there are any plans to commit this soon?
Comment 4 guyyur 2016-08-05 15:19:37 UTC
Updated patches for head (code moved to mkfs_msdos.c)

1. NetBSD cvs rev 1.40 to remove chs check.
https://github.com/guyyur/freebsd-src_patches/blob/master/newfs_msdos_remove_chs_align.patch

2. Only check bpbHugeSectors without bpb.bpbHiddenSecs for bpb.bpbSectors/bpb.bpbHugeSectors usage.
(Not needed for BeagleBone Black booting, just matches Windows format command behavior)
https://github.com/guyyur/freebsd-src_patches/blob/master/newfs_msdos_huge_sec_limit_check.patch


The BeagleBone Black also supports loading the MLO from offset 128k instead of the FAT partition so it should bypass the filesystem check.
I tested MLO at 128k with u-boot 2016.05, FAT sector count was not trimmed so I don't know if the filesystem check was bypassed.

dd if=MLO of=/path/to/device bs=128k seek=1 conv=sync,notrunc
dd if=u-boot.img of=/path/to/device bs=384k seek=1 conv=sync,notrunc

This also allows using GPT instead of MBR.
gpart create -s GPT SDCARD
gpart add -t ms-basic-data -b 8192 -s SIZE SDCARD
gpart add -t freebsd-ufs SDCARD
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-08-30 18:01:41 UTC
A commit references this bug:

Author: imp
Date: Tue Aug 30 18:01:20 UTC 2016
New revision: 305074
URL: https://svnweb.freebsd.org/changeset/base/305074

Log:
  Remove CHS alignment. It's not needed and causes problems for the BBB
  boot partition. NetBSD removed it in 1.10 in their repo some time ago.

  Submitted by: Guy Yur
  PR: 183234

Changes:
  head/sbin/newfs_msdos/mkfs_msdos.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-08-30 18:01:44 UTC
A commit references this bug:

Author: imp
Date: Tue Aug 30 18:01:26 UTC 2016
New revision: 305075
URL: https://svnweb.freebsd.org/changeset/base/305075

Log:
  The code only converts from bpbHugeSectors to bpbSectors if the sum of
  the hidden and huge sectors is less than or equal MAXU16. When
  formatting in Windows bpbSectors is still used for 63488 sectors and
  2048 hidden (sum > MAXU16). The hidden sectors count is the number of
  sectors before the FAT16 Boot Record so it shouldn't affect the sector
  count. Attached patch (huge_sec_conversion.patch) to only check for
  bpb.bpbHugeSectors <= MAXU16 when converting to bpbSectors.

  Submitted by: Guy Yur
  PR: 183234

Changes:
  head/sbin/newfs_msdos/mkfs_msdos.c
Comment 7 Andrey Fesenko 2016-09-04 16:13:53 UTC
Thanks it work for me

FreeBSD 12.0-CURRENT #0 r305366
hw.platform: am335x

# gpart show
=>     33  3751903  mmcsd0  MBR  (1.8G)
       33      991          - free -  (496K)
     1024     4096       1  !12  [active]  (2.0M)
     5120  3745792       2  freebsd  (1.8G)
  3750912     1024          - free -  (512K)
Comment 8 Eitan Adler freebsd_committer freebsd_triage 2018-05-28 19:44:50 UTC
batch change:

For bugs that match the following
-  Status Is In progress 
AND
- Untouched since 2018-01-01.
AND
- Affects Base System OR Documentation

DO:

Reset to open status.


Note:
I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
Comment 9 guyyur 2018-05-29 20:34:01 UTC
This bug can be closed.
Comments 5 & 6 contain the commit references that fix this bug.
Comment 10 Ed Maste freebsd_committer freebsd_triage 2018-05-29 20:50:07 UTC
Let me check whether MFC is needed, and will close after.
Comment 11 Warner Losh freebsd_committer freebsd_triage 2018-10-04 02:13:06 UTC
(In reply to Ed Maste from comment #10)
Likely should be MFC'd. It fell off my radar.
Comment 12 commit-hook freebsd_committer freebsd_triage 2019-01-15 15:47:27 UTC
A commit references this bug:

Author: kevans
Date: Tue Jan 15 15:47:02 UTC 2019
New revision: 343044
URL: https://svnweb.freebsd.org/changeset/base/343044

Log:
  MFC r305074-r305075, r327275, r327570: newfs_msdos updates

  r305074:
  Remove CHS alignment. It's not needed and causes problems for the BBB
  boot partition. NetBSD removed it in 1.10 in their repo some time ago.

  r305075:
  The code only converts from bpbHugeSectors to bpbSectors if the sum of
  the hidden and huge sectors is less than or equal MAXU16. When
  formatting in Windows bpbSectors is still used for 63488 sectors and
  2048 hidden (sum > MAXU16). The hidden sectors count is the number of
  sectors before the FAT16 Boot Record so it shouldn't affect the sector
  count. Attached patch (huge_sec_conversion.patch) to only check for
  bpb.bpbHugeSectors <= MAXU16 when converting to bpbSectors.

  r327275:
  Close fd and fd1 before returning now that we're done with them.

  r327570:
  Only call close if fd and fd1 are not -1.

  PR: 183234

Changes:
_U  stable/11/
  stable/11/sbin/newfs_msdos/mkfs_msdos.c
Comment 13 Kyle Evans freebsd_committer freebsd_triage 2019-01-15 16:13:09 UTC
Closing now that it's been MFC'd