Skip to content

ShellPkg: Add rdmsr Shell Command for x86 MSR Readback#12432

Open
RaajKumar-code wants to merge 1 commit intotianocore:masterfrom
RaajKumar-code:shellpkg-rdmsr-command
Open

ShellPkg: Add rdmsr Shell Command for x86 MSR Readback#12432
RaajKumar-code wants to merge 1 commit intotianocore:masterfrom
RaajKumar-code:shellpkg-rdmsr-command

Conversation

@RaajKumar-code
Copy link
Copy Markdown

Description

This PR adds a new Shell Level 3 built-in command, rdmsr, to read x86 Model Specific Registers (MSRs) from the UEFI Shell.

Why this change was made:

  • Provides a simple built-in command for low-level MSR inspection during firmware debugging and validation.
  • Aligns command naming with the x86 RDMSR instruction for clarity.

Scope of changes:

  • Added command implementation for parsing the MSR index and printing 64-bit MSR values.
  • Registered the command in the Shell Level 3 command library.
  • Added required declaration and build integration entries.
  • Added help and output strings.

Impact:

  • Breaking change?
    • No. This adds a new command and does not change existing command behavior, build flow, or boot behavior.
  • Impacts security?
    • No direct security impact. It is a diagnostic/read command with architecture guards.
  • Includes tests?
    • No explicit new test code

How This Was Tested

Ran PatchCheck on the latest commit and it passed
Built and verified integration in local EDK II workflow.
Manually validated command behavior in shell:

  • Help output for rdmsr.
  • Valid invocation with a known MSR index (for example, 0x10).
  • Invalid/missing parameter handling.

Integration Instructions

N/A

Signed-off-by: raajkumars <raajkumars@ami.com>
@pierregondois
Copy link
Copy Markdown
Contributor

Hello,
I assume that UEFI shell commands are supposed to me arch agnostics, so it doesn't seem than an rdmsr could be added to the UEFI Shell spec. Also, dynamically reading an unknown register would not work for arm64, we would need to have a set of supported registers first.

Even though having the ability to read some registers from the uefi shell seems useful, adding new commands should require more discussions with the edk2 stewards and spec. owners.

(not requesting any change, but the ability to read specific registers seems more related to debugging, so maybe the UefiShellDebug1CommandsLib might be more appropriate than Level3)

@makubacki
Copy link
Copy Markdown
Member

My concern is that we start accumulating a lot of code to maintain for simple operations that have historically been performed in more flexible places. For example, the use case given is "firmware debugging and validation". MSRs can be read at any point in boot with a software/hardware debugger (when debugging) and validation has used EFI applications in shell to read MSRs.

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