Regression: small sram, non power of 2 RAM size, firmware XIP (iCE40 8K: TinyFPGA BX/HX8K)

[ewenmcneill]

TL;DR: Regressions/questions:

1. Is sram (or other memory regions) that are of a size that is not a power of 2 still supported? This used to work (see below), but since September 2019 seems to have been implicitly rounded up in size in a way that causes the software linker to try to, eg, use RAM that doesn't actually exist. If only powers of 2 are supported, there should probably be a build failure when trying to pass any non-power-of-2 memory region size, rather than implicitly changing the size and hoping it works out.

2. Is XIP (execute in place) of the firmware still supported? The BIOS boot support for this still seems to be in the BIOS source (eg, https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/bios/boot.c#L361), but the triggering compiler define (FLASH_BOOT_ADDRESS) doesn't seem to have been exported since the changes in September 2019 (eg,
   8be5824#diff-d35fa8f6bd2523ded61b8e55c30cf6c2L68-L74). Was FLASH_BOOT_ADDRESS / XIP firmware deliberately removed, or is this a regression that can be fixed (I think it just needs the FLASH_BOOT_ADDRESS value exported out to the compiler defines again).

More detail below. These a huge amount of debugging of the issues in timvideos/litex-buildenv#277 where I was tracking my progress debugging what changed to break the TinyFPGA BX/HX8K.

Personally I think it'd be nice to again support non-power-of-2 memory regions -- as far as the compiler is concerned -- and just require the address decoding to be at powers of 2 boundaries so that the memory map itself was easy to handle. Ie, that there might be gaps which are not covered by any memory, that the compiler knows not to use, but that the RTL might think exists. I think that is implicitly how it used to work before (ie, prior to September 2019).

I also think it'd be very useful to continue to support the firmware being executed in place out of SPI flash. Otherwise these tiny FPGA targets (iCE 40 8K) basically can't be used to do anything meaningful with litex, as they're too light on RAM to have the application stored anywhere else.

---

litex-buildenv has build targets for a couple of small iCE40 8K targets, the TinyFPGA BX and the HX8K EVN board. These have worked with litex-buildenv/litex for a couple of years, including running MIcroPython.

Both boards have no external RAM (main_ram), and so are reliant on RAM blocks inside the iCE40 8k itself. As a result the port to them was done with a pretty small RAM size and reliant on both the BIOS and the firmware (MicroPython) being XIP (execute in place) out of the spiflash. This isn't particularly quick, but up through September 2019 it did work if one was patient enough to wait several seconds for the BIOS/MicroPython boot (there's also no CPU cache, for the same RAM constraints).

One of the side effects of this tension between "maximum user RAM" and "small FPGA" was that the boards were defined with a sram size of 0x2800 (10kB):

https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/tinyfpga_bx/base.py#L68-L69
https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/ice40_hx8k_b_evn/base.py#L60-L61

which worked through September 2019.

As of late October 2019 (found independently by @niklasnisbeth and myself around December 2019/January 2020), when litex-buildenv pulled in the July to October litex changes in timvideos/litex-buildenv@3a9127d, these two boards stopped working. In particular, with some minor changes the gateware could be built and the BIOS could be built, but there was never any serial console output from the boards, not even the LiteX opening banner after this set of changes.

After a lot of debugging I identified the first litex commit that didn't work any more, 8be5824, and then figured out what about that change was causing problems, ie that the sram size was being changed from 0x2800 (10kB) to 0x4000 (16kB) after it had been defined. (See timvideos/litex-buildenv#277 (comment) and timvideos/litex-buildenv#277 (comment).) This new size was being exported out to the linker, which I believe was then trying to use the expanded RAM range (eg, for the stack at the top), which was failing miserably. (The lm32 "exception handler" is mostly just hanging the CPU in a tight loop, so an exception early on would cause a lockup; and I suspect trying to read from the memory bus at a non-mapped address would also potentially just hang.)

I was able to get the TinyFPGA BX booting as far as litex BIOS, by forcing the sram size down to 0x2000 (8kB), which is the next lowest power of 2 (rather than the next highest power of 2 that litex was rounding up to. See timvideos/litex-buildenv#277 (comment).

But the firmware application is not found, because on these boards it's in the spiflash, and intended to be executed in place, but the FLASH_BOOT_ADDRESS variable used to trigger that is no longer exported to the BIOS build; see timvideos/litex-buildenv#277 (comment). (It was removed by 8be5824#diff-d35fa8f6bd2523ded61b8e55c30cf6c2L68-L74, and I don't see it being put back anywhere since.)

So there are two regressions/things not supported, as per the list at the top.

