Added additional SNP checks on the host and guest via verification of SNP bit status from instruction set #12
Conversation
LakshmiSaiHarika
commented
Jun 26, 2024
LakshmiSaiHarika
commented
- Added check on the host for all host SEV features in CPU via cpuid 0x8000001f instruction set
- Added additional check on the guest to see if guest sev, sev-es, and snp bits are active from MSR 0xc0010131 (MSR_AMD64_SEV)
There was a problem hiding this comment.
Will you please run these through shellcheck? There are a number of changes which I would like to see applied for this PR before we approve it.
|
Hi @larrydewey, I made few changes and did shellcheck... Please let me know if any further changes are required. |
LakshmiSaiHarika
force-pushed
the
add-snp-check-via-instruction-set
branch
from
4ba7408
to
4ff4dcd
Compare
|
Hi @larrydewey I addressed the changes as per the conversation above, please let me know if any additional changes are required |
docs/snp.md
Outdated
| Read the dedicated host cpuid Fn8000_001F[EAX] instruction set to verify if the SNP is on and supported on the host: | ||
| ``` | ||
| ./snp.sh check-host-snp-cpuid | ||
| ``` | ||
|
|
||
|
|
There was a problem hiding this comment.
Do we want this to be it's own individual command? I see that it is being called when people are setting up the host and launching the guest. Is there additional functionality to be it's own individual command?
tools/snp.sh
Outdated
| local host_cpuid_eax | ||
| host_cpuid_eax=$(cpuid -1 -r -l 0x8000001f | grep -oE "eax=[[:alnum:]]+ " | cut -c5- | tr -d '[:space:]') | ||
|
|
There was a problem hiding this comment.
I had created a function that does this for you called get_cpuid if you want to use that instead.
Line 933 in 4419036
tools/snp.sh
Outdated
| declare -A actual_sev_snp_bit_status=( | ||
| [SME]=$((${host_cpuid_eax} & 1)) | ||
| [SEV]=$((${host_cpuid_eax} & 2)) | ||
| [SEV-ES]=$((${host_cpuid_eax} & 8)) | ||
| [SNP]=$((${host_cpuid_eax} & 16)) | ||
| ) | ||
|
|
||
| # Map all the expected host sev/snp bit values in a single associative array | ||
| declare -A expected_sev_snp_bit_value=( | ||
| [SME]=1 | ||
| [SEV]=2 | ||
| [SEV-ES]=8 | ||
| [SNP]=16 | ||
| ) |
There was a problem hiding this comment.
You could just grab the specific bit for each of the features and that way you don't have to create two different arrays.
tools/snp.sh
Outdated
| declare -A actual_sev_snp_bit_status=( | ||
| [SEV]=$((${guest_msr_read} & 1)) | ||
| [SEV-ES]=$((${guest_msr_read} & 2)) | ||
| [SNP]=$((${guest_msr_read} & 4)) | ||
| ) | ||
|
|
||
| declare -A expected_sev_snp_bit_value=( | ||
| [SEV]=1 | ||
| [SEV-ES]=2 | ||
| [SNP]=4 | ||
| ) |
There was a problem hiding this comment.
Same thing as above. You can just get the specific bit for each feature and that way you don't need 2 different arrays.
| @@ -1228,6 +1325,7 @@ main() { | |||
|
|
|||
| setup_and_launch_guest | |||
| wait_and_retry_command verify_snp_guest | |||
| wait_and_retry_command verify_platform_snp_bit_status guest | |||
There was a problem hiding this comment.
This should maybe replace verify_snp_guest. They are essentially doing the same thing, the only difference is that the original function is checking dmesg, while the bit_status is looking at the guest MSR to verify enablement. No need to run both of them if the MSR check is working correctly.
…able Read the dedicated host cpuid 0x8000001f instruction set to check if SNP is on and supported Signed-off-by: Harika <[email protected]> Add shift operator for SNP host check Signed-off-by: Harika Nittala <[email protected]>
This check reads the dedicated guest MSR(MSR_AMD64_SEV, MSR 0xc0010131) to determine if SNP is on and supported. Signed-off-by: Harika Nittala <[email protected]>
LakshmiSaiHarika
force-pushed
the
add-snp-check-via-instruction-set
branch
from
4ff4dcd
to
699d8b1
Compare
