[U-Boot] [PATCH v1] DOS_PBR block type is also valid dos block type.

From: Sonic Zhang sonic.zhang@analog.com
- Should return 0 for both DOS_MBR and DOS_PBR block types in test_part_dos().
Signed-off-by: Sonic Zhang sonic.zhang@analog.com
--- disk/part_dos.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
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;

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

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.

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 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 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

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?
participants (4)
-
Sonic Zhang
-
sonic.adi@gmail.com
-
Stephen Warren
-
Tom Rini