You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/07/19 02:01:36 UTC

[GitHub] [tvm] mkatanbaf opened a new pull request, #12125: Zephyr fvp support

mkatanbaf opened a new pull request, #12125:
URL: https://github.com/apache/tvm/pull/12125

   This PR adds corstone 300 fvp to the platforms supported by the zephyr.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943936148


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   also, I checked and the str cast is needed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943908640


##########
apps/microtvm/zephyr/template_project/CMakeLists.txt.template:
##########
@@ -21,7 +21,7 @@ cmake_minimum_required(VERSION 3.13.1)
 
 set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
 
-set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+set(QEMU_PIPE <QEMU_PIPE> CACHE PATH "path to qemu pipe")

Review Comment:
   Do you mean to replace "path to qemu pipe" with "Path to QEMU pipe"?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r929414402


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -885,5 +945,186 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    FVP_INIT = "fvp_initialized"
+    MAKE_FAILED = "make_failed"
+    EOF = "eof"
+
+
+class BlockingStream:
+    """Reimplementation of Stream class from Iris with blocking semantics."""
+
+    def __init__(self):
+        self.q = queue.Queue()
+        self.unread = None
+
+    def read(self, n=-1, timeout_sec=None):
+        assert (
+            n != -1
+        ), "expect firmware to open stdin using raw mode, and therefore expect sized read requests"
+
+        data = b""
+        if self.unread:
+            data = data + self.unread
+            self.unread = None
+
+        while len(data) < n:
+            try:
+                # When there is some data to return, fetch as much as possible, then return what we can.
+                # When there is no data yet to return, block.
+                data += self.q.get(block=not len(data), timeout=timeout_sec)
+            except queue.Empty:
+                break
+
+        if len(data) > n:
+            self.unread = data[n:]
+            data = data[:n]
+
+        return data
+
+    readline = read
+
+    def write(self, data):
+        self.q.put(data)
+
+
+class ZephyrFvpTransport:
+    """A transport class that communicates with the ARM FVP via Iris server."""
+
+    def __init__(self, options):
+        self.options = options
+        self.proc = None
+        self._queue = queue.Queue()
+
+        self._import_iris()
+
+    def _import_iris(self):
+        # Location as seen in the FVP_Corstone_SSE-300_11.15_24 tar.
+        iris_lib_path = (
+            pathlib.Path(self.options["arm_fvp_path"]).parent.parent.parent
+            / "Iris"
+            / "Python"
+            / "iris"
+        )
+
+        sys.path.insert(0, str(iris_lib_path.parent))
+        try:
+            import iris.NetworkModelInitializer
+        finally:
+            sys.path.pop(0)
+
+        self._iris_lib = iris
+
+        def _convertStringToU64Array(strValue):
+            numBytes = len(strValue)
+            if numBytes == 0:
+                return []
+
+            numU64 = (numBytes + 7) // 8
+            # Extend the string ending with '\0', so that the string length is multiple of 8.
+            # E.g. 'hello' is extended to: 'hello'+\0\0\0
+            strExt = strValue.ljust(8 * numU64, b"\0")
+            # Convert the string to a list of uint64_t in little endian
+            return struct.unpack("<{}Q".format(numU64), strExt)
+
+        iris.iris.convertStringToU64Array = _convertStringToU64Array
+
+    def open(self):
+        args = ["ninja"]
+        if self.options.get("verbose"):
+            args.append("-v")
+        args.append("run")
+        env = dict(os.environ)
+        env["FVP_BIN_PATH"] = str(pathlib.Path(self.options["arm_fvp_path"]).parent)
+        env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+        self.proc = subprocess.Popen(
+            args,
+            cwd=BUILD_DIR,
+            env=env,
+            stdout=subprocess.PIPE,
+        )
+        threading.Thread(target=self._fvp_check_stdout, daemon=True).start()
+
+        self.iris_port = self._wait_for_fvp()
+        _LOG.info("IRIS started on port %d", self.iris_port)
+        NetworkModelInitializer = self._iris_lib.NetworkModelInitializer.NetworkModelInitializer
+        self._model_init = NetworkModelInitializer(
+            host="localhost", port=self.iris_port, timeout_in_ms=1000
+        )
+        self._model = self._model_init.start()
+        self._target = self._model.get_target("component.FVP_MPS3_Corstone_SSE_300.cpu0")
+
+        self._target.handle_semihost_io()
+        self._target._stdout = BlockingStream()
+        self._target._stdin = BlockingStream()
+        self._model.run(blocking=False, timeout=100)
+        self._wait_for_semihost_init()
+
+        return server.TransportTimeouts(
+            session_start_retry_timeout_sec=2.0,
+            session_start_timeout_sec=10.0,
+            session_established_timeout_sec=10.0,
+        )
+
+    def _fvp_check_stdout(self):
+        for line in self.proc.stdout:
+            line = str(line, "utf-8")
+            m = re.match(r"Iris server started listening to port ([0-9]+)\n", line)
+            n = re.match("microTVM Zephyr runtime - running", line)
+            if m:
+                self._queue.put((ZephyrFvpMakeResult.FVP_STARTED, int(m.group(1))))
+            elif n:
+                self._queue.put((ZephyrFvpMakeResult.FVP_INIT, None))
+            else:
+                line = re.sub("[^a-zA-Z0-9 \n]", "", line)
+                pattern = r"recipe for target (\w*) failed"
+                if re.search(pattern, line, re.IGNORECASE):
+                    self._queue.put((ZephyrFvpMakeResult.MAKE_FAILED, None))
+
+        self._queue.put((ZephyrFvpMakeResult.EOF, None))
+
+    def _wait_for_fvp(self):
+        while True:
+            try:
+                item = self._queue.get(timeout=120)
+            except Exception:
+                raise TimeoutError("FVP setup timeout.")
+
+            if item[0] == ZephyrFvpMakeResult.FVP_STARTED:
+                return item[1]
+
+            if item[0] in [ZephyrFvpMakeResult.MAKE_FAILED, ZephyrFvpMakeResult.EOF]:
+                raise RuntimeError("FVP setup failed.")
+
+            raise ValueError(f"{item} not expected.")
+
+    def _wait_for_semihost_init(self):
+        while True:
+            try:
+                item = self._queue.get(timeout=120)
+            except Exception:
+                raise TimeoutError("semihost init timeout.")
+
+            if item[0] == ZephyrFvpMakeResult.FVP_INIT:
+                return
+
+            raise ValueError(f"{item} not expected.")

