DemesneGH commented on code in PR #245: URL: https://github.com/apache/teaclave-trustzone-sdk/pull/245#discussion_r2476294732
########## cargo-optee/src/main.rs: ########## @@ -0,0 +1,158 @@ +// 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. + +use clap::{Parser, Subcommand}; +use std::env; +use std::path::PathBuf; +use std::process::abort; + +mod ca_builder; +mod common; +mod ta_builder; + +use common::Arch; + +#[derive(Debug, Parser)] +#[clap(version = env!("CARGO_PKG_VERSION"))] +#[clap(about = "Build tool for OP-TEE Rust projects")] +pub(crate) struct Cli { + #[clap(subcommand)] + cmd: Command, +} + +#[derive(Debug, Parser)] +struct BuildTypeCommonOptions { + /// Path to the app directory (default: current directory) + #[arg(long = "path", default_value = ".")] + path: PathBuf, + + /// Target architecture: aarch64 or arm (default: aarch64) + #[arg(long = "arch", default_value = "aarch64")] + arch: Arch, + + /// Path to the UUID file (default: ../uuid.txt) + #[arg(long = "uuid_path", default_value = "../uuid.txt")] + uuid_path: PathBuf, + + /// Build in debug mode (default is release) + #[arg(long = "debug")] + debug: bool, +} + +#[derive(Debug, Subcommand)] +enum BuildCommand { + /// Build a Trusted Application + TA { + /// Enable std feature for the TA + #[arg(long = "std")] + std: bool, + + /// Path to the TA dev kit directory (mandatory) + #[arg(long = "ta_dev_kit_dir", required = true)] + ta_dev_kit_dir: PathBuf, + + /// Path to the TA signing key (default: $(TA_DEV_KIT_DIR)/keys/default_ta.pem) + #[arg(long = "signing_key")] + signing_key: Option<PathBuf>, + + #[command(flatten)] + common: BuildTypeCommonOptions, + }, + /// Build a Client Application (Host) + CA { + /// Path to the OP-TEE client export directory (mandatory) + #[arg(long = "optee_client_export", required = true)] + optee_client_export: PathBuf, + + #[command(flatten)] + common: BuildTypeCommonOptions, + }, +} + +#[derive(Debug, Subcommand)] +enum Command { Review Comment: I've reviewed the `cargo-sgx` code snippet — although it reads metadata, it still accepts parameters via command line with default values. I assume you're suggesting we should move some params to `[package.metadata]` in `Cargo.toml` instead of passing them as command-line arguments? I actually mentioned the idea in the initial message of this PR, but there are a few considerations. Let's go through each: #### `arch` / `std` We should allow building the same app for different arch and modes. If these values are hardcoded in `Cargo.toml`, we lose the flexibility. For example, in CI: > `cargo-trustzone` should automatically infer whether to build with `std` based on the feature declared in `Cargo.toml`. But what should we write in the common examples’ `Cargo.toml`? If we set `"no-std"`, how would we build it in `std` mode for CI? by modifying `Cargo.toml` each time? I don't have the answer. Therefore, I believe **`arch`** and **`std`** should remain as **command-line parameters**, each with default values. #### `ta_dev_kit_dir` Seems this might suitable for `Cargo.toml`, but it’s different for `arch` — for example: ``` optee_os/out/arm-plat-vexpress/export-ta_arm64 optee_os/out/arm-plat-vexpress/export-ta_arm32 ``` If we put it in `Cargo.toml`, we’d have to edit that file each time we change the build architecture. Maybe try this? ``` [metadata] ta_dev_kit.64 = "xxxx_export-ta_arm64" ta_dev_kit.32 = "xxxx_export-ta_arm32" ``` use the different entry decided by cmd param `arch`. #### `uuid_path` (or `uuid`) Instead of storing a `uuid_path`, it may make more sense to store a `uuid` string directly in the metadata. Since `uuid.txt` is both used by TA build tools and read by the proto (and imported by the CA when invoking the TA). We could try to write into the Cargo.toml, but to which Cargo.toml? Maybe we should have a workspace? I’m still thinking about it. #### signing_key It makes sense to write into the Cargo.toml. ---- > and seamless integration of loadable trusted components with untrusted applications Does it mean the plugin if in optee? I don't really get the point. > Similarly, `cargo-trustzone` should support automatic discovery of the default `ta_dev_kit_dir` (e.g., from an environment variable, configuration file, or standard installation path) to provide a smooth and familiar developer experience consistent with the normal cargo workflow. Consider the scenario: - OP-TEE OS developers export the ta_dev_kit, and send it to the TA developer - TA developer receives it and build the TA For this tool, I want it less dependent with the environment, for production use cases or experienced developers, the TA developers typically only need a toolchain setup and already know the location of their ta_dev_kit; with those information they can build the TA directly. Additionally, I plan to provide a helper script for the dev/emulation env for easy use, together with preparing the OP-TEE libraries. ------ Please feel free to share your suggestions, thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