I'm happy to provide any more information that might be useful. If you want to try to replicate this, the https://github.com/timvideos/litex-buildenv/wiki/HowTo-FuPy-on-iCE40-Boards guide is what I've been following to build/test, on a TinyFPGA BX, with the serial port connected up to a USB serial device. And then fiddling with litex-buildenv and litex checkout versions as described in the later messages in timvideos/litex-buildenv#277.

Ewen

PS: AFAICT the BIOS XIP (execute in place) still works, as it's controlled by different compiler defines, and those do still seem to be exported.

/cc @mithro

[enjoy-digital]

Thanks @ewenmcneill for the detailed report/investigation. I will look at that and include targets for small boards in LiteX and Travis-CI to be sure to catch this more easily.

[enjoy-digital]

For 1. i don't think there was a regression. Before, the internal value used for the decoder was rounded to 256MB, while now it's rounded to the next power or 2. With b02c233, rounding in hardware is indicated to user when happening and this rounded value is also used to do the internal checks.
So i think it's the behavior you want: In the gateware, the rounded value is used but the exact value is still provided to the software. @ewenmcneill can you confirm this is ok for you?

I'll look at 2. soon.

[enjoy-digital]

Still for 1. if there was a regression, it has probably been fixed when introducing the new SoC Class. For example if we do:
litex_sim --integrated-rom-size=0x8004
The size used internally is rounded to 0x10000, so the next power or 2, but the size generated for the software is 0x8004.

[enjoy-digital]

For 2., this is still supported and the behavior has been made similar to ROM_BOOT_ADDRESS, when user want the BIOS to use it, the constant just need to be defined by the user:
soc.add_constant("FLASH_BOOT_ADDRESS", 0x11223344)

[ewenmcneill]

So i think it's the behavior you want: In the gateware, the rounded value is used but the exact value is still provided to the software. @ewenmcneill can you confirm this is ok for you?

Yes, I believe that's how it worked before: the exact value was provided to the linker (software) so the software was laid out correctly in the actual RAM available, but the address decoding used a larger chunk.

It sounds like good news that a newer commit has re-introduced this support (with the SoC classes, which I think is 6baa07a); I found that commit earlier, but couldn't tell from patch inspection whether or not the size in the linker table was still being rounded up.

litex-buildenv is still pinned to an older litex, from 2019-11-18, so it's currently only got the "rounds up to power of 2 before linker output' version. I did try updating the litex submodule independently, but ran into a different problem (which I think was added to work around the way linking worked previously, by trying to place the rom section in the same address space as the spiflash):

ERROR:SoCBusHandler:rom already declared as Region:

so the TinyFPGA BX target in litex-buildenv will need some more refactoring until I can test it with the newer litex.

The relevant code is at https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/tinyfpga_bx/base.py#L96-L102, and seems to be trying to use ROM_DISABLE to "remove" the ROM, and then add it again.

I'll try to find some time this week to update the TinyFPGA BX target to work with the newer litex API, and test this.

For 2., this is still supported and the behavior has been made similar to ROM_BOOT_ADDRESS, when user want the BIOS to use it, the constant just need to be defined by the user:
soc.add_constant("FLASH_BOOT_ADDRESS", 0x11223344)

Thanks for investigating. It looks like that's already been done in newer litex-buildenv than I was testing (eg, https://github.com/timvideos/litex-buildenv/blob/master/targets/tinyfpga_bx/base.py#L102; I was testing around the commit where it broke). So providing we can sort out the RAM size handling part it sounds like with newer litex it should just boot again.

Ewen