Review Comment:
   The API is waiting for a specific line to appear on the stdout. I added a comment to make it more clear.



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -64,8 +67,89 @@ static size_t g_num_bytes_requested = 0;
 static size_t g_num_bytes_written = 0;
 static size_t g_num_bytes_in_rx_buffer = 0;
 
+#ifdef FVP

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943933792


##########
apps/microtvm/zephyr/template_project/CMakeLists.txt.template:
##########
@@ -21,7 +21,7 @@ cmake_minimum_required(VERSION 3.13.1)
 
 set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
 
-set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+set(QEMU_PIPE <QEMU_PIPE> CACHE PATH "path to qemu pipe")

Review Comment:
   @mkatanbaf yeah, replace the docstring for `set`  using better caps



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213317914

   > @mkatanbaf Got it! Well, feels like déjà-vu to me (when using QEMU). When you ran locally in a loop did you use any container? 
   yes, I'm using the cortexm docker container (formerly qemu). What I've seen (and trying to reproduce) is that it gets stuck when it tries to close the transport. My guess is that something prevents the subprocess termination.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1206684390

   cc @mehrdadh 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943883500


##########
apps/microtvm/zephyr/template_project/src/host_driven/fvp/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+
+#include <kernel.h>
+#include <unistd.h>
+#include <zephyr.h>
+
+static uint32_t semihost_cmd(uint32_t opcode, void* arg);

Review Comment:
   Thanks for the explanation, and for including the warning message.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943999664


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   @mkatanbaf Could you please add a comment on top of the `chmod` code explaining it's a CI requirement as per David's explanation? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213594352

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943912859


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+            env["ARMFVP_BIN_PATH"] = os.path.realpath(env["ARMFVP_BIN_PATH"])
+            st = os.stat(env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55")
+            os.chmod(
+                env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55",
+                st.st_mode | stat.S_IEXEC,
+            )
 
-        args = ["make", "-j2"]
+        print("ENV", env)

Review Comment:
   yes, and removed!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mehrdadh commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r939310189


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -564,21 +599,49 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = self._find_board_from_cmake_file()

Review Comment:
   can you clarify why `cmake ..` and `make` doesn't work anymore?



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -44,6 +43,12 @@
 #include <unistd.h>
 #include <zephyr.h>
 
+#ifdef FVP
+#include <irq.h>
+

Review Comment:
   remove newline?



##########
apps/microtvm/zephyr/template_project/src/host_driven/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*

Review Comment:
   can you move fvp related files to a sub-directory like: `apps/microtvm/zephyr/template_project/src/host_driven/fvp`
   it would make it more readable. 
   Also using this style, we could also separate the zephyr transport files from main.c and move it to a different sub directory . That could be for a separate PR.



##########
tests/scripts/task_python_microtvm.sh:
##########
@@ -54,4 +55,4 @@ python3 gallery/how_to/work_with_microtvm/micro_tflite.py
 python3 gallery/how_to/work_with_microtvm/micro_autotune.py
 python3 gallery/how_to/work_with_microtvm/micro_aot.py
 
-run_pytest ctypes python-relay-strategy-arm_cpu tests/python/relay/strategy/arm_cpu --enable-corstone300-tests
+run_pytest ctypes python-relay-strategy-arm_cpu tests/python/relay/strategy/arm_cpu --enable-corstone300-tests

Review Comment:
   add a new line. It's weird that CI didn't get an error here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943911684


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -166,6 +169,16 @@ def _find_board_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
     return zephyr_board
 
 
+def _find_platform_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
+    emu_platform = None
+    with open(API_SERVER_DIR / CMAKELIST_FILENAME) as cmake_f:
+        for line in cmake_f:
+            if line.startswith("set(EMU_PLATFORM"):
+                emu_platform = line.strip("\n").strip("set(EMU_PLATFORM ").strip(")")

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943917402


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   The `FVP_Corstone_SSE-300_Ethos-U55` was not an executable on the CI and the tests were failing. I had to explicitly change that with `chmod`. It was OK on my local machine though!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r929416081


##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -218,11 +303,42 @@ void uart_irq_cb(const struct device* dev, void* user_data) {
   }
 }
 
+#ifdef FVP

Review Comment:
   we are using the uart to write some log messages, and it might be a good idea to initialize the fvp uart correctly. but technically we don't use the fvp uart in RX mode, and it won't cause an error if we remove these lines (unless someone accidentally sends data to the corresponding TCP port, which could cause an overflow!)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943947844


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   let me see if I can understand what's happening, this is odd.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943948778


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -344,6 +357,18 @@ def _get_nrf_device_args(options):
         type="str",
         help="Path to the CMSIS directory.",
     ),
