| Message ID | 20220209164254.8664-1-varad.gautam@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v2] x86/efi: Allow specifying AMD SEV/SEV-ES guest launch policy to run | expand |
On Wed, Feb 9, 2022 at 8:43 AM Varad Gautam <varad.gautam@suse.com> 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@suse.com> > --- > 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 )) 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 >
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@suse.com> 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@suse.com> >> --- >> 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@amd.com/ 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 >> >
On Mon, Feb 14, 2022 at 5:34 AM Varad Gautam <varad.gautam@suse.com> wrote: > > 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@suse.com> 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@suse.com> > >> --- > >> 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@amd.com/ > > Thanks, > Varad Ah. Got it now. I had forgotten about Tom's reply. And you make good points about the cpuid binary this patch is building being unique from other binaries in that it does not run under the guest... Also, I agree with minimizing dependencies on the machine under test. If we go with the current approach, is there any reason not to just split out the C code into a standalone .c file? Also, any reason not to build it when we build the rest of the x86 binaries? I agree it should not reside in the x86/ directory, since it is not a guest-side program as you mentioned. But I'm wondering if we can compile it in advance of running the test, rather than while running the test.
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 + + 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"
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@suse.com> --- x86/efi/README.md | 5 +++++ x86/efi/run | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+)