Skip to content

Add std-only examples and polish doc for main#144

Merged
DemesneGH merged 7 commits intoapache:mainfrom
DemesneGH:main
Oct 25, 2024
Merged

Add std-only examples and polish doc for main#144
DemesneGH merged 7 commits intoapache:mainfrom
DemesneGH:main

Conversation

@DemesneGH
Copy link
Copy Markdown
Contributor

  • Add examples: serde, message_passing_interface, tcp_client, udp_socket, tls_client, tls_server
  • Add test scripts and CI for those examples
  • Add migration guide from master branch
  • Polish README in main page

Comment thread docs/migrating-to-new-building-env.md Outdated

### Step 1: Migrate Old Code to the New Environment

To migrate an example project (e.g., `tls_server-rs` in `examples/`), you
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current migration strategy seems to ask developers to duplicate the hello-world example and manually copy their source code into the corresponding subdirectories. However, this approach doesn't clearly highlight what has actually changed and what developers need to focus on during the migration.

The target audience for this section is developers currently using the master branch who have their own code repositories. The key point we need to emphasize is that no code changes are required during the migration—only the build scripts need to be updated.

The message we want to convey is simple: developers should be aware that the build process has changed compared to the legacy master branch, and the build scripts will need to be replaced. Could we clearly outline the differences in the build scripts (e.g. Cargo.toml, build.rs, Makefile, etc), step-by-step, and perhaps reference a specific commit for further clarification? This way, developers can focus on adapting their build environment without being concerned about unnecessary code modifications.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, we should rather be describing the build environment for the main branch as to how it's structured. This will help developers writing new TAs or updating existing TAs to the build environment on main branch.

A few words for each file under following directory structure should be sufficient:

examples/acipher-rs/
├── host
│   ├── Cargo.toml
│   ├── Makefile
│   └── src
│       └── main.rs
├── Makefile
├── proto
│   ├── build.rs
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── ta
│   ├── build.rs
│   ├── Cargo.toml
│   ├── Makefile
│   ├── src
│   │   └── main.rs
│   ├── ta_static.rs
│   └── Xargo.toml
└── uuid.txt

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc updated, thanks for all comments

CROSS_COMPILE_HOST=$(CROSS_COMPILE_HOST)
$(q)make -C ta TARGET_TA=$(TARGET_TA) \
CROSS_COMPILE_TA=$(CROSS_COMPILE_TA)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that at the top level, TARGET_HOST and TARGET_TA are used to differentiate between the build parameters for the host and trusted application (TA), which makes sense. However, at the sub-level (host/Makefile and ta/Makefile), it may be unnecessary to keep differentiating these parameters. The sub-level Makefiles could rely on the variables passed down from the top-level Makefile and use more generic variable names, such as TARGET and CROSS_COMPILE, similar as the @CROSS_COMPILE usage for building ring.

Instead of maintaining the separate TARGET_HOST and TARGET_TA at each level, we could streamline the build process by using:

@make -C host TARGET=$(TARGET_HOST) CROSS_COMPILE=$(CROSS_COMPILE_HOST)
@make -C ta TARGET=$(TARGET_TA) CROSS_COMPILE=$(CROSS_COMPILE_TA)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sub-level Makefiles could rely on the variables passed down from the top-level Makefile and use more generic variable names, such as TARGET and CROSS_COMPILE, similar as the @CROSS_COMPILE usage for building ring.

That makes sense because the CROSS_COMPILE is needed for building inner C libraries for TA. But I have some consideration for independent building.
Keeping the CROSS_COMPILE_{HOST, TA} suffix helps to build CA or TA independently. The scenario is common, like the CA is finished and we're developing TA and willing to rebuild it.

Currently, the CROSS_COMPILE_{HOST, TA} is set by source environment on env setup stage of our SDK (https://github.com/DemesneGH/rust-optee-trustzone-sdk/blob/a84e96d4199289844285d545ac5916fb35f1e9fb/environment) .
After setup we can build only TA for one example: e.g. run make -C examples/hello_world-rs/ta.

If changed to this:

@make -C host TARGET=$(TARGET_HOST) CROSS_COMPILE=$(CROSS_COMPILE_HOST)
@make -C ta TARGET=$(TARGET_TA) CROSS_COMPILE=$(CROSS_COMPILE_TA)

The example/*/ta/Makefile needs to read CROSS_COMPILE set by example/*/Makefile.
If we want to build TA only, we have to build the entire example (using example/*/Makefile) to set correct CROSS_COMPILE, or set the CROSS_COMPILE explicitly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make host, make ta should be the recommended way for independent building.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread examples/message_passing_interface-rs/Makefile
Comment thread examples/message_passing_interface-rs/ta/ta_static.rs Outdated
- remove wildcard imports: "use core::ffi::*;"
- since "c_size_t" is an unstable feature for current Rustc
  (2024-05-14), replace them with "usize", which is equivalent
  on ARM32 and ARM64.
@DemesneGH DemesneGH force-pushed the main branch 2 times, most recently from d57ad65 to 3ccfadc Compare October 18, 2024 05:44
OUT_DIR := $(CURDIR)/target/$(TARGET)/release


all: host strip
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's a client towards STD TA, there is no value in building it without ${STD} enabled, so we should gate it as well as we do for STD TAs. This holds true for other STD TAs clients as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Updated in new commits, thanks for your comment

Comment thread examples/serde-rs/ta/src/main.rs Outdated

#![no_main]
// this is the workaround for the error:
// error[E0658]: use of unstable library feature 'c_size_t'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment redundant now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in new commit

@b49020
Copy link
Copy Markdown
Contributor

b49020 commented Oct 18, 2024

Apart from few comments above, the overall cleanup looks good to me.

Host and TA read `TARGET` and `CROSS_COMPILE` from the top level
Makefile of the example. That meets the env need of building inner
C libraries such as ring for TA.
Run `make -C examples/xxxx ta` will build TA independently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants