enjoy-digital / litex

Build your hardware, easily!
Other
2.89k stars 555 forks source link

Add the ability to insert boot methods to the BIOS's bootlist, from an external library #1270

Open DaveBerkeley opened 2 years ago

DaveBerkeley commented 2 years ago

Background : I want to add a SPI Flash boot using my own bit-banged interface to a SPI Flash, not using the current Litex SPI Flash driver.

The only way seems to be to modify the source of litex/soc/software/bios/boot.c or main.c to manually add a call to an external library function. This can be added by creating a soft link to the external library at, eg. litex/soc/software/libother

In other projects I've used gcc's constructor attributes to create a function that gets run during cstart, after the memory is initialised and before main() is called.

eg:

__attribute__((constructor))
static void lib_init(void)
{
        // TODO : run code here
        // Gets called before main()
}

See https://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Function-Attributes.html

I've used this to insert a static struct in a linked list. The function lib_init() gets called during cstart. It links a boot method into a list. The boot code in main.c boot_sequence() can then simply iterate over this list, calling each boot method in turn. This mechanism allows new methods to be dynamically inserted into the list without having to modify the BIOS source.

However, attribute((constructor)) requires support in the linker and cstart code. The linker definition file, linker.ld, needs .ctors and .dtors sections adding to it. This is also required for any static initialisation of C++ classes, so eases the way to providing full C++ support.

See https://blog.thea.codes/the-most-thoroughly-commented-linker-script/ for an example linker script.

The init code needs to add a function to a linked list. I would like to add a simple structure and some support code, which allows adding and iterating of the list.

Existing boot methods could be adapted to use the same technique, so the main boot sequence function could be reduced to eg

static void boot_sequence(void)
{
#ifdef CSR_UART_BASE
    if (serialboot() == 0)
        return;
#endif
    fnlist_run(bootlist);
}

The order of the boot methods could be determined by using the 'priority' parameter of the constructor attribute.

The same technique could be used by the BIOS to call init methods for peripherals etc.

I have a working prototype of this proposed system. I'd welcome comments on the idea.

A secondary problem is adding external libraries to the BIOS build. This is partially solved by calling eg :

for lib in [ "libother" ]:
    builder.add_software_package(lib)
    builder.add_software_library(lib)

But there is no way to tell the BIOS builder where the code is. It currently has to be located in the litex code tree.

DaveBerkeley commented 2 years ago

The priority (order) of boot methods could be passed as a define. This also allows a decoupling of having a device and booting from it. Currently boot_sequence() is hard coded and boot methods are inserted if the hardware is present.

By using an order list of boot methods we can prevent a boot method from being run. For example, we might want an Ethernet interface, but do not want it try and boot over Ethernet.

    self.add_constant("SDCARD_BOOT_PRIORITY", 202)
    self.add_constant("NET_BOOT_PRIORITY", 201)
    self.add_constant("SPI_BOOT_PRIORITY", 200)

..

if defined(NET_BOOT_PRIORITY)

static void netboot_fn(void*obj) { netboot(0, NULL); }
FNLIST_ADD(& bootlist, netboot_fn, 0, NET_BOOT_PRIORITY);
#endif 
fjullien commented 2 years ago

We already use something like you described to "dynamically inserted into the list without having to modify the BIOS source." That's how commands are added:

https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/bios/command.h#L35-L43

This is here the commands pointer end up:

https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/bios/linker.ld#L40-L45

and how we enumerate the list of commands:

https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/bios/helpers.c#L122-L127

DaveBerkeley commented 2 years ago

Thanks, I noticed that in the linker.ld script. It serves a slightly different purpose to what I'm suggesting, but uses a very similar technique.

The attribute((constructor, priority)) technique allows the order of commands to be specified. For boot methods this is important.

I'd also like to be able to add hardware, eg ethernet interface, without it being automatically added to the boot methods. Network boot, for example, can be very slow to time out, which could add a long delay to every boot.

This could be done in a number of ways, but simply using add_constant() to specify the priority in the boot order does achieve this. The same constant could be used for conditional compilation in each driver, so the boot method is only added if required.

Supporting C++ static initialisation of objects is a different issue, but one that is solved with the simple addition of the following to the linker.ld script :

.init_array :
{
    . = ALIGN(4);
    __init_array_start = .;
    KEEP(*(SORT(.init_array.*)))
    KEEP(*(.init_array))
    __init_array_end = .;
} > rom

And adding a call just prior to calling main in the crt0.S code :

call __libc_init_array
call main

I noticed that this also added an existing function, __stack_chk_init(), which will not currently be run.

I would propose added the init_array support, regardless of the merits, or otherwise, of my proposal.

Should I do a pull request for just the C++ static init changes?

fjullien commented 2 years ago

If we add this in the code base, it means we will depend on C++ if we want to use this functionality (I know we don't use it now so we don't depend on it now).

I wonder how this "priority" is implemented. May be we could do the same thing in pure C.

If it's not possible then yes, adding init_array to the linker script is not a big deal I guess.

DaveBerkeley commented 2 years ago

It would not add any dependency on C++. The .init_array would be empty if not used. The only overhead would be the call to __libc_init_array() which would , in that case, return without doing anything.

The technique is used in C as shown : use the attribute ((constructor, priority)) to mark a function. The priority sets the order of the items in the init_array.

I can't see a downside to adding support for .init_array. It allows the use of static initialisation of C++ objects, but does not in any way add a dependency on C++.

It would allow the decoupling of presence of hardware from adding a boot method for that hardware. And it allows a way to set the boot method order (including turning items off), from the top level python script.

fjullien commented 2 years ago

Oh, you mean we can use attribute ((constructor, priority)) with C. I thought it was C++ only.

DaveBerkeley commented 2 years ago

I've added a pull request for the first part : supporting init_array for vexriscv

https://github.com/enjoy-digital/litex/pull/1278