[PATCH honister 0/9] arm-bsp/machine: Backport runfvp config for Corstone1000 and update runfvp script

[PATCH honister 0/9] arm-bsp/machine: Backport runfvp config for Corstone1000 and update runfvp script

[harry.moulton]

From: Harry Moulton <harry.moulton@arm.com>

These commits add the runfvp configuration for corstone1000 to Honister,
and patches for the runfvp script itself to fix issue with running cs1000.

Signed-off-by: Harry Moulton <harry.moulton@arm.com>

Harry Moulton (1):
  arm-bsp/machine: Add runfvp config for corstone1000

Ross Burton (8):
  runfvp: add an asyncio TODO
  runfvp: handle the fvp quitting before we kill it
  runfvp: don't hide output when using terminals
  runfvp: don't use f-string when there's no expressions
  runfvp: refactor terminal selection
  runfvp: fix undefined variable in terminal selection login
  runfvp: propagate the exit code correctly
  runfvp: check for telnet

 ci/corstone1000-fvp.yml                       |  1 +
 kas/corstone1000-fvp.yml                      |  7 ++
 .../conf/machine/corstone1000-fvp.conf        | 34 ++++++-
 scripts/runfvp                                | 98 ++++++++++++++-----
 4 files changed, 115 insertions(+), 25 deletions(-)

-- 
2.17.1

[PATCH honister 1/9] arm-bsp/machine: Add runfvp config for corstone1000

[harry.moulton]

From: Harry Moulton <harry.moulton@arm.com>

Add the runfvp config for corstone1000-fvp.

Signed-off-by: Harry Moulton <harry.moulton@arm.com>
---
 ci/corstone1000-fvp.yml                       |  1 +
 kas/corstone1000-fvp.yml                      |  7 ++++
 .../conf/machine/corstone1000-fvp.conf        | 34 ++++++++++++++++++-
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/ci/corstone1000-fvp.yml b/ci/corstone1000-fvp.yml
index 66cdec6e..b6ae242f 100644
--- a/ci/corstone1000-fvp.yml
+++ b/ci/corstone1000-fvp.yml
@@ -10,3 +10,4 @@ local_conf_header:
         INITRAMFS_IMAGE:remove = "corstone1000-initramfs-image"
 
 machine: corstone1000-fvp
+
diff --git a/kas/corstone1000-fvp.yml b/kas/corstone1000-fvp.yml
index 531fd1ac..2420aa8e 100644
--- a/kas/corstone1000-fvp.yml
+++ b/kas/corstone1000-fvp.yml
@@ -12,5 +12,12 @@ repos:
 
 machine: corstone1000-fvp
 
+local_conf_header:
+    fvp-config: |
+        # Remove Dropbear SSH as it will not fit into the corstone1000 image.
+        IMAGE_FEATURES:remove = " ssh-server-dropbear"
+        INHERIT = " ${@bb.utils.contains('BUILD_ARCH', 'x86_64', 'fvpboot', '', d)}"
+        LICENSE_FLAGS_WHITELIST:append = " Arm-FVP-EULA"
+
 target:
   - corstone1000-image
diff --git a/meta-arm-bsp/conf/machine/corstone1000-fvp.conf b/meta-arm-bsp/conf/machine/corstone1000-fvp.conf
index fc69fe84..eb122f35 100644
--- a/meta-arm-bsp/conf/machine/corstone1000-fvp.conf
+++ b/meta-arm-bsp/conf/machine/corstone1000-fvp.conf
@@ -1,9 +1,41 @@
 #@TYPE: Machine
 #@NAME: corstone1000-fvp machine
 #@DESCRIPTION: Machine configuration for Corstone1000 64-bit FVP
-
 require conf/machine/include/corstone1000.inc
 
 TFA_TARGET_PLATFORM = "fvp"
 
 TFM_PLATFORM_IS_FVP = "TRUE"
