On Thu, Jan 20, 2022 at 4:52 AM Varad Gautam <varad.gautam@xxxxxxxx> wrote:
>
> Make x86/efi/run check for AMDSEV envvar and set SEV/SEV-ES parameters
> on the qemu cmdline.
>
> 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 | 16 ++++++++++++++++
> 2 files changed, 21 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..b48f626 100755
> --- a/x86/efi/run
> +++ b/x86/efi/run
> @@ -43,6 +43,21 @@ fi
> mkdir -p "$EFI_CASE_DIR"
> cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
>
> +amdsev_opts=
> +if [ -n "$AMDSEV" ]; then
> + policy=
> + if [ "$AMDSEV" = "sev" ]; then
> + policy="0x1"
> + elif [ "$AMDSEV" = "sev-es" ]; then
> + policy="0x5"
Rather than bare constants, I think we should define these as bit
fields. Something like:
$ git diff
diff --git a/x86/efi/run b/x86/efi/run
index b48f626b0435..427865807720 100755
--- a/x86/efi/run
+++ b/x86/efi/run
@@ -19,6 +19,10 @@ source config.mak
: "${EFI_SMP:=1}"
: "${EFI_CASE:=$(basename $1 .efi)}"
+# Define guest policy bits, used to form QEMU command line.
+readonly SEV_POLICY_NODBG=$(( 1 << 0 ))
+readonly SEV_POLICY_ES=$(( 1 << 2 ))
+
if [ ! -f "$EFI_UEFI" ]; then
echo "UEFI firmware not found: $EFI_UEFI"
echo "Please install the UEFI firmware to this path"
@@ -47,9 +51,9 @@ amdsev_opts=
if [ -n "$AMDSEV" ]; then
policy=
if [ "$AMDSEV" = "sev" ]; then
- policy="0x1"
+ policy="$(( $GUEST_POLICY_NODBG ))"
elif [ "$AMDSEV" = "sev-es" ]; then
- policy="0x5"
+ policy="$(( $GUEST_POLICY_NODBG | $GUEST_POLICY_ES ))"
else
echo "Cannot set AMDSEV policy. AMDSEV must be one of
'sev', 'sev-es'."
exit 2
> + 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=51,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 +76,5 @@ cp "$EFI_SRC/$EFI_CASE.efi" "$EFI_CASE_BINARY"
> -nographic \
> -m 256 \
> "$@" \
> + $amdsev_opts \
> -smp "$EFI_SMP"
> --
> 2.32.0
>
We don't have QEMU working on our internal setup because it doesn't
support INIT_EX. So I wasn't able to test this patch in its entirety.
But this patch seems pretty reasonable, independent of the rest of the
series. Depending on maintainer review bandwidth, it might make sense
to send this patch on its own (with Tom's feedback incorporated),
outside of the series, so it can get merged quicker.