(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ make clean
rm -f build/cache.mk
rm -rf build/tinyfpga_bx_base_lm32.minimal/
py3clean . 2>/dev/null || rm -rf $(find -name __pycache__)
(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ git log -1
commit d6cffe26a17af015821142a8b199fc6460f149d8 (HEAD -> master, origin/master, origin/HEAD)
Merge: c9661bf e7f98ba
Author: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Date:   Fri Mar 6 23:14:28 2020 +0000

    Merge pull request #365 from timvideos/dependabot/submodules/third_party/litex-renode-2ed761f
(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ (cd third_party/litex && git log -1)
commit b02c23391a75c809dbb6b0dfc17e49a88214a11d (HEAD -> master, origin/master, origin/HEAD)
Author: Florent Kermarrec <[email protected]>
Date:   Sun Mar 8 19:17:31 2020 +0100

    integration/soc/SoCRegion: add size_pow2 and use this internally for checks since decoder is using rounded size to next power or 2.
(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ make gateware

***************************************************************************
WARNING: the following submodules do not match expected commit:

+b02c23391a75c809dbb6b0dfc17e49a88214a11d third_party/litex (tinyfpga-bx-works-2019-08-10-648-gb02c2339)

If you are not developing in submodules you may need to run:

git submodule update --init --recursive

manually to bring everything back in sync with upstream
***************************************************************************

mkdir -p build/tinyfpga_bx_base_lm32.minimal/
time python -u ./make.py --platform=tinyfpga_bx --target=base --cpu-type=lm32 --iprange=192.168.100   --cpu-variant=minimal --cpu-variant=minimal  \
	2>&1 | tee -a /src/fpga/litex-buildenv/build/tinyfpga_bx_base_lm32.minimal//output.20200309-085407.log; (exit ${PIPESTATUS[0]})
INFO:SoC:        __   _ __      _  __  
INFO:SoC:       / /  (_) /____ | |/_/  
INFO:SoC:      / /__/ / __/ -_)>  <    
INFO:SoC:     /____/_/\__/\__/_/|_|  
INFO:SoC:  Build your hardware, easily!
INFO:SoC:--------------------------------------------------------------------------------
INFO:SoC:Creating SoC... (2020-03-09 08:54:08)
INFO:SoC:--------------------------------------------------------------------------------
INFO:SoCBusHandler:Creating Bus Handler...
INFO:SoCBusHandler:32-bit wishbone Bus, 4.0GiB Address Space.
INFO:SoCBusHandler:Adding reserved Bus Regions...
INFO:SoCBusHandler:Bus Handler created.
INFO:SoCCSRHandler:Creating CSR Handler...
INFO:SoCCSRHandler:8-bit CSR Bus, 32-bit Aligned, 16.0KiB Address Space, 2048B Paging (Up to 32 Locations).
INFO:SoCCSRHandler:Adding reserved CSRs...
INFO:SoCCSRHandler:spiflash CSR added at Location 0.
INFO:SoCCSRHandler:cas CSR added at Location 1.
INFO:SoCCSRHandler:CSR Handler created.
INFO:SoCIRQHandler:Creating IRQ Handler...
INFO:SoCIRQHandler:IRQ Handler (up to 32 Locations).
INFO:SoCIRQHandler:Adding reserved IRQs...
INFO:SoCIRQHandler:IRQ Handler created.
INFO:SoC:--------------------------------------------------------------------------------
INFO:SoC:Initial SoC:
INFO:SoC:--------------------------------------------------------------------------------
INFO:SoC:32-bit wishbone Bus, 4.0GiB Address Space.
INFO:SoC:8-bit CSR Bus, 32-bit Aligned, 16.0KiB Address Space, 2048B Paging (Up to 32 Locations).
CSR Locations: (2)
- spiflash : 0
- cas      : 1
INFO:SoC:IRQ Handler (up to 32 Locations).
INFO:SoC:--------------------------------------------------------------------------------
INFO:SoCCSRHandler:ctrl CSR allocated at Location 2.
INFO:SoCBusHandler:io0 Region added at Origin: 0x80000000, Size: 0x80000000, Mode: RW, Cached: False Linker: False.
INFO:SoCCSRHandler:Alignment updated from 32-bit to 32-bit.
INFO:SoCBusHandler:cpu_bus0 added as Bus Master.
INFO:SoCBusHandler:cpu_bus1 added as Bus Master.
INFO:SoCCSRHandler:cpu CSR allocated at Location 3.
INFO:SoCBusHandler:rom Region added at Origin: 0x00000000, Size: 0x00008000, Mode: R, Cached: True Linker: False.
INFO:SoCBusHandler:rom added as Bus Slave.
INFO:SoC:RAM rom added Origin: 0x00000000, Size: 0x00008000, Mode: R, Cached: True Linker: False.
INFO:SoCBusHandler:sram Region added at Origin: 0x01000000, Size: 0x00001000, Mode: RW, Cached: True Linker: False.
INFO:SoCBusHandler:sram added as Bus Slave.
INFO:SoC:RAM sram added Origin: 0x01000000, Size: 0x00001000, Mode: RW, Cached: True Linker: False.
INFO:SoCCSRHandler:identifier_mem CSR allocated at Location 4.
INFO:SoCCSRHandler:uart_phy CSR allocated at Location 5.
INFO:SoCCSRHandler:uart CSR allocated at Location 6.
INFO:SoCIRQHandler:uart IRQ allocated at Location 0.
INFO:SoCCSRHandler:timer0 CSR allocated at Location 7.
INFO:SoCIRQHandler:timer0 IRQ allocated at Location 1.
INFO:SoCBusHandler:csr Region added at Origin: 0x82000000, Size: 0x00010000, Mode: RW, Cached: False Linker: False.
INFO:SoCBusHandler:csr added as Bus Slave.
INFO:SoCCSRHandler:bridge added as CSR Master.
INFO:SoCBusHandler:spiflash Region added at Origin: 0x20000000, Size: 0x00100000, Mode: RW, Cached: True Linker: False.
INFO:SoCBusHandler:spiflash added as Bus Slave.
ERROR:SoCBusHandler:rom already declared as Region:
ERROR:SoCBusHandler:32-bit wishbone Bus, 4.0GiB Address Space.
IO Regions: (1)
io0                 : Origin: 0x80000000, Size: 0x80000000, Mode: RW, Cached: False Linker: False
Bus Regions: (4)
rom                 : Origin: 0x00000000, Size: 0x00008000, Mode: R, Cached: True Linker: False
sram                : Origin: 0x01000000, Size: 0x00001000, Mode: RW, Cached: True Linker: False
spiflash            : Origin: 0x20000000, Size: 0x00100000, Mode: RW, Cached: True Linker: False
csr                 : Origin: 0x82000000, Size: 0x00010000, Mode: RW, Cached: False Linker: False
Bus Masters: (2)
- cpu_bus0
- cpu_bus1
Bus Slaves: (4)
- rom
- sram
- csr
- spiflash
Traceback (most recent call last):
  File "./make.py", line 169, in <module>
    main()
  File "./make.py", line 123, in main
    soc = get_soc(args, platform)
  File "./make.py", line 57, in get_soc
    soc = SoC(platform, ident=SoC.__name__, **soc_sdram_argdict(args), **dict(args.target_option))
  File "/src/fpga/litex-buildenv/targets/tinyfpga_bx/base.py", line 100, in __init__
    type="cached+linker")
  File "/src/fpga/litex-buildenv/third_party/litex/litex/soc/integration/soc_core.py", line 219, in add_memory_region
    linker="linker" in type))
  File "/src/fpga/litex-buildenv/third_party/litex/litex/soc/integration/soc.py", line 166, in add_region
    raise