+
+# testimage config
+TEST_TARGET = "OEFVPTarget"
+TEST_SUITES = "noop"
+
+# FVP Config
+FVP_PROVIDER ?= "fvp-corstone1000-native"
+FVP_EXE ?= "FVP_Corstone-1000"
+FVP_CONSOLE ?= "host_terminal_0"
+
+# FVP Parameters
+FVP_CONFIG[se.trustedBootROMloader.fname] ?= "${DEPLOY_DIR_IMAGE}/bl1.bin"
+FVP_CONFIG[board.xnvm_size] ?= "64"
+FVP_CONFIG[se.trustedSRAM_config] ?= "6"
+FVP_CONFIG[se.BootROM_config] ?= "3"
+FVP_CONFIG[board.hostbridge.interfaceName] ?= "tap0"
+FVP_CONFIG[board.smsc_91c111.enabled] ?= "1"
+FVP_CONFIG[board.hostbridge.userNetworking] ?= "true"
+FVP_CONFIG[board.hostbridge.userNetPorts] ?= "5555=5555,8080=80,8022=22"
+FVP_CONFIG[board.se_flash_size] ?= "8192"
+FVP_CONFIG[diagnostics] ?= "4"
+FVP_CONFIG[disable_visualisation] ?= "true"
+FVP_CONFIG[se.nvm.update_raw_image] ?= "0"
+
+# Boot image
+FVP_DATA ?= "board.flash0=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.wic.nopt@0x68100000"
+
+# FVP Terminals
+FVP_TERMINALS[host.host_terminal_0] ?= "Normal World Console"
+FVP_TERMINALS[host.host_terminal_1] ?= "Secure World Console"
+FVP_TERMINALS[se.secenc_terminal] ?= "Secure Enclave Console"
+FVP_TERMINALS[extsys0.extsys_terminal] ?= "Cortex M3"
+
-- 
2.17.1

[PATCH honister 2/9] runfvp: add an asyncio TODO

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 scripts/runfvp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/runfvp b/scripts/runfvp
index 66a76cc5..524de24d 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -205,6 +205,7 @@ def runfvp(cli_args):
             telnet = await asyncio.create_subprocess_exec("telnet", "localhost", str(port), stdin=sys.stdin, stdout=sys.stdout)
             await telnet.wait()
             logger.debug(f"Telnet quit, cancelling tasks")
+            # TODO: this is 3.7+
             for t in asyncio.all_tasks():
                 logger.debug(f"Cancelling {t}")
                 t.cancel()
-- 
2.17.1

[PATCH honister 3/9] runfvp: handle the fvp quitting before we kill it

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Don't raise an exception if the FVP has quit before we get around to
killing it.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 scripts/runfvp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index 524de24d..877e80cb 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -179,7 +179,10 @@ async def start_fvp(cli, console_cb):
     finally:
         # If we get cancelled or throw an exception, kill the FVP
         logger.debug(f"Killing FVP PID {fvp_process.pid}")
-        fvp_process.terminate()
+        try:
+            fvp_process.terminate()
+        except ProcessLookupError:
+            pass
 
     if await fvp_process.wait() != 0:
         logger.info(f"{cli[0]} quit with code {fvp_process.returncode}")
-- 
2.17.1

[PATCH honister 4/9] runfvp: don't hide output when using terminals

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Only pass a console_cb if we're hooking up a console, so that the output
from the FVP is visible on the terminal.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 scripts/runfvp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index 877e80cb..77668248 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -216,7 +216,8 @@ def runfvp(cli_args):
     try:
         # When we can assume Py3.7+, this can simply be asyncio.run()
         loop = asyncio.get_event_loop()
-        loop.run_until_complete(asyncio.gather(start_fvp(cli, console_cb=console_started)))
+        console_cb = expected_terminal and console_started or None
+        loop.run_until_complete(asyncio.gather(start_fvp(cli, console_cb=console_cb)))
     except asyncio.CancelledError:
         pass
 
-- 
2.17.1

[PATCH honister 5/9] runfvp: don't use f-string when there's no expressions

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 scripts/runfvp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index 77668248..fb334795 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -169,7 +169,7 @@ async def start_fvp(cli, console_cb):
 
             # Look for serial connections opening
             if console_cb:
-                m = re.match(fr"^(\S+): Listening for serial connection on port (\d+)$", line)
+                m = re.match(r"^(\S+): Listening for serial connection on port (\d+)$", line)
                 if m:
                     terminal = m.group(1)
                     port = int(m.group(2))
-- 
2.17.1

[PATCH honister 6/9] runfvp: refactor terminal selection

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Rewrite the terminal code to have a priority list of terminals when
selecting a default, allow the user to pick a default with a
configuration file, and add gnome-terminal to the list.

