Hi Marc,
On 2/13/22 4:48 AM, Marc Orr wrote:
> On Wed, Feb 9, 2022 at 8:43 AM Varad Gautam <varad.gautam@xxxxxxxx> wrote:
>>
>> Make x86/efi/run check for AMDSEV envvar and set corresponding
>> SEV/SEV-ES parameters on the qemu cmdline, to make it convenient
>> to launch SEV/SEV-ES tests.
>>
>> Since the C-bit position depends on the runtime host, fetch it
>> via cpuid before guest launch.
>>
>> AMDSEV can be set to `sev` or `sev-es`.
>>
>> Signed-off-by: Varad Gautam <varad.gautam@xxxxxxxx>
>> ---
>> x86/efi/README.md | 5 +++++
>> x86/efi/run | 38 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/x86/efi/README.md b/x86/efi/README.md
>> index a39f509..1222b30 100644
>> --- a/x86/efi/README.md
>> +++ b/x86/efi/README.md
>> @@ -30,6 +30,11 @@ the env variable `EFI_UEFI`:
>>
>> EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/msr.efi
>>
>> +To run the tests under AMD SEV/SEV-ES, set env variable `AMDSEV=sev` or
>> +`AMDSEV=sev-es`. This adds the desired guest policy to qemu command line.
>> +
>> + AMDSEV=sev-es EFI_UEFI=/path/to/OVMF.fd ./x86/efi/run ./x86/amd_sev.efi
>> +
>> ## Code structure
>>
>> ### Code from GNU-EFI
>> diff --git a/x86/efi/run b/x86/efi/run
>> index ac368a5..9bf0dc8 100755
>> --- a/x86/efi/run
>> +++ b/x86/efi/run
>> @@ -43,6 +43,43 @@ fi
>> mkdir -p "$EFI_CASE_DIR"
>> cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
>>
>> +amdsev_opts=
>> +if [ -n "$AMDSEV" ]; then
>> + # Guest policy bits, used to form QEMU command line.
>> + readonly AMDSEV_POLICY_NODBG=$(( 1 << 0 ))
>> + readonly AMDSEV_POLICY_ES=$(( 1 << 2 ))
>> +
>> + gcc -x c -o getcbitpos - <<EOF
>> + /* CPUID Fn8000_001F_EBX bits 5:0 */
>> + int get_cbit_pos(void)
>> + {
>> + int ebx;
>> + __asm__("mov \$0x8000001f , %eax\n\t");
>> + __asm__("cpuid\n\t");
>> + __asm__("mov %%ebx, %0\n\t":"=r" (ebx));
>> + return (ebx & 0x3f);
>> + }
>> + int main(void)
>> + {
>> + return get_cbit_pos();
>> + }
>> +EOF
>
> We could do this in bash directly, using the cpuid driver:
> https://man7.org/linux/man-pages/man4/cpuid.4.html
>
> I'm not a Linux shell wizard, but I found an example of a script using
> this module here:
> https://git.irsamc.ups-tlse.fr/dsanchez/spectre-meltdown-checker/src/branch/master/spectre-meltdown-checker.sh
>
> After studying that script (for like an hour, lol), I was able to
> extract the cbit position. Below, I explain how to do this with the
> cpuid driver. My only complaint about using the cpuid driver is that
> it's awfully hard to follow. So I'd be OK to stick with the C code
> that you've got. Though it may be better to break out the C code into
> an actual .c file, rather than embed it in the script like this, and
> magically build it and clean it up, which seems pretty hacky. I know I
> was doing something similar with a dummy.c file embedded in the run
> script file to get the run_tests.sh script to work, and Paolo ended up
> moving that into an explicit build target in the x86/ directory.
>
> Getting the c bit position in pure bash, using the cpuid driver.
> $ ebx=$(dd if=/dev/cpu/0/cpuid bs=16 skip=134217729 count=16
> 2>/dev/null | od -j 240 -An -t u4 | awk '{print $'"2"'}')
> $ echo $(( $ebx&0x3f ))
>
Tom also suggested magic using the cpuid module earlier [1], but I would
like to avoid a dependency on CONFIG_X86_CPUID here. Besides the readability
of the C snippet, I believe gcc (ie userspace) is easier to install on a set
of test hosts already prepared with CONFIG_X86_CPUID=n, than to
enable/deploy/install the cpuid kmod there, which becomes important when
testing a large number of hosts at once.
Unlike x86/dummy.c, the C code doesn't run in a guest context, which is why
I'm hesitant to place it as a build target under x86/. I "hid" it within
the run script since it's only needed when constructing the qemu cmdline,
and packaging a 'getcbitpos' binary didn't make much sense. Thoughts?
[1] https://lore.kernel.org/kvm/1a79ea5b-71dd-2782-feba-0d733f8c2fbf@xxxxxxx/
Thanks,
Varad
> Breaking it down:
>
> # Use dd to read the 0x8000001f leaf via the cpuid driver:
> # bs=16: block size of 16 bytes; required by the driver per it's man page
> # skip=134217729: This is the CPUID leaf, 0x8000001f as a decimal number,
> # divided by the block size
> # count=16: We actually only want to read a count=1 16 byte block
> # because {eax, ebx, ecx, edx} is a single 16 byte block.
> However, our CPUID leaf,
> # 0x8000001f, doesn't divide evenly by 16. It has a remainder of
> 15. So read the
> # previous 15 sixteen-byte blocks, plus the block we actually want to read.
> $ dd if=/dev/cpu/0/cpuid bs=16 skip=134217729 count=16 2>/dev/null
>
> # Use od to convert the binary data returned by the cpuid driver into ascii.
> # -j 240: Skip the first 15 sixteen-byte blocks that we only read to
> appease the 16 byte block size. (15 * 16 = 240).
> # -An: Don't label the output with indexes.
> # -t u4: Output the data as 4-byte unsigned decimal #'s.
> od -j 240 -An -t u4
>
> The od command above outputs the four CPUID values {eax, ebx, ecx,
> edx}. On my machine:
> 65551 367 509 100
>
> Finally, use awk to take the second one -- ebx. And then take the
> lower 6 bits for the c-bit position.
>
>> +
>> + cbitpos=$(./getcbitpos ; echo $?) || rm ./getcbitpos
>> + policy=
>> + if [ "$AMDSEV" = "sev" ]; then
>> + policy="$(( $AMDSEV_POLICY_NODBG ))"
>> + elif [ "$AMDSEV" = "sev-es" ]; then
>> + policy="$(( $AMDSEV_POLICY_NODBG | $AMDSEV_POLICY_ES ))"
>> + else
>> + echo "Cannot set AMDSEV policy. AMDSEV must be one of 'sev', 'sev-es'."
>> + exit 2
>> + fi
>> +
>> + amdsev_opts="-object sev-guest,id=sev0,cbitpos=${cbitpos},reduced-phys-bits=1,policy=${policy} \
>> + -machine memory-encryption=sev0"
>> +fi
>> +
>> # Run test case with 256MiB QEMU memory. QEMU default memory size is 128MiB.
>> # After UEFI boot up and we call `LibMemoryMap()`, the largest consecutive
>> # memory region is ~42MiB. Although this is sufficient for many test cases to
>> @@ -61,4 +98,5 @@ cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
>> -nographic \
>> -m 256 \
>> "$@" \
>> + $amdsev_opts \
>> -smp "$EFI_SMP"
>> --
>> 2.34.1
>>
>
