| Message ID | 1362995768-30954-1-git-send-email-sonic.adi@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: > From: Sonic Zhang <sonic.zhang@analog.com> > > - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). What problem does this solve? I don't believe this change is correct. The purpose of test_part_dos() is to determine whether a block device contains an MS-DOS partition table. Such a partition table is present in an MBR, but not a PBR. A PBR contains a *FAT file-system, and does not include a partition table.
Hi Stephen, On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >> From: Sonic Zhang <sonic.zhang@analog.com> >> >> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). > > What problem does this solve? > > I don't believe this change is correct. The purpose of test_part_dos() > is to determine whether a block device contains an MS-DOS partition table. > > Such a partition table is present in an MBR, but not a PBR. A PBR > contains a *FAT file-system, and does not include a partition table. The SD card formated by windows 7 into one FAT partition can't be initialized correct in u-boot function init_part() after you reuse the function test_block_type() in function test_part_dos(). So, files on that partition can't be displayed when running command "fatls mmc 0". The only difference in your change is to mark dos partition with flag DOS_PBR invalid. Regards, Sonic
Hi Stephen, On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/11/2013 08:57 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> >> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>> >>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>> >>> What problem does this solve? >>> >>> I don't believe this change is correct. The purpose of test_part_dos() >>> is to determine whether a block device contains an MS-DOS partition table. >>> >>> Such a partition table is present in an MBR, but not a PBR. A PBR >>> contains a *FAT file-system, and does not include a partition table. >> >> The SD card formated by windows 7 into one FAT partition can't be >> initialized correct in u-boot function init_part() after you reuses >> function test_block_type() in function test_part_dos(). So, files on >> that partition can't be displayed when run command "fatls mmc 0". >> >> The only difference in your change is to mark dos partition with flag >> DOS_PBR invalid. > > I did test a raw FAT filesystem on an SD card without any partition > table, and it worked fine. Admittedly I created the layout/filesystem > with Linux rather than Windows, but I don't think the layout would be > any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; > does that make any difference? "fatls mmc 0:0" makes no difference. Regards, Sonic
On 03/12/2013 08:58 PM, Sonic Zhang wrote: > Hi Stephen, > > On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 03/11/2013 08:57 PM, Sonic Zhang wrote: >>> Hi Stephen, >>> >>> >>> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>>> >>>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>>> >>>> What problem does this solve? >>>> >>>> I don't believe this change is correct. The purpose of test_part_dos() >>>> is to determine whether a block device contains an MS-DOS partition table. >>>> >>>> Such a partition table is present in an MBR, but not a PBR. A PBR >>>> contains a *FAT file-system, and does not include a partition table. >>> >>> The SD card formated by windows 7 into one FAT partition can't be >>> initialized correct in u-boot function init_part() after you reuses >>> function test_block_type() in function test_part_dos(). So, files on >>> that partition can't be displayed when run command "fatls mmc 0". >>> >>> The only difference in your change is to mark dos partition with flag >>> DOS_PBR invalid. >> >> I did test a raw FAT filesystem on an SD card without any partition >> table, and it worked fine. Admittedly I created the layout/filesystem >> with Linux rather than Windows, but I don't think the layout would be >> any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; >> does that make any difference? > > "fatls mmc 0:0" makes no difference. I have reproduced this. However, I believe it's not a simple "the code is wrong" issue, but rather some kind of issue with stale state sticking around. In other words, the following works just fine: ======== Reset the board Insert an SD card with a raw FAT filesystem; no partitions Tegra20 (SeaBoard) # ls mmc 1 3637576 zimage 0 raw-fat-no-partititions 2 file(s), 0 dir(s) ======== (I have a file named raw-fat-no-partitions on the card so I can easily identify it) However, the following fails: ======== Reset the board Insert an SD card with DOS partition table, two partitions, each containing a FAT filesystem Tegra20 (SeaBoard) # ls mmc 1 3637576 zimage 0 part-id-1 2 file(s), 0 dir(s) Insert an SD card with a raw FAT filesystem; no partitions Tegra20 (SeaBoard) # mmc rescan 1 Tegra20 (SeaBoard) # ls mmc 1 ** Can't read partition table on 1:0 ** ** Invalid partition 1 ** ======== (Again, I have a file named part-id-1 on the other card so I can easily identify it) This reproduces all the way back to at least commit d1efb64 "disk: part_dos: don't claim whole-disk FAT filesystems", which is where I fixed raw-FAT-filesystem-without-partition-tables, and is the change you wanted to revert. I'll see if I can track down what's going on.
On 03/13/2013 10:51 AM, Stephen Warren wrote: > On 03/12/2013 08:58 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> On Tue, Mar 12, 2013 at 11:09 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 08:57 PM, Sonic Zhang wrote: >>>> Hi Stephen, >>>> >>>> >>>> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>>>> >>>>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>>>> >>>>> What problem does this solve? >>>>> >>>>> I don't believe this change is correct. The purpose of test_part_dos() >>>>> is to determine whether a block device contains an MS-DOS partition table. >>>>> >>>>> Such a partition table is present in an MBR, but not a PBR. A PBR >>>>> contains a *FAT file-system, and does not include a partition table. >>>> >>>> The SD card formated by windows 7 into one FAT partition can't be >>>> initialized correct in u-boot function init_part() after you reuses >>>> function test_block_type() in function test_part_dos(). So, files on >>>> that partition can't be displayed when run command "fatls mmc 0". >>>> >>>> The only difference in your change is to mark dos partition with flag >>>> DOS_PBR invalid. >>> >>> I did test a raw FAT filesystem on an SD card without any partition >>> table, and it worked fine. Admittedly I created the layout/filesystem >>> with Linux rather than Windows, but I don't think the layout would be >>> any difference. What if you "fatls mmc 0:0" rather than "fatls mmc 0"; >>> does that make any difference? >> >> "fatls mmc 0:0" makes no difference. > > I have reproduced this. However, I believe it's not a simple "the code > is wrong" issue, but rather some kind of issue with stale state sticking > around. Oh, actually perhaps I haven't. What is the exact error message that you see? If I apply your patch, it doesn't solve the problem that I described; I suspect the problem you're seeing is something different.
On 03/11/2013 08:59 PM, Sonic Zhang wrote: > Hi Stephen, > > On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>> From: Sonic Zhang <sonic.zhang@analog.com> >>> >>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >> >> What problem does this solve? >> >> I don't believe this change is correct. The purpose of test_part_dos() >> is to determine whether a block device contains an MS-DOS partition table. >> >> Such a partition table is present in an MBR, but not a PBR. A PBR >> contains a *FAT file-system, and does not include a partition table. > > The SD card formated by windows 7 into one FAT partition can't be > initialized correct in u-boot function init_part() after you reuse the > function test_block_type() in function test_part_dos(). So, files on > that partition can't be displayed when running command "fatls mmc 0". > > The only difference in your change is to mark dos partition with flag > DOS_PBR invalid. Hmmm. I obtained an SD card that had been formatted in Windows 7 (inserted SD card, right-clicked on it in Explorer, selected Format, selected default FAT32 options), and could not reproduce this issue. Can you give more explicit instructions on how to reproduce this problem? Perhaps a hexdump of the first sector would also help, or uploading a heavily compressed image of the SD card that I can dd onto mine. Also, what branch/commit of U-Boot are you using?
Hi Stephen, On Thu, Mar 14, 2013 at 1:11 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/11/2013 08:59 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>> >>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>> >>> What problem does this solve? >>> >>> I don't believe this change is correct. The purpose of test_part_dos() >>> is to determine whether a block device contains an MS-DOS partition table. >>> >>> Such a partition table is present in an MBR, but not a PBR. A PBR >>> contains a *FAT file-system, and does not include a partition table. >> >> The SD card formated by windows 7 into one FAT partition can't be >> initialized correct in u-boot function init_part() after you reuse the >> function test_block_type() in function test_part_dos(). So, files on >> that partition can't be displayed when running command "fatls mmc 0". >> >> The only difference in your change is to mark dos partition with flag >> DOS_PBR invalid. > > Hmmm. I obtained an SD card that had been formatted in Windows 7 > (inserted SD card, right-clicked on it in Explorer, selected Format, > selected default FAT32 options), and could not reproduce this issue. > > Can you give more explicit instructions on how to reproduce this > problem? Perhaps a hexdump of the first sector would also help, or > uploading a heavily compressed image of the SD card that I can dd onto mine. > > Also, what branch/commit of U-Boot are you using? You should create a FAT partition on your SD card other than FAT32. Regards, Sonic
On 03/13/2013 08:51 PM, Sonic Zhang wrote: > Hi Stephen, > > On Thu, Mar 14, 2013 at 1:11 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 03/11/2013 08:59 PM, Sonic Zhang wrote: >>> Hi Stephen, >>> >>> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>>> >>>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>>> >>>> What problem does this solve? >>>> >>>> I don't believe this change is correct. The purpose of test_part_dos() >>>> is to determine whether a block device contains an MS-DOS partition table. >>>> >>>> Such a partition table is present in an MBR, but not a PBR. A PBR >>>> contains a *FAT file-system, and does not include a partition table. >>> >>> The SD card formated by windows 7 into one FAT partition can't be >>> initialized correct in u-boot function init_part() after you reuse the >>> function test_block_type() in function test_part_dos(). So, files on >>> that partition can't be displayed when running command "fatls mmc 0". >>> >>> The only difference in your change is to mark dos partition with flag >>> DOS_PBR invalid. >> >> Hmmm. I obtained an SD card that had been formatted in Windows 7 >> (inserted SD card, right-clicked on it in Explorer, selected Format, >> selected default FAT32 options), and could not reproduce this issue. >> >> Can you give more explicit instructions on how to reproduce this >> problem? Perhaps a hexdump of the first sector would also help, or >> uploading a heavily compressed image of the SD card that I can dd onto mine. >> >> Also, what branch/commit of U-Boot are you using? > > You should create a FAT partition on your SD card other than FAT32. Windows didn't give me that option. Do I need a smaller SD card to get that option? Can you simply upload a compressed disk image or a hex dump of the first sector instead?
Hi Stephen, On Thu, Mar 14, 2013 at 12:36 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/13/2013 08:51 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> On Thu, Mar 14, 2013 at 1:11 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 08:59 PM, Sonic Zhang wrote: >>>> Hi Stephen, >>>> >>>> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>>>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>>>> >>>>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>>>> >>>>> What problem does this solve? >>>>> >>>>> I don't believe this change is correct. The purpose of test_part_dos() >>>>> is to determine whether a block device contains an MS-DOS partition table. >>>>> >>>>> Such a partition table is present in an MBR, but not a PBR. A PBR >>>>> contains a *FAT file-system, and does not include a partition table. >>>> >>>> The SD card formated by windows 7 into one FAT partition can't be >>>> initialized correct in u-boot function init_part() after you reuse the >>>> function test_block_type() in function test_part_dos(). So, files on >>>> that partition can't be displayed when running command "fatls mmc 0". >>>> >>>> The only difference in your change is to mark dos partition with flag >>>> DOS_PBR invalid. >>> >>> Hmmm. I obtained an SD card that had been formatted in Windows 7 >>> (inserted SD card, right-clicked on it in Explorer, selected Format, >>> selected default FAT32 options), and could not reproduce this issue. >>> >>> Can you give more explicit instructions on how to reproduce this >>> problem? Perhaps a hexdump of the first sector would also help, or >>> uploading a heavily compressed image of the SD card that I can dd onto mine. >>> >>> Also, what branch/commit of U-Boot are you using? >> >> You should create a FAT partition on your SD card other than FAT32. > > Windows didn't give me that option. Do I need a smaller SD card to get > that option? Can you simply upload a compressed disk image or a hex dump > of the first sector instead? > Yes, you can use a small SD card such as 256M, 512M. Or create a small partition on your large SD card. Regards, Sonic
On 03/14/2013 01:31 AM, Sonic Zhang wrote: ... >> Windows didn't give me that option. Do I need a smaller SD card to get >> that option? Can you simply upload a compressed disk image or a hex dump >> of the first sector instead? >> > > Yes, you can use a small SD card such as 256M, 512M. OK. The smallest card I have is 2GB, and I really don't want to lose its content for a test. Aside from that, I only have 8GB and 16GB... > Or create a small partition on your large SD card. Now I'm confused. I thought you said this problem happened when there was a raw filesystem placed directly onto the card, without any kind of partition table? If so, then I don't see how creating a small partition on my card would help reproduce the problem. Again, can you simply send me a compressed image that demonstrates this problem. Something like: (WARNING: this will wipe any data on your SD card) dd if=/dev/zero of=/dev/whatever-your-sd-card-is bs=32768 Then, format/... the card in Windows Then, validate that the problem reproduces with it (or just skip all the above if your current SD card content isn't confidential, and there's little enough data that it will compress well) dd if=/dev/whatever-your-sd-card-is of=sd.img bs=32768 bzip2 sd.img Then, mail me sd.img.bz2 Thanks.
On 03/11/2013 08:59 PM, Sonic Zhang wrote: > Hi Stephen, > > On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>> From: Sonic Zhang <sonic.zhang@analog.com> >>> >>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >> >> What problem does this solve? >> >> I don't believe this change is correct. The purpose of test_part_dos() >> is to determine whether a block device contains an MS-DOS partition table. >> >> Such a partition table is present in an MBR, but not a PBR. A PBR >> contains a *FAT file-system, and does not include a partition table. > > The SD card formated by windows 7 into one FAT partition can't be > initialized correct in u-boot function init_part() after you reuse the > function test_block_type() in function test_part_dos(). So, files on > that partition can't be displayed when running command "fatls mmc 0". > > The only difference in your change is to mark dos partition with flag > DOS_PBR invalid. Thanks for sending me the disk image. The image is a mess; it's been manipulated by a variety of tools at different times that have left rather a lot of cruft there. The first sector does appear to be an actual MBR, containing a single partition starting at LBA 0x10 (byte offset 0x2000), and quite large in size. At LBA 0x10, I do see what may be the start of a FAT16 file-system. So far, so good. However, the partition table contains the string "FAT32" at 0x52, and also the string "mkdosfs" at 0x03. I believe that in the past, mkdosfs was used on this card to create a raw FAT filesystem without any partition table. Then later, some partitioning tool was run to create the partition I mentioned above. Finally you said that Windows was used to create the FAT filesystem within the partition. However, the partitioning tool didn't wipe out the region of the MBR that contains the boot code, and hence didn't wipe out the "FAT32" filesystem signature. Finally, in LBA 3 (byte offset 0x600), I see another sector that looks remarkably like the start of a (presumably long-gone) FAT filesystem. Perhaps an old partition table on this device contained a partition that started in this (non-cylinder-aligned) sector. This sector contains the same "mkdosfs" and "FAT32" signatures. If we take your patch, we end up with the following situation: With your strange partition table: ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so works, and picks partition 1. ls mmc 0:0 -> Explicit request for "partition" 0 (whole-disk). This option doesn't make sense here, since the whole-disk is not a file-system, but rather a partitioned device. With a real raw FAT filesystem; no partitions: ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so tries to access a non- existent partition table entryrather than the whole disk, so automatic mode fails. ls mmc 0:0 -> Explicit request for "partition" 0 (whole-disk), so works. So the issue is that the automatic handling of raw FAT filesystems (i.e. use of the entire disk rather than the first partition) fails with your patch. Perhaps it's acceptable that people with raw FAT filesystems must explicitly specify ":0" to access the whole disk, and we accept that automatic mode won't work? I'll let Tom or Wolfgang make the call. As far as I can tell, the Linux kernel never looks at the "FAT" or "FAT32" strings in the MBR, and hence accepts your disk as having a partition table. And since in Linux you must always use a specific device (/dev/sda or /dev/sdaN), this issue doesn't arise. U-Boot's automatic partition-or-whole-device selection is something Linux doesn't do. One other thing to note: commands such as "mmc part" or "part list" won't work for your disk. After my patch d1efb64 "disk: part_dos: don't claim whole-disk FAT filesystems", if test_block_type()!=DOS_MBR (i.e. in your case), then print_partition_extended() will simply print an error. Before that patch, if test_block_type()==DOS_PBR (i.e. in your case) then print_partition_extended() would print a fake partition table entry that covered the whole disk. Neither action is correct for your disk since it imagine that there was a raw FAT filesystem covering the entire disk. In other words, U-Boot's partition table printing commands never worked correctly on your disk, even if accessing the file-system (accidentally?) used to! Another solution here is for you to simply: # Back up your MBR in case something goes wrong. dd if=/dev/whatever of=backup.bin bs=1 count=512 # Zero out the boot code portion of your MBR, # which will also zero out the false "FAT32" signature. dd if=/dev/zero of=/dev/whatever bs=1 count=446 conv=notrunc Alternatively, if there's still some command in Windows that will install a regular MS-DOS/Windows MBR boot code onto your disk, use that (fdisk /mbr???). Presumably such a command would over-write only those same first 446 bytes, and leave the actual partition table in tact.
Hi Stephen, On Fri, Mar 15, 2013 at 1:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 03/11/2013 08:59 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>> >>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). >>> >>> What problem does this solve? >>> >>> I don't believe this change is correct. The purpose of test_part_dos() >>> is to determine whether a block device contains an MS-DOS partition table. >>> >>> Such a partition table is present in an MBR, but not a PBR. A PBR >>> contains a *FAT file-system, and does not include a partition table. >> >> The SD card formated by windows 7 into one FAT partition can't be >> initialized correct in u-boot function init_part() after you reuse the >> function test_block_type() in function test_part_dos(). So, files on >> that partition can't be displayed when running command "fatls mmc 0". >> >> The only difference in your change is to mark dos partition with flag >> DOS_PBR invalid. > > Thanks for sending me the disk image. > > The image is a mess; it's been manipulated by a variety of tools at > different times that have left rather a lot of cruft there. > > The first sector does appear to be an actual MBR, containing a single > partition starting at LBA 0x10 (byte offset 0x2000), and quite large in > size. At LBA 0x10, I do see what may be the start of a FAT16 > file-system. So far, so good. > > However, the partition table contains the string "FAT32" at 0x52, and > also the string "mkdosfs" at 0x03. I believe that in the past, mkdosfs > was used on this card to create a raw FAT filesystem without any > partition table. Then later, some partitioning tool was run to create > the partition I mentioned above. Finally you said that Windows was used > to create the FAT filesystem within the partition. However, the > partitioning tool didn't wipe out the region of the MBR that contains > the boot code, and hence didn't wipe out the "FAT32" filesystem signature. > > Finally, in LBA 3 (byte offset 0x600), I see another sector that looks > remarkably like the start of a (presumably long-gone) FAT filesystem. > Perhaps an old partition table on this device contained a partition that > started in this (non-cylinder-aligned) sector. This sector contains the > same "mkdosfs" and "FAT32" signatures. > > If we take your patch, we end up with the following situation: > > With your strange partition table: > > ls mmc 0 > ls mmc 0:auto > -> Thinks there's a partition table, so works, and picks > partition 1. > ls mmc 0:0 > -> Explicit request for "partition" 0 (whole-disk). This option > doesn't make sense here, since the whole-disk is not a > file-system, but rather a partitioned device. > > With a real raw FAT filesystem; no partitions: > > ls mmc 0 > ls mmc 0:auto > -> Thinks there's a partition table, so tries to access a non- > existent partition table entryrather than the whole disk, > so automatic mode fails. > ls mmc 0:0 > -> Explicit request for "partition" 0 (whole-disk), so works. > > So the issue is that the automatic handling of raw FAT filesystems (i.e. > use of the entire disk rather than the first partition) fails with your > patch. Perhaps it's acceptable that people with raw FAT filesystems must > explicitly specify ":0" to access the whole disk, and we accept that > automatic mode won't work? I'll let Tom or Wolfgang make the call. > > As far as I can tell, the Linux kernel never looks at the "FAT" or > "FAT32" strings in the MBR, and hence accepts your disk as having a > partition table. And since in Linux you must always use a specific > device (/dev/sda or /dev/sdaN), this issue doesn't arise. U-Boot's > automatic partition-or-whole-device selection is something Linux doesn't do. > > One other thing to note: commands such as "mmc part" or "part list" > won't work for your disk. After my patch d1efb64 "disk: part_dos: don't > claim whole-disk FAT filesystems", if test_block_type()!=DOS_MBR (i.e. > in your case), then print_partition_extended() will simply print an > error. Before that patch, if test_block_type()==DOS_PBR (i.e. in your > case) then print_partition_extended() would print a fake partition table > entry that covered the whole disk. Neither action is correct for your > disk since it imagine that there was a raw FAT filesystem covering the > entire disk. In other words, U-Boot's partition table printing commands > never worked correctly on your disk, even if accessing the file-system > (accidentally?) used to! > > Another solution here is for you to simply: > > # Back up your MBR in case something goes wrong. > dd if=/dev/whatever of=backup.bin bs=1 count=512 > > # Zero out the boot code portion of your MBR, > # which will also zero out the false "FAT32" signature. > dd if=/dev/zero of=/dev/whatever bs=1 count=446 conv=notrunc > > Alternatively, if there's still some command in Windows that will > install a regular MS-DOS/Windows MBR boot code onto your disk, use that > (fdisk /mbr???). Presumably such a command would over-write only those > same first 446 bytes, and leave the actual partition table in tact. We can erase the first 3 blocks completely before formatting it again. But, nothing can prevent others from formatting the SD card with different tools.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/15/2013 01:10 AM, Stephen Warren wrote: > On 03/11/2013 08:59 PM, Sonic Zhang wrote: >> Hi Stephen, >> >> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren >> <swarren@wwwdotorg.org> wrote: >>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>> >>>> - Should return 0 for both DOS_MBR and DOS_PBR block types >>>> in test_part_dos(). >>> >>> What problem does this solve? >>> >>> I don't believe this change is correct. The purpose of >>> test_part_dos() is to determine whether a block device >>> contains an MS-DOS partition table. >>> >>> Such a partition table is present in an MBR, but not a PBR. A >>> PBR contains a *FAT file-system, and does not include a >>> partition table. >> >> The SD card formated by windows 7 into one FAT partition can't be >> initialized correct in u-boot function init_part() after you >> reuse the function test_block_type() in function >> test_part_dos(). So, files on that partition can't be displayed >> when running command "fatls mmc 0". >> >> The only difference in your change is to mark dos partition with >> flag DOS_PBR invalid. > > Thanks for sending me the disk image. > > The image is a mess; it's been manipulated by a variety of tools at > different times that have left rather a lot of cruft there. > > The first sector does appear to be an actual MBR, containing a > single partition starting at LBA 0x10 (byte offset 0x2000), and > quite large in size. At LBA 0x10, I do see what may be the start > of a FAT16 file-system. So far, so good. > > However, the partition table contains the string "FAT32" at 0x52, > and also the string "mkdosfs" at 0x03. I believe that in the past, > mkdosfs was used on this card to create a raw FAT filesystem > without any partition table. Then later, some partitioning tool > was run to create the partition I mentioned above. Finally you > said that Windows was used to create the FAT filesystem within the > partition. However, the partitioning tool didn't wipe out the > region of the MBR that contains the boot code, and hence didn't > wipe out the "FAT32" filesystem signature. > > Finally, in LBA 3 (byte offset 0x600), I see another sector that > looks remarkably like the start of a (presumably long-gone) FAT > filesystem. Perhaps an old partition table on this device > contained a partition that started in this (non-cylinder-aligned) > sector. This sector contains the same "mkdosfs" and "FAT32" > signatures. > > If we take your patch, we end up with the following situation: > > With your strange partition table: > > ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so > works, and picks partition 1. ls mmc 0:0 -> Explicit request for > "partition" 0 (whole-disk). This option doesn't make sense here, > since the whole-disk is not a file-system, but rather a > partitioned device. > > With a real raw FAT filesystem; no partitions: > > ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so > tries to access a non- existent partition table entryrather than > the whole disk, so automatic mode fails. ls mmc 0:0 -> Explicit > request for "partition" 0 (whole-disk), so works. > > So the issue is that the automatic handling of raw FAT filesystems > (i.e. use of the entire disk rather than the first partition) > fails with your patch. Perhaps it's acceptable that people with raw > FAT filesystems must explicitly specify ":0" to access the whole > disk, and we accept that automatic mode won't work? I'll let Tom > or Wolfgang make the call. Thanks for looking into all of this. What exactly fails with this image, without this patch applied? - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRQxgVAAoJENk4IS6UOR1WCtEQAIvLpSOD8ZgM2B0aKNUI92sr ZgULRoQ7bjBL12NPrHQEdQn09cS3RTGY7AijWECfH29m5RN6e23tri5/MSRrw9yw jxEPhIEWqNaJ0gEF9qteNm9QDlJyGq9AENMWIf33fqo+hF9jrAsFwzZKRmWnNm8U Euc+KGdIwqBkD2ke+YIwHhV7ohII4LWrUD7FTYTYT78By3YR0YNpqrrgNuJB5Bs6 1TDARj/qTYk5uGy+1Ep8EqTIMqWfWtmeUhE5k6wYONOYPaETnDTezoxvN4wAuIyP QF25XkJbNfU39UreGnXscWjeuS/3FHzEC6ArfT0n6uVv94fAbcH/w6gFUanNa3Ya 50/314K0ePiRH2rJP8/7tiCQixZtykvAfx5IfLr4HZsbDZtYMcLkKGvKiWkt+0Hp MKKm059GK4W1tPbc31sFToqrGNNo41bE8hcDBa8rjFsbKkdDbXnMdMyRm4J5qWBX MTGYsmw4veRVn8NAZVkCkyckZOIPu2I4CwLv4KyMBQZ/p/o70xzMMpOyBM6kXWWE +8uoFZEL0jjJh214TUjRNu6YfzKyTiIriO6zKRsU0Do1iGGQ9EhiJxWKeLsuxz2i C7o11B4ayFsrpJ+GfpV6JEl2UBRdBmiDmsp+9bwwH1eMBLkBMexH+KzNUTdW5eKI 7pumNMZepGQnQYXXHH0L =RjQu -----END PGP SIGNATURE-----
On Fri, Mar 15, 2013 at 02:36:21PM +0800, Sonic Zhang wrote: > Hi Stephen, > > On Fri, Mar 15, 2013 at 1:10 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > > On 03/11/2013 08:59 PM, Sonic Zhang wrote: > >> Hi Stephen, > >> > >> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > >>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: > >>>> From: Sonic Zhang <sonic.zhang@analog.com> > >>>> > >>>> - Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos(). > >>> > >>> What problem does this solve? > >>> > >>> I don't believe this change is correct. The purpose of test_part_dos() > >>> is to determine whether a block device contains an MS-DOS partition table. > >>> > >>> Such a partition table is present in an MBR, but not a PBR. A PBR > >>> contains a *FAT file-system, and does not include a partition table. > >> > >> The SD card formated by windows 7 into one FAT partition can't be > >> initialized correct in u-boot function init_part() after you reuse the > >> function test_block_type() in function test_part_dos(). So, files on > >> that partition can't be displayed when running command "fatls mmc 0". > >> > >> The only difference in your change is to mark dos partition with flag > >> DOS_PBR invalid. > > > > Thanks for sending me the disk image. > > > > The image is a mess; it's been manipulated by a variety of tools at > > different times that have left rather a lot of cruft there. > > > > The first sector does appear to be an actual MBR, containing a single > > partition starting at LBA 0x10 (byte offset 0x2000), and quite large in > > size. At LBA 0x10, I do see what may be the start of a FAT16 > > file-system. So far, so good. > > > > However, the partition table contains the string "FAT32" at 0x52, and > > also the string "mkdosfs" at 0x03. I believe that in the past, mkdosfs > > was used on this card to create a raw FAT filesystem without any > > partition table. Then later, some partitioning tool was run to create > > the partition I mentioned above. Finally you said that Windows was used > > to create the FAT filesystem within the partition. However, the > > partitioning tool didn't wipe out the region of the MBR that contains > > the boot code, and hence didn't wipe out the "FAT32" filesystem signature. > > > > Finally, in LBA 3 (byte offset 0x600), I see another sector that looks > > remarkably like the start of a (presumably long-gone) FAT filesystem. > > Perhaps an old partition table on this device contained a partition that > > started in this (non-cylinder-aligned) sector. This sector contains the > > same "mkdosfs" and "FAT32" signatures. > > > > If we take your patch, we end up with the following situation: > > > > With your strange partition table: > > > > ls mmc 0 > > ls mmc 0:auto > > -> Thinks there's a partition table, so works, and picks > > partition 1. > > ls mmc 0:0 > > -> Explicit request for "partition" 0 (whole-disk). This option > > doesn't make sense here, since the whole-disk is not a > > file-system, but rather a partitioned device. > > > > With a real raw FAT filesystem; no partitions: > > > > ls mmc 0 > > ls mmc 0:auto > > -> Thinks there's a partition table, so tries to access a non- > > existent partition table entryrather than the whole disk, > > so automatic mode fails. > > ls mmc 0:0 > > -> Explicit request for "partition" 0 (whole-disk), so works. > > > > So the issue is that the automatic handling of raw FAT filesystems (i.e. > > use of the entire disk rather than the first partition) fails with your > > patch. Perhaps it's acceptable that people with raw FAT filesystems must > > explicitly specify ":0" to access the whole disk, and we accept that > > automatic mode won't work? I'll let Tom or Wolfgang make the call. > > > > As far as I can tell, the Linux kernel never looks at the "FAT" or > > "FAT32" strings in the MBR, and hence accepts your disk as having a > > partition table. And since in Linux you must always use a specific > > device (/dev/sda or /dev/sdaN), this issue doesn't arise. U-Boot's > > automatic partition-or-whole-device selection is something Linux doesn't do. > > > > One other thing to note: commands such as "mmc part" or "part list" > > won't work for your disk. After my patch d1efb64 "disk: part_dos: don't > > claim whole-disk FAT filesystems", if test_block_type()!=DOS_MBR (i.e. > > in your case), then print_partition_extended() will simply print an > > error. Before that patch, if test_block_type()==DOS_PBR (i.e. in your > > case) then print_partition_extended() would print a fake partition table > > entry that covered the whole disk. Neither action is correct for your > > disk since it imagine that there was a raw FAT filesystem covering the > > entire disk. In other words, U-Boot's partition table printing commands > > never worked correctly on your disk, even if accessing the file-system > > (accidentally?) used to! > > > > Another solution here is for you to simply: > > > > # Back up your MBR in case something goes wrong. > > dd if=/dev/whatever of=backup.bin bs=1 count=512 > > > > # Zero out the boot code portion of your MBR, > > # which will also zero out the false "FAT32" signature. > > dd if=/dev/zero of=/dev/whatever bs=1 count=446 conv=notrunc > > > > Alternatively, if there's still some command in Windows that will > > install a regular MS-DOS/Windows MBR boot code onto your disk, use that > > (fdisk /mbr???). Presumably such a command would over-write only those > > same first 446 bytes, and leave the actual partition table in tact. > > We can erase the first 3 blocks completely before formatting it again. > But, nothing can prevent others from formatting the SD card with > different tools. Well, it sounds like this card has been put through a series of odd formats and it's not strictly a valid card, but just not something that doesn't break (because we're trying to be clever and it's causing us problems). Or do you have a number of cards which show this problem?
On 03/15/2013 06:46 AM, Tom Rini wrote: > On 03/15/2013 01:10 AM, Stephen Warren wrote: >> On 03/11/2013 08:59 PM, Sonic Zhang wrote: >>> Hi Stephen, >>> >>> On Tue, Mar 12, 2013 at 1:28 AM, Stephen Warren >>> <swarren@wwwdotorg.org> wrote: >>>> On 03/11/2013 03:56 AM, sonic.adi@gmail.com wrote: >>>>> From: Sonic Zhang <sonic.zhang@analog.com> >>>>> >>>>> - Should return 0 for both DOS_MBR and DOS_PBR block types >>>>> in test_part_dos(). >>>> >>>> What problem does this solve? >>>> >>>> I don't believe this change is correct. The purpose of >>>> test_part_dos() is to determine whether a block device >>>> contains an MS-DOS partition table. >>>> >>>> Such a partition table is present in an MBR, but not a PBR. A >>>> PBR contains a *FAT file-system, and does not include a >>>> partition table. >>> >>> The SD card formated by windows 7 into one FAT partition can't >>> be initialized correct in u-boot function init_part() after you >>> reuse the function test_block_type() in function >>> test_part_dos(). So, files on that partition can't be >>> displayed when running command "fatls mmc 0". >>> >>> The only difference in your change is to mark dos partition >>> with flag DOS_PBR invalid. > >> Thanks for sending me the disk image. > >> The image is a mess; it's been manipulated by a variety of tools >> at different times that have left rather a lot of cruft there. > >> The first sector does appear to be an actual MBR, containing a >> single partition starting at LBA 0x10 (byte offset 0x2000), and >> quite large in size. At LBA 0x10, I do see what may be the start >> of a FAT16 file-system. So far, so good. > >> However, the partition table contains the string "FAT32" at 0x52, >> and also the string "mkdosfs" at 0x03. I believe that in the >> past, mkdosfs was used on this card to create a raw FAT >> filesystem without any partition table. Then later, some >> partitioning tool was run to create the partition I mentioned >> above. Finally you said that Windows was used to create the FAT >> filesystem within the partition. However, the partitioning tool >> didn't wipe out the region of the MBR that contains the boot >> code, and hence didn't wipe out the "FAT32" filesystem >> signature. > >> Finally, in LBA 3 (byte offset 0x600), I see another sector that >> looks remarkably like the start of a (presumably long-gone) FAT >> filesystem. Perhaps an old partition table on this device >> contained a partition that started in this >> (non-cylinder-aligned) sector. This sector contains the same >> "mkdosfs" and "FAT32" signatures. > >> If we take your patch, we end up with the following situation: > >> With your strange partition table: > >> ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so >> works, and picks partition 1. ls mmc 0:0 -> Explicit request for >> "partition" 0 (whole-disk). This option doesn't make sense here, >> since the whole-disk is not a file-system, but rather a >> partitioned device. > >> With a real raw FAT filesystem; no partitions: > >> ls mmc 0 ls mmc 0:auto -> Thinks there's a partition table, so >> tries to access a non- existent partition table entryrather than >> the whole disk, so automatic mode fails. ls mmc 0:0 -> Explicit >> request for "partition" 0 (whole-disk), so works. > >> So the issue is that the automatic handling of raw FAT >> filesystems (i.e. use of the entire disk rather than the first >> partition) fails with your patch. Perhaps it's acceptable that >> people with raw FAT filesystems must explicitly specify ":0" to >> access the whole disk, and we accept that automatic mode won't >> work? I'll let Tom or Wolfgang make the call. > > Thanks for looking into all of this. What exactly fails with this > image, without this patch applied? Without Sonic's patch, if you have a disk that really contains a DOS partition table, but also with a FAT filesystem signature in the partition table's boot code area, then U-Boot will not recognize the partition table, and hence there is no way to access those partitions. There's no workaround that I know of without changing U-Boot code or disk content. With Sonic's patch, if you have a disk that contains a raw FAT filesystem, then U-Boot will consider it to have a DOS partition table (since his patch removes the condition that prevents raw FAT filesystems being considered valid partition tables), and hence this will break some aspects of the "automatic mode" I describe below. Historically, I believe U-Boot users have been able to type "ls mmc 0 /" (i.e. just specify a device and no explicit partition) and U-Boot will "do the right thing". Automatic mode is supposed to work like: On a device with partitions, this automatic mode will pick the first valid partition (i.e. partition 1), and access that partition. On a device without partitions, this automatic mode will pick the entire disk and access the entire disk. Without Sonic's patch, automatic mode works in detail as: On a device with a partition table, but also having a FAT filesystem signature in the MBR boot code area, U-Boot will consider the partition table invalid, and hence the partitions can't be accessed. On a device with a partition table, but NOT also having a FAT filesystem signature in the MBR boot code area, U-Boot will parse the partition table, and hence automatic mode will select the first valid partition table. On a device with a raw FAT filesystem without a partition table, a whole-device partition object is created internally to U-Boot, and hence that raw FAT filesystem will be accessed successfully. With Sonic's patch, automatic mode would become: On a device with a partition table, /irrespective/ of whether it also has a FAT filesystem signature in the MBR boot code area, U-Boot will parse the partition table, and hence automatic mode will select the first valid partition table. On a device with a raw FAT filesystem without a partition table, U-Boot will still consider the device to contain a partition table (since a partition table and FAT filesystem look so similar) and hence U-Boot will attempt to parse the non-existent partition table, and hence access will fail, since it will access the disk based on the invalid partition table information, rather than assuming the filesystem starts at block 0. This can be worked around by explicitly specifying ":0" as the partition ID. The issue here is boot scripts that want to load files from "the first valid partition". They couldn't use the WAR to explicitly specify ":0" as the partition, since they deliberately don't specify a partition in order to rely on U-Boot's automatic partition selection. Perhaps this isn't much of an issue though, since unpartitioned devices are probably somewhat rare (although Sonic's device was one at one time) and so we perhaps don't need to make using them so simple/automatic?
On 03/15/2013 12:36 AM, Sonic Zhang wrote: ... > We can erase the first 3 blocks completely before formatting it again. > But, nothing can prevent others from formatting the SD card with > different tools. Surely any tool that creates a partition table on a device when there wasn't any partition table there before (i.e. converts a raw filesystem to a partition table) should zero out the "boot code" area of that MBR, since there is no "boot code" area in a raw filesystem. This seems like a bug in whatever partitioning tool was used. If any device /always/ historically contained a real partition table, or /always/ contained a raw FAT filesystem, then there will be no issue. It's only when switching between the two, and not clearing out all the data in the MBR when doing so, that you get this problem.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/15/2013 01:09 PM, Stephen Warren wrote: > On 03/15/2013 12:36 AM, Sonic Zhang wrote: ... >> We can erase the first 3 blocks completely before formatting it >> again. But, nothing can prevent others from formatting the SD >> card with different tools. > > Surely any tool that creates a partition table on a device when > there wasn't any partition table there before (i.e. converts a raw > filesystem to a partition table) should zero out the "boot code" > area of that MBR, since there is no "boot code" area in a raw > filesystem. This seems like a bug in whatever partitioning tool was > used. > > If any device /always/ historically contained a real partition > table, or /always/ contained a raw FAT filesystem, then there will > be no issue. It's only when switching between the two, and not > clearing out all the data in the MBR when doing so, that you get > this problem. To me the question is, are we looking at a "torture card" that doesn't really contain a compliant set of choices, but happens to not fail in other cases (that don't play clever games like we do) or a really valid card and we need to make a harder choice. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRQ1p3AAoJENk4IS6UOR1WnqAQAKjwfCRf0o4Agn5zoEZKBiQN stPPQAbHXiIL0eHfLXfIHN13kVeM7KnN+C49jGn7s3e3y7kb3bkA/paQXcATLYIe 0oZAF0FOHJVvotuefQdZU6Nk/1flwpfNUXo0D0Lu0qQtbNnUaQwEdGy5PA4YaqkS PSRmlC/3JJHDh+bsvS9tIZBmOln1mYPJzR5VH2mnLHZbwqKN2z1hY39tPVyD0PUh 9cgcD0zhJllSr7ZgNzGXPpN6yMk36zzz2QKk6L2YzmusJYesUPiXJry1hmh/xg1R vU9TcpO3NpyxhljXzuX3soo1ED8NI/Ux0Q2Eg9SnWGJYnQkOHqAKiHgE69tOU3lF 72u1tzmXJLVEZYxKTLU9+wjoWmM2tQzXPCFVE2T2S02he801e69EPasmxfT2Jspn bqna3zCBKrsg41IUW6vXMCvWGmyxCUiPJkyq+uHv6SWDf7sgkgbLtVh/NQdhKRTH IgOmBcf4EyIcD+F0OIiZlMaqlcEG9K82O1NGRXfBI9LzeJpgco7gTyjDhrvPahxJ nH/OYJ9wnAXCl3gGiIXeoo9TxxMNMf/zACLlS6Z4fIxIoRCLvyS/tBaIGBP6Cs8k 8ckK7IU2VrT1IuCf7OS6TnEDWLWKmXOTdjoVvXRZBc2BTytj2lKZehg1es1JjWNe cb5F4wMFfMYzPApKoy3z =59H0 -----END PGP SIGNATURE-----
diff --git a/disk/part_dos.c b/disk/part_dos.c index 3fe901b..98f82ca 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -98,7 +98,7 @@ int test_part_dos (block_dev_desc_t *dev_desc) if (dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) return -1; - if (test_block_type(buffer) != DOS_MBR) + if (test_block_type(buffer) < 0) return -1; return 0;