[U-Boot] [PATCH 2/2] sunxi: Enable pre-console buffer

[Simon Glass]

Hi Siarhei,

On 11 January 2015 at 16:28, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
> On Sat, 10 Jan 2015 08:24:52 -0700
> Simon Glass <sjg at chromium.org> wrote:
>
>> Hi,
>>
>> On 10 January 2015 at 03:50, Ian Campbell <ijc at hellion.org.uk> wrote:
>> > On Fri, 2015-01-09 at 13:13 +0200, Siarhei Siamashka wrote:
>> >> > If yes then I think it is confusing to modify this comment, and the
>> >> > comment before the pre-console #defines should mention that the buffer
>> >> > is boottime only/short lived etc.
>> >>
>> >> Just in case if something goes really wrong (in theory it shouldn't,
>> >> but in practice you know...) it is still somewhat safer to keep the
>> >> buffer in its own dedicated area and keep everything else out.
>> >
>> > Nothing in those defines is protecting anything though, if the kernel is
>> > more than 15M it will still overwrite that area.
>> >
>> >> > Perhaps a better place for the pre-console buffer would be right before
>> >> > the framebuffer (or at the top of RAM if no video on the board), with
>> >> > modifications to bootm_size or not depending on the answer to the
>> >> > original question above.
>> >>
>> >> If this needs any kind of runtime address calculations instead of
>> >> compile time constants, then IMHO it becomes unnecessarily complicated.
>> >
>> > One for Hans I think, my understanding was that the framebuffer was at
>> > the top of RAM, but having bootm_size set to 0xf0000000 unconditionally
>> > doesn't match that. I suppose the idea is that it corresponds with the
>> > smallest board because it's not worth making it dynamic (I think I
>> > recall Hans saying something like that at the time).
>> >
>> > I think you could safely put the early buffer at 0xf000000-DELTA (and
>> > adjust bootm_size to match), rather than worrying about packing it up
>> > below the real framebuffer.
>> >
>> >> Anyway. The sunxi part of these changes just needs to assign some
>> >> memory area to the pre-console buffer. In the end it does not really
>> >> matter which one. The size also does not need to be too large.
>> >> For example 1920 * 1080 / (8 * 16) = 16200. It means that only ~16K
>> >> of the log buffer can fully cover the FullHD display using the 8x16
>> >> font. And this is even assuming no line breaks. I picked 1M only
>> >> because it was the smallest unit of the address space allocation in
>> >> sunxi-common.h :-)
>> >
>> > I don't think it needs to be allocated in 1M chunks, it just happens to
>> > have been arbitrarily chosen that way so far.
>> >
>> > If you want to keep the early buffer down in that region then I think
>> > it'd be better to steal a few KB from the end of the fdt, script or pxe
>> > (all of which will never be anywhere near 1MB) than stealing 1M off the
>> > end of the kernel (it's not totally inconceivable that a kernel might be
>> > approaching ~15M in size)
>>
>> I don't think it is a good idea to use the 'pre-console buffer' after
>> the console exists. It is a misnomer.
>
> If one is fixated on the idea that "console" == "UART serial console",
> then yes, keeping the pre-console buffer after the UART console
> is initialized looks like a misnomer. However there are other
> consoles too. And in the case of tablets, the "LCD console" may
> be the only console that is (reasonably easily) accessible. IMHO,
> the "pre-vga console buffer" interpretation is just as valid as the
> "pre-serial console buffer".
>
> Again, if we look at the pre-console buffer original idea. It just
> does initial logging in the memory buffer because the serial console
> becomes available relatively late and needs somewhat complicated
> initialization (complicated compared to trivial logging in the memory
> buffer). The vga console is in principle exactly the same, except that
> it brings "late" and "complicated" to the entirely new level.
>
> I understand that the pre-console buffer had been introduced to solve
> one particular problem. But giving it a fresh look, we can see that it
> already works perfectly fine for solving other problems too.

I'm sorry, but pre-console means pre-console in my book. Using it
after the console is inited by console_init_f() is going to cause
confusion. If you are planning to add this function beyond
console_init_f() then it should be renamed to something like console
recording.

>
>> Also, the reason that the pre-console buffer has pre-allocated memory
>> is to work around the lack of memory allocation before relocation. Now
>> that we have initf_mallloc() called very early in boot we could
>> consider allocating the space instead.
>
> My understanding is that one of the important reasons to have this
> buffer at a known predictable memory location is to simplify JTAG
> debugging.