Signed-off-by: Ross Burton <ross.burton@arm.com>
---
 scripts/runfvp | 69 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 17 deletions(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index fb334795..ca1e797c 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -1,6 +1,7 @@
 #! /usr/bin/env python3
 
 import asyncio
+import collections
 import json
 import os
 import re
@@ -12,25 +13,59 @@ import pathlib
 import logging
 logger = logging.getLogger("RunFVP")
 
-# TODO: option to switch between telnet and netcat
-connect_command = "telnet localhost %port"
+from typing import List, Optional
+
+def get_config_dir() -> pathlib.Path:
+    value = os.environ.get("XDG_CONFIG_HOME")
+    if value and os.path.isabs(value):
+        return pathlib.Path(value)
+    else:
+        return pathlib.Path.home() / ".config"
+
+class Terminals:
+    Terminal = collections.namedtuple("Terminal", ["priority", "name", "command"])
+
+    def __init__(self):
+        self.terminals = []
+
+    def add_terminal(self, priority, name, command):
+        self.terminals.append(Terminals.Terminal(priority, name, command))
+        # Keep this list sorted by priority
+        self.terminals.sort(reverse=True, key=lambda t: t.priority)
+        self.name_map = {t.name: t for t in self.terminals}
 
-terminal_map = {
-    "tmux": f"tmux new-window -n \"%title\" \"{connect_command}\"",
-    "xterm": f"xterm -title \"%title\" -e {connect_command}",
-    "none": ""
-    # TODO more terminals
-}
+    def configured_terminal(self) -> Optional[str]:
+        import configparser
 
-def get_default_terminal():
-    import shutil
-    import shlex
+        config = configparser.ConfigParser()
+        config.read(get_config_dir() / "runfvp.conf")
+        return config.get("RunFVP", "Terminal", fallback=None)
 
-    for terminal in "xterm",:
-        command = shlex.split(terminal_map[terminal])[0]
-        if shutil.which(command):
-            return terminal
-    return "none"
+    def preferred_terminal(self) -> str:
+        import shlex, shutil
+
+        preferred = self.configured_terminal()
+        if preferred:
+            return preferred
+
+        for t in self.terminals:
+            if t.command and shutil.which(shlex.split(t.command)[0]):
+                return t.name
+        return self.terminals[-1].name
+
+    def all_terminals(self) -> List[str]:
+        return self.name_map.keys()
+
+    def __getitem__(self, name: str):
+        return self.name_map[name]
+
+terminals = Terminals()
+# TODO: option to switch between telnet and netcat
+connect_command = "telnet localhost %port"
+terminals.add_terminal(2, "tmux", f"tmux new-window -n \"%title\" \"{connect_command}\""),
+terminals.add_terminal(2, "gnome-terminal", f"gnome-terminal --window --title \"%title\" --command \"{connect_command}\""),
+terminals.add_terminal(1, "xterm", f"xterm -title \"%title\" -e {connect_command}"),
+terminals.add_terminal(0, "none", None)
 
 def get_image_directory(machine=None):
     """
@@ -59,7 +94,7 @@ def parse_args(arguments):
     parser = argparse.ArgumentParser(description="Run images in a FVP")
     parser.add_argument("config", nargs="?", help="Machine name or path to .fvpconf file")
     group = parser.add_mutually_exclusive_group()
-    group.add_argument("-t", "--terminals", choices=terminal_map.keys(), default=get_default_terminal(), help="Automatically start terminals (default: %(default)s)")
+    group.add_argument("-t", "--terminals", choices=terminals.all_terminals(), default=terminals.preferred_terminal(), help="Automatically start terminals (default: %(default)s)")
     group.add_argument("-c", "--console", action="store_true", help="Attach the first uart to stdin/stdout")
     parser.add_argument("--verbose", action="store_true", help="Output verbose logging")
     parser.usage = f"{parser.format_usage().strip()} -- [ arguments passed to FVP ]"
-- 
2.17.1

[PATCH honister 7/9] runfvp: fix undefined variable in terminal selection login

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Since 10e60cc the terminal_map doesn't exist, this piece of code wasn't
updated.

Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Jon Mason <jon.mason@arm.com>
---
 scripts/runfvp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index ca1e797c..c20455fd 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -182,7 +182,7 @@ def parse_config(args, config):
             # TODO if raw mode
             # cli.extend(["--parameter", f"{terminal}.mode=raw"])
             # TODO put name into terminal title
-            cli.extend(["--parameter", f"{terminal}.terminal_command={terminal_map[args.terminals]}"])
+            cli.extend(["--parameter", f"{terminal}.terminal_command={terminals[args.terminals].command}"])
         else:
             # Disable terminal
             cli.extend(["--parameter", f"{terminal}.start_telnet=0"])
-- 
2.17.1

[PATCH honister 8/9] runfvp: propagate the exit code correctly

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

runfvp could encounter an error but the exit code remained as 0.

Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Jon Mason <jon.mason@arm.com>
---
 scripts/runfvp | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index c20455fd..5cab0949 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -221,6 +221,9 @@ async def start_fvp(cli, console_cb):
 
     if await fvp_process.wait() != 0:
         logger.info(f"{cli[0]} quit with code {fvp_process.returncode}")
+        return fvp_process.returncode
+    else:
+        return 0
 
 def runfvp(cli_args):
     args, fvp_args = parse_args(cli_args)
@@ -234,7 +237,7 @@ def runfvp(cli_args):
         expected_terminal = config["console"]
         if not expected_terminal:
             logger.error("--console used but FVP_CONSOLE not set in machine configuration")
-            sys.exit(1)
+            return 1
     else:
         expected_terminal = None
 
@@ -252,9 +255,10 @@ def runfvp(cli_args):
         # When we can assume Py3.7+, this can simply be asyncio.run()
         loop = asyncio.get_event_loop()
         console_cb = expected_terminal and console_started or None
-        loop.run_until_complete(asyncio.gather(start_fvp(cli, console_cb=console_cb)))
+        return loop.run_until_complete(start_fvp(cli, console_cb=console_cb))
     except asyncio.CancelledError:
-        pass
+        # This means telnet exited, which isn't an error
+        return 0
 
 if __name__ == "__main__":
     try:
@@ -264,6 +268,6 @@ if __name__ == "__main__":
         if sys.stdin.isatty():
             signal.signal(signal.SIGTTOU, signal.SIG_IGN)
             os.tcsetpgrp(sys.stdin.fileno(), os.getpgrp())
-        runfvp(sys.argv[1:])
+        sys.exit(runfvp(sys.argv[1:]))
     except KeyboardInterrupt:
         pass
-- 
2.17.1

[PATCH honister 9/9] runfvp: check for telnet

[harry.moulton]

From: Ross Burton <ross.burton@arm.com>

Check for telnet on startup to avoid mysterious failures later when
telnet isn't available.

Signed-off-by: Ross Burton <ross.burton@arm.com>
Signed-off-by: Jon Mason <jon.mason@arm.com>
---
 scripts/runfvp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/runfvp b/scripts/runfvp
index 5cab0949..0ebf873e 100755
--- a/scripts/runfvp
+++ b/scripts/runfvp
@@ -5,6 +5,7 @@ import collections
 import json
 import os
 import re
+import shutil
 import signal
 import sys
 import subprocess
@@ -42,7 +43,7 @@ class Terminals:
         return config.get("RunFVP", "Terminal", fallback=None)
 
     def preferred_terminal(self) -> str:
-        import shlex, shutil
+        import shlex
 
         preferred = self.configured_terminal()
         if preferred:
@@ -233,6 +234,11 @@ def runfvp(cli_args):
     cli.extend(fvp_args)
     logger.debug(f"Constructed FVP call: {cli}")
 
+    # Check that telnet is present
+    if not bool(shutil.which("telnet")):
+        logger.error("Cannot find telnet, this is needed to connect to the FVP.")
+        return 1
+
     if args.console:
         expected_terminal = config["console"]
         if not expected_terminal:
-- 
2.17.1

Re: [PATCH honister 0/9] arm-bsp/machine: Backport runfvp config for Corstone1000 and update runfvp script

[Jon Mason]

On Tue, 26 Apr 2022 13:33:50 +0100, harry.moulton@arm.com wrote:
> These commits add the runfvp configuration for corstone1000 to Honister,
> and patches for the runfvp script itself to fix issue with running cs1000.
> 
> Signed-off-by: Harry Moulton <harry.moulton@arm.com>
> 
> Harry Moulton (1):
>   arm-bsp/machine: Add runfvp config for corstone1000
> 
> [...]

Applied, thanks!

[1/9] arm-bsp/machine: Add runfvp config for corstone1000
      commit: 0ea4b2220f8463e8d46f1e13c3986aac213ffcba
[2/9] runfvp: add an asyncio TODO
      commit: cc1c99f53737bca1e503da4e08ba11e5b27d2ae5
[3/9] runfvp: handle the fvp quitting before we kill it
      commit: c72527d6a5c8be469136a4d5cde62d63283e11d9
[4/9] runfvp: don't hide output when using terminals
      commit: 55453024ee5cf5bcf396551e7de10c77d0cc3390
[5/9] runfvp: don't use f-string when there's no expressions
      commit: 5920774ada13e3f265913d17e6f44d56cb23ba26
[6/9] runfvp: refactor terminal selection
      commit: 0ffbe558a0bfe3aec7110cde7a25fbcadd91d01c
[7/9] runfvp: fix undefined variable in terminal selection login
      commit: 4b8aeb6edcc95fc747cdc8f56b3717830e6c9e5d
[8/9] runfvp: propagate the exit code correctly
      commit: 952e04cd581672ccaa3c9b58227af5b45bd1f870
[9/9] runfvp: check for telnet
      commit: 2623e69db362b357db45c343e6d504909552c2d5

Best regards,
-- 
Jon Mason <jon.mason@arm.com>