RuntimeError: No active exception to reraise

real	0m0.358s
user	0m0.346s
sys	0m0.014s
Makefile:266: recipe for target 'gateware' failed
make: *** [gateware] Error 1
(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ 

[enjoy-digital]

Thanks for the feedback. I'm was planning to work a bit on the icebreaker in the next days which has similar constraints so will look at that and try to provide an up to date solution.

[ewenmcneill]

FWIW, it looks like the TinyFPGA BX does try to set integrated_rom_size to 0 (which I think used to prevent auto-generated rom), but for some reason that's not working now and the rom segment is still appearing at 0x0000 anyway (hence the conflict):

https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/tinyfpga_bx/base.py#L66-L67

So I'll need to do some more digging into what's changed in the API.

FWIW, the iCEBreaker has been working throughout -- it's a slightly different iCE40 FPGA, with more internal RAM, so it's got a different memory setup. Looks like it's got 128kB of custom RAM built from SPRAM:

https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/icebreaker/base.py#L121-L124

which obviously is positively gigantic in comparison to the iCE40 8K part :-)

Ewen

[ewenmcneill]

Also FTR, it seems like the 0x8000 (32kB) ROM size is coming from litex-buildenv make.py argument parsing defaults, although I've not yet figured out where that default argument is coming from...

(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ ./make.py --help | grep -A 1 -- " --integrated-rom-size"
  --integrated-rom-size INTEGRATED_ROM_SIZE
                        size/enable the integrated (BIOS) ROM (default=32KB)
(LX P=tinyfpga_bx.minimal F=micropython) ewen@parthenon:/src/fpga/litex-buildenv$ 

It looks like that didn't used to be set by default, but now it is set by default. So platforms relying on overriding the default (but not a manually set value) probably need some refactoring.

I get much closer if I just unconditionally set the integrated_rom_size to 0, but then it looks like there's been a change in the way that toolchain overrides should be done as well, as:

https://github.com/timvideos/litex-buildenv/blob/d6cffe26a17af015821142a8b199fc6460f149d8/targets/icebreaker/base.py#L135-L138

also doesn't work any longer. (I'm not sure if it's even necessary now everything has switched from arachne-pnr to nextpnr.)

Ewen

[ewenmcneill]

I get much closer if I just unconditionally set the integrated_rom_size to 0

Unfortunately it looks like the result of doing that is that the BIOS isn't built (I suspect because cpu.use_rom isn't being set, but I haven't fully traced it), and the FuPy port of MIcroPython also doesn't build because the way to access CSRs has changed. So I can't directly test just forcing the integrated_rom_size to 0; it's clearly going to require more adjustments. And I should actually do some of my dayjob given it's Monday morning here :-)

FWIW, for this TinyFPGA BX special case (remapping rom as a linker section that is hosted by the spiflash) we still need to build the BIOS as usual, even though it won't be included in the gateware bitstream (eg, as pre-init RAM acting as ROM), but will be appended to the data written to the SPI flash. I'm not sure if there's a supported way to do that now (ie, still build the BIOS without the integrated_rom_size).

Ewen

[enjoy-digital]

Hi @ewenmcneill,

i tried to work yesterday to improve support for small iCE40 FPGAs and tried to create a minimal icebreaker target in https://github.com/enjoy-digital/litex/blob/master/litex/boards/targets/icebreaker.py.

In this target, the integrated_rom_size and integrated_ram_size are forced to 0 and a linker region is created for the ROM (Physically located in SPI Flash) and SPRAM is used for the SRAM. I also changed the condition to trigger the build of the BIOS: ba2f31d

The BIOS is now built if:

• the cpu_reset_address is not specified.
• the cpu_reset_address is specified and located in a defined rom region.

I hope it will help. The main difference with the TinyFPGA BX now is the SRAM.

[ewenmcneill]

The BIOS is now built if:
* the cpu_reset_address is not specified.
* the cpu_reset_address is specified and located in a defined rom region.

Thanks very much for that. It's definitely useful to have a "known working" example to compare against.

With the latest litex I'm now seeing the BIOS compiled for the TinyFPGA BX in litex-buildenv, so it feels like I'm getting close to what is needed. I'm running into a weird linking error with the software (litex-buildenv firmware.elf), which needs more debugging to figure out what's happening; I think I found where the flags are coming from: variables.mak, from

litex/litex/soc/cores/cpu/lm32/core.py

Lines 26 to 33 in 846a272

	@property
	def gcc_flags(self):
	flags = "-mbarrel-shift-enabled "
	flags += "-mmultiply-enabled "
	flags += "-mdivide-enabled "
	flags += "-msign-extend-enabled "
	flags += "-D__lm32__ "
	return flags

But it doesn't seem to have (meaningfully) changed in the last year or so, so it's not immediately obvious what's changed that's causing it not to link.

Ewen

[ewenmcneill]

I think I found where the flags are coming from: variables.mak, from [...]
But it doesn't seem to have (meaningfully) changed in the last year or so, so it's not immediately obvious what's changed that's causing it not to link.

It looks like the -msign-extend-enabled is what it doesn't like, which is ending up in the LDFLAGS via:

https://github.com/enjoy-digital/litex/blob/master/litex/soc/software/common.mak#L66

which includes CPUFLAGS (ie, the settings above). But I still haven't traced it far enough to know what changed that made that not work for linking.

FTR, this is the linking error I'm getting (with SHELL='sh -x' to get verboseness):

 LD       firmware.elf
+ lm32-elf-ld -nostdlib -nodefaultlibs -Os -mbarrel-shift-enabled -mmultiply-enabled -mdivide-enabled -msign-extend-enabled -D__lm32__ -L/src/fpga/litex-buildenv/build/tinyfpga_bx_base_lm32.minimal/software/include -T /src/fpga/litex-buildenv/firmware/stub/linker.ld -N -o firmware.elf ../libbase/crt0-lm32-xip.o main.o isr.o boot-helper-lm32.o -L../libbase -lbase-nofloat -L../libcompiler_rt -lcompiler_rt
lm32-elf-ld: unrecognised emulation mode: sign-extend-enabled
Supported emulations: elf32lm32 elf32lm32fd
/src/fpga/litex-buildenv/firmware/stub/Makefile:39: recipe for target 'firmware.elf' failed

Ewen

[enjoy-digital]

The change i can think of here is: https://github.com/enjoy-digital/litex/pull/401/files. Could you try with software before this PR?

[ewenmcneill]

The change i can think of here is: https://github.com/enjoy-digital/litex/pull/401/files. Could you try with software before this PR?

Yes, that's the same change that I found added $(CPUFLAGS) to $(LDFLAGS). Which in general seems a somewhat unsafe thing to do, since not all compiler (compile time) flags will work for linking (even linking initiated via gcc).

If I just revert the LDFLAGS bit of that by hand (ie, remove $(CPUFLAGS) from LDFLAGS in common.mak), then I'm able to get a booting BIOS with the latest litex commit (it doesn't boot beyond the BIOS, despite trying, because I put none in the firmware, since MicroPython needs CSR handling ported before that'll compile with the current litex, so the hang after "booting from flash" is expected).

The SRAM has been forced down to 4kB, but it seems like that's happening because the integrated-sram-size is now defaulted to 4kB, which means the "set if not set" logic in the litex-buildenv targets is skipping setting the desired default values. (Looks like that's another change in the default values; clearly litex-buildenv needs different logic to detect if values were actually set on the command line, and/or per-target default values.)