+    server.ProjectOption(
+        "arm_fvp_path",
+        optional=["generate_project"],

Review Comment:
   I added the "open_transport" to the list of domains the option is used.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943937552


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   hmm got it. odd. yeah it's ok on my local machine as well...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213315767

   We can re-trigger and hope for the best, but I wonder if the first issue -- in FVP hanging -- is blocker to merge it. @areusch Thoughts? :) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r929268089


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -885,5 +945,186 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    FVP_INIT = "fvp_initialized"
+    MAKE_FAILED = "make_failed"
+    EOF = "eof"
+
+
+class BlockingStream:
+    """Reimplementation of Stream class from Iris with blocking semantics."""
+
+    def __init__(self):
+        self.q = queue.Queue()
+        self.unread = None
+
+    def read(self, n=-1, timeout_sec=None):
+        assert (
+            n != -1
+        ), "expect firmware to open stdin using raw mode, and therefore expect sized read requests"
+
+        data = b""
+        if self.unread:
+            data = data + self.unread
+            self.unread = None
+
+        while len(data) < n:
+            try:
+                # When there is some data to return, fetch as much as possible, then return what we can.
+                # When there is no data yet to return, block.
+                data += self.q.get(block=not len(data), timeout=timeout_sec)
+            except queue.Empty:
+                break
+
+        if len(data) > n:
+            self.unread = data[n:]
+            data = data[:n]
+
+        return data
+
+    readline = read
+
+    def write(self, data):
+        self.q.put(data)
+
+
+class ZephyrFvpTransport:
+    """A transport class that communicates with the ARM FVP via Iris server."""
+
+    def __init__(self, options):
+        self.options = options
+        self.proc = None
+        self._queue = queue.Queue()
+
+        self._import_iris()
+
+    def _import_iris(self):
+        # Location as seen in the FVP_Corstone_SSE-300_11.15_24 tar.
+        iris_lib_path = (
+            pathlib.Path(self.options["arm_fvp_path"]).parent.parent.parent
+            / "Iris"
+            / "Python"
+            / "iris"
+        )
+
+        sys.path.insert(0, str(iris_lib_path.parent))
+        try:
+            import iris.NetworkModelInitializer
+        finally:
+            sys.path.pop(0)
+
+        self._iris_lib = iris
+
+        def _convertStringToU64Array(strValue):
+            numBytes = len(strValue)
+            if numBytes == 0:
+                return []
+
+            numU64 = (numBytes + 7) // 8
+            # Extend the string ending with '\0', so that the string length is multiple of 8.
+            # E.g. 'hello' is extended to: 'hello'+\0\0\0
+            strExt = strValue.ljust(8 * numU64, b"\0")
+            # Convert the string to a list of uint64_t in little endian
+            return struct.unpack("<{}Q".format(numU64), strExt)
+
+        iris.iris.convertStringToU64Array = _convertStringToU64Array
+
+    def open(self):
+        args = ["ninja"]
+        if self.options.get("verbose"):
+            args.append("-v")
+        args.append("run")
+        env = dict(os.environ)
+        env["FVP_BIN_PATH"] = str(pathlib.Path(self.options["arm_fvp_path"]).parent)
+        env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+        self.proc = subprocess.Popen(
+            args,
+            cwd=BUILD_DIR,
+            env=env,
+            stdout=subprocess.PIPE,
+        )
+        threading.Thread(target=self._fvp_check_stdout, daemon=True).start()
+
+        self.iris_port = self._wait_for_fvp()
+        _LOG.info("IRIS started on port %d", self.iris_port)
+        NetworkModelInitializer = self._iris_lib.NetworkModelInitializer.NetworkModelInitializer
+        self._model_init = NetworkModelInitializer(
+            host="localhost", port=self.iris_port, timeout_in_ms=1000
+        )
+        self._model = self._model_init.start()
+        self._target = self._model.get_target("component.FVP_MPS3_Corstone_SSE_300.cpu0")
+
+        self._target.handle_semihost_io()
+        self._target._stdout = BlockingStream()
+        self._target._stdin = BlockingStream()
+        self._model.run(blocking=False, timeout=100)
+        self._wait_for_semihost_init()
+
+        return server.TransportTimeouts(
+            session_start_retry_timeout_sec=2.0,
+            session_start_timeout_sec=10.0,
+            session_established_timeout_sec=10.0,
+        )
+
+    def _fvp_check_stdout(self):
+        for line in self.proc.stdout:
+            line = str(line, "utf-8")
+            m = re.match(r"Iris server started listening to port ([0-9]+)\n", line)
+            n = re.match("microTVM Zephyr runtime - running", line)
+            if m:
+                self._queue.put((ZephyrFvpMakeResult.FVP_STARTED, int(m.group(1))))
+            elif n:
+                self._queue.put((ZephyrFvpMakeResult.FVP_INIT, None))
+            else:
+                line = re.sub("[^a-zA-Z0-9 \n]", "", line)
+                pattern = r"recipe for target (\w*) failed"
+                if re.search(pattern, line, re.IGNORECASE):
+                    self._queue.put((ZephyrFvpMakeResult.MAKE_FAILED, None))
+
+        self._queue.put((ZephyrFvpMakeResult.EOF, None))
+
+    def _wait_for_fvp(self):
+        while True:
+            try:
+                item = self._queue.get(timeout=120)
+            except Exception:
+                raise TimeoutError("FVP setup timeout.")
+
+            if item[0] == ZephyrFvpMakeResult.FVP_STARTED:
+                return item[1]
+
+            if item[0] in [ZephyrFvpMakeResult.MAKE_FAILED, ZephyrFvpMakeResult.EOF]:
+                raise RuntimeError("FVP setup failed.")
+
+            raise ValueError(f"{item} not expected.")
+
+    def _wait_for_semihost_init(self):
+        while True:
+            try:
+                item = self._queue.get(timeout=120)
+            except Exception:
+                raise TimeoutError("semihost init timeout.")
+
+            if item[0] == ZephyrFvpMakeResult.FVP_INIT:
+                return
+
+            raise ValueError(f"{item} not expected.")

Review Comment:
   could you add some context to these two errors? i.e. what is the API server doing right now and what does it expect?



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -885,5 +945,186 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    FVP_INIT = "fvp_initialized"

Review Comment:
   how about we call this MICROTVM_API_SERVER_INIT?



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -218,11 +303,42 @@ void uart_irq_cb(const struct device* dev, void* user_data) {
   }
 }
 
+#ifdef FVP

Review Comment:
   is this code needed any longer?



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -64,8 +67,89 @@ static size_t g_num_bytes_requested = 0;
 static size_t g_num_bytes_written = 0;
 static size_t g_num_bytes_in_rx_buffer = 0;
 
+#ifdef FVP

