Measure the Cost of Traps, World Switches, and Misaligned Operat…

[francois141]

…ions with the Protect Payload Policy

This commit extends the tracing firmware with three new latency measurements in the Protect Payload policy and ensures that no additional performance losses are introduced.

[francois141]

@CharlyCst Let's first merge this pr and then the other. So we can see the cycle gain for the decoder

[CharlyCst]

Thanks for the changes, it's cool to have more stats :)

So turns out tracing_firmware and tracing_firmware_protect_payload are basically the same file at the exception of the name of the policy according to diff:

➜  firmware git:(851f56c5) ✗ diff tracing_firmware/main.rs tracing_firmware_protect_payload/main.rs
36c36
<         "Default",
---
>         "Protect",
74c74
<         "Default",
---
>         "Protect",
80c80
<         "Default",
---
>         "Protect",

But can simply read the policy name from the config, as we do in Miralis:

pub const POLICY_NAME: &str = parse_str_or(option_env!("MIRALIS_POLICY_NAME"), "default_policy");

(if the firmware don't have access to it then we should fix that and expose the environment variables too)

Using this we can simply have a single tracing firmware (and so we don't need the library anymore), which will make things much easier to maintain in the long run :)

[CharlyCst]

I am writing some comments here, but as explained in the main review feedback we can simply get rid of this library by having a single tracing firmware and keeping things simple.

So the comments on this file are simply informational.

[CharlyCst]

This is a copy paste from another library, it doesn't explain what this library is doing nor how it should be used. The current doc comment might cause confusion and make the crate less maintainable.

[CharlyCst]

This macro uses locally-defined function, so it will fail with a compile error when used by another crate or module. There is a simple way to fix this, which is to use an absolute path for each local function used. There is a magic variable $crate to do that:

let stats = $crate::get_statistics(values)

With the above version the macro can be used without importing the get_statistics function. In general macros in libraries should encapsulate all of their import so that they work like standard functions.

[CharlyCst]

In libraries we don't want to insert global assembly, because it can override existing symbols and cause at best linker errors, at worst arbitrarily bad things.

[francois141]

@CharlyCst Thank you for all the nice and relevants comments. I did the changes. Feel free to review again ;)

[CharlyCst]

Thanks for the changes, looks good to me :)