Initial port to powerpc bare metal #5093

mikey · 2019-09-11T07:22:47Z

This runs in qemu using:

$ ./qemu-system-ppc64 -M powernv -cpu POWER9 -nographic -bios build/firmware.bin

It can also run in microwatt (new POWER softcore) using GHDL or on an FGPA. Instructions on how to run in GHDL are here:
https://github.com/antonblanchard/microwatt/blob/master/README.md

Thanks to Anton Blanchard and Jordan Niethe for helping with this port.

dpgeorge · 2019-09-16T06:38:06Z

Thanks for the contribution. It's nice that it's a minimal port to start with, makes it easier to review!

I guess MicroPython already runs on PowerPC via the unix port, and here is a real bare-metal port. I'm just wonder though if this scales to eventually add bare-metal ports to other archs like x86, or if these could all be merged into one "bare-cpu" port. It depends how much they'd have in common (probably not that much) and whether peripherals will be supported or not. Are there plans to add peripheral support (eg GPIO, SPI) to this port?

What is the reason to add the skiboot code? Seems like this should be provided by the toolchain?

mikey · 2019-09-16T07:02:28Z

Thanks for the contribution. It's nice that it's a minimal port to start with, makes it easier to review!

Thanks!

I guess MicroPython already runs on PowerPC via the unix port, and here is a real bare-metal port. I'm just wonder though if this scales to eventually add bare-metal ports to other archs like x86, or if these could all be merged into one "bare-cpu" port. It depends how much they'd have in common (probably not that much) and whether peripherals will be supported or not.

The core of the port is mostly architecture specific. head.S, the nlr code, the uart, the linker script would all need to change for a different arch like x86. So I think the core code reuse would be minimal.

Are there plans to add peripheral support (eg GPIO, SPI) to this port?

I'd like to but at this stage we are concentrating on microwatt which doesn't have any I/O as yet (other than the UART). The qemu port was just to make it easier to test (for you and me).

FWIW There is some info on microwatt and micropython here https://lwn.net/Articles/796796/ (see "Demo time") and https://www.youtube.com/watch?v=JdMTLs7EMM0

What is the reason to add the skiboot code? Seems like this should be provided by the toolchain?

The skiboot code was there to get going quicker and with a toolchain that doesn't have the libc libraries compiled in (which makes building cross toolchains easier). I could try to get rid of them if they are a problem.

dpgeorge · 2019-09-16T07:56:38Z

The skiboot code was there to get going quicker and with a toolchain that doesn't have the libc libraries compiled in (which makes building cross toolchains easier). I could try to get rid of them if they are a problem.

I only took a brief look and the main thing that stood out was licensing/copyright of these files being different from core MicroPython. We try to consolidate such files to lib/ (eg in a submodule) if they are really necessary.

For general points related to new ports, see also this comment: #4557 (comment)

mikey · 2019-10-01T11:42:02Z

(sorry for the slow update)

I only took a brief look and the main thing that stood out was licensing/copyright of these files being different from core MicroPython. We try to consolidate such files to lib/ (eg in a submodule) if they are really necessary.

I've updated the code to remove these using more compiler builtins now.

All new files are MIT licensed.

For general points related to new ports, see also this comment: #4557 (comment)

are the chips widely available to the public and will they be for the foreseeable future?

You can either use qemu or the microwatt softcore from https://github.com/antonblanchard/microwatt

I've added instructions on how to use qemu in ports/powerpc/README.md

are development boards easily available?

microwatt is a softcore which can be put on numerous FPGA development boards like the Arty A7, Nexys A7, Nexys Video.

is the toolchain open source (or at least free to obtain)?

Yes, we are using GCC.

are there plans to support multiple MCUs/SoCs?

Yes. We support microwatt on various FPGA boards. We'd like to support more IO and soft IP which can be built into SOCs

will there be someone around for the long term to maintain this port?

Yes me! I've been working on Linux on powerpc for 10+ years:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=author&q=neuling

the license (and copyright) must be appropriate.

All MIT (fixed from initial PR).

Other things I've changed this the initial PR:

Added a travis powerpc test
Cleaned up the makefile CFLAGS options
Removed the divide emulation code (since microwatt now support divide instructions)
Rebased on latest master branch (small conflict in nlr.h and updated frozentest.mpy).

shenki

Looks good! I have made some minor comments that are my opinion. If you're unsure then we should defer to the micropython experts.

@dpgeorge, I will also be available to help maintain this port.

shenki · 2019-10-02T10:11:47Z

ports/powerpc/main.c

+    #endif
+    //do_str("print('hello world!', list(x+1 for x in range(10)), end='eol\\n')", MP_PARSE_SINGLE_INPUT);
+    //do_str("for i in range(10):\r\n  print(i)", MP_PARSE_FILE_INPUT);
+    #else


Should this commented out code be removed?

I'll remove. This was left over from the ports/minimal copy.

please remove

shenki · 2019-10-02T10:12:55Z

ports/powerpc/uart_core.c

+
+/*
+ * Core UART functions to implement for a port
+ */


This comment could go