Review Comment:
   what if we break this out into a separate file and modify CMakeLists.txt to include it (we can still e.g.
   ```
   #ifdef FVP
   #include "semihost.h"
   #endif
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943964049


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   @driazati Any clue why this file here: https://github.com/apache/tvm/blob/ebd98ce8215eb7499ead0d509457fd432eca7d32/apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55  (which is indeed an executable) could turn into a non executable  when copied inside the CI container as @mkatanbaf said above? It's copied this way to the new location:
   
   https://github.com/apache/tvm/pull/12125/files#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R561



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943983138


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   If the script gets sent between a build and a test step in Jenkins it goes through AWS S3 which doesn't have any notion of file permissions, so we add them back in here: https://github.com/apache/tvm/blob/main/ci/jenkins/Build.groovy.j2#L36-L43. Changing that is a little tricky since those changes wont be run in CI unless the source branch is under apache/tvm and not from a fork, so keeping the chmod in code here seems reasonable for now. We can follow up with a change that cleans it up and moves it out to `Build.groovy.j2`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r940626946


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -564,21 +599,49 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = self._find_board_from_cmake_file()

Review Comment:
   I believe the main motivation for using ninja was to reduce the build time.



##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -44,6 +43,12 @@
 #include <unistd.h>
 #include <zephyr.h>
 
+#ifdef FVP
+#include <irq.h>
+

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213076210

   @mkatanbaf @areusch @mehrdadh I've re-triggered the tests, but apparently @mkatanbaf have done it already once... it seems that one of the FVP process got stuck for about ~3h:
   
   ```
   [2022-08-12T07:40:53.224Z] INFO:__main__:dummy log...
   [2022-08-12T07:40:53.224Z] 
   [2022-08-12T07:40:53.224Z] INFO:__main__:microTVM Zephyr runtime - running
   [2022-08-12T07:40:53.224Z] 
   [2022-08-12T07:40:53.224Z] INFO:__main__:IRIS semihosting initialized.
   [2022-08-12T07:40:53.224Z] [07:40:53] /workspace/src/runtime/micro/micro_session.cc:368: remote: microTVM Zephyr runtime - running
   [2022-08-12T10:25:46.234Z] Sending interrupt signal to process
   [2022-08-12T10:25:50.665Z] script returned exit code 143
   ```
   :( 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r929413667


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -885,5 +945,186 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    FVP_INIT = "fvp_initialized"

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r944009772


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r944008143


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -166,6 +169,18 @@ def _find_board_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
     return zephyr_board
 
 
+def _find_platform_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
+    emu_platform = None
+    with open(API_SERVER_DIR / CMAKELIST_FILENAME) as cmake_f:
+        for line in cmake_f:
+            if line.startswith("set(EMU_PLATFORM"):

Review Comment:
   @mkatanbaf This line can be removed now that `re.match`  is in place.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r931659205


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -1062,8 +1062,18 @@ def open(self):
         self._target.handle_semihost_io()
         self._target._stdout = BlockingStream()
         self._target._stdin = BlockingStream()
-        self._model.run(blocking=False, timeout=100)
-        self._wait_for_semihost_init()
+        for i in range(3):
+            try:
+                _LOG.info("Iteration %d", i)
+                self._model.run(blocking=False, timeout=100)
+                self._wait_for_semihost_init()
+                break
+            except self._iris_lib.error.IrisError:
+                continue
+            else:

Review Comment:
   note: indent 1 less level for this block



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1215960287

   ok let's give this a shot, we can always disable the test in CI if it becomes flaky.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1216012645

   @areusch really?! We did that for the mps3 board with QEMU ... 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1194800215

   cc @gromero 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943964049


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   @driazati Any clue why this file here: https://github.com/apache/tvm/blob/ebd98ce8215eb7499ead0d509457fd432eca7d32/apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55  (which is indeed an executable) could turn into an non executable  when copied inside the CI container as @mkatanbaf said above? It's copied this way to the new location:
   
   https://github.com/apache/tvm/pull/12125/files#diff-e9070632c64e33062450a5c887a7a6b683d7294064584407aeff7aef24031dc1R561



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943920872


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+            env["ARMFVP_BIN_PATH"] = os.path.realpath(env["ARMFVP_BIN_PATH"])
+            st = os.stat(env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55")
+            os.chmod(
+                env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55",
+                st.st_mode | stat.S_IEXEC,
+            )
 
-        args = ["make", "-j2"]
+        print("ENV", env)
+        check_call(["cmake", "-GNinja", ".."], cwd=BUILD_DIR, env=env)
+
+        args = ["ninja"]
         if options.get("verbose"):
-            args.append("VERBOSE=1")
-        check_call(args, cwd=BUILD_DIR)
+            args.append("-v")
+        check_call(args, cwd=BUILD_DIR, env=env)
 
     # A list of all zephyr_board values which are known to launch using QEMU. Many platforms which
     # launch through QEMU by default include "qemu" in their name. However, not all do. This list
     # includes those tested platforms which do not include qemu.
-    _KNOWN_QEMU_ZEPHYR_BOARDS = ("mps2_an521", "mps3_an547")
+    _KNOWN_QEMU_ZEPHYR_BOARDS = ["mps2_an521", "mps3_an547"]
+
+    # A list of all zephyr_board values which are known to launch using ARM FVP (this script configures
+    # Zephyr to use that launch method).
+    _KNOWN_FVP_ZEPHYR_BOARDS = ["mps3_an547"]
 
     @classmethod
-    def _is_qemu(cls, board: str) -> bool:
-        return "qemu" in board or board in cls._KNOWN_QEMU_ZEPHYR_BOARDS
+    def _is_fvp(cls, board, use_fvp):
+        assert not (

Review Comment:
   Yes, this is a better way to express the function. Thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943725515


##########
apps/microtvm/zephyr/template_project/src/host_driven/fvp/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+
+#include <kernel.h>
+#include <unistd.h>
+#include <zephyr.h>
+
+static uint32_t semihost_cmd(uint32_t opcode, void* arg);
+
+void init_semihosting();
+
+ssize_t read_semihost(uint8_t* data, size_t size);
+
+ssize_t write_semihost(void* unused_context, const uint8_t* data, size_t size);
+
+int32_t get_sim_clk();

Review Comment:
   I don't see this function being used. Could we remove it? (also from `semihost.c`)? 



##########
apps/microtvm/zephyr/template_project/CMakeLists.txt.template:
##########
@@ -21,7 +21,7 @@ cmake_minimum_required(VERSION 3.13.1)
 
 set(ENV{QEMU_BIN_PATH} "${CMAKE_SOURCE_DIR}/qemu-hack")
 
-set(QEMU_PIPE "\${QEMU_PIPE}")  # QEMU_PIPE is set by the calling TVM instance.
+set(QEMU_PIPE <QEMU_PIPE> CACHE PATH "path to qemu pipe")

Review Comment:
   Could we s/path to qemu pipe/Path to QEMU pipe/? 



##########
apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55:
##########
@@ -0,0 +1,44 @@
+#!/bin/bash -e
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -x
+
+ARGS=( "$(basename $0)" )
+
+if [ "${FVP_BIN_PATH}"  != "" ]; then
+    ARGS=( ${FVP_BIN_PATH}/${ARGS[0]} )
+fi
+
+ARGS=( "${ARGS[@]}" 
+    --iris-server 
+    --print-port-number 

Review Comment:
   remove trailing space



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)

Review Comment:
   I think this cast to `dict()` is not necessary
   For instance, if `env = os.environ` you can access `env[ZEPHYR_BASE]`.



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -166,6 +169,16 @@ def _find_board_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
     return zephyr_board
 
 
+def _find_platform_from_cmake_file(cmake_file: Union[str, pathlib.Path]) -> str:
+    emu_platform = None
+    with open(API_SERVER_DIR / CMAKELIST_FILENAME) as cmake_f:
+        for line in cmake_f:
+            if line.startswith("set(EMU_PLATFORM"):
+                emu_platform = line.strip("\n").strip("set(EMU_PLATFORM ").strip(")")

Review Comment:
   It's possible to use a capture group as I suggested in:
   
   https://github.com/apache/tvm/pull/12338#discussion_r940861161
   
   but it's up to you, won't block on this.
   



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -904,5 +982,192 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    MICROTVM_API_SERVER_INIT = "fvp_initialized"
+    MAKE_FAILED = "make_failed"
+    EOF = "eof"
+
+
+class BlockingStream:
+    """Reimplementation of Stream class from Iris with blocking semantics."""
+
+    def __init__(self):
+        self.q = queue.Queue()
+        self.unread = None
+
+    def read(self, n=-1, timeout_sec=None):
+        assert (
+            n != -1
+        ), "expect firmware to open stdin using raw mode, and therefore expect sized read requests"
+
+        data = b""
+        if self.unread:
+            data = data + self.unread
+            self.unread = None
+
+        while len(data) < n:
+            try:
+                # When there is some data to return, fetch as much as possible, then return what we can.
+                # When there is no data yet to return, block.
+                data += self.q.get(block=not len(data), timeout=timeout_sec)
+            except queue.Empty:
+                break
+
+        if len(data) > n:
+            self.unread = data[n:]
+            data = data[:n]
+
+        return data
+
+    readline = read
+
+    def write(self, data):
+        self.q.put(data)
+
+
+class ZephyrFvpTransport:
+    """A transport class that communicates with the ARM FVP via Iris server."""
+
+    def __init__(self, options):
+        self.options = options
+        self.proc = None
+        self._queue = queue.Queue()
+        self._import_iris()
+
+    def _import_iris(self):
+        # Location as seen in the FVP_Corstone_SSE-300_11.15_24 tar.
+        iris_lib_path = (
+            pathlib.Path(self.options["arm_fvp_path"]).parent.parent.parent
+            / "Iris"
+            / "Python"
+            / "iris"
+        )
+
+        sys.path.insert(0, str(iris_lib_path.parent))
+        try:
+            import iris.NetworkModelInitializer
+        finally:
+            sys.path.pop(0)
+
+        self._iris_lib = iris
+
+        def _convertStringToU64Array(strValue):
+            numBytes = len(strValue)
+            if numBytes == 0:
+                return []
+
+            numU64 = (numBytes + 7) // 8
+            # Extend the string ending with '\0', so that the string length is multiple of 8.
+            # E.g. 'hello' is extended to: 'hello'+\0\0\0
+            strExt = strValue.ljust(8 * numU64, b"\0")
+            # Convert the string to a list of uint64_t in little endian
+            return struct.unpack("<{}Q".format(numU64), strExt)
+
+        iris.iris.convertStringToU64Array = _convertStringToU64Array
+
+    def open(self):
+        args = ["ninja"]
+        if self.options.get("verbose"):
+            args.append("-v")
+        args.append("run")
+        env = dict(os.environ)
+        env["FVP_BIN_PATH"] = str(pathlib.Path(self.options["arm_fvp_path"]).parent)

Review Comment:
   Should we add `arm_fvp_path`  as required or optional in the server options? It seems required here -- or is a check missing that would use `options.get()`?  But if it's a required option by FVP I'm not sure how to add it since this option doesn't apply to QEMU or other boards that don't use FVP. I think currently there is no way to inform that complexity using the server options ... so if it's indeed require at least assert or throw an error if `arm_fvp_path` is not given here?
   
   I also wonder why we have both `FVP_BIN_PATH` and `ARMFVP_BIN_PATH` here. The fvp-hack script uses FVP_BIN_PATH. Couldn't  we use only the `ARMFVP_BIN_PATH` here and in the fvp-hack. In this case the Python code here would set `ARMFVP_BIN_PATH` to use the fvp-hack script and the fvp-hack, by its turn, would be called by Zephyr build / run system (when subprocess() is called). Then the fvp-hack, when called by Zephyr, would add the iris flags etc?
   
   Or at least add a comment why there are two similar bin paths to the FVP, saying how that's used by the fvp-hack script..



##########
apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55:
##########
@@ -0,0 +1,44 @@
+#!/bin/bash -e
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -x
+
+ARGS=( "$(basename $0)" )
+
+if [ "${FVP_BIN_PATH}"  != "" ]; then
+    ARGS=( ${FVP_BIN_PATH}/${ARGS[0]} )
+fi
+
+ARGS=( "${ARGS[@]}" 

Review Comment:
   remove trailing space in this line



##########
apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55:
##########
@@ -0,0 +1,44 @@
+#!/bin/bash -e
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -x
+
+ARGS=( "$(basename $0)" )
+
+if [ "${FVP_BIN_PATH}"  != "" ]; then
+    ARGS=( ${FVP_BIN_PATH}/${ARGS[0]} )
+fi
+
+ARGS=( "${ARGS[@]}" 
+    --iris-server 

Review Comment:
   remove trailing space
   



##########
apps/microtvm/zephyr/template_project/src/host_driven/fvp/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+
+#include <kernel.h>
+#include <unistd.h>
+#include <zephyr.h>
+
+static uint32_t semihost_cmd(uint32_t opcode, void* arg);
+
+void init_semihosting();
+
+ssize_t read_semihost(uint8_t* data, size_t size);

Review Comment:
   this should be `semihost_read()`



##########
apps/microtvm/zephyr/template_project/src/host_driven/fvp/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+
+#include <kernel.h>
+#include <unistd.h>
+#include <zephyr.h>
+
+static uint32_t semihost_cmd(uint32_t opcode, void* arg);
+
+void init_semihosting();
+
+ssize_t read_semihost(uint8_t* data, size_t size);
+
+ssize_t write_semihost(void* unused_context, const uint8_t* data, size_t size);

Review Comment:
   this should be `semihost_write()`



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -483,10 +509,14 @@ def _generate_cmake_args(self, mlf_extracted_path, options) -> str:
         if options.get("west_cmd"):
             cmake_args += f"set(WEST {options['west_cmd']})\n"
 
-        if self._is_qemu(options["zephyr_board"]):
+        if self._is_qemu(options["zephyr_board"], options.get("use_fvp")):
             # Some boards support more than one emulator, so ensure QEMU is set.
             cmake_args += f"set(EMU_PLATFORM qemu)\n"
 
+        if self._is_fvp(options["zephyr_board"], options.get("use_fvp")):
+            cmake_args += f"set(EMU_PLATFORM armfvp)\n"

Review Comment:
   no s-tring is necessary here



##########
apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55:
##########
@@ -0,0 +1,44 @@
+#!/bin/bash -e
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -x
+
+ARGS=( "$(basename $0)" )
+
+if [ "${FVP_BIN_PATH}"  != "" ]; then
+    ARGS=( ${FVP_BIN_PATH}/${ARGS[0]} )
+fi
+
+ARGS=( "${ARGS[@]}" 
+    --iris-server 
+    --print-port-number 
+    -C cpu0.semihosting-enable=1 

Review Comment:
   remove trailing space



##########
apps/microtvm/zephyr/template_project/src/host_driven/fvp/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+#define TVM_APPS_MICROTVM_ZEPHYR_HOST_DRIVEN_SEMIHOST_H_
+
+#include <kernel.h>
+#include <unistd.h>
+#include <zephyr.h>
+
+static uint32_t semihost_cmd(uint32_t opcode, void* arg);

Review Comment:
   Usually static functions should not go into header files, since they are defined and used as per each translation unit (source file). But I suspect you didn't want it to be static, so `static`could be removed  from here and from `semihost.c`?
   
   Otherwise the following warning happens in `main.c` which also includes `semihost.h` but doesn't define -- because it does't use it --  `semihost_cmd` in:
   
   ```
   In file included from /home/gromero/git/tvm/tests/micro/zephyr/workspace_mps3_an547/2022-08-11T14-40-30/project/src/main.c:49:
   /home/gromero/git/tvm/tests/micro/zephyr/workspace_mps3_an547/2022-08-11T14-40-30/project/src/main.c: At top level:
   /home/gromero/git/tvm/tests/micro/zephyr/workspace_mps3_an547/2022-08-11T14-40-30/project/src/semihost.h:32:17: warning: 'semihost_cmd' declared 'static' but never defined [-Wunused-function]
      32 | static uint32_t semihost_cmd(uint32_t opcode, void* arg);
         |                 ^~~~
   ```



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -483,10 +509,14 @@ def _generate_cmake_args(self, mlf_extracted_path, options) -> str:
         if options.get("west_cmd"):
             cmake_args += f"set(WEST {options['west_cmd']})\n"
 
-        if self._is_qemu(options["zephyr_board"]):
+        if self._is_qemu(options["zephyr_board"], options.get("use_fvp")):
             # Some boards support more than one emulator, so ensure QEMU is set.
             cmake_args += f"set(EMU_PLATFORM qemu)\n"
 
+        if self._is_fvp(options["zephyr_board"], options.get("use_fvp")):
+            cmake_args += f"set(EMU_PLATFORM armfvp)\n"
+            cmake_args += f"set(ARMFVP_FLAGS -I)\n"

Review Comment:
   likewise regarding s-string



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+            env["ARMFVP_BIN_PATH"] = os.path.realpath(env["ARMFVP_BIN_PATH"])
+            st = os.stat(env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55")
+            os.chmod(
+                env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55",
+                st.st_mode | stat.S_IEXEC,
+            )
 
-        args = ["make", "-j2"]
+        print("ENV", env)

Review Comment:
   Is this a leftover and should be removed?



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
+            env["ARMFVP_BIN_PATH"] = os.path.realpath(env["ARMFVP_BIN_PATH"])
+            st = os.stat(env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55")
+            os.chmod(
+                env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55",
+                st.st_mode | stat.S_IEXEC,
+            )
 
-        args = ["make", "-j2"]
+        print("ENV", env)
+        check_call(["cmake", "-GNinja", ".."], cwd=BUILD_DIR, env=env)
+
+        args = ["ninja"]
         if options.get("verbose"):
-            args.append("VERBOSE=1")
-        check_call(args, cwd=BUILD_DIR)
+            args.append("-v")
+        check_call(args, cwd=BUILD_DIR, env=env)
 
     # A list of all zephyr_board values which are known to launch using QEMU. Many platforms which
     # launch through QEMU by default include "qemu" in their name. However, not all do. This list
     # includes those tested platforms which do not include qemu.
-    _KNOWN_QEMU_ZEPHYR_BOARDS = ("mps2_an521", "mps3_an547")
+    _KNOWN_QEMU_ZEPHYR_BOARDS = ["mps2_an521", "mps3_an547"]
+
+    # A list of all zephyr_board values which are known to launch using ARM FVP (this script configures
+    # Zephyr to use that launch method).
+    _KNOWN_FVP_ZEPHYR_BOARDS = ["mps3_an547"]
 
     @classmethod
-    def _is_qemu(cls, board: str) -> bool:
-        return "qemu" in board or board in cls._KNOWN_QEMU_ZEPHYR_BOARDS
+    def _is_fvp(cls, board, use_fvp):
+        assert not (

Review Comment:
   Could this assert be simplified by:
   
   ```
   if use_fvp:
       assert board in cls._KNOWN_FVP_ZEPHYR_BOARDS, "FVP can be used to emulate this board on Zephyr"
       return True
   
   return False
   ```
   ?



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   I think you can avoid str() cast since `API_SERVER_DIR` is already of `pathlib.Path()` type.
   
   Why it's necessary to `chmod` the `FVP_Corstone_SSE-300_Ethos-U55` bash script? It's already add as an executable to the repo.
   
   So, if all this holds, I think the code from like 631 to 637 can be simply something like:
   
   ```
   -            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")
   -            env["ARMFVP_BIN_PATH"] = os.path.realpath(env["ARMFVP_BIN_PATH"])
   -            st = os.stat(env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55")
   -            os.chmod(
   -                env["ARMFVP_BIN_PATH"] + "/FVP_Corstone_SSE-300_Ethos-U55",
   -                st.st_mode | stat.S_IEXEC,
   -            )
   +            env["ARMFVP_BIN_PATH"] = (API_SERVER_DIR / "fvp-hack").resolve()
   ```
   ?



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -344,6 +357,18 @@ def _get_nrf_device_args(options):
         type="str",
         help="Path to the CMSIS directory.",
     ),
+    server.ProjectOption(
+        "arm_fvp_path",
+        optional=["generate_project"],

Review Comment:
   I see  this options is accessed directly in open() (so used by `open_transport` when FVP is used), hence it seems a required option for `open_transport` Project API method? Could you please clarify its domain / use?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943942967


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -904,5 +982,192 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    MICROTVM_API_SERVER_INIT = "fvp_initialized"
+    MAKE_FAILED = "make_failed"
+    EOF = "eof"
+
+
+class BlockingStream:
+    """Reimplementation of Stream class from Iris with blocking semantics."""
+
+    def __init__(self):
+        self.q = queue.Queue()
+        self.unread = None
+
+    def read(self, n=-1, timeout_sec=None):
+        assert (
+            n != -1
+        ), "expect firmware to open stdin using raw mode, and therefore expect sized read requests"
+
+        data = b""
+        if self.unread:
+            data = data + self.unread
+            self.unread = None
+
+        while len(data) < n:
+            try:
+                # When there is some data to return, fetch as much as possible, then return what we can.
+                # When there is no data yet to return, block.
+                data += self.q.get(block=not len(data), timeout=timeout_sec)
+            except queue.Empty:
+                break
+
+        if len(data) > n:
+            self.unread = data[n:]
+            data = data[:n]
+
+        return data
+
+    readline = read
+
+    def write(self, data):
+        self.q.put(data)
+
+
+class ZephyrFvpTransport:
+    """A transport class that communicates with the ARM FVP via Iris server."""
+
+    def __init__(self, options):
+        self.options = options
+        self.proc = None
+        self._queue = queue.Queue()
+        self._import_iris()
+
+    def _import_iris(self):
+        # Location as seen in the FVP_Corstone_SSE-300_11.15_24 tar.
+        iris_lib_path = (
+            pathlib.Path(self.options["arm_fvp_path"]).parent.parent.parent
+            / "Iris"
+            / "Python"
+            / "iris"
+        )
+
+        sys.path.insert(0, str(iris_lib_path.parent))
+        try:
+            import iris.NetworkModelInitializer
+        finally:
+            sys.path.pop(0)
+
+        self._iris_lib = iris
+
+        def _convertStringToU64Array(strValue):
+            numBytes = len(strValue)
+            if numBytes == 0:
+                return []
+
+            numU64 = (numBytes + 7) // 8
+            # Extend the string ending with '\0', so that the string length is multiple of 8.
+            # E.g. 'hello' is extended to: 'hello'+\0\0\0
+            strExt = strValue.ljust(8 * numU64, b"\0")
+            # Convert the string to a list of uint64_t in little endian
+            return struct.unpack("<{}Q".format(numU64), strExt)
+
+        iris.iris.convertStringToU64Array = _convertStringToU64Array
+
+    def open(self):
+        args = ["ninja"]
+        if self.options.get("verbose"):
+            args.append("-v")
+        args.append("run")
+        env = dict(os.environ)
+        env["FVP_BIN_PATH"] = str(pathlib.Path(self.options["arm_fvp_path"]).parent)

Review Comment:
   regarding the second point, we can only use the `ARMFVP_BIN_PATH`. The other one was left-over code from my attempts to understand why tests were failing on CI addressed [here](https://github.com/apache/tvm/pull/12125#discussion_r943748705). I removed the `FVP_BIN_PATH`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943948149


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -904,5 +982,192 @@ def _wait_for_qemu(self):
             raise ValueError(f"{item} not expected.")
 
 
+class ZephyrFvpMakeResult(enum.Enum):
+    FVP_STARTED = "fvp_started"
+    MICROTVM_API_SERVER_INIT = "fvp_initialized"
+    MAKE_FAILED = "make_failed"
+    EOF = "eof"
+
+
+class BlockingStream:
+    """Reimplementation of Stream class from Iris with blocking semantics."""
+
+    def __init__(self):
+        self.q = queue.Queue()
+        self.unread = None
+
+    def read(self, n=-1, timeout_sec=None):
+        assert (
+            n != -1
+        ), "expect firmware to open stdin using raw mode, and therefore expect sized read requests"
+
+        data = b""
+        if self.unread:
+            data = data + self.unread
+            self.unread = None
+
+        while len(data) < n:
+            try:
+                # When there is some data to return, fetch as much as possible, then return what we can.
+                # When there is no data yet to return, block.
+                data += self.q.get(block=not len(data), timeout=timeout_sec)
+            except queue.Empty:
+                break
+
+        if len(data) > n:
+            self.unread = data[n:]
+            data = data[:n]
+
+        return data
+
+    readline = read
+
+    def write(self, data):
+        self.q.put(data)
+
+
+class ZephyrFvpTransport:
+    """A transport class that communicates with the ARM FVP via Iris server."""
+
+    def __init__(self, options):
+        self.options = options
+        self.proc = None
+        self._queue = queue.Queue()
+        self._import_iris()
+
+    def _import_iris(self):
+        # Location as seen in the FVP_Corstone_SSE-300_11.15_24 tar.
+        iris_lib_path = (
+            pathlib.Path(self.options["arm_fvp_path"]).parent.parent.parent
+            / "Iris"
+            / "Python"
+            / "iris"
+        )
+
+        sys.path.insert(0, str(iris_lib_path.parent))
+        try:
+            import iris.NetworkModelInitializer
+        finally:
+            sys.path.pop(0)
+
+        self._iris_lib = iris
+
+        def _convertStringToU64Array(strValue):
+            numBytes = len(strValue)
+            if numBytes == 0:
+                return []
+
+            numU64 = (numBytes + 7) // 8
+            # Extend the string ending with '\0', so that the string length is multiple of 8.
+            # E.g. 'hello' is extended to: 'hello'+\0\0\0
+            strExt = strValue.ljust(8 * numU64, b"\0")
+            # Convert the string to a list of uint64_t in little endian
+            return struct.unpack("<{}Q".format(numU64), strExt)
+
+        iris.iris.convertStringToU64Array = _convertStringToU64Array
+
+    def open(self):
+        args = ["ninja"]
+        if self.options.get("verbose"):
+            args.append("-v")
+        args.append("run")
+        env = dict(os.environ)
+        env["FVP_BIN_PATH"] = str(pathlib.Path(self.options["arm_fvp_path"]).parent)

Review Comment:
   regarding the first point,you are correct to say that the option is required in open, but only for the FVP projects. (we still need the `arm_fvp_path` in open transport in `_import_iris` function. ) As you suggested, I added an assert there to make it clear.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1214413854

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch merged pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
areusch merged PR #12125:
URL: https://github.com/apache/tvm/pull/12125


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1214644806

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r930227788


##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -218,11 +303,42 @@ void uart_irq_cb(const struct device* dev, void* user_data) {
   }
 }
 
+#ifdef FVP

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r931684026


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -1062,8 +1062,18 @@ def open(self):
         self._target.handle_semihost_io()
         self._target._stdout = BlockingStream()
         self._target._stdin = BlockingStream()
-        self._model.run(blocking=False, timeout=100)
-        self._wait_for_semihost_init()
+        for i in range(3):
+            try:
+                _LOG.info("Iteration %d", i)
+                self._model.run(blocking=False, timeout=100)
+                self._wait_for_semihost_init()
+                break
+            except self._iris_lib.error.IrisError:
+                continue
+            else:

Review Comment:
   Oh, thanks for the note!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r940627164


##########
apps/microtvm/zephyr/template_project/src/host_driven/semihost.h:
##########
@@ -0,0 +1,42 @@
+/*

Review Comment:
   done



##########
tests/scripts/task_python_microtvm.sh:
##########
@@ -54,4 +55,4 @@ python3 gallery/how_to/work_with_microtvm/micro_tflite.py
 python3 gallery/how_to/work_with_microtvm/micro_autotune.py
 python3 gallery/how_to/work_with_microtvm/micro_aot.py
 
-run_pytest ctypes python-relay-strategy-arm_cpu tests/python/relay/strategy/arm_cpu --enable-corstone300-tests
+run_pytest ctypes python-relay-strategy-arm_cpu tests/python/relay/strategy/arm_cpu --enable-corstone300-tests

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r943964049


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -578,49 +623,78 @@ def generate_project(self, model_library_format_path, standalone_crt_dir, projec
     def build(self, options):
         BUILD_DIR.mkdir()
 
-        check_call(["cmake", ".."], cwd=BUILD_DIR)
+        zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+        emu_platform = _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
+
+        env = dict(os.environ)
+        if self._is_fvp(zephyr_board, emu_platform == "armfvp"):
+            env["ARMFVP_BIN_PATH"] = str(API_SERVER_DIR / "fvp-hack")

Review Comment:
   @driazati Any clue why this file here: https://github.com/apache/tvm/blob/ebd98ce8215eb7499ead0d509457fd432eca7d32/apps/microtvm/zephyr/template_project/fvp-hack/FVP_Corstone_SSE-300_Ethos-U55  (which is indeed an executable) could turn into a non executable  when copied inside the CI container as @mkatanbaf said above? It's copied this way to the new location:
   
   https://github.com/apache/tvm/blob/ebd98ce8215eb7499ead0d509457fd432eca7d32/apps/microtvm/zephyr/template_project/microtvm_api_server.py#L561



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213309932

   @mkatanbaf Got it! Well, feels like déjà-vu to me (when using QEMU).  When you run locally in a loop did you use any container?  Anyways, I know how hard it can be. But how about the last error in `test_binary_add_with_non_4d_shapes` :
   
   https://ci.tlcpack.ai/blue/rest/organizations/jenkins/pipelines/tvm/branches/PR-12125/runs/56/nodes/432/steps/1490/log/?start=0
   
   It seems not related to that change, so I'm confused... 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213302544

   @gromero Thanks for re-triggering the tests. This has happened locally a few times before (out of many runs!), and I talked with @areusch about it. This is difficult to reproduce, I ran the tests in a loop overnight for the past few nights, and this issue didn't come up!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on a diff in pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12125:
URL: https://github.com/apache/tvm/pull/12125#discussion_r929445344


##########
apps/microtvm/zephyr/template_project/src/host_driven/main.c:
##########
@@ -218,11 +303,42 @@ void uart_irq_cb(const struct device* dev, void* user_data) {
   }
 }
 
+#ifdef FVP

Review Comment:
   yeah i think if possible let's rely on Zephyr for UART TX



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: Zephyr fvp support

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1212290085

   thanks, @mehrdadh. I tested this commit with microTVM hardware CI, and compared the errors against nightly build 316. No new error was introduced.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213074527

   @tvm-bot rerun 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
gromero commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213311462

   > There are some failing ethosu codegen tests on the rerun, but I don't think those are caused by this PR
   
   @mkatanbaf yeah I agree. So it means the CI is not "quite" stable ... and that other glitches do happen :( 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1213309154

   There are some failing ethosu codegen tests on the rerun, but I don't think those are caused by this PR


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1214526021

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mkatanbaf commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
mkatanbaf commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1215362563

   @tvm-bot rerun


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #12125: [microTVM] Zephyr: Add support for FVP

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #12125:
URL: https://github.com/apache/tvm/pull/12125#issuecomment-1220026288

   @gromero i think we should try it, and CI Monitoring rotation will catch any flakes a lot faster.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org