Date: Mon, 17 Mar 2014 11:25:37 -0400 From: Patrick Kelsey <kelsey@ieee.org> To: Ian Lepore <ian@FreeBSD.org> Cc: freebsd-arm <freebsd-arm@FreeBSD.org> Subject: Re: Booting FreeBSD from eMMC on BeagleBone Black Message-ID: <80EF2F0E-B4E3-4E06-933A-685F78F2EBD7@ieee.org> In-Reply-To: <1395067218.1149.570.camel@revolution.hippie.lan> References: <CAD44qMXrKbZqXT_Z1UL2LXuVON4Gb49m-GygW_6Y14Zz-egTFw@mail.gmail.com> <E1283934-0065-4979-92AA-99D1056BFBD5@FreeBSD.org> <CAD44qMVxoM=MdvDfosJgxQycYYNmxgMii3_0z91PGwHKXmszMg@mail.gmail.com> <1395064661.1149.565.camel@revolution.hippie.lan> <CAD44qMU0iDE=sJTyLfh2AD2aPRyM5sCgeq8U3MVgaCSpVGxsvw@mail.gmail.com> <1395067218.1149.570.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mar 17, 2014, at 10:40 AM, Ian Lepore <ian@FreeBSD.org> wrote:
> On Mon, 2014-03-17 at 10:21 -0400, Patrick Kelsey wrote:
>> On Mon, Mar 17, 2014 at 9:57 AM, Ian Lepore <ian@freebsd.org> wrote:
>>=20
>>> On Sun, 2014-03-16 at 22:16 -0400, Patrick Kelsey wrote:
>>>> On Sun, Mar 16, 2014 at 9:33 PM, Rui Paulo <rpaulo@freebsd.org> wrote:
>>>>=20
>>>>> On 16 Mar 2014, at 14:59, Patrick Kelsey <kelsey@ieee.org> wrote:
>>>>>=20
>>>>>> - Improved disk probing support that will now by default find and
>>> use the
>>>>>> first suitable partition among the available storage devices.
>>>>>=20
>>>>> I think this introduced a bug where, if you have a non-responsive boot=
>>>>> device, ubldr will stop and won't try network booting:
>>>>>=20
>>>>> ## Starting application at 0x01000054 ...
>>>>> Consoles: U-Boot console
>>>>> Compatible API signature found @1d800a8
>>>>> Number of U-Boot devices: 2
>>>>>=20
>>>>> FreeBSD/armv6 U-Boot loader, Revision 1.2
>>>>> (rpaulo@zedfs.local, Fri Mar 14 22:35:47 PDT 2014)
>>>>> DRAM: 256MB
>>>>> Unknown device type '' <------------ this is new
>>>>> Found U-Boot device: disk
>>>>> Probing all storage devices...
>>>>> Checking unit=3D0 slice=3D0 partition=3D-1...disk0: read failed, error=
=3D1
>>>>>=20
>>>>> Checking unit=3D1 slice=3D0 partition=3D-1...
>>>>> Checking unit=3D2 slice=3D0 partition=3D-1...
>>>>> Checking unit=3D3 slice=3D0 partition=3D-1...
>>>>> Checking unit=3D4 slice=3D0 partition=3D-1...
>>>>> Checking unit=3D5 slice=3D0 partition=3D-1...
>>>>>=20
>>>>> can't load 'kernel'
>>>>>=20
>>>>> Type '?' for a list of commands, 'help' for more detailed help.
>>>>> loader>
>>>>>=20
>>>>> It stops here and doesn't try net0 booting.
>>>> I think the problem is that some of the conditionals in
>>>> sys/boot/uboot/common/main.c:main() are broken. I believe I sowed the
>>> seed
>>>> for this in the original patch I sent to Ian, which appears to have had=
>>> an
>>>> out-of-order set of tests in the disk conditional, which in hindsight
>>>> turned out to work due to a friendly coincidence (namely disk appearing=
>>>> before net in the devsw). That bad-pattern conditional seems to have
>>>> gotten munged a bit further and propagated in some of the refactoring I=
an
>>>> did when integrating my patch.
>>>>=20
>>>> I believe sys/boot/uboot/common/main.c, starting around line 442, shoul=
d
>>>> look like this:
>>>>=20
>>>> if (strcmp(devsw[i]->dv_name, "disk") =3D=3D 0 &&
>>>> (load_type =3D=3D -1 || (load_type & DEV_TYP_STOR))) {=
>>>> if (probe_disks(i, load_type, load_unit,
>>> load_slice,
>>>> load_partition) =3D=3D 0)
>>>> break;
>>>> }
>>>>=20
>>>> if (strcmp(devsw[i]->dv_name, "net") =3D=3D 0 &&
>>>> (load_type =3D=3D -1 || (load_type & DEV_TYP_NET)))
>>>> break;
>>>>=20
>>>>=20
>>>> Can you give that a try?
>>>>=20
>>>> -Patrick
>>>=20
>>> This was my bad. I think I was so enmeshed in whether the strange
>>> original strncmp() calls were really just a silly form of strcmp() (they=
>>> were) that I didn't even notice I screwed up the overall logic.
>>>=20
>>> Rather than putting the strcmp() first as shown above, I just adjusted
>>> the parens in the network if() to be what I should have done originally.=
>> I don't believe that approach will fix it. It's not just the parens in t=
he
>> "net" conditional that are the issue - I had the order of the tests in th=
e
>> "disk" conditional backwards to begin with. To get the desired behavior
>> (proper fallback to net), the test of devsw[]->dv_name needs to be first s=
o
>> that a load_type of -1 cannot short circuit it in a loop iteration that i=
s
>> for another type. Otherwise, when load_type is -1, coming out of an
>> unsuccessful probe_disks() will result in an exit from the loop via the
>> "net" conditional, and you will wind up out of the loop, but without
>> currdev set up properly for the net device (it will still be set up for
>> "disk", and with a possibly non-zero unit number). Putting the test of
>> devsw[]->dv_name first ensures that the loop won't be exited on behalf of=
a
>> given type without currdev being set up for that type.
>>=20
>> -Patrick
>=20
> I don't think so. With the parens nested correctly now, there's no
> difference between
>=20
> if ((condition A) && (condition B)) versus=20
> if ((condition B) && (condition A))
>=20
> I've just always had a prejudice for testing the simple local-var
> conditions first with && conditions (even when performance doesn't
> matter, as in this case).
>=20
> Plus I've done something radical and actually tested it. :)
>=20
> MMC: no card present
> Number of U-Boot devices: 2
> U-Boot env: loaderdev not set, will probe all devices.
> Found U-Boot device: disk
> Probing all disk devices...
> Checking unit=3D0 slice=3D<auto> partition=3D<auto>...disk0: real size !=3D=
size
>=20
> Checking unit=3D1 slice=3D<auto> partition=3D<auto>...
> Checking unit=3D2 slice=3D<auto> partition=3D<auto>...
> Checking unit=3D3 slice=3D<auto> partition=3D<auto>...
> Checking unit=3D4 slice=3D<auto> partition=3D<auto>...
> Checking unit=3D5 slice=3D<auto> partition=3D<auto>...
> Found U-Boot device: net
> |
> /boot/kernel/kernel data=3D0x4e3a08+0x305f8 syms=3D[0x4+0x7ee30+0x4+0x4e3d=
9]
> Hit [Enter] to boot immediately, or any other key for command prompt.
>=20
Speaking of short-circuit... I'm with you (and moving bedtime up a couple of=
hours tonight). The saving grace is that I was wrong about my being wrong?=
I guess I should have just thrown you under the bus alone :)=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?80EF2F0E-B4E3-4E06-933A-685F78F2EBD7>