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

[Stephen Warren]

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 at wwwdotorg.org> wrote:
>>>> On 03/11/2013 03:56 AM, sonic.adi at gmail.com wrote:
>>>>> From: Sonic Zhang <sonic.zhang at 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?