-
Notifications
You must be signed in to change notification settings - Fork 21
refactor: add amt enable/disable functionality to CLI #1094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
Unit tests (Go)837 tests +23 837 ✅ +23 2s ⏱️ ±0s Results for commit 875b3d2. ± Comparison against base commit 3a1aca6. This pull request removes 3 and adds 26 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
Functionality check(AMT 21.0.2) $ sudo ./rpc-v1 amtinfo $ sudo ./rpc-v3 configure enable-amt user@localhost:~/shradha$ sudo ./rpc-v3 configure disable-amt $ sudo ./rpc-v3 amtinfo $ sudo ./rpc-v3 configure enable-amt |
1038049 to
31b25d8
Compare
5ede741 to
31b25d8
Compare
|
Seeing this as well: |
65af3c0 to
caabded
Compare
|
aliases for enable & disable is changed as discussed. user@localhost: |
sudhir-intc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a code review, but need to test the usage. Will provide further feedback after doing the testing
|
I see these multiple logs, please remove the duplicate one |
875b3d2 to
315bd41
Compare
|
@sudhir-intc incorporated the code changes shared by you for log updates. Pls review. |
|
|
||
| // Step 2: Attempt to use MHC_SetAmtOperationalState to enable AMT | ||
| if err := ctx.AMTCommand.EnableAMT(); err != nil { | ||
| return fmt.Errorf("failed to enable AMT; retry after resetting device: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitpick: I’ve seen customers ask what “reset” means. Would it be clearer to say “restarting the device” (or “rebooting the device”) instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified code to display "reboot the device" message
4857cab to
0fc7405
Compare
- Modify rpc configure to support amt enable & disable features using SetAmtOperationalState HECI command - Added test files for enableamt.go & disableamt.go - Modified relevant files for a more readable function names addresses : #1002 Signed-off-by: ShradhaGupta31 <shradha.gupta@intel.com>
addresses : #1002
Implementation results:
Note: After disabling AMT to enable it back OS refresh is needed