search

enarx-archive / sevctl

Administrative utility for AMD SEV
Apache License 2.0
16 stars 5 forks source link

Implement 'ok' subcommand #61

Closed tylerfanelli closed 3 years ago

tylerfanelli commented 3 years ago

Initial draft for the 'ok' subcommand, which probes the processor, sysfs, and KVM for SEV enablement and support. I've yet to test on SEV hardware, so some details could change this week. I'd just like some initial thoughts/critiques on code quality. Also, if there are additional features anyone would like to see added, feel free to reach out.

connorkuehl commented 3 years ago

Your first commit subject doesn't share the same imperative voice that your other commits do. Please update it to something like "Add initial boilerplate for sevctl-ok"

Right now, all of the results are flattened, and there's no way to understand which tests depend on which tests. I would suggest arranging the tests in a tree like structure so that you can update the code that prints the tests and their results to be formatted according to their dependencies. This makes it easier to change how the results are displayed as opposed to a line, a blank line, ... etc

Just for example:

[ OK ] AMD CPU
[ OK ] - AMD SEV
[ OK ]   - /dev/sev readable
[ OK ]   - /dev/sev writable
[ OK ]   - SEV enabled in kernel
...

and so on, so forth.

Or

[ FAIL ] AMD CPU
[ SKIP ] - AMD SEV
[ SKIP ]   - /dev/sev readable
...
connorkuehl commented 3 years ago

Also: don't forget to document this new feature.

We generate the README with cargo-readme. This means you have to update the crate doc comment in src/main.rs. Then you run cargo readme > README.md and commit the change.

You'll have to cargo install cargo-readme