Date: Mon, 17 Mar 2014 08:40:18 -0600 From: Ian Lepore <ian@FreeBSD.org> To: Patrick Kelsey <kelsey@ieee.org> Cc: freebsd-arm <freebsd-arm@FreeBSD.org> Subject: Re: Booting FreeBSD from eMMC on BeagleBone Black Message-ID: <1395067218.1149.570.camel@revolution.hippie.lan> In-Reply-To: <CAD44qMU0iDE=sJTyLfh2AD2aPRyM5sCgeq8U3MVgaCSpVGxsvw@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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:
>
> > 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:
> > >
> > > > On 16 Mar 2014, at 14:59, Patrick Kelsey <kelsey@ieee.org> wrote:
> > > >
> > > > > - Improved disk probing support that will now by default find and
> > use the
> > > > > first suitable partition among the available storage devices.
> > > >
> > > > I think this introduced a bug where, if you have a non-responsive boot
> > > > device, ubldr will stop and won't try network booting:
> > > >
> > > > ## Starting application at 0x01000054 ...
> > > > Consoles: U-Boot console
> > > > Compatible API signature found @1d800a8
> > > > Number of U-Boot devices: 2
> > > >
> > > > 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=0 slice=0 partition=-1...disk0: read failed, error=1
> > > >
> > > > Checking unit=1 slice=0 partition=-1...
> > > > Checking unit=2 slice=0 partition=-1...
> > > > Checking unit=3 slice=0 partition=-1...
> > > > Checking unit=4 slice=0 partition=-1...
> > > > Checking unit=5 slice=0 partition=-1...
> > > >
> > > > can't load 'kernel'
> > > >
> > > > Type '?' for a list of commands, 'help' for more detailed help.
> > > > loader>
> > > >
> > > > 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 Ian
> > > did when integrating my patch.
> > >
> > > I believe sys/boot/uboot/common/main.c, starting around line 442, should
> > > look like this:
> > >
> > > if (strcmp(devsw[i]->dv_name, "disk") == 0 &&
> > > (load_type == -1 || (load_type & DEV_TYP_STOR))) {
> > > if (probe_disks(i, load_type, load_unit,
> > load_slice,
> > > load_partition) == 0)
> > > break;
> > > }
> > >
> > > if (strcmp(devsw[i]->dv_name, "net") == 0 &&
> > > (load_type == -1 || (load_type & DEV_TYP_NET)))
> > > break;
> > >
> > >
> > > Can you give that a try?
> > >
> > > -Patrick
> >
> > 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.
> >
> > 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 the
> "net" conditional that are the issue - I had the order of the tests in the
> "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 so
> that a load_type of -1 cannot short circuit it in a loop iteration that is
> 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.
>
> -Patrick
I don't think so. With the parens nested correctly now, there's no
difference between
if ((condition A) && (condition B)) versus
if ((condition B) && (condition A))
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).
Plus I've done something radical and actually tested it. :)
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=0 slice=<auto> partition=<auto>...disk0: real size != size
Checking unit=1 slice=<auto> partition=<auto>...
Checking unit=2 slice=<auto> partition=<auto>...
Checking unit=3 slice=<auto> partition=<auto>...
Checking unit=4 slice=<auto> partition=<auto>...
Checking unit=5 slice=<auto> partition=<auto>...
Found U-Boot device: net
|
/boot/kernel/kernel data=0x4e3a08+0x305f8 syms=[0x4+0x7ee30+0x4+0x4e3d9]
Hit [Enter] to boot immediately, or any other key for command prompt.
-- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1395067218.1149.570.camel>