mbox series

[kvm-unit-tests,v3,00/17] x86_64 UEFI and AMD SEV/SEV-ES support

Message ID 20211004204931.1537823-1-zxwang42@gmail.com (mailing list archive)
Headers show
Series x86_64 UEFI and AMD SEV/SEV-ES support | expand

Message

Zixuan Wang Oct. 4, 2021, 8:49 p.m. UTC
Hello,

This patch series updates the x86_64 KVM-Unit-Tests to run under UEFI
and culminates in enabling AMD SEV/SEV-ES. The patches are organized as
four parts.

The first part (patch 1) refactors the current Multiboot start-up code
by converting assembly data structures into C. This enables the
follow-up UEFI patches to reuse these data structures without redefining
or duplicating them in assembly.

The second part (patches 2-3) copies code from Varad's patch set [1]
that builds EFI stubs without depending on GNU-EFI. Part 3 and 4 are
built on top of this part.

The third part (patches 4-11) enables the x86_64 test cases to run
under UEFI. In particular, these patches allow the x86_64 test cases to
be built as EFI executables and take full control of the guest VM. The
efi_main() function sets up the KVM-Unit-Tests framework to run under
UEFI and then launches the test cases' main() functions. To date, we
have 38/43 test cases running with UEFI using this approach.

The fourth part of the series (patches 12-17) focuses on SEV. In
particular, these patches introduce SEV/SEV-ES set up code into the EFI
set up process, including checking if SEV is supported, setting c-bits
for page table entries, and (notably) reusing the UEFI #VC handler so
that the set up process does not need to re-implement it (a test case
can always implement a new #VC handler and load it after set up is
finished). Using this approach, we are able to launch the x86_64 test
cases under SEV-ES and exercise KVM's VMGEXIT handler.

Note, a previous feedback [3] indicated that long-term we'd like to
instrument KVM-Unit-Tests with it's own #VC handler. However, we still
believe that the current approach is good as an intermediate solution,
because it unlocks a lot of testing and we do not expect that testing
to be inherently tied to the UEFI's #VC handler. Rather, test cases
should be tied to the underlying GHCB spec implemented by an
arbitrary #VC handler.

See the Part 1 to Part 4 summaries, below, for a high-level breakdown
of how the patches are organized.

Part 1 Summary:
Commit 1 refactors boot-related data structures from assembly to C.

Part 2 Summary:
Commits 2-3 copy code from Varad's patch set [1] that implements
EFI-related helper functions to replace the GNU-EFI library.

Part 3 Summary:
Commits 4-5 introduce support to build test cases with EFI support.

Commits 6-10 set up KVM-Unit-Tests to run under UEFI. In doing so, these
patches incrementally enable most existing x86_64 test cases to run
under UEFI.

Commit 11 fixes several test cases that fail to compile with EFI due
to UEFI's position independent code (PIC) requirement.

Part 4 Summary:
Commits 12-13 introduce support for SEV by adding code to set the SEV
c-bit in page table entries.

Commits 14-16 introduce support for SEV-ES by reusing the UEFI #VC
handler in KVM-Unit-Tests. They also fix GDT and IDT issues that occur
when reusing UEFI functions in KVM-Unit-Tests.

Commit 17 adds additional test cases for SEV-ES.

Changes V2 -> V3:
V3 Patch #  Changes
----------  -------
     01/17  (New patch) refactors assembly data structures in C
     02/17  Adds a missing alignment attribute
            Renames the file uefi.h to efi.h
     03/17  Adds an SPDX header, fixes a comment style issue
     06/17  Removes assembly data structure definitions
     07/17  Removes assembly data structure definitions
     12/17  Simplifies an if condition code
     14/17  Simplifies an if condition code
     15/17  Removes GDT copying for SEV-ES #VC handler

Notes on page table set up code:
Paolo suggested unifying  the page table definitions in cstart64.S and
UEFI start-up code [5]. We tried but found it hard to implement due to
the real/long mode issue: a page table set up function written in C is
by default compiled to run in long mode. However, cstart64.S requires
page table setup before entering long mode. Calling a long mode function
from real/protected mode crashes the guest VM. Thus we chose not to
implement this feature in this patch set. More details can be found in
our off-list GitHub review [6].

Changes V1 -> V2:
1. Merge Varad's patches [1] as the foundation of our V2 patch set [4].
2. Remove AMD SEV/SEV-ES config flags and macros (patches 11-17)
3. Drop one commit 'x86 UEFI: Move setjmp.h out of desc.h' because we do
not link GNU-EFI library.

Notes on authorships and attributions:
The first two commits are from Varad's patch set [1], so they are
tagged as 'From:' and 'Signed-off-by:' Varad. Commits 3-7 are from our
V1 patch set [2], and since Varad implemented similar code [1], these
commits are tagged as 'Co-developed-by:' and 'Signed-off-by:' Varad.