It looks to me like with something to handle appropriate/no CPUFLAGS in LDFLAGS, plus a bunch of "catch up with new API" porting for litex-buildenv that the latest litex will work again for litex-buildenv.

Ewen

        __   _ __      _  __
       / /  (_) /____ | |/_/
      / /__/ / __/ -_)>  <
     /____/_/\__/\__/_/|_|
   Build your hardware, easily!

 (c) Copyright 2012-2020 Enjoy-Digital
 (c) Copyright 2007-2015 M-Labs

 BIOS built on Mar 11 2020 21:20:18
 BIOS CRC passed (7d93a841)

 Migen git sha1: d11565a
 LiteX git sha1: 846a2720

--=============== SoC ==================--
CPU:       LM32 @ 16MHz
ROM:       32KB
SRAM:      4KB

--============== Boot ==================--
Booting from serial...
Press Q or ESC to abort boot completely.
sL5DdSMmkekro
Timeout
Booting from flash...
Executing booted program at 0x20058008

--============= Liftoff! ===============--

ewen@parthenon:/src/fpga/litex-buildenv/third_party/litex/litex/soc/software$ git diff common.mak
diff --git a/litex/soc/software/common.mak b/litex/soc/software/common.mak
index 9fd0e307..c0771cb8 100644
--- a/litex/soc/software/common.mak
+++ b/litex/soc/software/common.mak
@@ -63,7 +63,9 @@ COMMONFLAGS += -flto -fuse-linker-plugin
 endif
 CFLAGS   = $(COMMONFLAGS) -fexceptions -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes
 CXXFLAGS = $(COMMONFLAGS) -std=c++11 -I$(SOC_DIRECTORY)/software/include/basec++ -fexceptions -fno-rtti -ffreestanding
