8xx: fix initialation bug/race condition between u-boot & linux
PATCH 1/1 8xx: fix initialation bug/race condition between u-boot & linux
Hi all
Sorry in advance for cross-posting but this is
a problem that affects both the boot loader and
the kernel.
In my 8xx board we use u-boot in order to boot linux.
My troubles began when for dpram conflict reasons we
moved from using a console on an SMC to a max3100 SPI/UART converter chip.
I was still using the SMC console in u-boot, but when tried
to use this chip as a driver for the boot console and removing the CPM console, strange panics appeared in the ethernet driver.
After some interesting debugging sessions the problem
was discovered to be this:
The u-boot loader upon finishing the decompression of the
kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.
The attached patch fixes the common problem by performing the CP
reset every time.
This is only a bandaid however, because under network load
at the time of booting, i.e. broadcast packets or rebooting after a crash and another end sending non TCP packets, we still crash.
It is still possible for a packet to be received and placed
in memory by DMA before the kernel reaching the point where the ethernet controller is initialized.
[boot jumps into kernel ]
[kernel starts executing ]
<--- incoming packet
write in memory
XXX possible corruption XXX
[init ethernet controller]
The proper way to address the problem is for the loader
to stop any DMA capable peripherals before executing the kernel.
The fix for my case is a simple reset of the CP,
but I believe that this is a problem that should be looked upon by the maintainers.
IMHO it is good practice for the kernel too to issue a
reset to it's peripherals as soon as possible in order to minimize the chances of that happening.
Regards
Pantelis
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: one-liner.diff
Url: http://lists.denx.de/pipermail/u-boot/attachments/20030617/a113ad9a/attachment.txt
Quoted 6 lines · Pantelis Antoniou
The u-boot loader upon finishing the decompression of the
kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.
I believe that the assumption here, which other firmwares follow, is that this would be taken care of already. Dan, do you recall making any assumptions about this either way?
[snip]
Quoted 3 lines
This is only a bandaid however, because under network load
at the time of booting, i.e. broadcast packets or rebooting after a crash and another end sending non TCP packets, we still crash.
[snip]
Quoted 3 lines
The proper way to address the problem is for the loader
to stop any DMA capable peripherals before executing the kernel.
Does doing this fix, fix all of your problems?
Signature · 2 lines
Tom Rini http://gate.crashing.org/~trini/
Quoted 21 lines · Tom Rini
Quoted 7 lines · Pantelis Antoniou
The u-boot loader upon finishing the decompression of the kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.
I believe that the assumption here, which other firmwares follow, is that this would be taken care of already. Dan, do you recall making any assumptions about this either way?
[snip]
Quoted 4 lines
This is only a bandaid however, because under network load at the time of booting, i.e. broadcast packets or rebooting after a crash and another end sending non TCP packets, we still crash.
Quoted 10 lines
[snip]
Quoted 4 lines
The proper way to address the problem is for the loader to stop any DMA capable peripherals before executing the kernel.
Does doing this fix, fix all of your problems?
Yes, this fixed my problems. But this is mostly luck.
It just happens that the reception buffer for the
ethernet packet is high enough in memory that no memory corruption takes place.
I'm not sure however if we're completely safe in all cases.
I think it's just proper defensive programming to always
do a CP reset in that case, and as soon as possible. The best thing we could do is do the reset at head_8xx.S.
Regards
Pantelis
Quoted 3 lines · In message <[email protected]> you wrote:
I think it's just proper defensive programming to always
do a CP reset in that case, and as soon as possible.
Then probably the boot loader should do it before starting the Linux kernel.
Quoted 1 line
The best thing we could do is do the reset at
head_8xx.S.
Note that such a change will kill all attempts for ealy debug output over the serial console port.
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de The one who says it cannot be done should never interrupt the one who is doing it.
Quoted 9 lines · Wolfgang Denk
Quoted 3 lines · In message <[email protected]> you wrote:
I think it's just proper defensive programming to always
do a CP reset in that case, and as soon as possible.Then probably the boot loader should do it before starting the Linux kernel.
Yes, this is the best thing.
I propose we add two new functions to u-boot.
1. cpu_peripheral_reset()
Do a reset for the on-chip peripherals of the cpu. For the 8xx family a CP reset is sufficient.
2. board_peripheral_reset()
A conditional function controlled by CONFIG_HAVE_BOARD_PERIPHERAL_RESET. Do the reset for board specific peripherals.
And have them called before jumping into the kernel.
But this is a far reaching change for u-boot since at least the cpu_peripheral_reset() must be implemented for all the cpus that u-boot supports.
And I'm not talking about the boards.
The decision is yours Wolfgang.
Quoted 8 lines
Quoted 3 lines
The best thing we could do is do the reset at
head_8xx.S.Note that such a change will kill all attempts for ealy debug output over the serial console port.
That is truly unfortunate. Perhaps we can avoid the full CP reset and do only the DMA peripheral reset in this case. The buffers for the uart are in dpram anyway, so there no chance for a crash.
Quoted 4 lines
Wolfgang Denk
Regards
Pantelis
Hi Wolfgang,
Quoted 8 lines · Wolfgang Denk
Quoted 3 lines · In message <[email protected]> you wrote:
I think it's just proper defensive programming to always
do a CP reset in that case, and as soon as possible.Then probably the boot loader should do it before starting the Linux kernel.
This would be a mostly hidden requirement for any boot loader towards the ppc linux kernel.
Quoted 5 lines
Quoted 2 lines
The best thing we could do is do the reset at
head_8xx.S.Note that such a change will kill all attempts for ealy debug output over the serial console port.
That would also be the case when the boot loader does it. The kernel will initialize the uart again very early isn't it?
And is a CP reset enough? I think I remember you also have to unlock the pram section that is used for uCode.
just some thoughts,
Jaap-Jan
Quoted 10 lines
Wolfgang Denk
--
Software Engineering: Embedded and Realtime Systems, Embedded Linux
Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de The one who says it cannot be done should never interrupt the one who is doing it.** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/
Signature · 4 lines
J.G.J. Boor Anton Philipsweg 1 Software Engineer 1223 KZ Hilversum AimSys bv tel. +31 35 689 1941 Postbus 2194, 1200 CD Hilversum mailto:jjboor at aimsys.nl
Quoted 7 lines · Jaap-Jan Boor
Hi Wolfgang,
Quoted 3 lines · Wolfgang Denk
Quoted 1 line · In message <[email protected]> you wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
I think it's just proper defensive programming to always
do a CP reset in that case, and as soon as possible.Then probably the boot loader should do it before starting the Linux kernel.
This would be a mostly hidden requirement for any boot loader towards the ppc linux kernel.
... or this is already the case (which is why I ping'ed Dan who would probably recall such a thing :)). If so, it would probably be a good idea to make a note in head_8xx.S about that assumption.
Signature · 2 lines
Tom Rini http://gate.crashing.org/~trini/
Quoted 6 lines · Pantelis Antoniou
The u-boot loader upon finishing the decompression of the
kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.
The UART console has a number of subtle assumptions built around the initialization to support kgdb/xmon debugging. The quick answer is we don't want to reset the CPM so we can proper support this debugging. This has all been described in archives before...........
When the kernel is first booted, kgdb/xmon use the CPM as it was set up by the boot rom. There is a second initialization of the UART driver, but before the console is initialized. This changes the BDs, but the UART still operates for kgdb/xmon. The final stage initialization occurs when the console is initialized, and all of the "normal path" debugging and messages can occur after this point.
Quoted 2 lines
The attached patch fixes the common problem by performing the CP
reset every time.
We don't want to do this and shouldn't need to do so except in the case of loading microcode patches.
Quoted 3 lines
This is only a bandaid however, because under network load
at the time of booting, i.e. broadcast packets or rebooting after a crash and another end sending non TCP packets, we still crash.
This is a common problem of all bootloaders that don't disable the Ethernet controller after they have performed the network load operation. You must disable any kind of DMA operation that could write into memory while any software outside of the boot rom is initializing. Performing the CPM reset in the kernel doesn't solve this problem, it only makes the window of opportunity smaller. With sufficient network traffic and coincidental buffer locations, you will always crash even with the CPM reset in the kernel.
Thanks.
-- Dan
[U-Boot-Users] Re: [PATCH] 8xx: fix initialization bug/race condition between u-boot & linux
Quoted 15 lines · Dan Malek
Quoted 6 lines · Pantelis Antoniou
The u-boot loader upon finishing the decompression of the
kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.The UART console has a number of subtle assumptions built around the initialization to support kgdb/xmon debugging. The quick answer is we don't want to reset the CPM so we can proper support this debugging. This has all been described in archives before...........
Quoted 6 lines
When the kernel is first booted, kgdb/xmon use the CPM as it was set up by the boot rom. There is a second initialization of the UART driver, but before the console is initialized. This changes the BDs, but the UART still operates for kgdb/xmon. The final stage initialization occurs when the console is initialized, and all of the "normal path" debugging and messages can occur after this point.
OK. Shouldn't there be at least a comment in the code to mention that?
Quoted 13 lines
Quoted 3 lines
The attached patch fixes the common problem by performing the CP
reset every time.We don't want to do this and shouldn't need to do so except in the case of loading microcode patches.
Quoted 3 lines
This is only a bandaid however, because under network load
at the time of booting, i.e. broadcast packets or rebooting after a crash and another end sending non TCP packets, we still crash.
Quoted 8 lines
This is a common problem of all bootloaders that don't disable the Ethernet controller after they have performed the network load operation. You must disable any kind of DMA operation that could write into memory while any software outside of the boot rom is initializing. Performing the CPM reset in the kernel doesn't solve this problem, it only makes the window of opportunity smaller. With sufficient network traffic and coincidental buffer locations, you will always crash even with the CPM reset in the kernel.
Yep, it is a job for the bootloader. Wolfgang?
Quoted 10 lines
Thanks.
-- Dan
[U-Boot-Users] Re: [PATCH] 8xx: fix initialization bug/race condition between u-boot & linux
Quoted 2 lines · In message <[email protected]> you wrote:
OK. Shouldn't there be at least a comment in the code to mention that?
Feel free to submit a patch.
Quoted 10 lines
Quoted 8 lines
This is a common problem of all bootloaders that don't disable the Ethernet controller after they have performed the network load operation. You must disable any kind of DMA operation that could write into memory while any software outside of the boot rom is initializing. Performing the CPM reset in the kernel doesn't solve this problem, it only makes the window of opportunity smaller. With sufficient network traffic and coincidental buffer locations, you will always crash even with the CPM reset in the kernel.
Yep, it is a job for the bootloader. Wolfgang?
Can you please explain again under which conditions the problem happened? As Dan explained there is good reason NOT to perform a
global CPM reset, but to leave the console UART enabled (which should be not critical). Regarding network transfers I think that U-Boot
will call eth_halt() after all (attempted) network transfers;eth_halt() in turn calls driver functions (like scc_halt() or fec_halt()) that should disable the network controller.
Quoted 3 lines · You
My troubles began when for dpram conflict reasons we
moved from using a console on an SMC to a max3100 SPI/UART converter chip.
I have never seen a crash like the one you reported, and in fact I am not convinced that it is a global problem. Maybe the problem is only introduced by your own SPI/UART driver code?
In any case: if you find a place where a CPM device is not shut down before booting Linux please submit a patch. But I would like to avoid a global CPM reset.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de You can't evaluate a man by logic alone. -- McCoy, "I, Mudd", stardate 4513.3
[U-Boot-Users] Re: [PATCH] 8xx: fix initialization bug/race condition between u-boot & linux
Quoted 7 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:
OK. Shouldn't there be at least a comment in the code to mention that?
Feel free to submit a patch.
OK
Quoted 29 lines
Quoted 10 lines
Quoted 10 lines
This is a common problem of all bootloaders that don't disable the Ethernet controller after they have performed the network load operation. You must disable any kind of DMA operation that could write into memory while any software outside of the boot rom is initializing. Performing the CPM reset in the kernel doesn't solve this problem, it only makes the window of opportunity smaller. With sufficient network traffic and coincidental buffer locations, you will always crash even with the CPM reset in the kernel.
Quoted 2 lines
Yep, it is a job for the bootloader. Wolfgang?
Can you please explain again under which conditions the problem happened? As Dan explained there is good reason NOT to perform aglobal CPM reset, but to leave the console UART enabled (which should be not critical). Regarding network transfers I think that U-Boot
will calleth_halt()after all (attempted) network transfers;eth_halt()in turn calls driver functions (likescc_halt()orfec_halt()) that should disable the network controller.Quoted 4 lines · You
My troubles began when for dpram conflict reasons we
moved from using a console on an SMC to a max3100 SPI/UART converter chip.
Quoted 8 lines
I have never seen a crash like the one you reported, and in fact I am not convinced that it is a global problem. Maybe the problem is only introduced by your own SPI/UART driver code?
In any case: if you find a place where a CPM device is not shut down before booting Linux please submit a patch. But I would like to avoid a global CPM reset.
The problem was introduced while testing the SPI/UART driver and using the SMC uart on u-boot.
Since the SMC uart was disabled in linux it's dpram memory area was used by the next CP driver, the ethernet. However it was still active and both the ethernet and SMC ended up using the same dpram.
When both u-boot and the kernel use the SPI/UART for console there is no problem.
Granted it is an obscure case, but the whole thing was surprising.
And since the whole mess was produced by kgdb, which was not configured in could we at least reset the CP if we don't use kgdb? It will save some poor sucker some hair.
Quoted 6 lines
Best regards,
Wolfgang Denk
Regards
Pantelis
[U-Boot-Users] Re: [PATCH] 8xx: fix initialization bug/race condition between u-boot & linux
Quoted 6 lines · In message <[email protected]> you wrote:
The problem was introduced while testing the SPI/UART driver and using the SMC uart on u-boot.
Since the SMC uart was disabled in linux it's dpram memory area was used by the next CP driver, the ethernet. However it was still active and both the ethernet and SMC ended up using the same dpram.
It is indeed an unusual configuration to use different devices for console in the boot loader and in Linux.
Quoted 4 lines
When both u-boot and the kernel use the SPI/UART for console there is no problem.
Granted it is an obscure case, but the whole thing was surprising.
If you do such a thing you are supposed to know what you are doing. I don't think it is worth the effort to protect against all possible things.
Quoted 3 lines
And since the whole mess was produced by kgdb, which was not configured in could we at least reset the CP if we don't use kgdb? It will save some poor sucker some hair.
I disagree. As Dan Malek has explained, there is good reason NOT to perform a CPM reset. Instead, the console port _shall_ be passed initialized and usable to Linux. I think it is not wise to assume that only kgdb may use it.
A CPM reset is a too drastic measure. Neither U-Boot nor Linux are designed this way. Instead, normally a driver will enable just a
single port/channel at init, and disable it again in the
release/close function - see for example the ethernet drivers. The fact that this is not done for the console is intentional. If you use a SMC UART port in U-Boot and do not pass this as console up to Linux, it is IMHO your responsibility to disable exactly this channel only.
But this is an exotic setup, and IMHO not worth a patch.
Best regards,
Wolfgang Denk
Signature · 4 lines
Software Engineering: Embedded and Realtime Systems, Embedded Linux Phone: (+49)-8142-4596-87 Fax: (+49)-8142-4596-88 Email: wd at denx.de You're dead, Jim. -- McCoy, "Amok Time", stardate 3372.7
[U-Boot-Users] Re: [PATCH] 8xx: fix initialization bug/race condition between u-boot & linux
In reply to Pantelis AntoniouQuoted 17 lines · Pantelis Antoniou
Quoted 15 lines · Dan Malek
Quoted 6 lines · Pantelis Antoniou
The u-boot loader upon finishing the decompression of the kernel image it just jumps into the kernel without resetting the CP. The kernel however resets the CP only when configured to apply a u-patch. The UART in the CP is still active however, and the descriptors in dpram end up overlapping.
The UART console has a number of subtle assumptions built around the initialization to support kgdb/xmon debugging. The quick answer is we don't want to reset the CPM so we can proper support this debugging. This has all been described in archives before...........
Quoted 8 lines
Quoted 6 lines
When the kernel is first booted, kgdb/xmon use the CPM as it was set up by the boot rom. There is a second initialization of the UART driver, but before the console is initialized. This changes the BDs, but the UART still operates for kgdb/xmon. The final stage initialization occurs when the console is initialized, and all of the "normal path" debugging and messages can occur after this point.
OK. Shouldn't there be at least a comment in the code to mention that?
Yes, there should. Throwing that into head_8xx.S now..
Signature · 2 lines
Tom Rini http://gate.crashing.org/~trini/