shenki · 2019-10-02T10:13:08Z

ports/powerpc/uart_core.c

+ * Core UART functions to implement for a port
+ */
+
+#if MICROPY_MIN_USE_STM32_MCU


This looks unrelated to the ppc port

oops, copy and paste from other ports

shenki · 2019-10-02T10:14:08Z

ports/powerpc/uart_core.c

+#define USART1 ((periph_uart_t*)0x40011000)
+#endif
+
+static int potato_console;


It would be clearer if the potato and the qemu uarts go in different files.

Also mention what/where the potato uart is (I think it's some IP that microwatt is using?)

Good point.
Yeah potato uart is the uart we use for microwatt. It's from https://github.com/skordal/potato. I'll make that clearer in the code.

does the selection between lpc and potato need to be dynamic, or can it be configured at compile time? (in the latter case there's no need for these variables)

mikey · 2019-10-02T11:35:46Z

Updated branch to address @shenki comments.

dpgeorge · 2019-10-10T11:18:32Z

I tested this on Arch Linux which only has a powerpc toolchain in the AUR, so instead I tried using powerpc64le-linux-musl-cross (gcc 9.2.1) from https://musl.cc . It compiled except I needed to change the 3rd arg to main.c:__assert_fail from unsigned int to int.

Is it possible to use int here with the Ubuntu toolchain?

It also executed correctly in qemu-system-ppc64 which is great! But I did notice it used 100% CPU at an idle prompt. Is it possible to insert an idle instruction in the UART busy wait loop?

Also, I think it'd be good to put in the README some notes about what toolchain to use and/or where to get it.

dpgeorge

For code style, tabs are not used, instead 4 spaces. And some other things. To make it easy, just run astyle on the files:

$ astyle --options=astyle.fmt *.?

where astyle.fmt contains:

--style=google
--indent=spaces=4
--attach-closing-while
--add-brackets
--indent-switches
--indent-after-parens
--indent-labels
--indent-preproc-define
--indent-preproc-cond
--pad-header
--unpad-paren
--add-braces
--convert-tabs

ports/powerpc/Makefile

dpgeorge · 2019-10-10T11:22:13Z

ports/powerpc/uart_core.c

+// Receive single character
+int mp_hal_stdin_rx_chr(void) {
+    unsigned char c = 0;
+#if MICROPY_MIN_USE_STDOUT


I think this can go

Yep, removing.

dpgeorge · 2019-10-10T11:22:20Z

ports/powerpc/uart_core.c

+void mp_hal_stdout_tx_strn(const char *str, mp_uint_t len) {
+#if MICROPY_MIN_USE_STDOUT
+    int r = write(1, str, len);
+    (void)r;


Can probably go

Yep, removing

dpgeorge · 2019-10-10T11:22:46Z

ports/powerpc/uart_potato.c

+#define   POTATO_CONSOLE_STATUS_RX_FULL			0x04
+#define   POTATO_CONSOLE_STATUS_TX_FULL			0x08
+#define POTATO_CONSOLE_CLOCK_DIV	0x18
+#define POTATO_CONSOLE_IRQ_EN		0x20


Please line these up

mikey · 2019-10-11T01:06:29Z

I tested this on Arch Linux which only has a powerpc toolchain in the AUR, so instead I tried using powerpc64le-linux-musl-cross (gcc 9.2.1) from https://musl.cc . It compiled except I needed to change the 3rd arg to main.c:__assert_fail from unsigned int to int.

Is it possible to use int here with the Ubuntu toolchain?

This is a difference between the definitions of __assert_fail() between glibc and musl. It's a musl issue AFAICT https://www.openwall.com/lists/musl/2019/03/04/6

It also executed correctly in qemu-system-ppc64 which is great! But I did notice it used 100% CPU at an idle prompt. Is it possible to insert an idle instruction in the UART busy wait loop?

Unfortunately without adding support to exceptions, we can't do this bare metal on POWER. It's something we could add in the future.

Also, I think it'd be good to put in the README some notes about what toolchain to use and/or where to get it.

Will do and I'll say to avoid musl for now.

mikey · 2019-10-11T03:14:56Z

Pushed update to address commends from @dpgeorge

Fixed formatting with astyle
Removed some dead code
Update README to say avoid musl based libc and give suggestions on which cross compilers to use
Included link to microwatt

mikey · 2019-10-16T23:24:17Z

Pushed minor fix for whitespace breakage in ports/powerpc/head.S

mikey · 2019-10-17T00:34:18Z

Repushed to fix the travis failure once merged.

dpgeorge

I tested it with the toolchain from https://toolchains.bootlin.com/ and it works well.

Just a few minor items to address, then it should be good to go in.

dpgeorge · 2019-10-21T02:43:05Z

ports/powerpc/Makefile

+INC += -I.
+INC += -I$(TOP)
+INC += -I$(BUILD)
+INC += -Iskiboot


this line can go

dpgeorge · 2019-10-21T02:44:19Z

ports/powerpc/README.md

@@ -0,0 +1,40 @@
+# The minimal port


should be something like "The PowerPC port"

dpgeorge · 2019-10-21T02:44:52Z

ports/powerpc/README.md

@@ -0,0 +1,40 @@
+# The minimal port
+
+This port is intended to be a minimal MicroPython port that runs in


"... MicroPython port to the PowerPC architecture that runs ..."

Oops thanks :-)

dpgeorge · 2019-10-21T02:46:21Z

ports/powerpc/main.c

+    #endif
+    //do_str("print('hello world!', list(x+1 for x in range(10)), end='eol\\n')", MP_PARSE_SINGLE_INPUT);
+    //do_str("for i in range(10):\r\n  print(i)", MP_PARSE_FILE_INPUT);
+    #else


please remove

dpgeorge · 2019-10-21T02:47:40Z

ports/powerpc/powerpc.lds

+                KEEP(*(.head))
+        }
+
+        . = 0x1700;


maybe add a comment as to why this number / what this line does?

yep, it for putting it right after the exception vectors. I'll add the comment in the code

dpgeorge · 2019-10-21T02:48:45Z

ports/powerpc/uart_core.c

+#define USART1 ((periph_uart_t*)0x40011000)
+#endif
+
+static int potato_console;


does the selection between lpc and potato need to be dynamic, or can it be configured at compile time? (in the latter case there's no need for these variables)

dpgeorge · 2019-10-21T02:50:09Z

py/nlrpowerpc.c

+        :
+    );
+
+    assert(0); /* should never get here */


can this be replaced with MP_UNREACHABLE?

Yep.

(github won't let me comment on the console question so commenting here)
The console is dynamically selected (not compile time), so we need those variables.

Runs in microwatt (GHDL and FPGA) + qemu Port done initially by Michael Neuling, with help from Anton Blanchard and Jordan Niethe. Signed-off-by: Michael Neuling <mikey@neuling.org>

mikey · 2019-10-22T09:57:19Z

New push to address comments from @dpgeorge and rebase to latest master

Fixed readme text
Removed do_str code from main.c
documented linker offset
use MP_UNREACHABLE in nlr code rather than assert(0)

dpgeorge · 2019-10-22T11:48:53Z

Thank's for updating. Merged in 079cc94 with some fixups to code style, mainly indenting. Also simplified a bit the Makefile because there's already a rule in py/mkrules.mk to build .S files.

Great contribution, thank you!

CI: Add `Print failure info`

mikey force-pushed the powerpc-upstream branch 2 times, most recently from d1c7f57 to ddfbf48 Compare October 1, 2019 11:28

shenki reviewed Oct 2, 2019

View reviewed changes

mikey force-pushed the powerpc-upstream branch from ddfbf48 to 8daea61 Compare October 2, 2019 11:34

dpgeorge reviewed Oct 10, 2019

View reviewed changes

dpgeorge added the ports Relates to multiple ports, or a new/proposed port label Oct 10, 2019

mikey force-pushed the powerpc-upstream branch from 8daea61 to ce17d88 Compare October 11, 2019 03:14

mikey force-pushed the powerpc-upstream branch from ce17d88 to d27e5f2 Compare October 16, 2019 23:23

mikey force-pushed the powerpc-upstream branch 2 times, most recently from 8ed9525 to bb430f1 Compare October 17, 2019 00:32

mikey force-pushed the powerpc-upstream branch from bb430f1 to c9f488a Compare October 17, 2019 00:50

dpgeorge reviewed Oct 21, 2019

View reviewed changes

mikey force-pushed the powerpc-upstream branch from c9f488a to 7ce0a59 Compare October 22, 2019 09:53

Initial port to powerpc bare metal

9916104

Runs in microwatt (GHDL and FPGA) + qemu Port done initially by Michael Neuling, with help from Anton Blanchard and Jordan Niethe. Signed-off-by: Michael Neuling <mikey@neuling.org>

mikey force-pushed the powerpc-upstream branch from 7ce0a59 to 9916104 Compare October 22, 2019 09:55

dpgeorge closed this Oct 22, 2019

tannewt added a commit to tannewt/circuitpython that referenced this pull request Aug 4, 2021

Merge pull request micropython#5093 from microDev1/update-ci

b5fa6b5

CI: Add `Print failure info`

		@@ -0,0 +1,40 @@
		# The minimal port

		This port is intended to be a minimal MicroPython port that runs in

Initial port to powerpc bare metal #5093

Initial port to powerpc bare metal #5093

Conversation

mikey commented Sep 11, 2019

dpgeorge commented Sep 16, 2019

mikey commented Sep 16, 2019

dpgeorge commented Sep 16, 2019

mikey commented Oct 1, 2019

shenki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikey commented Oct 2, 2019

dpgeorge commented Oct 10, 2019

dpgeorge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikey commented Oct 11, 2019

mikey commented Oct 11, 2019 • edited Loading

mikey commented Oct 16, 2019

mikey commented Oct 17, 2019

dpgeorge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikey commented Oct 22, 2019

dpgeorge commented Oct 22, 2019

mikey commented Oct 11, 2019 •

edited

Loading