-LDFLAGS  = -nostdlib -nodefaultlibs -Os $(CPUFLAGS) -L$(BUILDINC_DIRECTORY)
+# XXX: CPUFLAGS might not work for the linker...
+#LDFLAGS  = -nostdlib -nodefaultlibs -Os $(CPUFLAGS) -L$(BUILDINC_DIRECTORY)
+LDFLAGS  = -nostdlib -nodefaultlibs -Os -L$(BUILDINC_DIRECTORY)
 
 define compilexx
 $(CX) -c $(CXXFLAGS) $(1) $< -o $@
ewen@parthenon:/src/fpga/litex-buildenv/third_party/litex/litex/soc/software$ 

(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ cat build/tinyfpga_bx_base_lm32.minimal/software/include/generated/regions.ld 
MEMORY {
	sram : ORIGIN = 0x01000000, LENGTH = 0x00001000
	csr : ORIGIN = 0x82000000, LENGTH = 0x00010000
	spiflash : ORIGIN = 0x20000000, LENGTH = 0x00100000
	rom : ORIGIN = 0x20050000, LENGTH = 0x00008000
	user_flash : ORIGIN = 0x20058000, LENGTH = 0x000a7f00
}
(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ grep -i sram targets/tinyfpga_bx/
base.py      Makefile.mk  
(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ grep -i sram targets/tinyfpga_bx/base.py 
        if 'integrated_sram_size' not in kwargs:
            kwargs['integrated_sram_size']=0x2800
(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ 

(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ ./make.py --help | grep SRAM
               [--integrated-sram-size INTEGRATED_SRAM_SIZE]
  --integrated-sram-size INTEGRATED_SRAM_SIZE
                        size/enable the integrated SRAM (default=4KB)
(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ 

[ewenmcneill]

For completeness, if I unconditionally have the target set integrated_sram_size to 10kB, then I do get the correct amount of RAM. So that confirms the problem we first started with, sram size being rounded up/down, is solved.

My very hacky test edit of litex-buildenv targets/tinyfpga_bx/base.py is shown in the patch below (against litex-buildenv top) for reference. It's "working" give or take the compiling issue and needing to update MicroPython. But obviously needs a bunch of cleaning up before I'd submit it to litex-buildenv :-)

Ewen

        __   _ __      _  __
       / /  (_) /____ | |/_/
      / /__/ / __/ -_)>  <
     /____/_/\__/\__/_/|_|
   Build your hardware, easily!

 (c) Copyright 2012-2020 Enjoy-Digital
 (c) Copyright 2007-2015 M-Labs

 BIOS built on Mar 11 2020 21:39:58
 BIOS CRC passed (dd3b7f80)

 Migen git sha1: d11565a
 LiteX git sha1: 846a2720

--=============== SoC ==================--
CPU:       LM32 @ 16MHz
ROM:       32KB
SRAM:      10KB

--============== Boot ==================--
Booting from serial...
Press Q or ESC to abort boot completely.
sL5DdSMmkekro
Timeout
Booting from flash...

(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ git diff targets/tinyfpga_bx/
diff --git a/targets/tinyfpga_bx/base.py b/targets/tinyfpga_bx/base.py
index bcfed41..23c76b3 100755
--- a/targets/tinyfpga_bx/base.py
+++ b/targets/tinyfpga_bx/base.py
@@ -63,10 +63,18 @@ class BaseSoC(SoCCore):
     mem_map.update(SoCCore.mem_map)
 
     def __init__(self, platform, **kwargs):
-        if 'integrated_rom_size' not in kwargs:
-            kwargs['integrated_rom_size']=0
-        if 'integrated_sram_size' not in kwargs:
-            kwargs['integrated_sram_size']=0x2800
+        #breakpoint()
+        #if 'integrated_rom_size' not in kwargs:
+        #    kwargs['integrated_rom_size']=0
+        # XXX: Unconditionally force integrated rom to 0; we use rom for spiflash
+        # XXX: kwargs has a 32kB size, from argument parsing defaults
+        kwargs['integrated_rom_size']=0
+
+        #if 'integrated_sram_size' not in kwargs:
+        #    kwargs['integrated_sram_size']=0x2800
+        # XXX: Unconditionally force integrated sram to 10kB; 
+        # XXX: kwargs has a 4kB size, from argument parsing defaults
+        kwargs['integrated_sram_size']=0x2800
 
         # FIXME: Force either lite or minimal variants of CPUs; full is too big.
 
@@ -118,7 +126,8 @@ class BaseSoC(SoCCore):
         # template anyway just in case.
         # Disable final deep-sleep power down so firmware words are loaded
         # onto softcore's address bus.
-        platform.toolchain.build_template[3] = "icepack -s {build_name}.txt {build_name}.bin"
-        platform.toolchain.nextpnr_build_template[2] = "icepack -s {build_name}.txt {build_name}.bin"
+        # XXX: Does not work with 2020 litex, just skip it
+        #platform.toolchain.build_template[3] = "icepack -s {build_name}.txt {build_name}.bin"
+        #platform.toolchain.nextpnr_build_template[2] = "icepack -s {build_name}.txt {build_name}.bin"
 
 SoC = BaseSoC
(LX P=tinyfpga_bx.minimal F=none) ewen@parthenon:/src/fpga/litex-buildenv$ 

[ewenmcneill]

FTR, -m defines the emulation when passed to ld, and presumably there's only one so I think it's complaining about the last one after it's done lexing the command line. See, eg, https://linux.die.net/man/1/ld.

Since linking is invoked via $(TARGET_PREFIX)ld I'm not actually sure how putting ${CPUFLAGS) in LDFLAGS ever worked... maybe the only tested CPU architectures use different/safer/more "gcc and ld common" flags? From a quick glance it doesn't seem like lm32 was tested with that change :-) Because basically only one of the cpu_flags for lm32 seems "linker safe":

litex/litex/soc/cores/cpu/lm32/core.py

Lines 26 to 33 in 846a272

	@property
	def gcc_flags(self):
	flags = "-mbarrel-shift-enabled "
	flags += "-mmultiply-enabled "
	flags += "-mdivide-enabled "
	flags += "-msign-extend-enabled "
	flags += "-D__lm32__ "
	return flags

Possibly these per-CPU flags need to be split into "compile flags" and "link flags"? (Or at least "common flags" and "compile only" flags?)

Ewen

-m emulation
    Emulate the emulation linker. You can list the available emulations with the --verbose or -V options.

    If the -m option is not used, the emulation is taken from the "LDEMULATION" environment variable, if that is defined.

    Otherwise, the default emulation depends upon how the linker was configured. 

ewen@parthenon:/src/fpga/litex-buildenv/third_party/litex/litex/soc/software$ grep LD common.mak 
LD_normal      := $(TARGET_PREFIX)ld
LD_quiet      = @echo " LD      " $@ && $(LD_normal)
	LD = $(LD_normal)
	LD = $(LD_quiet)
INCLUDES    = -I$(SOC_DIRECTORY)/software/include/base -I$(SOC_DIRECTORY)/software/include -I$(SOC_DIRECTORY)/common -I$(BUILDINC_DIRECTORY)
#LDFLAGS  = -nostdlib -nodefaultlibs -Os $(CPUFLAGS) -L$(BUILDINC_DIRECTORY)
LDFLAGS  = -nostdlib -nodefaultlibs -Os -L$(BUILDINC_DIRECTORY)
ewen@parthenon:/src/fpga/litex-buildenv/third_party/litex/litex/soc/software$ 

[enjoy-digital]

Thanks for the tests/feedback. I also saw some issues with LTO (with small VexRiscv variants), so for now i reverted the changes and disabled it with 979f98e and re-opened: #139. This will need to be tested more before re-integrating it.

[ewenmcneill]

In general I think that LTO is a great idea, especially for smaller/more RAM constrained targets (which have the most to benefit from removing any unnecessary code, etc). It just seems like the patch adding it needs a wee bit more tweaking to more carefully separate "compile time" LTO flags and "link time" LTO flags, so they don't get mixed together. (And to avoid accidentally passing compile time flags to ld on non-LTO targets; it looks like the lm32 wasn't "LTO enabled" according to #401, so maybe that's why it wasn't re-tested then?)

Possibly just having a "build without LTO flags being added" option would provide a sufficient work around for those targets where it doesn't work? To allow #401 to be reintegrated again and get more testing.

Ewen

[ewenmcneill]

Of note, reading the revert (979f98e) it looks to me like the BIOS Makefile was changed to use gcc as the linker (rather than $(LD)), but that wasn't visible through common.mak so other things using the same litex generated compile infrastructure got the $(CPUFLAGS) in their LDFLAGS but didn't get the switch from using ld to gcc....

The build issue that I was seeing I think was in the litex-buildenv "stub firmware" (ie 'none'), which got confused linking. So the fix to get #139 reintegrated might be as simple as:

(a) providing a way to force LTO off as a build option (to test targets where the link time optimisation itself might be going wrong); and

(b) updating the definition of $(LD) in common.mak instead of changing it locally in just the BIOS Makefile (ie, so that everything using the litex generated compile/build flags gets the same tools at all stages). I see that was done with $(AR), but apparently not with $(LD).

Ewen

[ewenmcneill]

@enjoy-digital, out of interest (found while trying to figure out how to handle this new API better in the tinyfpga BX target), why is integrated_rom_size set to 0 here:

litex/litex/soc/integration/soc_core.py

Lines 68 to 70 in b5bddc2

	# ROM parameters
	integrated_rom_size = 0,
	integrated_rom_init = [],

but set to 32kB explicitly later when the arguments parsing is set up?

litex/litex/soc/integration/soc_core.py

Lines 261 to 265 in b5bddc2

	# ROM parameters
	parser.add_argument("--integrated-rom-size", default=0x8000, type=auto_int,
	help="size/enable the integrated (BIOS) ROM (default=32KB)")
	parser.add_argument("--integrated-rom-file", default=None, type=str,
	help="integrated (BIOS) ROM binary file")

Is that intentional or an oversight?

(integrated_sram_size is also set in both locations, but at least for integrated_sram_size the size that is hard coded in both places is the same...)

FWIW, it seems to me that it'd be better to have a "default defaults" table somewhere else, and set the default __init__ arguments and the default parser arguments from the same "default default" table values to avoid them getting out of sync.

Ewen

[enjoy-digital]

@ewenmcneill: it is probably different to avoid regression on some designs that were relying on the default values, but i agree with you, we should probably defines a default table and use it for both.

[ewenmcneill]

Progress update: I've been able to build, from litex-buildenv a TinyFPGA / lm32 target, with MicroPython, which boots to a MIcroPython prompt, and MicroPython seems to work as well as before, using the current litex.

So I think all the regressions have been resolved (the last set by reverting #401, and reopening #139), and I'm closing this issue. Any further discussion is likely to be in timvideos/litex-buildenv#277 and/or timvideos/litex-buildenv#366 about how to update litex-buildenv to the current (March 2020) litex.

WIP versions of my changes for litex-buildenv for the TinyFPGA BX target are at:

https://github.com/ewenmcneill/litex-buildenv/tree/refactor-tinyfpga-bx

and the one tiny change for the MicroPython port (remove old cached litex hw include files, to let litex-buildenv supply the current litex hw includes via a symlink instead) is at:

https://github.com/ewenmcneill/fupy-micropython/tree/modern-litex

if anyone wants to follow along at home.

On the SRAM size overrides, the main change is to set the default SRAM size for the TinyFPGA target from the Makefile (and thus command line) to the target desired default size, rather than trying to set it later when the SoC is being initialised (as was done before if it was still 0). The target SoC definition ends up with fewer special cases as a result.

Thanks very much for your help in getting this far.

Ewen

        __   _ __      _  __
       / /  (_) /____ | |/_/
      / /__/ / __/ -_)>  <
     /____/_/\__/\__/_/|_|
   Build your hardware, easily!

 (c) Copyright 2012-2020 Enjoy-Digital
 (c) Copyright 2007-2015 M-Labs

 BIOS built on Mar 13 2020 22:35:21
 BIOS CRC passed (57d4b565)

 Migen git sha1: d11565a
 LiteX git sha1: b5bddc23

--=============== SoC ==================--
CPU:       LM32 @ 16MHz
ROM:       32KB
SRAM:      10KB

--============== Boot ==================--
Booting from serial...
Press Q or ESC to abort boot completely.
sL5DdSMmkekro
Timeout
Booting from flash...
Executing booted program at 0x20058008

--============= Liftoff! ===============--
MicroPython v1.9.4-1430-ge23216bf2 on 2020-03-13; litex with lm32
>>> print("Hello World")
Hello World
>>> 

[enjoy-digital]

Thanks for the patience. In the future i'll try to do more tests with small targets and micropython/circuitpython to ensure things are not broken and provide examples if changes are required.