OK, I didn't know that. Still, the base of the malloc() area is fixed
for each board so in fact I doubt there would be any benefit to not
just using malloc(). Worth trying.

This is the first discussion I'm aware of on the topic.

http://patchwork.ozlabs.org/patch/112119/

Are you aware of these patches?

http://patchwork.ozlabs.org/patch/421220/
http://patchwork.ozlabs.org/patch/421224/

>
>> I think this patch is a good feature to implement, but I agree with
>> others that hard-coded memory locations for U-Boot features should not
>> exist except in exceptional circumstances (e.g. very early boot).
>
> The platform code is in a perfect control of its address space. It can
> make decisions about how much RAM to use (for example, assume that
> 256MB is the minimum possible amount of RAM on sunxi hardware), chop
> off some amount of RAM for simplefb, etc. We are just having a hair
> splitting argument with Ian about the suitable location and size of
> the pre-console buffer. But no problem exists there in principle.
>
> Doing the fixed memory location reservation provides absolutely the
> best console log buffer. No malloc, UART hardware or anything else
> is required. You can start logging right away. This could possibly
> be enough to justify the exception.

So is the real problem you are trying to solve that you don't have a
UART? But you do have JTAG? What kind of board is this?

malloc() is a convenient way to allocate memory - if you think of a
new board use it is much easier to just 'enable console recording' and
perhaps select a size than to try to figure out where to put it, and
perhaps overwrite something else.

Perhaps a solution would be not to require the address to be set. If
it is not set, then it can be malloc()'d.

Also I encourage you to look at the board_init_f() init sequence:

static init_fnc_t init_sequence_f[] = {
#ifdef CONFIG_SANDBOX
setup_ram_buf,
#endif
setup_mon_len,
setup_fdt,
#ifdef CONFIG_TRACE
trace_early_init,
#endif
initf_malloc,

I don't believe any board code can run before initf_malloc(). Before
this sequence there is no C stack so the console can't work.

So there really does not seem to be any point to a console that avoids
using malloc() so you can write stuff in these 4 functions.

>
> Also because of assigning a fixed location to the pre-console buffer,
> it is trivially easy to additionally extract and prepend the log from
> SPL with really minimal changes. I can submit an updated patch with SPL
> log extraction support just to demonstrate how it works.

>
>> Re passing the U-Boot console to the kernel, see as an example the
>> cbmem_console.c driver. It only works on x86 at present and only with
>> coreboot.
>
> Cool. It is good to know that this is already at least partially
> solved. What makes it x86 and coreboot specific? Is it difficult
> to make this usable on all platforms?

It is coreboot-specific because it puts the console data in a coreboot
table that is left around in RAM by coreboot. To make it useable on
ARM, for example, it should be changed to stick the data in the device
tree. See for example bootstage_fdt_add_report().

>
>> It works as a stdio driver, so skips the first part of U-Boot's output.
>
> By applying my pre-console buffer tweak and enabling pre-console in
> the platform header file, the first part of U-Boot's output should be
> available for cbmem_console too.

Yes I think so.

>
>> So I suggest:
>>
>> 1. Remove the pre-console address and just have a size. Allocate the
>> space after initf_malloc(). Store the details (buffer start, size and
>> current pointer) in global_data
>> 2. Add a general mechanism to record the console into a buffer by
>> renaming and adjusting the existing code. It can then be set up
>> pre-console, post-console but pre-stdio, and then post-stdio for
>> recording what is passed to Linux.
>
> If this means getting rid of the fixed address for the pre-console
> buffer reservation entirely, then we lose the ability to log before
> malloc, JTAG debugging becomes more complicated and extracting the SPL
> log is also not easy anymore. Or am I missing something?

Do you have a patch for SPL recording? Perhaps we should worry about that later?

But again with SPL, there should be essentially nothing before
malloc() is available so the address can be optional. Again, there
would be no stack before board_init_f(). We cannot call putc() until
global_data exists. I recently sent a series that sets things up for
driver model in SPL, so early malloc() is available.

>
> If we keep the fixed pre-console buffer reservation, but migrate the
> pre-console buffer into a new malloc allocated entity as soon as
> malloc becomes available, then what do we really gain by doing this?

My point is that there really isn't any benefit now to recording a
console before malloc() is available. See above.

It would also be nice to use this feature on platforms that 'lose'
their memory on relocation. In that case, any console recorded will no
longer be available.

Regards,
Simon