Notes on patch sets merging strategy:
We understand that the current merging strategy (reorganizing and
squeezing Varad's patches into two) reduces Varad's authorships, and we
hope the additional attribution tags make up for it. We see another
approach which is to build our patch set on top of Varad's original
patch set, but this creates some noise in the final patch set, e.g.,
x86/cstart64.S is modified in Varad's part and later reverted in our
part as we implement start up code in C. For the sake of the clarity of
the code history, we believe the current approach is the best effort so
far, and we are open to all kinds of opinions.

[1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
[2] https://lore.kernel.org/kvm/20210818000905.1111226-1-zixuanwang@google.com/
[3] https://lore.kernel.org/kvm/YSA%2FsYhGgMU72tn+@google.com/
[4] https://lore.kernel.org/kvm/20210827031222.2778522-1-zixuanwang@google.com/
[5] https://lore.kernel.org/kvm/3fd467ae-63c9-adba-9d29-09b8a7beb92d@redhat.com/
[6] https://github.com/marc-orr/KVM-Unit-Tests-dev-fork/pull/1

Regards,
Zixuan Wang

Varad Gautam (2):
  x86 UEFI: Copy code from Linux
  x86 UEFI: Implement UEFI function calls

Zixuan Wang (15):
  x86: Move IDT, GDT and TSS to desc.c
  x86 UEFI: Copy code from GNU-EFI
  x86 UEFI: Boot from UEFI
  x86 UEFI: Load IDT after UEFI boot up
  x86 UEFI: Load GDT and TSS after UEFI boot up
  x86 UEFI: Set up memory allocator
  x86 UEFI: Set up RSDP after UEFI boot up
  x86 UEFI: Set up page tables
  x86 UEFI: Convert x86 test cases to PIC
  x86 AMD SEV: Initial support
  x86 AMD SEV: Page table with c-bit
  x86 AMD SEV-ES: Check SEV-ES status
  x86 AMD SEV-ES: Copy UEFI #VC IDT entry
  x86 AMD SEV-ES: Set up GHCB page
  x86 AMD SEV-ES: Add test cases

 .gitignore                 |   3 +
 Makefile                   |  29 +-
 README.md                  |   6 +
 configure                  |   6 +
 lib/efi.c                  | 118 ++++++++
 lib/efi.h                  |  22 ++
 lib/linux/efi.h            | 539 +++++++++++++++++++++++++++++++++++++
 lib/x86/acpi.c             |  38 ++-
 lib/x86/acpi.h             |  11 +
 lib/x86/amd_sev.c          | 174 ++++++++++++
 lib/x86/amd_sev.h          |  63 +++++
 lib/x86/asm/page.h         |  28 +-
 lib/x86/asm/setup.h        |  35 +++
 lib/x86/desc.c             |  46 +++-
 lib/x86/desc.h             |   6 +-
 lib/x86/setup.c            | 246 +++++++++++++++++
 lib/x86/usermode.c         |   3 +-
 lib/x86/vm.c               |  18 +-
 x86/Makefile.common        |  68 +++--
 x86/Makefile.i386          |   5 +-
 x86/Makefile.x86_64        |  58 ++--
 x86/access.c               |   9 +-
 x86/amd_sev.c              |  94 +++++++
 x86/cet.c                  |   8 +-
 x86/cstart64.S             |  77 +-----
 x86/efi/README.md          |  63 +++++
 x86/efi/crt0-efi-x86_64.S  |  79 ++++++
 x86/efi/efistart64.S       |  77 ++++++
 x86/efi/elf_x86_64_efi.lds |  81 ++++++
 x86/efi/reloc_x86_64.c     |  96 +++++++
 x86/efi/run                |  63 +++++
 x86/emulator.c             |   5 +-
 x86/eventinj.c             |   6 +-
 x86/run                    |  16 +-
 x86/smap.c                 |   8 +-
 x86/umip.c                 |  10 +-
 x86/vmx.c                  |   8 +-
 37 files changed, 2067 insertions(+), 155 deletions(-)
 create mode 100644 lib/efi.c
 create mode 100644 lib/efi.h
 create mode 100644 lib/linux/efi.h
 create mode 100644 lib/x86/amd_sev.c
 create mode 100644 lib/x86/amd_sev.h
 create mode 100644 lib/x86/asm/setup.h
 create mode 100644 x86/amd_sev.c
 create mode 100644 x86/efi/README.md
 create mode 100644 x86/efi/crt0-efi-x86_64.S
 create mode 100644 x86/efi/efistart64.S
 create mode 100644 x86/efi/elf_x86_64_efi.lds
 create mode 100644 x86/efi/reloc_x86_64.c
 create mode 100755 x86/efi/run

Comments

Paolo Bonzini Oct. 21, 2021, 2:10 p.m. UTC | #1
On 04/10/21 22:49, Zixuan Wang wrote:
> Hello,

WHOA IT WORKS! XD

There are still a few rough edges around the build system (and in 
general, the test harness is starting to really show its limits), but 
this is awesome work.  Thanks Drew, Varad and Zixuan (in alphabetic and 
temporal order) for the combined contribution!

For now I've placed it at a 'uefi' branch on gitlab, while I'm waiting 
for some reviews of my GDT cleanup work.  Any future improvements can be 
done on top.

Paolo

> This patch series updates the x86_64 KVM-Unit-Tests to run under UEFI
> and culminates in enabling AMD SEV/SEV-ES. The patches are organized as
> four parts.
> 
> The first part (patch 1) refactors the current Multiboot start-up code
> by converting assembly data structures into C. This enables the
> follow-up UEFI patches to reuse these data structures without redefining
> or duplicating them in assembly.
> 
> The second part (patches 2-3) copies code from Varad's patch set [1]
> that builds EFI stubs without depending on GNU-EFI. Part 3 and 4 are
> built on top of this part.
> 
> The third part (patches 4-11) enables the x86_64 test cases to run
> under UEFI. In particular, these patches allow the x86_64 test cases to
> be built as EFI executables and take full control of the guest VM. The
> efi_main() function sets up the KVM-Unit-Tests framework to run under
> UEFI and then launches the test cases' main() functions. To date, we
> have 38/43 test cases running with UEFI using this approach.
> 
> The fourth part of the series (patches 12-17) focuses on SEV. In
> particular, these patches introduce SEV/SEV-ES set up code into the EFI
> set up process, including checking if SEV is supported, setting c-bits
> for page table entries, and (notably) reusing the UEFI #VC handler so
> that the set up process does not need to re-implement it (a test case
> can always implement a new #VC handler and load it after set up is
> finished). Using this approach, we are able to launch the x86_64 test
> cases under SEV-ES and exercise KVM's VMGEXIT handler.
> 
> Note, a previous feedback [3] indicated that long-term we'd like to
> instrument KVM-Unit-Tests with it's own #VC handler. However, we still
> believe that the current approach is good as an intermediate solution,
> because it unlocks a lot of testing and we do not expect that testing
> to be inherently tied to the UEFI's #VC handler. Rather, test cases
> should be tied to the underlying GHCB spec implemented by an
> arbitrary #VC handler.
> 
> See the Part 1 to Part 4 summaries, below, for a high-level breakdown
> of how the patches are organized.
> 
> Part 1 Summary:
> Commit 1 refactors boot-related data structures from assembly to C.
> 
> Part 2 Summary:
> Commits 2-3 copy code from Varad's patch set [1] that implements
> EFI-related helper functions to replace the GNU-EFI library.
> 
> Part 3 Summary:
> Commits 4-5 introduce support to build test cases with EFI support.
> 
> Commits 6-10 set up KVM-Unit-Tests to run under UEFI. In doing so, these
> patches incrementally enable most existing x86_64 test cases to run
> under UEFI.
> 
> Commit 11 fixes several test cases that fail to compile with EFI due
> to UEFI's position independent code (PIC) requirement.
> 
> Part 4 Summary:
> Commits 12-13 introduce support for SEV by adding code to set the SEV
> c-bit in page table entries.
> 
> Commits 14-16 introduce support for SEV-ES by reusing the UEFI #VC
> handler in KVM-Unit-Tests. They also fix GDT and IDT issues that occur
> when reusing UEFI functions in KVM-Unit-Tests.
> 
> Commit 17 adds additional test cases for SEV-ES.
> 
> Changes V2 -> V3:
> V3 Patch #  Changes
> ----------  -------
>       01/17  (New patch) refactors assembly data structures in C
>       02/17  Adds a missing alignment attribute
>              Renames the file uefi.h to efi.h
>       03/17  Adds an SPDX header, fixes a comment style issue
>       06/17  Removes assembly data structure definitions
>       07/17  Removes assembly data structure definitions
>       12/17  Simplifies an if condition code
>       14/17  Simplifies an if condition code
>       15/17  Removes GDT copying for SEV-ES #VC handler
> 
> Notes on page table set up code:
> Paolo suggested unifying  the page table definitions in cstart64.S and
> UEFI start-up code [5]. We tried but found it hard to implement due to
> the real/long mode issue: a page table set up function written in C is
> by default compiled to run in long mode. However, cstart64.S requires
> page table setup before entering long mode. Calling a long mode function
> from real/protected mode crashes the guest VM. Thus we chose not to
> implement this feature in this patch set. More details can be found in
> our off-list GitHub review [6].
> 
> Changes V1 -> V2:
> 1. Merge Varad's patches [1] as the foundation of our V2 patch set [4].
> 2. Remove AMD SEV/SEV-ES config flags and macros (patches 11-17)
> 3. Drop one commit 'x86 UEFI: Move setjmp.h out of desc.h' because we do
> not link GNU-EFI library.
> 
> Notes on authorships and attributions:
> The first two commits are from Varad's patch set [1], so they are
> tagged as 'From:' and 'Signed-off-by:' Varad. Commits 3-7 are from our
> V1 patch set [2], and since Varad implemented similar code [1], these
> commits are tagged as 'Co-developed-by:' and 'Signed-off-by:' Varad.
> 
> Notes on patch sets merging strategy:
> We understand that the current merging strategy (reorganizing and
> squeezing Varad's patches into two) reduces Varad's authorships, and we
> hope the additional attribution tags make up for it. We see another
> approach which is to build our patch set on top of Varad's original
> patch set, but this creates some noise in the final patch set, e.g.,
> x86/cstart64.S is modified in Varad's part and later reverted in our
> part as we implement start up code in C. For the sake of the clarity of
> the code history, we believe the current approach is the best effort so
> far, and we are open to all kinds of opinions.
> 
> [1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
> [2] https://lore.kernel.org/kvm/20210818000905.1111226-1-zixuanwang@google.com/
> [3] https://lore.kernel.org/kvm/YSA%2FsYhGgMU72tn+@google.com/
> [4] https://lore.kernel.org/kvm/20210827031222.2778522-1-zixuanwang@google.com/
> [5] https://lore.kernel.org/kvm/3fd467ae-63c9-adba-9d29-09b8a7beb92d@redhat.com/
> [6] https://github.com/marc-orr/KVM-Unit-Tests-dev-fork/pull/1
> 
> Regards,
> Zixuan Wang
> 
> Varad Gautam (2):
>    x86 UEFI: Copy code from Linux
>    x86 UEFI: Implement UEFI function calls
> 
> Zixuan Wang (15):
>    x86: Move IDT, GDT and TSS to desc.c
>    x86 UEFI: Copy code from GNU-EFI
>    x86 UEFI: Boot from UEFI
>    x86 UEFI: Load IDT after UEFI boot up
>    x86 UEFI: Load GDT and TSS after UEFI boot up
>    x86 UEFI: Set up memory allocator
>    x86 UEFI: Set up RSDP after UEFI boot up
>    x86 UEFI: Set up page tables
>    x86 UEFI: Convert x86 test cases to PIC
>    x86 AMD SEV: Initial support
>    x86 AMD SEV: Page table with c-bit
>    x86 AMD SEV-ES: Check SEV-ES status
>    x86 AMD SEV-ES: Copy UEFI #VC IDT entry
>    x86 AMD SEV-ES: Set up GHCB page
>    x86 AMD SEV-ES: Add test cases
> 
>   .gitignore                 |   3 +
>   Makefile                   |  29 +-
>   README.md                  |   6 +
>   configure                  |   6 +
>   lib/efi.c                  | 118 ++++++++
>   lib/efi.h                  |  22 ++
>   lib/linux/efi.h            | 539 +++++++++++++++++++++++++++++++++++++
>   lib/x86/acpi.c             |  38 ++-
>   lib/x86/acpi.h             |  11 +
>   lib/x86/amd_sev.c          | 174 ++++++++++++
>   lib/x86/amd_sev.h          |  63 +++++
>   lib/x86/asm/page.h         |  28 +-
>   lib/x86/asm/setup.h        |  35 +++
>   lib/x86/desc.c             |  46 +++-
>   lib/x86/desc.h             |   6 +-
>   lib/x86/setup.c            | 246 +++++++++++++++++
>   lib/x86/usermode.c         |   3 +-
>   lib/x86/vm.c               |  18 +-
>   x86/Makefile.common        |  68 +++--
>   x86/Makefile.i386          |   5 +-
>   x86/Makefile.x86_64        |  58 ++--
>   x86/access.c               |   9 +-
>   x86/amd_sev.c              |  94 +++++++
>   x86/cet.c                  |   8 +-
>   x86/cstart64.S             |  77 +-----
>   x86/efi/README.md          |  63 +++++
>   x86/efi/crt0-efi-x86_64.S  |  79 ++++++
>   x86/efi/efistart64.S       |  77 ++++++
>   x86/efi/elf_x86_64_efi.lds |  81 ++++++
>   x86/efi/reloc_x86_64.c     |  96 +++++++
>   x86/efi/run                |  63 +++++
>   x86/emulator.c             |   5 +-
>   x86/eventinj.c             |   6 +-
>   x86/run                    |  16 +-
>   x86/smap.c                 |   8 +-
>   x86/umip.c                 |  10 +-
>   x86/vmx.c                  |   8 +-
>   37 files changed, 2067 insertions(+), 155 deletions(-)
>   create mode 100644 lib/efi.c
>   create mode 100644 lib/efi.h
>   create mode 100644 lib/linux/efi.h
>   create mode 100644 lib/x86/amd_sev.c
>   create mode 100644 lib/x86/amd_sev.h
>   create mode 100644 lib/x86/asm/setup.h
>   create mode 100644 x86/amd_sev.c
>   create mode 100644 x86/efi/README.md
>   create mode 100644 x86/efi/crt0-efi-x86_64.S
>   create mode 100644 x86/efi/efistart64.S
>   create mode 100644 x86/efi/elf_x86_64_efi.lds
>   create mode 100644 x86/efi/reloc_x86_64.c
>   create mode 100755 x86/efi/run
>
Marc Orr Oct. 21, 2021, 2:22 p.m. UTC | #2
On Thu, Oct 21, 2021 at 7:10 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 04/10/21 22:49, Zixuan Wang wrote:
> > Hello,
>
> WHOA IT WORKS! XD
>
> There are still a few rough edges around the build system (and in
> general, the test harness is starting to really show its limits), but
> this is awesome work.  Thanks Drew, Varad and Zixuan (in alphabetic and
> temporal order) for the combined contribution!
>
> For now I've placed it at a 'uefi' branch on gitlab, while I'm waiting
> for some reviews of my GDT cleanup work.  Any future improvements can be
> done on top.

Phenomenal! And +1 on thanking Drew, Varad, and Zixuan! (And thanks
Paolo for reviewing, testing, and setting up a branch for this work
:-).)

Zixuan has actually had a v4 ready to go since last week that
incorporates Drew's last round of reviews, but we were holding off on
posting it because we kept getting more comments and Zixuan wanted to
incorporate all of the feedback :-).

Should we go ahead and post it to the list (and perhaps update the
branch in the gitlab so everyone can work off of that)? Or would it be
easier to wait for the GDT cleanup work to finalize, and then post it
afterward?

>
> Paolo
>
> > This patch series updates the x86_64 KVM-Unit-Tests to run under UEFI
> > and culminates in enabling AMD SEV/SEV-ES. The patches are organized as
> > four parts.
> >
> > The first part (patch 1) refactors the current Multiboot start-up code
> > by converting assembly data structures into C. This enables the
> > follow-up UEFI patches to reuse these data structures without redefining
> > or duplicating them in assembly.
> >
> > The second part (patches 2-3) copies code from Varad's patch set [1]
> > that builds EFI stubs without depending on GNU-EFI. Part 3 and 4 are
> > built on top of this part.
> >
> > The third part (patches 4-11) enables the x86_64 test cases to run
> > under UEFI. In particular, these patches allow the x86_64 test cases to
> > be built as EFI executables and take full control of the guest VM. The
> > efi_main() function sets up the KVM-Unit-Tests framework to run under
> > UEFI and then launches the test cases' main() functions. To date, we
> > have 38/43 test cases running with UEFI using this approach.
> >
> > The fourth part of the series (patches 12-17) focuses on SEV. In
> > particular, these patches introduce SEV/SEV-ES set up code into the EFI
> > set up process, including checking if SEV is supported, setting c-bits
> > for page table entries, and (notably) reusing the UEFI #VC handler so
> > that the set up process does not need to re-implement it (a test case
> > can always implement a new #VC handler and load it after set up is
> > finished). Using this approach, we are able to launch the x86_64 test
> > cases under SEV-ES and exercise KVM's VMGEXIT handler.
> >
> > Note, a previous feedback [3] indicated that long-term we'd like to
> > instrument KVM-Unit-Tests with it's own #VC handler. However, we still
> > believe that the current approach is good as an intermediate solution,
> > because it unlocks a lot of testing and we do not expect that testing
> > to be inherently tied to the UEFI's #VC handler. Rather, test cases
> > should be tied to the underlying GHCB spec implemented by an
> > arbitrary #VC handler.
> >
> > See the Part 1 to Part 4 summaries, below, for a high-level breakdown
> > of how the patches are organized.
> >
> > Part 1 Summary:
> > Commit 1 refactors boot-related data structures from assembly to C.
> >
> > Part 2 Summary:
> > Commits 2-3 copy code from Varad's patch set [1] that implements
> > EFI-related helper functions to replace the GNU-EFI library.
> >
> > Part 3 Summary:
> > Commits 4-5 introduce support to build test cases with EFI support.
> >
> > Commits 6-10 set up KVM-Unit-Tests to run under UEFI. In doing so, these
> > patches incrementally enable most existing x86_64 test cases to run
> > under UEFI.
> >
> > Commit 11 fixes several test cases that fail to compile with EFI due
> > to UEFI's position independent code (PIC) requirement.
> >
> > Part 4 Summary:
> > Commits 12-13 introduce support for SEV by adding code to set the SEV
> > c-bit in page table entries.
> >
> > Commits 14-16 introduce support for SEV-ES by reusing the UEFI #VC
> > handler in KVM-Unit-Tests. They also fix GDT and IDT issues that occur
> > when reusing UEFI functions in KVM-Unit-Tests.
> >
> > Commit 17 adds additional test cases for SEV-ES.
> >
> > Changes V2 -> V3:
> > V3 Patch #  Changes
> > ----------  -------
> >       01/17  (New patch) refactors assembly data structures in C
> >       02/17  Adds a missing alignment attribute
> >              Renames the file uefi.h to efi.h
> >       03/17  Adds an SPDX header, fixes a comment style issue
> >       06/17  Removes assembly data structure definitions
> >       07/17  Removes assembly data structure definitions
> >       12/17  Simplifies an if condition code
> >       14/17  Simplifies an if condition code
> >       15/17  Removes GDT copying for SEV-ES #VC handler
> >
> > Notes on page table set up code:
> > Paolo suggested unifying  the page table definitions in cstart64.S and
> > UEFI start-up code [5]. We tried but found it hard to implement due to
> > the real/long mode issue: a page table set up function written in C is
> > by default compiled to run in long mode. However, cstart64.S requires
> > page table setup before entering long mode. Calling a long mode function
> > from real/protected mode crashes the guest VM. Thus we chose not to
> > implement this feature in this patch set. More details can be found in
> > our off-list GitHub review [6].
> >
> > Changes V1 -> V2:
> > 1. Merge Varad's patches [1] as the foundation of our V2 patch set [4].
> > 2. Remove AMD SEV/SEV-ES config flags and macros (patches 11-17)
> > 3. Drop one commit 'x86 UEFI: Move setjmp.h out of desc.h' because we do
> > not link GNU-EFI library.
> >
> > Notes on authorships and attributions:
> > The first two commits are from Varad's patch set [1], so they are
> > tagged as 'From:' and 'Signed-off-by:' Varad. Commits 3-7 are from our
> > V1 patch set [2], and since Varad implemented similar code [1], these
> > commits are tagged as 'Co-developed-by:' and 'Signed-off-by:' Varad.
> >
> > Notes on patch sets merging strategy:
> > We understand that the current merging strategy (reorganizing and
> > squeezing Varad's patches into two) reduces Varad's authorships, and we
> > hope the additional attribution tags make up for it. We see another
> > approach which is to build our patch set on top of Varad's original
> > patch set, but this creates some noise in the final patch set, e.g.,
> > x86/cstart64.S is modified in Varad's part and later reverted in our
> > part as we implement start up code in C. For the sake of the clarity of
> > the code history, we believe the current approach is the best effort so
> > far, and we are open to all kinds of opinions.
> >
> > [1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
> > [2] https://lore.kernel.org/kvm/20210818000905.1111226-1-zixuanwang@google.com/
> > [3] https://lore.kernel.org/kvm/YSA%2FsYhGgMU72tn+@google.com/
> > [4] https://lore.kernel.org/kvm/20210827031222.2778522-1-zixuanwang@google.com/
> > [5] https://lore.kernel.org/kvm/3fd467ae-63c9-adba-9d29-09b8a7beb92d@redhat.com/
> > [6] https://github.com/marc-orr/KVM-Unit-Tests-dev-fork/pull/1
> >
> > Regards,
> > Zixuan Wang
> >
> > Varad Gautam (2):
> >    x86 UEFI: Copy code from Linux
> >    x86 UEFI: Implement UEFI function calls
> >
> > Zixuan Wang (15):
> >    x86: Move IDT, GDT and TSS to desc.c
> >    x86 UEFI: Copy code from GNU-EFI
> >    x86 UEFI: Boot from UEFI
> >    x86 UEFI: Load IDT after UEFI boot up
> >    x86 UEFI: Load GDT and TSS after UEFI boot up
> >    x86 UEFI: Set up memory allocator
> >    x86 UEFI: Set up RSDP after UEFI boot up
> >    x86 UEFI: Set up page tables
> >    x86 UEFI: Convert x86 test cases to PIC
> >    x86 AMD SEV: Initial support
> >    x86 AMD SEV: Page table with c-bit
> >    x86 AMD SEV-ES: Check SEV-ES status
> >    x86 AMD SEV-ES: Copy UEFI #VC IDT entry
> >    x86 AMD SEV-ES: Set up GHCB page
> >    x86 AMD SEV-ES: Add test cases
> >
> >   .gitignore                 |   3 +
> >   Makefile                   |  29 +-
> >   README.md                  |   6 +
> >   configure                  |   6 +
> >   lib/efi.c                  | 118 ++++++++
> >   lib/efi.h                  |  22 ++
> >   lib/linux/efi.h            | 539 +++++++++++++++++++++++++++++++++++++
> >   lib/x86/acpi.c             |  38 ++-
> >   lib/x86/acpi.h             |  11 +
> >   lib/x86/amd_sev.c          | 174 ++++++++++++
> >   lib/x86/amd_sev.h          |  63 +++++
> >   lib/x86/asm/page.h         |  28 +-
> >   lib/x86/asm/setup.h        |  35 +++
> >   lib/x86/desc.c             |  46 +++-
> >   lib/x86/desc.h             |   6 +-
> >   lib/x86/setup.c            | 246 +++++++++++++++++
> >   lib/x86/usermode.c         |   3 +-
> >   lib/x86/vm.c               |  18 +-
> >   x86/Makefile.common        |  68 +++--
> >   x86/Makefile.i386          |   5 +-
> >   x86/Makefile.x86_64        |  58 ++--
> >   x86/access.c               |   9 +-
> >   x86/amd_sev.c              |  94 +++++++
> >   x86/cet.c                  |   8 +-
> >   x86/cstart64.S             |  77 +-----
> >   x86/efi/README.md          |  63 +++++
> >   x86/efi/crt0-efi-x86_64.S  |  79 ++++++
> >   x86/efi/efistart64.S       |  77 ++++++
> >   x86/efi/elf_x86_64_efi.lds |  81 ++++++
> >   x86/efi/reloc_x86_64.c     |  96 +++++++
> >   x86/efi/run                |  63 +++++
> >   x86/emulator.c             |   5 +-
> >   x86/eventinj.c             |   6 +-
> >   x86/run                    |  16 +-
> >   x86/smap.c                 |   8 +-
> >   x86/umip.c                 |  10 +-
> >   x86/vmx.c                  |   8 +-
> >   37 files changed, 2067 insertions(+), 155 deletions(-)
> >   create mode 100644 lib/efi.c
> >   create mode 100644 lib/efi.h
> >   create mode 100644 lib/linux/efi.h
> >   create mode 100644 lib/x86/amd_sev.c
> >   create mode 100644 lib/x86/amd_sev.h
> >   create mode 100644 lib/x86/asm/setup.h
> >   create mode 100644 x86/amd_sev.c
> >   create mode 100644 x86/efi/README.md
> >   create mode 100644 x86/efi/crt0-efi-x86_64.S
> >   create mode 100644 x86/efi/efistart64.S
> >   create mode 100644 x86/efi/elf_x86_64_efi.lds
> >   create mode 100644 x86/efi/reloc_x86_64.c
> >   create mode 100755 x86/efi/run
> >
>
Paolo Bonzini Oct. 21, 2021, 2:27 p.m. UTC | #3
On 21/10/21 16:22, Marc Orr wrote:
> Should we go ahead and post it to the list (and perhaps update the 
> branch in the gitlab so everyone can work off of that)? Or would it
> be easier to wait for the GDT cleanup work to finalize, and then post
> it afterward?

I would say, just post it on top of the 'uefi' branch.  I'm not sure how 
and when it will be merged in 'master', but for the next few weeks I 
suppose it's okay if I get incremental patches, and either squash them 
or commit them on top.

Paolo
Varad Gautam Nov. 25, 2021, 3:21 p.m. UTC | #4
On 10/21/21 4:10 PM, Paolo Bonzini wrote:
> On 04/10/21 22:49, Zixuan Wang wrote:
>> Hello,
> 
> WHOA IT WORKS! XD
> 
> There are still a few rough edges around the build system (and in general, the test harness is starting to really show its limits), but this is awesome work.  Thanks Drew, Varad and Zixuan (in alphabetic and temporal order) for the combined contribution!
> 
> For now I've placed it at a 'uefi' branch on gitlab, while I'm waiting for some reviews of my GDT cleanup work.  Any future improvements can be done on top.
> 

While doing the #VC handler support for test binaries [1], I realised I can't seem
to run any of the tests from the uefi branch [2] that write to cr3 via setup_vm()
on SEV-ES. These tests (eg., tscdeadline_latency) crash with SEV-ES, and work with
uefi without SEV-ES (policy=0x0). I'm wondering if I am missing something, is
setup_vm->setup_mmu->write_cr3() known to work on SEV-ES elsewhere?

[1] https://lore.kernel.org/all/20211117134752.32662-1-varad.gautam@suse.com/
[2] https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/tree/uefi

Thanks,
Varad

> Paolo
> 
>> This patch series updates the x86_64 KVM-Unit-Tests to run under UEFI
>> and culminates in enabling AMD SEV/SEV-ES. The patches are organized as
>> four parts.
>>
>> The first part (patch 1) refactors the current Multiboot start-up code
>> by converting assembly data structures into C. This enables the
>> follow-up UEFI patches to reuse these data structures without redefining
>> or duplicating them in assembly.
>>
>> The second part (patches 2-3) copies code from Varad's patch set [1]
>> that builds EFI stubs without depending on GNU-EFI. Part 3 and 4 are
>> built on top of this part.
>>
>> The third part (patches 4-11) enables the x86_64 test cases to run
>> under UEFI. In particular, these patches allow the x86_64 test cases to
>> be built as EFI executables and take full control of the guest VM. The
>> efi_main() function sets up the KVM-Unit-Tests framework to run under
>> UEFI and then launches the test cases' main() functions. To date, we
>> have 38/43 test cases running with UEFI using this approach.
>>
>> The fourth part of the series (patches 12-17) focuses on SEV. In
>> particular, these patches introduce SEV/SEV-ES set up code into the EFI
>> set up process, including checking if SEV is supported, setting c-bits
>> for page table entries, and (notably) reusing the UEFI #VC handler so
>> that the set up process does not need to re-implement it (a test case
>> can always implement a new #VC handler and load it after set up is
>> finished). Using this approach, we are able to launch the x86_64 test
>> cases under SEV-ES and exercise KVM's VMGEXIT handler.
>>
>> Note, a previous feedback [3] indicated that long-term we'd like to
>> instrument KVM-Unit-Tests with it's own #VC handler. However, we still
>> believe that the current approach is good as an intermediate solution,
>> because it unlocks a lot of testing and we do not expect that testing
>> to be inherently tied to the UEFI's #VC handler. Rather, test cases
>> should be tied to the underlying GHCB spec implemented by an
>> arbitrary #VC handler.
>>
>> See the Part 1 to Part 4 summaries, below, for a high-level breakdown
>> of how the patches are organized.
>>
>> Part 1 Summary:
>> Commit 1 refactors boot-related data structures from assembly to C.
>>
>> Part 2 Summary:
>> Commits 2-3 copy code from Varad's patch set [1] that implements
>> EFI-related helper functions to replace the GNU-EFI library.
>>
>> Part 3 Summary:
>> Commits 4-5 introduce support to build test cases with EFI support.
>>
>> Commits 6-10 set up KVM-Unit-Tests to run under UEFI. In doing so, these
>> patches incrementally enable most existing x86_64 test cases to run
>> under UEFI.
>>
>> Commit 11 fixes several test cases that fail to compile with EFI due
>> to UEFI's position independent code (PIC) requirement.
>>
>> Part 4 Summary:
>> Commits 12-13 introduce support for SEV by adding code to set the SEV
>> c-bit in page table entries.
>>
>> Commits 14-16 introduce support for SEV-ES by reusing the UEFI #VC
>> handler in KVM-Unit-Tests. They also fix GDT and IDT issues that occur
>> when reusing UEFI functions in KVM-Unit-Tests.
>>
>> Commit 17 adds additional test cases for SEV-ES.
>>
>> Changes V2 -> V3:
>> V3 Patch #  Changes
>> ----------  -------
>>       01/17  (New patch) refactors assembly data structures in C
>>       02/17  Adds a missing alignment attribute
>>              Renames the file uefi.h to efi.h
>>       03/17  Adds an SPDX header, fixes a comment style issue
>>       06/17  Removes assembly data structure definitions
>>       07/17  Removes assembly data structure definitions
>>       12/17  Simplifies an if condition code
>>       14/17  Simplifies an if condition code
>>       15/17  Removes GDT copying for SEV-ES #VC handler
>>
>> Notes on page table set up code:
>> Paolo suggested unifying  the page table definitions in cstart64.S and
>> UEFI start-up code [5]. We tried but found it hard to implement due to
>> the real/long mode issue: a page table set up function written in C is
>> by default compiled to run in long mode. However, cstart64.S requires
>> page table setup before entering long mode. Calling a long mode function
>> from real/protected mode crashes the guest VM. Thus we chose not to
>> implement this feature in this patch set. More details can be found in
>> our off-list GitHub review [6].
>>
>> Changes V1 -> V2:
>> 1. Merge Varad's patches [1] as the foundation of our V2 patch set [4].
>> 2. Remove AMD SEV/SEV-ES config flags and macros (patches 11-17)
>> 3. Drop one commit 'x86 UEFI: Move setjmp.h out of desc.h' because we do
>> not link GNU-EFI library.
>>
>> Notes on authorships and attributions:
>> The first two commits are from Varad's patch set [1], so they are
>> tagged as 'From:' and 'Signed-off-by:' Varad. Commits 3-7 are from our
>> V1 patch set [2], and since Varad implemented similar code [1], these
>> commits are tagged as 'Co-developed-by:' and 'Signed-off-by:' Varad.
>>
>> Notes on patch sets merging strategy:
>> We understand that the current merging strategy (reorganizing and
>> squeezing Varad's patches into two) reduces Varad's authorships, and we
>> hope the additional attribution tags make up for it. We see another
>> approach which is to build our patch set on top of Varad's original
>> patch set, but this creates some noise in the final patch set, e.g.,
>> x86/cstart64.S is modified in Varad's part and later reverted in our
>> part as we implement start up code in C. For the sake of the clarity of
>> the code history, we believe the current approach is the best effort so
>> far, and we are open to all kinds of opinions.
>>
>> [1] https://lore.kernel.org/kvm/20210819113400.26516-1-varad.gautam@suse.com/
>> [2] https://lore.kernel.org/kvm/20210818000905.1111226-1-zixuanwang@google.com/
>> [3] https://lore.kernel.org/kvm/YSA%2FsYhGgMU72tn+@google.com/
>> [4] https://lore.kernel.org/kvm/20210827031222.2778522-1-zixuanwang@google.com/
>> [5] https://lore.kernel.org/kvm/3fd467ae-63c9-adba-9d29-09b8a7beb92d@redhat.com/
>> [6] https://github.com/marc-orr/KVM-Unit-Tests-dev-fork/pull/1
>>
>> Regards,
>> Zixuan Wang
>>
>> Varad Gautam (2):
>>    x86 UEFI: Copy code from Linux
>>    x86 UEFI: Implement UEFI function calls
>>
>> Zixuan Wang (15):
>>    x86: Move IDT, GDT and TSS to desc.c
>>    x86 UEFI: Copy code from GNU-EFI
>>    x86 UEFI: Boot from UEFI
>>    x86 UEFI: Load IDT after UEFI boot up
>>    x86 UEFI: Load GDT and TSS after UEFI boot up
>>    x86 UEFI: Set up memory allocator
>>    x86 UEFI: Set up RSDP after UEFI boot up
>>    x86 UEFI: Set up page tables
>>    x86 UEFI: Convert x86 test cases to PIC
>>    x86 AMD SEV: Initial support
>>    x86 AMD SEV: Page table with c-bit
>>    x86 AMD SEV-ES: Check SEV-ES status
>>    x86 AMD SEV-ES: Copy UEFI #VC IDT entry
>>    x86 AMD SEV-ES: Set up GHCB page
>>    x86 AMD SEV-ES: Add test cases
>>
>>   .gitignore                 |   3 +
>>   Makefile                   |  29 +-
>>   README.md                  |   6 +
>>   configure                  |   6 +
>>   lib/efi.c                  | 118 ++++++++
>>   lib/efi.h                  |  22 ++
>>   lib/linux/efi.h            | 539 +++++++++++++++++++++++++++++++++++++
>>   lib/x86/acpi.c             |  38 ++-
>>   lib/x86/acpi.h             |  11 +
>>   lib/x86/amd_sev.c          | 174 ++++++++++++
>>   lib/x86/amd_sev.h          |  63 +++++
>>   lib/x86/asm/page.h         |  28 +-
>>   lib/x86/asm/setup.h        |  35 +++
>>   lib/x86/desc.c             |  46 +++-
>>   lib/x86/desc.h             |   6 +-
>>   lib/x86/setup.c            | 246 +++++++++++++++++
>>   lib/x86/usermode.c         |   3 +-
>>   lib/x86/vm.c               |  18 +-
>>   x86/Makefile.common        |  68 +++--
>>   x86/Makefile.i386          |   5 +-
>>   x86/Makefile.x86_64        |  58 ++--
>>   x86/access.c               |   9 +-
>>   x86/amd_sev.c              |  94 +++++++
>>   x86/cet.c                  |   8 +-
>>   x86/cstart64.S             |  77 +-----
>>   x86/efi/README.md          |  63 +++++
>>   x86/efi/crt0-efi-x86_64.S  |  79 ++++++
>>   x86/efi/efistart64.S       |  77 ++++++
>>   x86/efi/elf_x86_64_efi.lds |  81 ++++++
>>   x86/efi/reloc_x86_64.c     |  96 +++++++
>>   x86/efi/run                |  63 +++++
>>   x86/emulator.c             |   5 +-
>>   x86/eventinj.c             |   6 +-
>>   x86/run                    |  16 +-
>>   x86/smap.c                 |   8 +-
>>   x86/umip.c                 |  10 +-
>>   x86/vmx.c                  |   8 +-
>>   37 files changed, 2067 insertions(+), 155 deletions(-)
>>   create mode 100644 lib/efi.c
>>   create mode 100644 lib/efi.h
>>   create mode 100644 lib/linux/efi.h
>>   create mode 100644 lib/x86/amd_sev.c
>>   create mode 100644 lib/x86/amd_sev.h
>>   create mode 100644 lib/x86/asm/setup.h
>>   create mode 100644 x86/amd_sev.c
>>   create mode 100644 x86/efi/README.md
>>   create mode 100644 x86/efi/crt0-efi-x86_64.S
>>   create mode 100644 x86/efi/efistart64.S
>>   create mode 100644 x86/efi/elf_x86_64_efi.lds
>>   create mode 100644 x86/efi/reloc_x86_64.c
>>   create mode 100755 x86/efi/run
>>
>
Marc Orr Nov. 29, 2021, 2:44 p.m. UTC | #5
On Thu, Nov 25, 2021 at 7:21 AM Varad Gautam <varad.gautam@suse.com> wrote:
>
> On 10/21/21 4:10 PM, Paolo Bonzini wrote:
> > On 04/10/21 22:49, Zixuan Wang wrote:
> >> Hello,
> >
> > WHOA IT WORKS! XD
> >
> > There are still a few rough edges around the build system (and in general, the test harness is starting to really show its limits), but this is awesome work.  Thanks Drew, Varad and Zixuan (in alphabetic and temporal order) for the combined contribution!
> >
> > For now I've placed it at a 'uefi' branch on gitlab, while I'm waiting for some reviews of my GDT cleanup work.  Any future improvements can be done on top.
> >
>
> While doing the #VC handler support for test binaries [1], I realised I can't seem
> to run any of the tests from the uefi branch [2] that write to cr3 via setup_vm()
> on SEV-ES. These tests (eg., tscdeadline_latency) crash with SEV-ES, and work with
> uefi without SEV-ES (policy=0x0). I'm wondering if I am missing something, is
> setup_vm->setup_mmu->write_cr3() known to work on SEV-ES elsewhere?
>
> [1] https://lore.kernel.org/all/20211117134752.32662-1-varad.gautam@suse.com/
> [2] https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/tree/uefi

I've only been running amd_sev under SEV-ES up to now. I just tried
tscdeadline_latency on my setup, and can confirm that it does indeed
fail under SEV-ES.
Tom Lendacky Nov. 29, 2021, 3:24 p.m. UTC | #6
On 11/29/21 8:44 AM, Marc Orr wrote:
> On Thu, Nov 25, 2021 at 7:21 AM Varad Gautam <varad.gautam@suse.com> wrote:
>>
>> On 10/21/21 4:10 PM, Paolo Bonzini wrote:
>>> On 04/10/21 22:49, Zixuan Wang wrote:
>>>> Hello,
>>>
>>> WHOA IT WORKS! XD
>>>
>>> There are still a few rough edges around the build system (and in general, the test harness is starting to really show its limits), but this is awesome work.  Thanks Drew, Varad and Zixuan (in alphabetic and temporal order) for the combined contribution!
>>>
>>> For now I've placed it at a 'uefi' branch on gitlab, while I'm waiting for some reviews of my GDT cleanup work.  Any future improvements can be done on top.
>>>
>>
>> While doing the #VC handler support for test binaries [1], I realised I can't seem
>> to run any of the tests from the uefi branch [2] that write to cr3 via setup_vm()
>> on SEV-ES. These tests (eg., tscdeadline_latency) crash with SEV-ES, and work with
>> uefi without SEV-ES (policy=0x0). I'm wondering if I am missing something, is
>> setup_vm->setup_mmu->write_cr3() known to work on SEV-ES elsewhere?

When writing a new CR3 value, do the new page tables have the GHCB(s) 
mapped shared?

Thanks,
Tom

>>
>> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20211117134752.32662-1-varad.gautam%40suse.com%2F&amp;data=04%7C01%7CThomas.Lendacky%40amd.com%7C30e4810784c9456a7c4208d9b346bfe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637737938743453221%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=%2Fo0aGSzTWbVwLId4gEsnDpYfDsyMWNibjocX6whDK14%3D&amp;reserved=0
>> [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fkvm-unit-tests%2Fkvm-unit-tests%2F-%2Ftree%2Fuefi&amp;data=04%7C01%7CThomas.Lendacky%40amd.com%7C30e4810784c9456a7c4208d9b346bfe9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637737938743463179%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=k2kQzSZwmSWNVWWV%2BHJI0cfT71zva3Ify3UHFbSEOyA%3D&amp;reserved=0
> 
> I've only been running amd_sev under SEV-ES up to now. I just tried
> tscdeadline_latency on my setup, and can confirm that it does indeed
> fail under SEV-ES.
>