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 2020/10/17 04:16:16 UTC
[GitHub] [incubator-tvm] areusch opened a new pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
areusch opened a new pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703
This PR adds two Vagrantfiles:
- a µTVM base box in `tools/microtvm/base-box` intended to support general µTVM development. it includes all the dependencies necessary to build the Zephyr runtime and test it with attached hardware (I.e. use USB port forwarding). This means it includes cross-compilers for RISC-V, ARM, and x86, among others (see Zephyr SDK).
- a specialization of the base box which mounts the local `tvm` directory using Host-VM shared folders, builds your local copy of TVM inside the VM, then creates a poetry (Python) virtualenv containing all TVM and Zephyr dependencies. You can use this VM to test µTVM against real hardware, for example:
`tvm@microtvm:/Users/andrew/ws/tvm2$ TVM_LIBRARY_PATH=build-microtvm poetry run python3 tests/micro/qemu/test_zephyr.py --microtvm-platforms=stm32f746xx -s`
This PR also includes additional transports needed to talk to real hardware, specifically a pySerial-based RPC transport layer plus utilities to invoke GDB to debug e.g. runtime problems, bad operator implementations, and to help with porting to new architectures. Because µTVM aims to be platform-agnostic, µTVM assumes only that some shell command exists to launch GDB and connect to the SoC's debug port. Due to this constraint, an additional RPC server is included: `tvm.exec.microtvm_debug_shell`, which uses the event-driven RPC server to host the debugger in a dedicated shell, so that signals can be forwarded to the inferior GDB.
cc @tmoreau89 @tqchen @u99127 @tom-gall @liangfu @mshawcroft
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510371947
##########
File path: apps/microtvm-vm/base-box/setup-tvm-user.sh
##########
@@ -0,0 +1,39 @@
+#!/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.
+
+# Zephyr
+pip3 install --user -U west
+echo 'export PATH=$HOME/.local/bin:"$PATH"' >> ~/.profile
+source ~/.profile
+echo PATH=$PATH
+west init --mr v2.4.0 ~/zephyr
+cd ~/zephyr
+west update
+west zephyr-export
+
Review comment:
actually `west` is the Zephyr command-line tool, not a username.
adding other RTOSes may present a challenge. i'd advocate we cross that bridge when we get there--likely, if we need to maintain multiple python venv's, it would be something like we pip install west into a venv, then symlink west to the venv's bin. I don't know if that will fully work, but we'll have to see.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r511323349
##########
File path: apps/microtvm-vm/base-box/Vagrantfile
##########
@@ -0,0 +1,23 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ config.vm.box = "generic/ubuntu2004"
Review comment:
downgraded to 18.04
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch edited a comment on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch edited a comment on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715743438
@manupa-arm
> From a high-level the need for the Vagrant boxes arise from the need not being able to use serial COM ports of Windows via the docker image, right ? (ref : docker/for-win#1018).
correct. however, it's still a bit messy to configure serial ports, because the Zephyr udev rules need to be installed in the docker host/guest VM. further, they need to be individually forwarded with -v, so you need to prediect the /dev/ttyACM* names.
> Out of curiosity, have you thought about spinning a docker up , possibly extending ci_qemu, to include the dependencies of Zehpyr and use it inside the Vagrant (perhaps via Docker Provisioner ? ) -- therefore the Vagrant will provide just the thing the docker is missing which is the serial COM port exposure (in Windows) inside the box.
due to the aforementioned messiness, it seems better to just stick with one virtualization layer.
@manupa-arm @u99127
I've split out the `-mfpu` change, I agree that's fairly separate from this one.
now with respect to the rest of the PR--I see that we could theoretically split it, but I don't know it gets us much. because much of this is python code that deals with talking to real hardware, it's not simple to develop and maintain unit tests for it. we can certainly add some, but the real utility is in an end-to-end test. that's what this PR adds--the end to end test and the minimum infrastructure needed to support it. further, it proves the end-to-end test is, at least at the user script level, so similar to the QEMU test ran in the CI, that it can be parameterized.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510464859
##########
File path: apps/microtvm-vm/base-box/Vagrantfile.packer-template
##########
@@ -0,0 +1,45 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ # From hashicorp default template:
+ # https://github.com/hashicorp/packer/blob/master/builder/vagrant/step_create_vagrantfile.go#L23-L37
+
+ config.vm.define "source" do |source|
+ source.vm.box = "{{.SourceBox}}"
+ config.ssh.insert_key = {{.InsertKey}}
Review comment:
Ack. as long as it doesn't get baked into public source I guess that's ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-714421195
hey Andrew,
Thanks a lot for your reply.
>
>
> @u99127 some early replies.
>
> > I've spent sometime today attempting to review this PR. I would like to understand more about how this pushes things forward with the uTVM integration. Can you link this to an RFC that describes this for posterity and how this links with the overall plan for uTVM ?
> > sure, happy to write one. briefly: this makes it easy to use Zephyr, so it generally aligns with [Standalone µTVM roadmap](https://discuss.tvm.apache.org/t/rfc-tvm-standalone-tvm-roadmap/6987), though happy to admit an RFC with an added level of detail would help.
>
> > At the end of this PR getting merged, is the expectation that a user of TVM gets a shell like interface to interact with a board and be able to interactively run machine learning models on the board using Zephyr and uTVM (well, really autotuning at this point of time) ?
>
> correct. Zephyr has a lot of dependencies, and additionally, setting up the TVM virtualenv is not trivial now and is made especially hard by needing to install the additional zephyr dependencies. the VM solves this in a cross-platform way. it's needed (vs a docker container) because docker can't correctly virtualize any piece of attached hardware which can't be controlled solely with libusb (I.e. virtual com ports can't be used from docker containers).
>
> > This appears to mix multiple changes which could be staged into one giant PR.
> > I can see the following changes here .
> >
> > 1. New CI infrastructure with vagrant boxes,
>
> I don't believe we're adding any CI infrastructure here. The [previous change](https://github.com/apache/incubator-tvm/pull/6603) added QEMU support to the CI plus the current test. This PR proves that the test can be used with both QEMU and real hardware with a minor 2-line change (this is why this PR adds parameterization to `test_zephyr`).
>
> > 1. changes to existing uTVM transport server protocols
>
> No protocol level changes were made here, but I did add the `SerialTransport` implementation and a way to automatically launch GDB to debug the attached Cortex board. I think more documentation would make this clear. I can add a doc explaining how to use this change.
>
I think this is worth splitting out into its own PR since it seems independent ? The smaller the PR the easier it is for review.
> > 1. changes to llvm's target parsing which is really common between the Arm and AArch64 targets AFAIK and the behaviour of mfpu is very different between the 2. Thus that can be an indpendent PR in its own right.
>
> actually I can split this out. originally I was thinking that `test_zephyr` required floating point support, but it uses integers.
Please can we split it out. Unrelated changes in the same PR make review difficult and painful and mean things slip with review and is harder on the reviewer.
>
> > 4 ... I'm sure I've missed some changes relating to Zephyr or others in there.
>
> A couple of bugfixes:
>
> * if an Exception is raised while constructing the Session but after the transport has been initialized, de-initialize the transport
>
> * fix behavior of MicroSession when using it concurrently with an attached debugger. Specifically, if no retry timeout is specified, StartSession would still timeout after the retry timeout.
Can we please pull it out ? One PR per bug fix is really what I would suggest.
>
>
> > I'm not going to be able to review items 2, 3 and 4 on this list tonight and will be restraining myself to just looking at the vagrant bits and understand more about how this looks.
> > My understanding of the choice of vagrant is that this is useful as a different way of connecting to peripherals which is hard to manage in a docker container . Is that right ?
>
> Vagrant is mainly useful to codify the virtual machine configuration. you're right that we use the VM to connect to peripherals because docker isn't any good for that. there are 2 virtual machine configs stored: a "base box" which is stored [here](https://app.vagrantup.com/areusch/boxes/microtvm-staging) (we need to find a better home for it), and a "vm" which is what the user's supposed to use. the base box just needs to be updated like the CI docker containers: when dependencies change. the vm is built by every user by issuing:
> `$ vagrant up --provider={parallels,virtualbox,...}`
> in the `apps/microtvm-vm/vm` directory.
>
> this builds the user's TVM installation using the VM's gcc toolchain.
I'll need to read through this again but it won't be till later tonight.
>
> > With the vagrant box setup what is the thinking with respect to those developers or those developers using CI that runs already inside virtual machines. Would this naturally nest and work out of the box ? For instance if there as a developer wanting to develop uTVM on a GNU/Linux VM on a Windows laptop, how would this set up work for them ? Or indeed if one were to run CI systems that used openstack, would vagrant play nicely on those CI systems ?
> > Right now the VM doesn't have any relation to the CI. You could imagine a future where part of the CI is spinning up the VM and launching a Jenkins instance. Instead, we build a docker image with QEMU and Zephyr, but no utilities for connecting to peripherals.
>
> I believe it should be possible to launch the CI from inside the VM if you installed docker--we configure this now. Vagrant is supported on Windows and it should be possible to use it with VirtualBox to develop for µTVM. I haven't tested that, though. I don't know if the VM can nest on openstack, but as we don't plan to use it in CI yet, I don't think this adds any additional concerns to the problem of running the TVM CI on openstack. Let me know if I've misunderstood your question.
>
We do internally run our TVM CI on openstack or other VM's for development. So the deployment flow here would certainly need to be thought through. I'm not sure about other organizations and how they organize their CI but we've certainly tried to use the building blocks from upstream CI launching stuff off on internal workers that are either VM guests or machines.
@leandron should certainly look at this too.
> > A first pass through this to see what it looks like but I'm not terribly happy with my own understanding of this .
> > Finally , are there plans for a tutorial in the source base about how to actually use this since this PR seems to be short on any documentation about folks wanting to try it out. How can a developer try this out ?
>
> Yes, definitely. I have one nearly ready--I should just merge it with this PR. I'll iterate on that.
I think this should be split up into atleast 4 or 5 PRs to manage?
1. Vagrant box changes with the tutorial as I think this is the biggest item here.
2. GDB changes.
3. LLVM changes.
4. Zephyr changes
5. The 2 bug fixes you mention.
6. Anything else that I've missed.
regards
Ramana
>
> > regards
> > Ramana
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r508731936
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# 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.
+
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
Review comment:
I believe the goal here is to build the vmbox, like in the case of LLVM and other places, we download the pubkey from the source of truth, in this case from https://apt.kitware.com/
so that we don't have to maintain the pubkey in our own repo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-722478387
Thanks @areusch @u99127 @leandron @manupa-arm !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510463809
##########
File path: apps/microtvm-vm/base-box/setup-tvm-user.sh
##########
@@ -0,0 +1,39 @@
+#!/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.
+
+# Zephyr
+pip3 install --user -U west
+echo 'export PATH=$HOME/.local/bin:"$PATH"' >> ~/.profile
+source ~/.profile
+echo PATH=$PATH
+west init --mr v2.4.0 ~/zephyr
+cd ~/zephyr
+west update
+west zephyr-export
+
Review comment:
Apologies, it also looks like I might have misparsed the `pip3 install --user -U west' into thinking that it was specific for a `west' user , re-reading it again suggests my expectation there was a bit misplaced.
I suspect we agree on the point about it still being Zephyr specific :) So maybe it does make sense to make this Zephyr specific.
regards
Ramana
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-716838419
@u99127 @manupa-arm any further thoughts 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r509704644
##########
File path: apps/microtvm-vm/base-box/Vagrantfile.packer-template
##########
@@ -0,0 +1,45 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ # From hashicorp default template:
+ # https://github.com/hashicorp/packer/blob/master/builder/vagrant/step_create_vagrantfile.go#L23-L37
+
+ config.vm.define "source" do |source|
+ source.vm.box = "{{.SourceBox}}"
+ config.ssh.insert_key = {{.InsertKey}}
Review comment:
Where do users insert their ssh key and have you thought about security with this ? Is there a requirement to bake that into some image some where ?
##########
File path: apps/microtvm-vm/base-box/packer.json.template
##########
@@ -0,0 +1,43 @@
+# 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.
+{
+ "variables": {
+ "provider": null,
+ "version": null,
+ "access_token": null,
+ "skip_add": "false"
+ },
+ "builders": [
+ {
+ "type": "vagrant",
+ "output_dir": "output-vagrant-{{user `provider`}}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu2004",
+ "provider": "{{user `provider`}}",
+ "template": "Vagrantfile.packer-template",
+ "skip_add": "{{user `skip_add`}}"
+ }
+ ],
+ "post-processors": [
+ {
+ "type": "vagrant-cloud",
+ "box_tag": "areusch/microtvm-staging",
Review comment:
What is areusch/microtvm-staging ? Should we be merging this in ?
If we are putting this into main - I don't think this should have any information of this sort here.
##########
File path: python/tvm/target/target.py
##########
@@ -220,19 +220,24 @@ def intel_graphics(model="unknown", options=None):
return Target(" ".join(["opencl"] + opts))
-def micro(hardware="unknown", options=None):
+def micro(model="unknown", options=None):
Review comment:
Changes for target API ? Should this really be a separate PR ?
##########
File path: python/tvm/micro/debugger.py
##########
@@ -45,34 +56,102 @@ def stop(self):
"""Terminate the debugger."""
raise NotImplementedError()
+ def _run_on_terminate_callbacks(self):
+ for callback in self.on_terminate_callbacks:
+ try:
+ callback()
+ except Exception: # pylint: disable=broad-except
+ _LOG.warning("on_terminate_callback raised exception", exc_info=True)
+
class GdbDebugger(Debugger):
"""Handles launching, suspending signals, and potentially dealing with terminal issues."""
+ # Number of seconds to wait in stop() for a graceful shutdown. After this time has elapsed,
+ # the debugger is kill()'d.
+ _GRACEFUL_SHUTDOWN_TIMEOUT_SEC = 5.0
+
+ # The instance of GdbDebugger that's currently started.
+ _STARTED_INSTANCE = None
+
+ @classmethod
+ def _stop_all(cls):
+ if cls._STARTED_INSTANCE:
+ cls._STARTED_INSTANCE.stop()
+
+ def __init__(self):
+ super(GdbDebugger, self).__init__()
+ self._is_running = False
+ self._child_alive_lock = threading.RLock()
+ self._is_child_alive = False
+
@abc.abstractmethod
def popen_kwargs(self):
raise NotImplementedError()
- def _wait_restore_signal(self):
+ def _wait_for_child(self):
self.popen.wait()
- if not self.did_terminate.is_set():
- for callback in self.on_terminate_callbacks:
+ with self._child_alive_lock:
+ self._is_child_alive = True
+
+ @classmethod
+ def _sigint_handler(cls, signum, stack_frame): # pylint: disable=unused-argument
+ if cls._STARTED_INSTANCE is not None:
+ with cls._STARTED_INSTANCE._child_alive_lock:
+ exists = cls._STARTED_INSTANCE._is_child_alive
+ if exists:
try:
- callback()
- except Exception: # pylint: disable=broad-except
- logging.warn("on_terminate_callback raised exception", exc_info=True)
+ os.killpg(cls._STARTED_INSTANCE.child_pgid, signal.SIGINT)
+ return
+ except ProcessLookupError:
+ pass
+
+ raise Exception()
def start(self):
+ assert not self._is_running
+ assert not self._STARTED_INSTANCE
+
kwargs = self.popen_kwargs()
- self.did_terminate = threading.Event()
- self.old_signal = signal.signal(signal.SIGINT, signal.SIG_IGN)
+ self.did_start_new_session = kwargs.setdefault("start_new_session", True)
+
+ self.old_termios = termios.tcgetattr(sys.stdin.fileno())
self.popen = subprocess.Popen(**kwargs)
- threading.Thread(target=self._wait_restore_signal).start()
+ self._is_running = True
+ self.__class__._STARTED_INSTANCE = self
+ try:
+ self.child_pgid = os.getpgid(self.popen.pid)
+ except Exception:
+ self.stop()
+ raise
+ with self._child_alive_lock:
+ self._is_child_alive = True
+ self.old_sigint_handler = signal.signal(signal.SIGINT, self._sigint_handler)
+ t = threading.Thread(target=self._wait_for_child)
+ t.daemon = True
+ t.start()
def stop(self):
- self.did_terminate.set()
- self.popen.terminate()
- signal.signal(signal.SIGINT, self.old_signal)
+ if not self._is_running:
+ return
+
+ signal.signal(signal.SIGINT, self.old_sigint_handler)
+ termios.tcsetattr(sys.stdin.fileno(), termios.TCSAFLUSH, self.old_termios)
+
+ try:
+ children = psutil.Process(self.popen.pid).children(recursive=True)
+ for c in children:
+ c.terminate()
+ _, alive = psutil.wait_procs(children, timeout=self._GRACEFUL_SHUTDOWN_TIMEOUT_SEC)
+ for a in alive:
+ a.kill()
+ finally:
+ self.__class__._STARTED_INSTANCE = None
+ self._is_running = False
+ self._run_on_terminate_callbacks()
+
+
+atexit.register(GdbDebugger._stop_all)
Review comment:
More comments in this file would be useful.
##########
File path: apps/microtvm-vm/base-box/Vagrantfile
##########
@@ -0,0 +1,23 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ config.vm.box = "generic/ubuntu2004"
Review comment:
The rest of our CI requires 18.04 : why 20.04 here ?
##########
File path: apps/microtvm-vm/base-box/release-version.sh
##########
@@ -0,0 +1,53 @@
+#!/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.
+
Review comment:
Need a top level comment as to why this file exists. What's it's claim to fame, what's it supposed to be used for ?
##########
File path: src/runtime/rpc/rpc_endpoint.cc
##########
@@ -700,6 +700,14 @@ void RPCEndpoint::Init() {
});
}
+/*!
+ * \brief Create a new RPCEndpoint instance.
+ * \param channel RPCChannel used to communicate
+ * \param name
+ * \param The remote key reported during protocol initialization, or "%toinit" if the RPCEndpoint
+ * should handle this phase of the protocol for you. Some servers may prefer to access parts of
+ * the key to modify their behavior.
+ */
Review comment:
Related to the RPC mechanism, why isn't this an independent PR ?
##########
File path: apps/microtvm-vm/base-box/setup-tvm-user.sh
##########
@@ -0,0 +1,39 @@
+#!/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.
+
+# Zephyr
+pip3 install --user -U west
+echo 'export PATH=$HOME/.local/bin:"$PATH"' >> ~/.profile
+source ~/.profile
+echo PATH=$PATH
+west init --mr v2.4.0 ~/zephyr
+cd ~/zephyr
+west update
+west zephyr-export
+
Review comment:
This appears to be zephyr specific and not uTVM specific. Is there really an expectation that the vagrant box will have a user named west ?
What happens when we need other RTOS's ?
##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -40,6 +38,7 @@
from ..transport import debug
from ..transport import file_descriptor
+from ..transport import serial
Review comment:
why are changes in this file necessary with this PR , can this be split out into it's own Zephyr specific PR ?
##########
File path: apps/microtvm-vm/vm/rebuild-tvm.sh
##########
@@ -0,0 +1,33 @@
+#!/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 -e
+
+cd "$(dirname $0)"
+cd "$(git rev-parse --show-toplevel)"
+BUILD_DIR=build-microtvm
+
+if [ ! -e "${BUILD_DIR}" ]; then
+ mkdir "${BUILD_DIR}"
+fi
+cp cmake/config.cmake "${BUILD_DIR}"
+cd "${BUILD_DIR}"
+sed -i 's/USE_MICRO OFF/USE_MICRO ON/' config.cmake
+sed -i 's/USE_LLVM OFF/USE_LLVM ON/' config.cmake
Review comment:
This forces the use of uTVM with llvm only. how does the user choose between using llvm or the C backend or turning this off entirely ?
##########
File path: apps/microtvm-vm/base-box/packer.json.template
##########
@@ -0,0 +1,43 @@
+# 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.
+{
Review comment:
Please link to a schema at the top level or something that is a reference to this json schema .
What do these keys mean ? How much of an expert should a uTVM developer be to configure this or use it as is ?
##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -60,10 +61,11 @@ void InitializeLLVM() {
}
void ParseLLVMTargetOptions(const Target& target, std::string* triple, std::string* mcpu,
- std::string* mattr, llvm::TargetOptions* options) {
Review comment:
Please pull out the llvm target changes to a separate 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r507905398
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
my thinking is that this is more like a `docker` directory than an `apps` subdir. happy to move wherever, though. what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510463809
##########
File path: apps/microtvm-vm/base-box/setup-tvm-user.sh
##########
@@ -0,0 +1,39 @@
+#!/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.
+
+# Zephyr
+pip3 install --user -U west
+echo 'export PATH=$HOME/.local/bin:"$PATH"' >> ~/.profile
+source ~/.profile
+echo PATH=$PATH
+west init --mr v2.4.0 ~/zephyr
+cd ~/zephyr
+west update
+west zephyr-export
+
Review comment:
Apologies, it also looks like I might have misparsed the `pip3 install --user -U west' into thinking that it was specific for a `west' user , re-reading it again suggests my expectation there was a bit misplaced.
regards
Ramana
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] leandron commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r512854762
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
+import argparse
+import json
+import logging
+import os
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+
+_LOG = logging.getLogger(__name__)
+
+
+THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+
+
+# List of vagrant providers supported by this tool
+ALL_PROVIDERS = (
+ "parallels",
+ "virtualbox",
+)
+
+
+def parse_virtualbox_devices():
+ output = subprocess.check_output(["VBoxManage", "list", "usbhost"], encoding="utf-8")
+ devices = []
+ current_dev = {}
+ for line in output.split("\n"):
+ if not line.strip():
+ if current_dev:
+ if "VendorId" in current_dev and "ProductId" in current_dev:
+ devices.append(current_dev)
+ current_dev = {}
+
+ continue
+
+ key, value = line.split(":", 1)
+ value = value.lstrip(" ")
+ current_dev[key] = value
+
+ if current_dev:
+ devices.append(current_dev)
+ return devices
+
+
+VIRTUALBOX_VID_PID_RE = re.compile(r"0x([0-9A-Fa-f]{4}).*")
+
+
+def attach_virtualbox(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = parse_virtualbox_devices()
+ for dev in usb_devices:
+ m = VIRTUALBOX_VID_PID_RE.match(dev["VendorId"])
+ if not m:
+ _LOG.warning("Malformed VendorId: %s", dev["VendorId"])
+ continue
+
+ dev_vid_hex = m.group(1).lower()
+
+ m = VIRTUALBOX_VID_PID_RE.match(dev["ProductId"])
+ if not m:
+ _LOG.warning("Malformed ProductId: %s", dev["ProductId"])
+ continue
+
+ dev_pid_hex = m.group(1).lower()
+
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev["SerialNumber"])
+ ):
+ rule_args = [
+ "VBoxManage",
+ "usbfilter",
+ "add",
+ "0",
+ "--action",
+ "hold",
+ "--name",
+ "test device",
+ "--target",
+ uuid,
+ "--vendorid",
+ vid_hex,
+ "--productid",
+ pid_hex,
+ ]
+ if serial is not None:
+ rule_args.extend(["--serialnumber", serial])
+ subprocess.check_call(rule_args)
+ subprocess.check_call(["VBoxManage", "controlvm", uuid, "usbattach", dev["UUID"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+def attach_parallels(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = json.loads(
+ subprocess.check_output(["prlsrvctl", "usb", "list", "-j"], encoding="utf-8")
+ )
+ for dev in usb_devices:
+ _, dev_vid_hex, dev_pid_hex, _, _, dev_serial = dev["System name"].split("|")
+ dev_vid_hex = dev_vid_hex.lower()
+ dev_pid_hex = dev_pid_hex.lower()
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev_serial)
+ ):
+ subprocess.check_call(["prlsrvctl", "usb", "set", dev["Name"], uuid])
+ subprocess.check_call(["prlctl", "set", uuid, "--device-connect", dev["Name"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+ATTACH_USB_DEVICE = {
+ "parallels": attach_parallels,
+ "virtualbox": attach_virtualbox,
+}
+
+
+def generate_packer_config(file_path, providers):
+ builders = []
+ for provider_name in providers:
+ builders.append(
+ {
+ "type": "vagrant",
+ "output_dir": f"output-packer-{provider_name}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu1804",
+ "provider": provider_name,
+ "template": "Vagrantfile.packer-template",
+ }
+ )
+
+ with open(file_path, "w") as f:
+ json.dump(
+ {
+ "builders": builders,
+ },
+ f,
+ sort_keys=True,
+ indent=2,
+ )
+
+
+def build_command(args):
+ generate_packer_config(
+ os.path.join(THIS_DIR, args.platform, "base-box", "packer.json"),
+ args.provider.split(",") or ALL_PROVIDERS,
+ )
+ subprocess.check_call(
+ ["packer", "build", "packer.json"], cwd=os.path.join(THIS_DIR, args.platform, "base-box")
+ )
+
+
+REQUIRED_TEST_CONFIG_KEYS = {
+ "vid_hex": str,
+ "pid_hex": str,
+ "test_cmd": list,
+}
+
+
+VM_BOX_RE = re.compile(r'(.*\.vm\.box) = "(.*)"')
+
+
+# Paths, relative to the platform box directory, which will not be copied to release-test dir.
+SKIP_COPY_PATHS = [".vagrant", "base-box"]
+
+
+def test_command(args):
+ user_box_dir = os.path.join(THIS_DIR, args.platform)
+ base_box_dir = os.path.join(THIS_DIR, args.platform, "base-box")
+ test_config_file = os.path.join(base_box_dir, "test-config.json")
+ with open(test_config_file) as f:
+ test_config = json.load(f)
+ for key, expected_type in REQUIRED_TEST_CONFIG_KEYS.items():
+ assert key in test_config and isinstance(
+ test_config[key], expected_type
+ ), f"Expected key {key} of type {expected_type} in {test_config_file}: {test_config!r}"
+
+ test_config["vid_hex"] = test_config["vid_hex"].lower()
+ test_config["pid_hex"] = test_config["pid_hex"].lower()
+
+ providers = args.provider.split(",")
+ provider_passed = {p: False for p in providers}
+
+ release_test_dir = os.path.join(THIS_DIR, "release-test")
+
+ for provider_name in providers:
+ if os.path.exists(release_test_dir):
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ for dirpath, _, filenames in os.walk(user_box_dir):
+ rel_path = os.path.relpath(dirpath, user_box_dir)
+ if any(
+ rel_path == scp or rel_path.startswith(f"{scp}{os.path.sep}")
+ for scp in SKIP_COPY_PATHS
+ ):
+ continue
+
+ dest_dir = os.path.join(release_test_dir, rel_path)
+ os.makedirs(dest_dir)
+ for filename in filenames:
+ shutil.copy2(os.path.join(dirpath, filename), os.path.join(dest_dir, filename))
+
+ release_test_vagrantfile = os.path.join(release_test_dir, "Vagrantfile")
+ with open(release_test_vagrantfile) as f:
+ lines = list(f)
+
+ found_box_line = False
+ with open(release_test_vagrantfile, "w") as f:
+ for line in lines:
+ m = VM_BOX_RE.match(line)
+ if not m:
+ f.write(line)
+ continue
+
+ box_package = os.path.join(
+ base_box_dir, f"output-packer-{provider_name}", "package.box"
+ )
+ f.write(f'{m.group(1)} = "{os.path.relpath(box_package, release_test_dir)}"\n')
+ found_box_line = True
+
+ if not found_box_line:
+ _LOG.error(
+ "testing provider %s: couldn't find config.box.vm = line in Vagrantfile; unable to test",
+ provider_name,
+ )
+ continue
+
+ subprocess.check_call(
+ ["vagrant", "up", f"--provider={provider_name}"], cwd=release_test_dir
+ )
+ try:
+ with open(
+ os.path.join(
+ release_test_dir, ".vagrant", "machines", "default", provider_name, "id"
+ )
+ ) as f:
+ machine_uuid = f.read()
+ ATTACH_USB_DEVICE[provider_name](
+ machine_uuid,
+ vid_hex=test_config["vid_hex"],
+ pid_hex=test_config["pid_hex"],
+ serial=args.test_device_serial,
+ )
+ tvm_home = os.path.realpath(os.path.join(THIS_DIR, "..", "..", ".."))
+
+ def _quote_cmd(cmd):
+ return " ".join(shlex.quote(a) for a in cmd)
+
+ test_cmd = _quote_cmd(["cd", tvm_home]) + " && " + _quote_cmd(test_config["test_cmd"])
+ subprocess.check_call(
+ ["vagrant", "ssh", "-c", f"bash -ec '{test_cmd}'"], cwd=release_test_dir
+ )
+ provider_passed[provider_name] = True
+
+ finally:
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ if not all(provider_passed[p] for p in provider_passed.keys()):
+ sys.exit(
+ "some providers failed release test: "
+ + ",".join(name for name, passed in provider_passed if not passed)
+ )
+
+
+def release_command(args):
+ # subprocess.check_call(["vagrant", "cloud", "version", "create", f"tlcpack/microtvm-{args.platform}", args.version])
+ if not args.version:
+ sys.exit(f"--version must be specified")
+
+ for provider_name in args.provider.split(","):
+ subprocess.check_call(
+ [
+ "vagrant",
+ "cloud",
+ "publish",
+ "-f",
+ f"tlcpack/microtvm-{args.platform}",
+ args.version,
+ provider_name,
+ os.path.join(
+ THIS_DIR,
+ args.platform,
+ "base-box",
+ f"output-packer-{provider_name}/package.box",
+ ),
+ ]
+ )
+
+
+ALL_COMMANDS = {
+ "build": build_command,
+ "test": test_command,
+ "release": release_command,
+}
+
+
+def parse_args():
+ parser = argparse.ArgumentParser(
+ description="Automates building, testing, and releasing a base box"
+ )
+ parser.add_argument(
+ "command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action to perform."
+ )
+ parser.add_argument(
+ "platform",
+ help="Name of the platform VM to act on. Must be a sub-directory of this directory.",
+ )
+ parser.add_argument(
+ "--provider",
+ help="Name of the provider or providers to act on; if not specified, act on all",
+ )
+ parser.add_argument(
+ "--test-device-serial", help="If given, attach the test device with this USB serial number"
+ )
+ parser.add_argument("--version", help="Version to release. Must be specified with release.")
Review comment:
Maybe `--release`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715743438
@manupa-arm
> From a high-level the need for the Vagrant boxes arise from the need not being able to use serial COM ports of Windows via the docker image, right ? (ref : docker/for-win#1018).
correct. however, it's still a bit messy to configure serial ports, because the Zephyr udev rules need to be installed in the docker host/guest VM. further, they need to be individually forwarded with -v, so you need to prediect the /dev/ttyACM* names.
> Out of curiosity, have you thought about spinning a docker up , possibly extending ci_qemu, to include the dependencies of Zehpyr and use it inside the Vagrant (perhaps via Docker Provisioner ? ) -- therefore the Vagrant will provide just the thing the docker is missing which is the serial COM port exposure (in Windows) inside the box.
due to the aforementioned messiness, it seems better to just stick with one virtualization layer.
@manupa-arm @u99127
now with respect to the rest of the PR--I see that we could theoretically split it, but I don't know it gets us much. because much of this is python code that deals with talking to real hardware, it's not simple to develop and maintain unit tests for it. we can certainly add some, but the real utility is in an end-to-end test. that's what this PR adds--the end to end test and the minimum infrastructure needed to support it. further, it proves the end-to-end test is, at least at the user script level, so similar to the QEMU test ran in the CI, that it can be parameterized.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715921651
Thansks @u99127 @manupa-arm for reviewing, please see if the recent set of changes addressed the concern of general code path.
I agree with @areusch that given this is initial scafolding of a feature, we may not have to split the PR into smaller chunks to keep things simple. If there are future patches against the same set of the feature, then it makes sense to have a clear PR that is unit-tested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510374434
##########
File path: src/runtime/rpc/rpc_endpoint.cc
##########
@@ -700,6 +700,14 @@ void RPCEndpoint::Init() {
});
}
+/*!
+ * \brief Create a new RPCEndpoint instance.
+ * \param channel RPCChannel used to communicate
+ * \param name
+ * \param The remote key reported during protocol initialization, or "%toinit" if the RPCEndpoint
+ * should handle this phase of the protocol for you. Some servers may prefer to access parts of
+ * the key to modify their behavior.
+ */
Review comment:
i'm just adding documentation where it was helpful as I was reading the code. happy to split it out; the regression takes long.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r506937104
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
consider move to apps as we use that folder for conventional misc tools
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# 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.
+
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
Review comment:
can we download the public key somewhere instead of putting in in?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510372998
##########
File path: python/tvm/micro/contrib/zephyr.py
##########
@@ -40,6 +38,7 @@
from ..transport import debug
from ..transport import file_descriptor
+from ..transport import serial
Review comment:
Zephyr support was previously merged in #6603. this file adds a serial-specific transport, which is useful only when running Zephyr against real hardware. the VM "enables" this in TVM by providing a standardized developer environment. it's still possible to use this outside the VM, too, but I think for the purposes of reproducing bugs, we will prefer to use the VM as a reference where possible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510373932
##########
File path: python/tvm/target/target.py
##########
@@ -220,19 +220,24 @@ def intel_graphics(model="unknown", options=None):
return Target(" ".join(["opencl"] + opts))
-def micro(hardware="unknown", options=None):
+def micro(model="unknown", options=None):
Review comment:
here i'm really just changing the name of an argument. most people pass this arg as a positional arg, so I don't know that there is much user-visible change. if you really want me to move it outside this change, I can, but jenkins takes 4-6 hours these days and we'd have to write a test here to get it to fail with this change.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510365042
##########
File path: apps/microtvm-vm/base-box/Vagrantfile.packer-template
##########
@@ -0,0 +1,45 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ # From hashicorp default template:
+ # https://github.com/hashicorp/packer/blob/master/builder/vagrant/step_create_vagrantfile.go#L23-L37
+
+ config.vm.define "source" do |source|
+ source.vm.box = "{{.SourceBox}}"
+ config.ssh.insert_key = {{.InsertKey}}
Review comment:
this is actually not the user's ssh key--it's vagrant/packer's, and it's used to run scripts to provision the VM. this is pretty standard when it comes to scripting VM setup, and further these VMs are intended for local development use only with NAT networking, so I think the risk is minimal. it is required to configure this key, however.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r516959904
##########
File path: apps/microtvm/reference-vm/zephyr/base-box/setup.sh
##########
@@ -0,0 +1,99 @@
+#!/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 -e
+
+sudo apt update
+sudo apt install -y build-essential
+sudo apt-get --purge remove modemmanager # required to access serial ports.
+
+# Zephyr
+wget --no-verbose https://apt.kitware.com/keys/kitware-archive-latest.asc
+sudo apt-key add kitware-archive-latest.asc
+sudo apt-add-repository 'deb https://apt.kitware.com/ubuntu/ bionic main'
+sudo apt update
+sudo apt install -y --no-install-recommends git cmake ninja-build gperf \
+ ccache dfu-util device-tree-compiler wget \
+ python3-dev python3-pip python3-setuptools python3-tk python3-wheel xz-utils file \
+ make gcc gcc-multilib g++-multilib libsdl2-dev
+
+# Avahi, so that ssh microtvm works.
+# apt install -y avahi-daemon
+
+OLD_HOSTNAME=$(hostname)
+sudo hostnamectl set-hostname microtvm
+sudo sed -i.bak "s/${OLD_HOSTNAME}/microtvm.localdomain/g" /etc/hosts
+
+# Poetry deps
+sudo apt install -y python3-venv
+
+# TVM deps
+sudo apt install -y llvm
+
+# ONNX deps
+sudo apt install -y protobuf-compiler libprotoc-dev
+
+# nrfjprog
+cd ~
+mkdir -p nrfjprog
+wget --no-verbose -O nRFCommandLineTools1090Linuxamd64.tar.gz https://www.nordicsemi.com/-/media/Software-and-other-downloads/Desktop-software/nRF-command-line-tools/sw/Versions-10-x-x/10-9-0/nRFCommandLineTools1090Linuxamd64tar.gz
Review comment:
great suggestion! i've done that and we saved about 500MB.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r518159388
##########
File path: apps/microtvm/reference-vm/zephyr/pyproject.toml
##########
@@ -0,0 +1,140 @@
+# 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.
+
+[tool.black]
+line-length = 100
+target-version = ['py36']
+include = '(\.pyi?$)'
+exclude = '''
+
+(
+ /(
+ \.github
+ | \.tvm
+ | \.tvm_test_data
+ | \.vscode
+ | \.venv
+ | 3rdparty
+ | build\/
+ | cmake\/
+ | conda\/
+ | docker\/
+ | docs\/
+ | golang\/
+ | include\/
+ | jvm\/
+ | licenses\/
+ | nnvm\/
+ | rust\/
+ | src\/
+ | vta\/
+ | web\/
+ )/
+)
+'''
+[tool.poetry]
+name = "incubator-tvm"
+version = "0.1.0"
+description = ""
+authors = ["Your Name <yo...@example.com>"]
+packages = [
+ { include = "tvm", from = "python" },
+]
+
+[tool.poetry.dependencies]
+attrs = "^19"
+decorator = "^4.4"
+numpy = "~1.19"
+psutil = "^5"
+scipy = "^1.4"
+python = "^3.6"
+tornado = "^6"
+typed_ast = "^1.4"
Review comment:
@manupa-arm I definitely agree with you that the approach used here should mirror the one used in the Consolidating Python Dependencies RFC. I guess, in a perfect world i agree it would make sense to introduce `requirements.txt` here now, and `pypoetry.toml` later. but, it's not like there exists a `requirements.txt` to copy now, and this PR has been out for weeks now. i'd rather just merge this change now, and treat these two tasks as separate efforts. after we finish addressing the RFC, I will align the approach here with the one taken there. we still have not yet resolved whether to depend on poetry as a whole project yet; however, the VM, by contrast to the project, prefers to specify dependencies with more version constraints in the interest of maintaining a working build.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-722466586
@manupa-arm i'm not sure which requirement.txt you're referring to--we have not yet created any requirements.txt in spirit of the RFC. i'd prefer to just merge this change and treat this PR as separate from the RFC since it was released before I released the RFC. i think it's better to merge what we have now to make progress and then align the two after the RFC is implemented.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715291366
I also concur @u99127 that this PR needs breakdown.
From a high-level the need for the Vagrant boxes arise from the need not being able to use serial COM ports of Windows via the docker image, right ? (ref : https://github.com/docker/for-win/issues/1018).
Thus, the reason that I think that we need the Vagrant is to enable autotvm in which we perform compilation and running together.
Out of curiosity, have you thought about spinning a docker up , possibly extending ci_qemu, to include the dependencies of Zehpyr and use it inside the Vagrant (perhaps via Docker Provisioner ? ) -- therefore the Vagrant will provide just the thing the docker is missing which is the serial COM port exposure (in Windows) inside the box.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-714207644
@u99127 some early replies.
> I've spent sometime today attempting to review this PR. I would like to understand more about how this pushes things forward with the uTVM integration. Can you link this to an RFC that describes this for posterity and how this links with the overall plan for uTVM ?
sure, happy to write one. briefly: this makes it easy to use Zephyr, so it generally aligns with [Standalone µTVM roadmap](https://discuss.tvm.apache.org/t/rfc-tvm-standalone-tvm-roadmap/6987), though happy to admit an RFC with an added level of detail would help.
> At the end of this PR getting merged, is the expectation that a user of TVM gets a shell like interface to interact with a board and be able to interactively run machine learning models on the board using Zephyr and uTVM (well, really autotuning at this point of time) ?
correct. Zephyr has a lot of dependencies, and additionally, setting up the TVM virtualenv is not trivial now and is made especially hard by needing to install the additional zephyr dependencies. the VM solves this in a cross-platform way. it's needed (vs a docker container) because docker can't correctly virtualize any piece of attached hardware which can't be controlled solely with libusb (I.e. virtual com ports can't be used from docker containers).
> This appears to mix multiple changes which could be staged into one giant PR.
>
> I can see the following changes here .
>
> 1. New CI infrastructure with vagrant boxes,
I don't believe we're adding any CI infrastructure here. The [previous change](https://github.com/apache/incubator-tvm/pull/6603) added QEMU support to the CI plus the current test. This PR proves that the test can be used with both QEMU and real hardware with a minor 2-line change (this is why this PR adds parameterization to `test_zephyr`).
> 2. changes to existing uTVM transport server protocols
No protocol level changes were made here, but I did add the `SerialTransport` implementation and a way to automatically launch GDB to debug the attached Cortex board. I think more documentation would make this clear. I can add a doc explaining how to use this change.
> 3. changes to llvm's target parsing which is really common between the Arm and AArch64 targets AFAIK and the behaviour of mfpu is very different between the 2. Thus that can be an indpendent PR in its own right.
actually I can split this out. originally I was thinking that `test_zephyr` required floating point support, but it uses integers.
> 4 ... I'm sure I've missed some changes relating to Zephyr or others in there.
A couple of bugfixes:
- if an Exception is raised while constructing the Session but after the transport has been initialized, de-initialize the transport
- fix behavior of MicroSession when using it concurrently with an attached debugger. Specifically, if no retry timeout is specified, StartSession would still timeout after the retry timeout.
>
> I'm not going to be able to review items 2, 3 and 4 on this list tonight and will be restraining myself to just looking at the vagrant bits and understand more about how this looks.
>
> My understanding of the choice of vagrant is that this is useful as a different way of connecting to peripherals which is hard to manage in a docker container . Is that right ?
Vagrant is mainly useful to codify the virtual machine configuration. you're right that we use the VM to connect to peripherals because docker isn't any good for that. there are 2 virtual machine configs stored: a "base box" which is stored [here](https://app.vagrantup.com/areusch/boxes/microtvm-staging) (we need to find a better home for it), and a "vm" which is what the user's supposed to use. the base box just needs to be updated like the CI docker containers: when dependencies change. the vm is built by every user by issuing:
`$ vagrant up --provider={parallels,virtualbox,...}`
in the `apps/microtvm-vm/vm` directory.
this builds the user's TVM installation using the VM's gcc toolchain.
>
> With the vagrant box setup what is the thinking with respect to those developers or those developers using CI that runs already inside virtual machines. Would this naturally nest and work out of the box ? For instance if there as a developer wanting to develop uTVM on a GNU/Linux VM on a Windows laptop, how would this set up work for them ? Or indeed if one were to run CI systems that used openstack, would vagrant play nicely on those CI systems ?
Right now the VM doesn't have any relation to the CI. You could imagine a future where part of the CI is spinning up the VM and launching a Jenkins instance. Instead, we build a docker image with QEMU and Zephyr, but no utilities for connecting to peripherals.
I believe it should be possible to launch the CI from inside the VM if you installed docker--we configure this now. Vagrant is supported on Windows and it should be possible to use it with VirtualBox to develop for µTVM. I haven't tested that, though. I don't know if the VM can nest on openstack, but as we don't plan to use it in CI yet, I don't think this adds any additional concerns to the problem of running the TVM CI on openstack. Let me know if I've misunderstood your question.
>
> A first pass through this to see what it looks like but I'm not terribly happy with my own understanding of this .
>
> Finally , are there plans for a tutorial in the source base about how to actually use this since this PR seems to be short on any documentation about folks wanting to try it out. How can a developer try this out ?
Yes, definitely. I have one nearly ready--I should just merge it with this PR. I'll iterate on that.
>
> regards
> Ramana
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-710744702
also, vagrant boxes currently hosted at: https://app.vagrantup.com/areusch/boxes/microtvm-staging
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm edited a comment on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715291366
I also agree with @u99127 that this PR needs breakdown.
From a high-level the need for the Vagrant boxes arise from the need not being able to use serial COM ports of Windows via the docker image, right ? (ref : https://github.com/docker/for-win/issues/1018).
Thus, the reason that I think that we need the Vagrant is to enable autotvm in which we perform compilation and running together.
Out of curiosity, have you thought about spinning a docker up , possibly extending ci_qemu, to include the dependencies of Zehpyr and use it inside the Vagrant (perhaps via Docker Provisioner ? ) -- therefore the Vagrant will provide just the thing the docker is missing which is the serial COM port exposure (in Windows) inside the box.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] leandron commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-717369439
> > I believe it should be possible to launch the CI from inside the VM if you installed docker--we configure this now. Vagrant is supported on Windows and it should be possible to use it with VirtualBox to develop for µTVM. I haven't tested that, though. I don't know if the VM can nest on openstack, but as we don't plan to use it in CI yet, I don't think this adds any additional concerns to the problem of running the TVM CI on openstack. Let me know if I've misunderstood your question.
>
> We do internally run our TVM CI on openstack or other VM's for development. So the deployment flow here would certainly need to be thought through. I'm not sure about other organizations and how they organize their CI but we've certainly tried to use the building blocks from upstream CI launching stuff off on internal workers that are either VM guests or machines.
>
> @leandron should certainly look at this too.
@u99127 now that I had a chance to look into @areusch's patch, I don't think this will impact directly our internal CI for now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-722098764
@leandron please take a look when you have a minute and explicitly approve if you're good w/ this change
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] leandron commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r512553762
##########
File path: apps/microtvm/reference-vm/zephyr/pyproject.toml
##########
@@ -0,0 +1,140 @@
+# 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.
+
+[tool.black]
+line-length = 100
+target-version = ['py36']
+include = '(\.pyi?$)'
+exclude = '''
+
+(
+ /(
+ \.github
+ | \.tvm
+ | \.tvm_test_data
+ | \.vscode
+ | \.venv
+ | 3rdparty
+ | build\/
+ | cmake\/
+ | conda\/
+ | docker\/
+ | docs\/
+ | golang\/
+ | include\/
+ | jvm\/
+ | licenses\/
+ | nnvm\/
+ | rust\/
+ | src\/
+ | vta\/
+ | web\/
+ )/
+)
+'''
+[tool.poetry]
+name = "incubator-tvm"
+version = "0.1.0"
+description = ""
+authors = ["Your Name <yo...@example.com>"]
+packages = [
+ { include = "tvm", from = "python" },
+]
+
+[tool.poetry.dependencies]
+attrs = "^19"
+decorator = "^4.4"
+numpy = "~1.19"
+psutil = "^5"
+scipy = "^1.4"
+python = "^3.6"
+tornado = "^6"
+typed_ast = "^1.4"
Review comment:
Many dependencies listed here overlap with the ones described in TVM's `setup.py`, can we try to install TVM in a way that we get the dependencies from there, rather than duplicating them here? I think it would reduce a lot the maintenance needed to keep this working over time.
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
+import argparse
+import json
+import logging
+import os
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+
+_LOG = logging.getLogger(__name__)
+
+
+THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+
+
+# List of vagrant providers supported by this tool
+ALL_PROVIDERS = (
+ "parallels",
+ "virtualbox",
+)
+
+
+def parse_virtualbox_devices():
+ output = subprocess.check_output(["VBoxManage", "list", "usbhost"], encoding="utf-8")
+ devices = []
+ current_dev = {}
+ for line in output.split("\n"):
+ if not line.strip():
+ if current_dev:
+ if "VendorId" in current_dev and "ProductId" in current_dev:
+ devices.append(current_dev)
+ current_dev = {}
+
+ continue
+
+ key, value = line.split(":", 1)
+ value = value.lstrip(" ")
+ current_dev[key] = value
+
+ if current_dev:
+ devices.append(current_dev)
+ return devices
+
+
+VIRTUALBOX_VID_PID_RE = re.compile(r"0x([0-9A-Fa-f]{4}).*")
+
+
+def attach_virtualbox(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = parse_virtualbox_devices()
+ for dev in usb_devices:
+ m = VIRTUALBOX_VID_PID_RE.match(dev["VendorId"])
+ if not m:
+ _LOG.warning("Malformed VendorId: %s", dev["VendorId"])
+ continue
+
+ dev_vid_hex = m.group(1).lower()
+
+ m = VIRTUALBOX_VID_PID_RE.match(dev["ProductId"])
+ if not m:
+ _LOG.warning("Malformed ProductId: %s", dev["ProductId"])
+ continue
+
+ dev_pid_hex = m.group(1).lower()
+
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev["SerialNumber"])
+ ):
+ rule_args = [
+ "VBoxManage",
+ "usbfilter",
+ "add",
+ "0",
+ "--action",
+ "hold",
+ "--name",
+ "test device",
+ "--target",
+ uuid,
+ "--vendorid",
+ vid_hex,
+ "--productid",
+ pid_hex,
+ ]
+ if serial is not None:
+ rule_args.extend(["--serialnumber", serial])
+ subprocess.check_call(rule_args)
+ subprocess.check_call(["VBoxManage", "controlvm", uuid, "usbattach", dev["UUID"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+def attach_parallels(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = json.loads(
+ subprocess.check_output(["prlsrvctl", "usb", "list", "-j"], encoding="utf-8")
+ )
+ for dev in usb_devices:
+ _, dev_vid_hex, dev_pid_hex, _, _, dev_serial = dev["System name"].split("|")
+ dev_vid_hex = dev_vid_hex.lower()
+ dev_pid_hex = dev_pid_hex.lower()
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev_serial)
+ ):
+ subprocess.check_call(["prlsrvctl", "usb", "set", dev["Name"], uuid])
+ subprocess.check_call(["prlctl", "set", uuid, "--device-connect", dev["Name"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+ATTACH_USB_DEVICE = {
+ "parallels": attach_parallels,
+ "virtualbox": attach_virtualbox,
+}
+
+
+def generate_packer_config(file_path, providers):
+ builders = []
+ for provider_name in providers:
+ builders.append(
+ {
+ "type": "vagrant",
+ "output_dir": f"output-packer-{provider_name}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu1804",
+ "provider": provider_name,
+ "template": "Vagrantfile.packer-template",
+ }
+ )
+
+ with open(file_path, "w") as f:
+ json.dump(
+ {
+ "builders": builders,
+ },
+ f,
+ sort_keys=True,
+ indent=2,
+ )
+
+
+def build_command(args):
+ generate_packer_config(
+ os.path.join(THIS_DIR, args.platform, "base-box", "packer.json"),
+ args.provider.split(",") or ALL_PROVIDERS,
+ )
+ subprocess.check_call(
+ ["packer", "build", "packer.json"], cwd=os.path.join(THIS_DIR, args.platform, "base-box")
+ )
+
+
+REQUIRED_TEST_CONFIG_KEYS = {
+ "vid_hex": str,
+ "pid_hex": str,
+ "test_cmd": list,
+}
+
+
+VM_BOX_RE = re.compile(r'(.*\.vm\.box) = "(.*)"')
+
+
+# Paths, relative to the platform box directory, which will not be copied to release-test dir.
+SKIP_COPY_PATHS = [".vagrant", "base-box"]
+
+
+def test_command(args):
+ user_box_dir = os.path.join(THIS_DIR, args.platform)
+ base_box_dir = os.path.join(THIS_DIR, args.platform, "base-box")
+ test_config_file = os.path.join(base_box_dir, "test-config.json")
+ with open(test_config_file) as f:
+ test_config = json.load(f)
+ for key, expected_type in REQUIRED_TEST_CONFIG_KEYS.items():
+ assert key in test_config and isinstance(
+ test_config[key], expected_type
+ ), f"Expected key {key} of type {expected_type} in {test_config_file}: {test_config!r}"
+
+ test_config["vid_hex"] = test_config["vid_hex"].lower()
+ test_config["pid_hex"] = test_config["pid_hex"].lower()
+
+ providers = args.provider.split(",")
+ provider_passed = {p: False for p in providers}
+
+ release_test_dir = os.path.join(THIS_DIR, "release-test")
+
+ for provider_name in providers:
+ if os.path.exists(release_test_dir):
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ for dirpath, _, filenames in os.walk(user_box_dir):
+ rel_path = os.path.relpath(dirpath, user_box_dir)
+ if any(
+ rel_path == scp or rel_path.startswith(f"{scp}{os.path.sep}")
+ for scp in SKIP_COPY_PATHS
+ ):
+ continue
+
+ dest_dir = os.path.join(release_test_dir, rel_path)
+ os.makedirs(dest_dir)
+ for filename in filenames:
+ shutil.copy2(os.path.join(dirpath, filename), os.path.join(dest_dir, filename))
+
+ release_test_vagrantfile = os.path.join(release_test_dir, "Vagrantfile")
+ with open(release_test_vagrantfile) as f:
+ lines = list(f)
+
+ found_box_line = False
+ with open(release_test_vagrantfile, "w") as f:
+ for line in lines:
+ m = VM_BOX_RE.match(line)
+ if not m:
+ f.write(line)
+ continue
+
+ box_package = os.path.join(
+ base_box_dir, f"output-packer-{provider_name}", "package.box"
+ )
+ f.write(f'{m.group(1)} = "{os.path.relpath(box_package, release_test_dir)}"\n')
+ found_box_line = True
+
+ if not found_box_line:
+ _LOG.error(
+ "testing provider %s: couldn't find config.box.vm = line in Vagrantfile; unable to test",
+ provider_name,
+ )
+ continue
+
+ subprocess.check_call(
+ ["vagrant", "up", f"--provider={provider_name}"], cwd=release_test_dir
+ )
+ try:
+ with open(
+ os.path.join(
+ release_test_dir, ".vagrant", "machines", "default", provider_name, "id"
+ )
+ ) as f:
+ machine_uuid = f.read()
+ ATTACH_USB_DEVICE[provider_name](
+ machine_uuid,
+ vid_hex=test_config["vid_hex"],
+ pid_hex=test_config["pid_hex"],
+ serial=args.test_device_serial,
+ )
+ tvm_home = os.path.realpath(os.path.join(THIS_DIR, "..", "..", ".."))
+
+ def _quote_cmd(cmd):
+ return " ".join(shlex.quote(a) for a in cmd)
+
+ test_cmd = _quote_cmd(["cd", tvm_home]) + " && " + _quote_cmd(test_config["test_cmd"])
+ subprocess.check_call(
+ ["vagrant", "ssh", "-c", f"bash -ec '{test_cmd}'"], cwd=release_test_dir
+ )
+ provider_passed[provider_name] = True
+
+ finally:
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ if not all(provider_passed[p] for p in provider_passed.keys()):
+ sys.exit(
+ "some providers failed release test: "
+ + ",".join(name for name, passed in provider_passed if not passed)
+ )
+
+
+def release_command(args):
+ # subprocess.check_call(["vagrant", "cloud", "version", "create", f"tlcpack/microtvm-{args.platform}", args.version])
+ if not args.version:
+ sys.exit(f"--version must be specified")
+
+ for provider_name in args.provider.split(","):
+ subprocess.check_call(
+ [
+ "vagrant",
+ "cloud",
+ "publish",
+ "-f",
+ f"tlcpack/microtvm-{args.platform}",
+ args.version,
+ provider_name,
+ os.path.join(
+ THIS_DIR,
+ args.platform,
+ "base-box",
+ f"output-packer-{provider_name}/package.box",
+ ),
+ ]
+ )
+
+
+ALL_COMMANDS = {
+ "build": build_command,
+ "test": test_command,
+ "release": release_command,
+}
+
+
+def parse_args():
+ parser = argparse.ArgumentParser(
+ description="Automates building, testing, and releasing a base box"
+ )
+ parser.add_argument(
+ "command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action to perform."
+ )
+ parser.add_argument(
+ "platform",
+ help="Name of the platform VM to act on. Must be a sub-directory of this directory.",
+ )
+ parser.add_argument(
+ "--provider",
+ help="Name of the provider or providers to act on; if not specified, act on all",
Review comment:
```suggestion
parser.add_argument(
"--provider",
choices=ALL_PROVIDERS,
help="Name of the provider or providers (comma-separated) to act on.If not specified, act on all",
```
##########
File path: python/tvm/exec/microtvm_debug_shell.py
##########
@@ -0,0 +1,148 @@
+# 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.
+# pylint: disable=redefined-outer-name, invalid-name
+"""Start an RPC server intended for use as a microTVM debugger.
+
+microTVM aims to be runtime-agnostic, and to that end, frameworks often define command-line tools
+used to launch a debug flow. These tools often manage the process of connecting to an attached
+device using a hardware debugger, exposing a GDB server, and launching GDB connected to that
+server with a source file attached. It's also true that this debugger can typically not be executed
+concurrently with any flash tool, so this integration point is provided to allow TVM to launch and
+terminate any debuggers integrated with the larger microTVM compilation/autotuning flow.
+
+To use this tool, first launch this script in a separate terminal window. Then, provide the hostport
+to your compiler's Flasher instance.
+"""
+
+import argparse
+import logging
+import socket
+import struct
+
+import tvm.micro.debugger as _ # NOTE: imported to expose global PackedFuncs over RPC.
+
+from .._ffi.base import py_str
+from ..rpc import base
+from ..rpc import _ffi_api
+
+
+_LOG = logging.getLogger(__name__)
+
+
+def parse_args():
+ """Parse command line arguments to this script."""
+ parser = argparse.ArgumentParser(description="microTVM debug-tool runner")
+ parser.add_argument("--host", default="0.0.0.0", help="hostname to listen on")
+ parser.add_argument("--port", type=int, default=9090, help="hostname to listen on")
+ parser.add_argument(
+ "--impl",
+ help=(
+ "If given, name of a module underneath tvm.micro.contrib "
+ "which contains the Debugger implementation to use."
Review comment:
It looks like the module described here can go with full absolute namespace as well as relative (based on my interpretation of code on line ~125 here). Maybe an example here would be beneficial for users to understand this option.
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
+import argparse
+import json
+import logging
+import os
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+
+_LOG = logging.getLogger(__name__)
+
+
+THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+
+
+# List of vagrant providers supported by this tool
+ALL_PROVIDERS = (
+ "parallels",
+ "virtualbox",
+)
+
+
+def parse_virtualbox_devices():
+ output = subprocess.check_output(["VBoxManage", "list", "usbhost"], encoding="utf-8")
+ devices = []
+ current_dev = {}
+ for line in output.split("\n"):
+ if not line.strip():
+ if current_dev:
+ if "VendorId" in current_dev and "ProductId" in current_dev:
+ devices.append(current_dev)
+ current_dev = {}
+
+ continue
+
+ key, value = line.split(":", 1)
+ value = value.lstrip(" ")
+ current_dev[key] = value
+
+ if current_dev:
+ devices.append(current_dev)
+ return devices
+
+
+VIRTUALBOX_VID_PID_RE = re.compile(r"0x([0-9A-Fa-f]{4}).*")
+
+
+def attach_virtualbox(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = parse_virtualbox_devices()
+ for dev in usb_devices:
+ m = VIRTUALBOX_VID_PID_RE.match(dev["VendorId"])
+ if not m:
+ _LOG.warning("Malformed VendorId: %s", dev["VendorId"])
+ continue
+
+ dev_vid_hex = m.group(1).lower()
+
+ m = VIRTUALBOX_VID_PID_RE.match(dev["ProductId"])
+ if not m:
+ _LOG.warning("Malformed ProductId: %s", dev["ProductId"])
+ continue
+
+ dev_pid_hex = m.group(1).lower()
+
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev["SerialNumber"])
+ ):
+ rule_args = [
+ "VBoxManage",
+ "usbfilter",
+ "add",
+ "0",
+ "--action",
+ "hold",
+ "--name",
+ "test device",
+ "--target",
+ uuid,
+ "--vendorid",
+ vid_hex,
+ "--productid",
+ pid_hex,
+ ]
+ if serial is not None:
+ rule_args.extend(["--serialnumber", serial])
+ subprocess.check_call(rule_args)
+ subprocess.check_call(["VBoxManage", "controlvm", uuid, "usbattach", dev["UUID"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+def attach_parallels(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = json.loads(
+ subprocess.check_output(["prlsrvctl", "usb", "list", "-j"], encoding="utf-8")
+ )
+ for dev in usb_devices:
+ _, dev_vid_hex, dev_pid_hex, _, _, dev_serial = dev["System name"].split("|")
+ dev_vid_hex = dev_vid_hex.lower()
+ dev_pid_hex = dev_pid_hex.lower()
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev_serial)
+ ):
+ subprocess.check_call(["prlsrvctl", "usb", "set", dev["Name"], uuid])
+ subprocess.check_call(["prlctl", "set", uuid, "--device-connect", dev["Name"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+ATTACH_USB_DEVICE = {
+ "parallels": attach_parallels,
+ "virtualbox": attach_virtualbox,
+}
+
+
+def generate_packer_config(file_path, providers):
+ builders = []
+ for provider_name in providers:
+ builders.append(
+ {
+ "type": "vagrant",
+ "output_dir": f"output-packer-{provider_name}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu1804",
+ "provider": provider_name,
+ "template": "Vagrantfile.packer-template",
+ }
+ )
+
+ with open(file_path, "w") as f:
+ json.dump(
+ {
+ "builders": builders,
+ },
+ f,
+ sort_keys=True,
+ indent=2,
+ )
+
+
+def build_command(args):
+ generate_packer_config(
+ os.path.join(THIS_DIR, args.platform, "base-box", "packer.json"),
+ args.provider.split(",") or ALL_PROVIDERS,
+ )
+ subprocess.check_call(
+ ["packer", "build", "packer.json"], cwd=os.path.join(THIS_DIR, args.platform, "base-box")
+ )
+
+
+REQUIRED_TEST_CONFIG_KEYS = {
+ "vid_hex": str,
+ "pid_hex": str,
+ "test_cmd": list,
+}
+
+
+VM_BOX_RE = re.compile(r'(.*\.vm\.box) = "(.*)"')
+
+
+# Paths, relative to the platform box directory, which will not be copied to release-test dir.
+SKIP_COPY_PATHS = [".vagrant", "base-box"]
+
+
+def test_command(args):
+ user_box_dir = os.path.join(THIS_DIR, args.platform)
+ base_box_dir = os.path.join(THIS_DIR, args.platform, "base-box")
+ test_config_file = os.path.join(base_box_dir, "test-config.json")
+ with open(test_config_file) as f:
+ test_config = json.load(f)
+ for key, expected_type in REQUIRED_TEST_CONFIG_KEYS.items():
+ assert key in test_config and isinstance(
+ test_config[key], expected_type
+ ), f"Expected key {key} of type {expected_type} in {test_config_file}: {test_config!r}"
+
+ test_config["vid_hex"] = test_config["vid_hex"].lower()
+ test_config["pid_hex"] = test_config["pid_hex"].lower()
+
+ providers = args.provider.split(",")
+ provider_passed = {p: False for p in providers}
+
+ release_test_dir = os.path.join(THIS_DIR, "release-test")
+
+ for provider_name in providers:
+ if os.path.exists(release_test_dir):
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ for dirpath, _, filenames in os.walk(user_box_dir):
+ rel_path = os.path.relpath(dirpath, user_box_dir)
+ if any(
+ rel_path == scp or rel_path.startswith(f"{scp}{os.path.sep}")
+ for scp in SKIP_COPY_PATHS
+ ):
+ continue
+
+ dest_dir = os.path.join(release_test_dir, rel_path)
+ os.makedirs(dest_dir)
+ for filename in filenames:
+ shutil.copy2(os.path.join(dirpath, filename), os.path.join(dest_dir, filename))
+
+ release_test_vagrantfile = os.path.join(release_test_dir, "Vagrantfile")
+ with open(release_test_vagrantfile) as f:
+ lines = list(f)
+
+ found_box_line = False
+ with open(release_test_vagrantfile, "w") as f:
+ for line in lines:
+ m = VM_BOX_RE.match(line)
+ if not m:
+ f.write(line)
+ continue
+
+ box_package = os.path.join(
+ base_box_dir, f"output-packer-{provider_name}", "package.box"
+ )
+ f.write(f'{m.group(1)} = "{os.path.relpath(box_package, release_test_dir)}"\n')
+ found_box_line = True
+
+ if not found_box_line:
+ _LOG.error(
+ "testing provider %s: couldn't find config.box.vm = line in Vagrantfile; unable to test",
+ provider_name,
+ )
+ continue
+
+ subprocess.check_call(
+ ["vagrant", "up", f"--provider={provider_name}"], cwd=release_test_dir
+ )
+ try:
+ with open(
+ os.path.join(
+ release_test_dir, ".vagrant", "machines", "default", provider_name, "id"
+ )
+ ) as f:
+ machine_uuid = f.read()
+ ATTACH_USB_DEVICE[provider_name](
+ machine_uuid,
+ vid_hex=test_config["vid_hex"],
+ pid_hex=test_config["pid_hex"],
+ serial=args.test_device_serial,
+ )
+ tvm_home = os.path.realpath(os.path.join(THIS_DIR, "..", "..", ".."))
+
+ def _quote_cmd(cmd):
+ return " ".join(shlex.quote(a) for a in cmd)
+
+ test_cmd = _quote_cmd(["cd", tvm_home]) + " && " + _quote_cmd(test_config["test_cmd"])
+ subprocess.check_call(
+ ["vagrant", "ssh", "-c", f"bash -ec '{test_cmd}'"], cwd=release_test_dir
+ )
+ provider_passed[provider_name] = True
+
+ finally:
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ if not all(provider_passed[p] for p in provider_passed.keys()):
+ sys.exit(
+ "some providers failed release test: "
+ + ",".join(name for name, passed in provider_passed if not passed)
+ )
+
+
+def release_command(args):
+ # subprocess.check_call(["vagrant", "cloud", "version", "create", f"tlcpack/microtvm-{args.platform}", args.version])
+ if not args.version:
+ sys.exit(f"--version must be specified")
+
+ for provider_name in args.provider.split(","):
+ subprocess.check_call(
+ [
+ "vagrant",
+ "cloud",
+ "publish",
+ "-f",
+ f"tlcpack/microtvm-{args.platform}",
+ args.version,
+ provider_name,
+ os.path.join(
+ THIS_DIR,
+ args.platform,
+ "base-box",
+ f"output-packer-{provider_name}/package.box",
+ ),
+ ]
+ )
+
+
+ALL_COMMANDS = {
+ "build": build_command,
+ "test": test_command,
+ "release": release_command,
+}
+
+
+def parse_args():
+ parser = argparse.ArgumentParser(
+ description="Automates building, testing, and releasing a base box"
+ )
+ parser.add_argument(
+ "command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action to perform."
Review comment:
```suggestion
"command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action or actions (comma-separated) to perform."
```
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
Review comment:
I think `abc` is not used here, and can be removed?
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
+import argparse
+import json
+import logging
+import os
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+
+_LOG = logging.getLogger(__name__)
+
+
+THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+
+
+# List of vagrant providers supported by this tool
+ALL_PROVIDERS = (
+ "parallels",
+ "virtualbox",
+)
+
+
+def parse_virtualbox_devices():
+ output = subprocess.check_output(["VBoxManage", "list", "usbhost"], encoding="utf-8")
+ devices = []
+ current_dev = {}
+ for line in output.split("\n"):
+ if not line.strip():
+ if current_dev:
+ if "VendorId" in current_dev and "ProductId" in current_dev:
+ devices.append(current_dev)
+ current_dev = {}
+
+ continue
+
+ key, value = line.split(":", 1)
+ value = value.lstrip(" ")
+ current_dev[key] = value
+
+ if current_dev:
+ devices.append(current_dev)
+ return devices
+
+
+VIRTUALBOX_VID_PID_RE = re.compile(r"0x([0-9A-Fa-f]{4}).*")
+
+
+def attach_virtualbox(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = parse_virtualbox_devices()
+ for dev in usb_devices:
+ m = VIRTUALBOX_VID_PID_RE.match(dev["VendorId"])
+ if not m:
+ _LOG.warning("Malformed VendorId: %s", dev["VendorId"])
+ continue
+
+ dev_vid_hex = m.group(1).lower()
+
+ m = VIRTUALBOX_VID_PID_RE.match(dev["ProductId"])
+ if not m:
+ _LOG.warning("Malformed ProductId: %s", dev["ProductId"])
+ continue
+
+ dev_pid_hex = m.group(1).lower()
+
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev["SerialNumber"])
+ ):
+ rule_args = [
+ "VBoxManage",
+ "usbfilter",
+ "add",
+ "0",
+ "--action",
+ "hold",
+ "--name",
+ "test device",
+ "--target",
+ uuid,
+ "--vendorid",
+ vid_hex,
+ "--productid",
+ pid_hex,
+ ]
+ if serial is not None:
+ rule_args.extend(["--serialnumber", serial])
+ subprocess.check_call(rule_args)
+ subprocess.check_call(["VBoxManage", "controlvm", uuid, "usbattach", dev["UUID"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+def attach_parallels(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = json.loads(
+ subprocess.check_output(["prlsrvctl", "usb", "list", "-j"], encoding="utf-8")
+ )
+ for dev in usb_devices:
+ _, dev_vid_hex, dev_pid_hex, _, _, dev_serial = dev["System name"].split("|")
+ dev_vid_hex = dev_vid_hex.lower()
+ dev_pid_hex = dev_pid_hex.lower()
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev_serial)
+ ):
+ subprocess.check_call(["prlsrvctl", "usb", "set", dev["Name"], uuid])
+ subprocess.check_call(["prlctl", "set", uuid, "--device-connect", dev["Name"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+ATTACH_USB_DEVICE = {
+ "parallels": attach_parallels,
+ "virtualbox": attach_virtualbox,
+}
+
+
+def generate_packer_config(file_path, providers):
+ builders = []
+ for provider_name in providers:
+ builders.append(
+ {
+ "type": "vagrant",
+ "output_dir": f"output-packer-{provider_name}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu1804",
+ "provider": provider_name,
+ "template": "Vagrantfile.packer-template",
+ }
+ )
+
+ with open(file_path, "w") as f:
+ json.dump(
+ {
+ "builders": builders,
+ },
+ f,
+ sort_keys=True,
+ indent=2,
+ )
+
+
+def build_command(args):
+ generate_packer_config(
+ os.path.join(THIS_DIR, args.platform, "base-box", "packer.json"),
+ args.provider.split(",") or ALL_PROVIDERS,
+ )
+ subprocess.check_call(
+ ["packer", "build", "packer.json"], cwd=os.path.join(THIS_DIR, args.platform, "base-box")
+ )
+
+
+REQUIRED_TEST_CONFIG_KEYS = {
+ "vid_hex": str,
+ "pid_hex": str,
+ "test_cmd": list,
+}
+
+
+VM_BOX_RE = re.compile(r'(.*\.vm\.box) = "(.*)"')
+
+
+# Paths, relative to the platform box directory, which will not be copied to release-test dir.
+SKIP_COPY_PATHS = [".vagrant", "base-box"]
+
+
+def test_command(args):
+ user_box_dir = os.path.join(THIS_DIR, args.platform)
+ base_box_dir = os.path.join(THIS_DIR, args.platform, "base-box")
+ test_config_file = os.path.join(base_box_dir, "test-config.json")
+ with open(test_config_file) as f:
+ test_config = json.load(f)
+ for key, expected_type in REQUIRED_TEST_CONFIG_KEYS.items():
+ assert key in test_config and isinstance(
+ test_config[key], expected_type
+ ), f"Expected key {key} of type {expected_type} in {test_config_file}: {test_config!r}"
+
+ test_config["vid_hex"] = test_config["vid_hex"].lower()
+ test_config["pid_hex"] = test_config["pid_hex"].lower()
+
+ providers = args.provider.split(",")
+ provider_passed = {p: False for p in providers}
+
+ release_test_dir = os.path.join(THIS_DIR, "release-test")
+
+ for provider_name in providers:
+ if os.path.exists(release_test_dir):
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ for dirpath, _, filenames in os.walk(user_box_dir):
+ rel_path = os.path.relpath(dirpath, user_box_dir)
+ if any(
+ rel_path == scp or rel_path.startswith(f"{scp}{os.path.sep}")
+ for scp in SKIP_COPY_PATHS
+ ):
+ continue
+
+ dest_dir = os.path.join(release_test_dir, rel_path)
+ os.makedirs(dest_dir)
+ for filename in filenames:
+ shutil.copy2(os.path.join(dirpath, filename), os.path.join(dest_dir, filename))
+
+ release_test_vagrantfile = os.path.join(release_test_dir, "Vagrantfile")
+ with open(release_test_vagrantfile) as f:
+ lines = list(f)
+
+ found_box_line = False
+ with open(release_test_vagrantfile, "w") as f:
+ for line in lines:
+ m = VM_BOX_RE.match(line)
+ if not m:
+ f.write(line)
+ continue
+
+ box_package = os.path.join(
+ base_box_dir, f"output-packer-{provider_name}", "package.box"
+ )
+ f.write(f'{m.group(1)} = "{os.path.relpath(box_package, release_test_dir)}"\n')
+ found_box_line = True
+
+ if not found_box_line:
+ _LOG.error(
+ "testing provider %s: couldn't find config.box.vm = line in Vagrantfile; unable to test",
+ provider_name,
+ )
+ continue
+
+ subprocess.check_call(
+ ["vagrant", "up", f"--provider={provider_name}"], cwd=release_test_dir
+ )
+ try:
+ with open(
+ os.path.join(
+ release_test_dir, ".vagrant", "machines", "default", provider_name, "id"
+ )
+ ) as f:
+ machine_uuid = f.read()
+ ATTACH_USB_DEVICE[provider_name](
+ machine_uuid,
+ vid_hex=test_config["vid_hex"],
+ pid_hex=test_config["pid_hex"],
+ serial=args.test_device_serial,
+ )
+ tvm_home = os.path.realpath(os.path.join(THIS_DIR, "..", "..", ".."))
+
+ def _quote_cmd(cmd):
+ return " ".join(shlex.quote(a) for a in cmd)
+
+ test_cmd = _quote_cmd(["cd", tvm_home]) + " && " + _quote_cmd(test_config["test_cmd"])
+ subprocess.check_call(
+ ["vagrant", "ssh", "-c", f"bash -ec '{test_cmd}'"], cwd=release_test_dir
+ )
+ provider_passed[provider_name] = True
+
+ finally:
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ if not all(provider_passed[p] for p in provider_passed.keys()):
+ sys.exit(
+ "some providers failed release test: "
+ + ",".join(name for name, passed in provider_passed if not passed)
+ )
+
+
+def release_command(args):
+ # subprocess.check_call(["vagrant", "cloud", "version", "create", f"tlcpack/microtvm-{args.platform}", args.version])
+ if not args.version:
+ sys.exit(f"--version must be specified")
+
+ for provider_name in args.provider.split(","):
+ subprocess.check_call(
+ [
+ "vagrant",
+ "cloud",
+ "publish",
+ "-f",
+ f"tlcpack/microtvm-{args.platform}",
+ args.version,
+ provider_name,
+ os.path.join(
+ THIS_DIR,
+ args.platform,
+ "base-box",
+ f"output-packer-{provider_name}/package.box",
+ ),
+ ]
+ )
+
+
+ALL_COMMANDS = {
+ "build": build_command,
+ "test": test_command,
+ "release": release_command,
+}
+
+
+def parse_args():
+ parser = argparse.ArgumentParser(
+ description="Automates building, testing, and releasing a base box"
+ )
+ parser.add_argument(
+ "command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action to perform."
+ )
+ parser.add_argument(
+ "platform",
+ help="Name of the platform VM to act on. Must be a sub-directory of this directory.",
+ )
+ parser.add_argument(
+ "--provider",
+ help="Name of the provider or providers to act on; if not specified, act on all",
+ )
+ parser.add_argument(
+ "--test-device-serial", help="If given, attach the test device with this USB serial number"
Review comment:
I think an example is needed here, to clarify how this option looks like, from the user point of view.
##########
File path: apps/microtvm/reference-vm/base-box-tool.py
##########
@@ -0,0 +1,368 @@
+#!/usr/bin/env python3
+# 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.
+
+
+import abc
+import argparse
+import json
+import logging
+import os
+import re
+import shlex
+import shutil
+import subprocess
+import sys
+
+
+_LOG = logging.getLogger(__name__)
+
+
+THIS_DIR = os.path.realpath(os.path.dirname(__file__) or ".")
+
+
+# List of vagrant providers supported by this tool
+ALL_PROVIDERS = (
+ "parallels",
+ "virtualbox",
+)
+
+
+def parse_virtualbox_devices():
+ output = subprocess.check_output(["VBoxManage", "list", "usbhost"], encoding="utf-8")
+ devices = []
+ current_dev = {}
+ for line in output.split("\n"):
+ if not line.strip():
+ if current_dev:
+ if "VendorId" in current_dev and "ProductId" in current_dev:
+ devices.append(current_dev)
+ current_dev = {}
+
+ continue
+
+ key, value = line.split(":", 1)
+ value = value.lstrip(" ")
+ current_dev[key] = value
+
+ if current_dev:
+ devices.append(current_dev)
+ return devices
+
+
+VIRTUALBOX_VID_PID_RE = re.compile(r"0x([0-9A-Fa-f]{4}).*")
+
+
+def attach_virtualbox(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = parse_virtualbox_devices()
+ for dev in usb_devices:
+ m = VIRTUALBOX_VID_PID_RE.match(dev["VendorId"])
+ if not m:
+ _LOG.warning("Malformed VendorId: %s", dev["VendorId"])
+ continue
+
+ dev_vid_hex = m.group(1).lower()
+
+ m = VIRTUALBOX_VID_PID_RE.match(dev["ProductId"])
+ if not m:
+ _LOG.warning("Malformed ProductId: %s", dev["ProductId"])
+ continue
+
+ dev_pid_hex = m.group(1).lower()
+
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev["SerialNumber"])
+ ):
+ rule_args = [
+ "VBoxManage",
+ "usbfilter",
+ "add",
+ "0",
+ "--action",
+ "hold",
+ "--name",
+ "test device",
+ "--target",
+ uuid,
+ "--vendorid",
+ vid_hex,
+ "--productid",
+ pid_hex,
+ ]
+ if serial is not None:
+ rule_args.extend(["--serialnumber", serial])
+ subprocess.check_call(rule_args)
+ subprocess.check_call(["VBoxManage", "controlvm", uuid, "usbattach", dev["UUID"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+def attach_parallels(uuid, vid_hex=None, pid_hex=None, serial=None):
+ usb_devices = json.loads(
+ subprocess.check_output(["prlsrvctl", "usb", "list", "-j"], encoding="utf-8")
+ )
+ for dev in usb_devices:
+ _, dev_vid_hex, dev_pid_hex, _, _, dev_serial = dev["System name"].split("|")
+ dev_vid_hex = dev_vid_hex.lower()
+ dev_pid_hex = dev_pid_hex.lower()
+ if (
+ vid_hex == dev_vid_hex
+ and pid_hex == dev_pid_hex
+ and (serial is None or serial == dev_serial)
+ ):
+ subprocess.check_call(["prlsrvctl", "usb", "set", dev["Name"], uuid])
+ subprocess.check_call(["prlctl", "set", uuid, "--device-connect", dev["Name"]])
+ return
+
+ raise Exception(
+ f"Device with vid={vid_hex}, pid={pid_hex}, serial={serial!r} not found:\n{usb_devices!r}"
+ )
+
+
+ATTACH_USB_DEVICE = {
+ "parallels": attach_parallels,
+ "virtualbox": attach_virtualbox,
+}
+
+
+def generate_packer_config(file_path, providers):
+ builders = []
+ for provider_name in providers:
+ builders.append(
+ {
+ "type": "vagrant",
+ "output_dir": f"output-packer-{provider_name}",
+ "communicator": "ssh",
+ "source_path": "generic/ubuntu1804",
+ "provider": provider_name,
+ "template": "Vagrantfile.packer-template",
+ }
+ )
+
+ with open(file_path, "w") as f:
+ json.dump(
+ {
+ "builders": builders,
+ },
+ f,
+ sort_keys=True,
+ indent=2,
+ )
+
+
+def build_command(args):
+ generate_packer_config(
+ os.path.join(THIS_DIR, args.platform, "base-box", "packer.json"),
+ args.provider.split(",") or ALL_PROVIDERS,
+ )
+ subprocess.check_call(
+ ["packer", "build", "packer.json"], cwd=os.path.join(THIS_DIR, args.platform, "base-box")
+ )
+
+
+REQUIRED_TEST_CONFIG_KEYS = {
+ "vid_hex": str,
+ "pid_hex": str,
+ "test_cmd": list,
+}
+
+
+VM_BOX_RE = re.compile(r'(.*\.vm\.box) = "(.*)"')
+
+
+# Paths, relative to the platform box directory, which will not be copied to release-test dir.
+SKIP_COPY_PATHS = [".vagrant", "base-box"]
+
+
+def test_command(args):
+ user_box_dir = os.path.join(THIS_DIR, args.platform)
+ base_box_dir = os.path.join(THIS_DIR, args.platform, "base-box")
+ test_config_file = os.path.join(base_box_dir, "test-config.json")
+ with open(test_config_file) as f:
+ test_config = json.load(f)
+ for key, expected_type in REQUIRED_TEST_CONFIG_KEYS.items():
+ assert key in test_config and isinstance(
+ test_config[key], expected_type
+ ), f"Expected key {key} of type {expected_type} in {test_config_file}: {test_config!r}"
+
+ test_config["vid_hex"] = test_config["vid_hex"].lower()
+ test_config["pid_hex"] = test_config["pid_hex"].lower()
+
+ providers = args.provider.split(",")
+ provider_passed = {p: False for p in providers}
+
+ release_test_dir = os.path.join(THIS_DIR, "release-test")
+
+ for provider_name in providers:
+ if os.path.exists(release_test_dir):
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ for dirpath, _, filenames in os.walk(user_box_dir):
+ rel_path = os.path.relpath(dirpath, user_box_dir)
+ if any(
+ rel_path == scp or rel_path.startswith(f"{scp}{os.path.sep}")
+ for scp in SKIP_COPY_PATHS
+ ):
+ continue
+
+ dest_dir = os.path.join(release_test_dir, rel_path)
+ os.makedirs(dest_dir)
+ for filename in filenames:
+ shutil.copy2(os.path.join(dirpath, filename), os.path.join(dest_dir, filename))
+
+ release_test_vagrantfile = os.path.join(release_test_dir, "Vagrantfile")
+ with open(release_test_vagrantfile) as f:
+ lines = list(f)
+
+ found_box_line = False
+ with open(release_test_vagrantfile, "w") as f:
+ for line in lines:
+ m = VM_BOX_RE.match(line)
+ if not m:
+ f.write(line)
+ continue
+
+ box_package = os.path.join(
+ base_box_dir, f"output-packer-{provider_name}", "package.box"
+ )
+ f.write(f'{m.group(1)} = "{os.path.relpath(box_package, release_test_dir)}"\n')
+ found_box_line = True
+
+ if not found_box_line:
+ _LOG.error(
+ "testing provider %s: couldn't find config.box.vm = line in Vagrantfile; unable to test",
+ provider_name,
+ )
+ continue
+
+ subprocess.check_call(
+ ["vagrant", "up", f"--provider={provider_name}"], cwd=release_test_dir
+ )
+ try:
+ with open(
+ os.path.join(
+ release_test_dir, ".vagrant", "machines", "default", provider_name, "id"
+ )
+ ) as f:
+ machine_uuid = f.read()
+ ATTACH_USB_DEVICE[provider_name](
+ machine_uuid,
+ vid_hex=test_config["vid_hex"],
+ pid_hex=test_config["pid_hex"],
+ serial=args.test_device_serial,
+ )
+ tvm_home = os.path.realpath(os.path.join(THIS_DIR, "..", "..", ".."))
+
+ def _quote_cmd(cmd):
+ return " ".join(shlex.quote(a) for a in cmd)
+
+ test_cmd = _quote_cmd(["cd", tvm_home]) + " && " + _quote_cmd(test_config["test_cmd"])
+ subprocess.check_call(
+ ["vagrant", "ssh", "-c", f"bash -ec '{test_cmd}'"], cwd=release_test_dir
+ )
+ provider_passed[provider_name] = True
+
+ finally:
+ subprocess.check_call(["vagrant", "destroy", "-f"], cwd=release_test_dir)
+ shutil.rmtree(release_test_dir)
+
+ if not all(provider_passed[p] for p in provider_passed.keys()):
+ sys.exit(
+ "some providers failed release test: "
+ + ",".join(name for name, passed in provider_passed if not passed)
+ )
+
+
+def release_command(args):
+ # subprocess.check_call(["vagrant", "cloud", "version", "create", f"tlcpack/microtvm-{args.platform}", args.version])
+ if not args.version:
+ sys.exit(f"--version must be specified")
+
+ for provider_name in args.provider.split(","):
+ subprocess.check_call(
+ [
+ "vagrant",
+ "cloud",
+ "publish",
+ "-f",
+ f"tlcpack/microtvm-{args.platform}",
+ args.version,
+ provider_name,
+ os.path.join(
+ THIS_DIR,
+ args.platform,
+ "base-box",
+ f"output-packer-{provider_name}/package.box",
+ ),
+ ]
+ )
+
+
+ALL_COMMANDS = {
+ "build": build_command,
+ "test": test_command,
+ "release": release_command,
+}
+
+
+def parse_args():
+ parser = argparse.ArgumentParser(
+ description="Automates building, testing, and releasing a base box"
+ )
+ parser.add_argument(
+ "command", default=",".join(ALL_COMMANDS), choices=ALL_COMMANDS, help="Action to perform."
+ )
+ parser.add_argument(
+ "platform",
+ help="Name of the platform VM to act on. Must be a sub-directory of this directory.",
+ )
+ parser.add_argument(
+ "--provider",
+ help="Name of the provider or providers to act on; if not specified, act on all",
+ )
+ parser.add_argument(
+ "--test-device-serial", help="If given, attach the test device with this USB serial number"
+ )
+ parser.add_argument("--version", help="Version to release. Must be specified with release.")
Review comment:
I think an example is needed here, to clarify how `version` looks like.
Apart from that, maybe `--version` is expected by users to mean "what is the version of this tool?". I suggest renaming this to something more meaningful to the end user.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch edited a comment on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch edited a comment on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-714207644
@u99127 some early replies.
> I've spent sometime today attempting to review this PR. I would like to understand more about how this pushes things forward with the uTVM integration. Can you link this to an RFC that describes this for posterity and how this links with the overall plan for uTVM ?
sure, happy to write one. briefly: this makes it easy to use Zephyr, so it generally aligns with [Standalone µTVM roadmap](https://discuss.tvm.apache.org/t/rfc-tvm-standalone-tvm-roadmap/6987), though happy to admit an RFC with an added level of detail would help.
> At the end of this PR getting merged, is the expectation that a user of TVM gets a shell like interface to interact with a board and be able to interactively run machine learning models on the board using Zephyr and uTVM (well, really autotuning at this point of time) ?
correct. Zephyr has a lot of dependencies, and additionally, setting up the TVM virtualenv is not trivial now and is made especially hard by needing to install the additional zephyr dependencies. the VM solves this in a cross-platform way. it's needed (vs a docker container) because docker can't correctly virtualize any piece of attached hardware which can't be controlled solely with libusb (I.e. virtual com ports can't be used from docker containers).
> This appears to mix multiple changes which could be staged into one giant PR.
>
> I can see the following changes here .
>
> 1. New CI infrastructure with vagrant boxes,
I don't believe we're adding any CI infrastructure here. The [previous change](https://github.com/apache/incubator-tvm/pull/6603) added QEMU support to the CI plus the current test. This PR proves that the test can be used with both QEMU and real hardware with a minor 2-line change (this is why this PR adds parameterization to `test_zephyr`).
> 2. changes to existing uTVM transport server protocols
No protocol level changes were made here, but I did add the `SerialTransport` implementation and a way to automatically launch GDB to debug the attached Cortex board. I think more documentation would make this clear. I can add a doc explaining how to use this change.
> 3. changes to llvm's target parsing which is really common between the Arm and AArch64 targets AFAIK and the behaviour of mfpu is very different between the 2. Thus that can be an indpendent PR in its own right.
actually I can split this out. originally I was thinking that `test_zephyr` required floating point support, but it uses integers.
> 4 ... I'm sure I've missed some changes relating to Zephyr or others in there.
A couple of bugfixes:
- if an Exception is raised while constructing the Session but after the transport has been initialized, de-initialize the transport
- fix behavior of MicroSession when using it concurrently with an attached debugger. Specifically, if no retry timeout is specified, StartSession would still timeout after the retry timeout.
>
> I'm not going to be able to review items 2, 3 and 4 on this list tonight and will be restraining myself to just looking at the vagrant bits and understand more about how this looks.
>
> My understanding of the choice of vagrant is that this is useful as a different way of connecting to peripherals which is hard to manage in a docker container . Is that right ?
Vagrant is mainly useful to codify the virtual machine configuration. you're right that we use the VM to connect to peripherals because docker isn't any good for that. there are 2 virtual machine configs stored: a "base box" which is stored [here](https://app.vagrantup.com/areusch/boxes/microtvm-staging) (we need to find a better home for it), and a "vm" which is what the user's supposed to use. the base box just needs to be updated like the CI docker containers: when dependencies change. the vm is built by every user by issuing:
`$ vagrant up --provider={parallels,virtualbox,...}`
in the `apps/microtvm-vm/vm` directory.
this builds the user's TVM installation using the VM's gcc toolchain.
>
> With the vagrant box setup what is the thinking with respect to those developers or those developers using CI that runs already inside virtual machines. Would this naturally nest and work out of the box ? For instance if there as a developer wanting to develop uTVM on a GNU/Linux VM on a Windows laptop, how would this set up work for them ? Or indeed if one were to run CI systems that used openstack, would vagrant play nicely on those CI systems ?
Right now the VM doesn't have any relation to the CI. You could imagine a future where part of the CI is spinning up the VM and launching a Jenkins instance. Instead, we build a docker image with QEMU and Zephyr, but no utilities for connecting to peripherals.
I believe it should be possible to launch the CI from inside the VM if you installed docker--we configure this now. Vagrant is supported on Windows and it should be possible to use it with VirtualBox to develop for µTVM. I haven't tested that, though. I don't know if the VM can nest on openstack, but as we don't plan to use it in CI yet, I don't think this adds any additional concerns to the problem of running the TVM CI on openstack. Let me know if I've misunderstood your question.
>
> A first pass through this to see what it looks like but I'm not terribly happy with my own understanding of this .
>
> Finally , are there plans for a tutorial in the source base about how to actually use this since this PR seems to be short on any documentation about folks wanting to try it out. How can a developer try this out ?
Yes, definitely. I have one nearly ready--I should just merge it with this PR. I'll iterate on that.
>
> regards
> Ramana
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r517903713
##########
File path: apps/microtvm/reference-vm/zephyr/pyproject.toml
##########
@@ -0,0 +1,140 @@
+# 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.
+
+[tool.black]
+line-length = 100
+target-version = ['py36']
+include = '(\.pyi?$)'
+exclude = '''
+
+(
+ /(
+ \.github
+ | \.tvm
+ | \.tvm_test_data
+ | \.vscode
+ | \.venv
+ | 3rdparty
+ | build\/
+ | cmake\/
+ | conda\/
+ | docker\/
+ | docs\/
+ | golang\/
+ | include\/
+ | jvm\/
+ | licenses\/
+ | nnvm\/
+ | rust\/
+ | src\/
+ | vta\/
+ | web\/
+ )/
+)
+'''
+[tool.poetry]
+name = "incubator-tvm"
+version = "0.1.0"
+description = ""
+authors = ["Your Name <yo...@example.com>"]
+packages = [
+ { include = "tvm", from = "python" },
+]
+
+[tool.poetry.dependencies]
+attrs = "^19"
+decorator = "^4.4"
+numpy = "~1.19"
+psutil = "^5"
+scipy = "^1.4"
+python = "^3.6"
+tornado = "^6"
+typed_ast = "^1.4"
Review comment:
@areusch , After reading the RFC, do we want to align with a one of the requirement.txt ?. So we can have synergy between this and ci-qemu ? and would suggest introduction of poetry after next RFC if and when that happens, thus the changes will go here 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r507904373
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# 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.
+
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
Review comment:
I did what i'd done previously on `docker/install/ubuntu_install_zephyr.sh`, wouldn't downloading the key defeat the purpose of it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r517903713
##########
File path: apps/microtvm/reference-vm/zephyr/pyproject.toml
##########
@@ -0,0 +1,140 @@
+# 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.
+
+[tool.black]
+line-length = 100
+target-version = ['py36']
+include = '(\.pyi?$)'
+exclude = '''
+
+(
+ /(
+ \.github
+ | \.tvm
+ | \.tvm_test_data
+ | \.vscode
+ | \.venv
+ | 3rdparty
+ | build\/
+ | cmake\/
+ | conda\/
+ | docker\/
+ | docs\/
+ | golang\/
+ | include\/
+ | jvm\/
+ | licenses\/
+ | nnvm\/
+ | rust\/
+ | src\/
+ | vta\/
+ | web\/
+ )/
+)
+'''
+[tool.poetry]
+name = "incubator-tvm"
+version = "0.1.0"
+description = ""
+authors = ["Your Name <yo...@example.com>"]
+packages = [
+ { include = "tvm", from = "python" },
+]
+
+[tool.poetry.dependencies]
+attrs = "^19"
+decorator = "^4.4"
+numpy = "~1.19"
+psutil = "^5"
+scipy = "^1.4"
+python = "^3.6"
+tornado = "^6"
+typed_ast = "^1.4"
Review comment:
@areusch , After reading the RFC, do we want to align with a one of the requirement.txt ?. So we can synergy between this and ci-qemu ? and would suggest introduction of poetry after next RFC if and when that happens, thus the changes will go here 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-720723262
@leandron @u99127 @manupa-arm please take a look when you have a minute and explicitly approve if you're good w/ this change
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] leandron commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-722242281
> @leandron please take a look when you have a minute and explicitly approve if you're good w/ this change
I'm happy with the current version. Would like also to hear from @manupa-arm and @u99127, if possible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r508730442
##########
File path: tools/microtvm/vm/kitware-archive-latest.asc
##########
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review comment:
Let us still move to apps to reduce the amount of top-level directories.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] manupa-arm edited a comment on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
manupa-arm edited a comment on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-715291366
Hi Andrew,
Thanks for the PR.
I also agree with @u99127 that this PR needs breakdown.
From a high-level the need for the Vagrant boxes arise from the need not being able to use serial COM ports of Windows via the docker image, right ? (ref : https://github.com/docker/for-win/issues/1018).
Thus, the reason that I think that we need the Vagrant is to enable autotvm in which we perform compilation and running together.
Out of curiosity, have you thought about spinning a docker up , possibly extending ci_qemu, to include the dependencies of Zehpyr and use it inside the Vagrant (perhaps via Docker Provisioner ? ) -- therefore the Vagrant will provide just the thing the docker is missing which is the serial COM port exposure (in Windows) inside the box.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] u99127 commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510464859
##########
File path: apps/microtvm-vm/base-box/Vagrantfile.packer-template
##########
@@ -0,0 +1,45 @@
+# 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.
+
+Vagrant.configure("2") do |config|
+ # From hashicorp default template:
+ # https://github.com/hashicorp/packer/blob/master/builder/vagrant/step_create_vagrantfile.go#L23-L37
+
+ config.vm.define "source" do |source|
+ source.vm.box = "{{.SourceBox}}"
+ config.ssh.insert_key = {{.InsertKey}}
Review comment:
Ack. as long as it doesn't get baked into a vcs system I guess that's ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] leandron commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r516568426
##########
File path: apps/microtvm/reference-vm/zephyr/base-box/setup.sh
##########
@@ -0,0 +1,99 @@
+#!/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 -e
+
+sudo apt update
+sudo apt install -y build-essential
+sudo apt-get --purge remove modemmanager # required to access serial ports.
+
+# Zephyr
+wget --no-verbose https://apt.kitware.com/keys/kitware-archive-latest.asc
+sudo apt-key add kitware-archive-latest.asc
+sudo apt-add-repository 'deb https://apt.kitware.com/ubuntu/ bionic main'
+sudo apt update
+sudo apt install -y --no-install-recommends git cmake ninja-build gperf \
+ ccache dfu-util device-tree-compiler wget \
+ python3-dev python3-pip python3-setuptools python3-tk python3-wheel xz-utils file \
+ make gcc gcc-multilib g++-multilib libsdl2-dev
+
+# Avahi, so that ssh microtvm works.
+# apt install -y avahi-daemon
+
+OLD_HOSTNAME=$(hostname)
+sudo hostnamectl set-hostname microtvm
+sudo sed -i.bak "s/${OLD_HOSTNAME}/microtvm.localdomain/g" /etc/hosts
+
+# Poetry deps
+sudo apt install -y python3-venv
+
+# TVM deps
+sudo apt install -y llvm
+
+# ONNX deps
+sudo apt install -y protobuf-compiler libprotoc-dev
+
+# nrfjprog
+cd ~
+mkdir -p nrfjprog
+wget --no-verbose -O nRFCommandLineTools1090Linuxamd64.tar.gz https://www.nordicsemi.com/-/media/Software-and-other-downloads/Desktop-software/nRF-command-line-tools/sw/Versions-10-x-x/10-9-0/nRFCommandLineTools1090Linuxamd64tar.gz
Review comment:
I suggest to have something here to cleanup the files/packages being downloaded in this script.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r511323444
##########
File path: apps/microtvm-vm/base-box/packer.json.template
##########
@@ -0,0 +1,43 @@
+# 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.
+{
Review comment:
i've moved this into a new standardized release tool. uTVM developers are not expected to ever need to configure this. someone maintaining e.g. the Zephyr integration or another RTOS/platform integration may need to understand this, but the packer docs are [extensive](https://www.packer.io/docs).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r511323507
##########
File path: apps/microtvm-vm/base-box/release-version.sh
##########
@@ -0,0 +1,53 @@
+#!/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.
+
Review comment:
moved this logic to `apps/microtvm/reference-vm/base-box-tool.py`, documented in `apps/microtvm/reference-vm/README.md`.
##########
File path: src/target/llvm/llvm_common.cc
##########
@@ -60,10 +61,11 @@ void InitializeLLVM() {
}
void ParseLLVMTargetOptions(const Target& target, std::string* triple, std::string* mcpu,
- std::string* mattr, llvm::TargetOptions* options) {
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#issuecomment-718367233
@leandron @u99127 @manupa-arm moved the Zephyr portions to #6789 . will update this one after that merges
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510372195
##########
File path: apps/microtvm-vm/vm/rebuild-tvm.sh
##########
@@ -0,0 +1,33 @@
+#!/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 -e
+
+cd "$(dirname $0)"
+cd "$(git rev-parse --show-toplevel)"
+BUILD_DIR=build-microtvm
+
+if [ ! -e "${BUILD_DIR}" ]; then
+ mkdir "${BUILD_DIR}"
+fi
+cp cmake/config.cmake "${BUILD_DIR}"
+cd "${BUILD_DIR}"
+sed -i 's/USE_MICRO OFF/USE_MICRO ON/' config.cmake
+sed -i 's/USE_LLVM OFF/USE_LLVM ON/' config.cmake
Review comment:
the C target is still enabled here. this allows the user to also use the `llvm` backend
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] areusch commented on a change in pull request #6703: [µTVM] Add virtual machine, test zephyr runtime on real hardware
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #6703:
URL: https://github.com/apache/incubator-tvm/pull/6703#discussion_r510460548
##########
File path: apps/microtvm-vm/base-box/setup-tvm-user.sh
##########
@@ -0,0 +1,39 @@
+#!/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.
+
+# Zephyr
+pip3 install --user -U west
+echo 'export PATH=$HOME/.local/bin:"$PATH"' >> ~/.profile
+source ~/.profile
+echo PATH=$PATH
+west init --mr v2.4.0 ~/zephyr
+cd ~/zephyr
+west update
+west zephyr-export
+
Review comment:
i've thought about this some more and I'd like to actually consider this VM to be Zephyr-specific. I'd rather allow platforms the freedom to integrate in the way that makes sense for them. I'll update the path to this VM to be `apps/microtvm-vm/zephyr/base-box`, and provide a `README.md` explaining about the subdirectories in this tree.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org