mbox series

[kvm-unit-tests,0/6] Initial x86_64 UEFI support

Message ID 20210702114820.16712-1-varad.gautam@suse.com (mailing list archive)
Headers show
Series Initial x86_64 UEFI support | expand

Message

Varad Gautam July 2, 2021, 11:48 a.m. UTC
This series brings EFI support to a reduced subset of kvm-unit-tests
on x86_64. I'm sending it out for early review since it covers enough
ground to allow adding KVM testcases for EFI-only environments.

EFI support works by changing the test entrypoint to a stub entry
point for the EFI loader to jump to in long mode, where the test binary
exits EFI boot services, performs the remaining CPU bootstrapping,
and then calls the testcase main().

Since the EFI loader only understands PE objects, the first commit
introduces a `configure --efi` mode which builds each test as a shared
lib. This shared lib is repackaged into a PE via objdump.

Commit 2-4 take a trip from the asm entrypoint to C to exit EFI and
relocate ELF .dynamic contents.

Commit 5 adds post-EFI long mode x86_64 setup and calls the testcase.

Commit 6 patches out some broken tests for EFI. Testcases that refuse
to build as shared libs are also left disabled, these need some massaging.

git tree: https://github.com/varadgautam/kvm-unit-tests/commits/efi-stub

Varad Gautam (6):
  x86: Build tests as PE objects for the EFI loader
  x86: Call efi_main from _efi_pe_entry
  x86: efi_main: Get EFI memory map and exit boot services
  x86: efi_main: Self-relocate ELF .dynamic addresses
  cstart64.S: x86_64 bootstrapping after exiting EFI
  x86: Disable some breaking tests for EFI and modify vmexit test

 .gitignore          |   2 +
 Makefile            |  16 ++-
 configure           |  11 ++
 lib/linux/uefi.h    | 337 ++++++++++++++++++++++++++++++++++++++++++++
 x86/Makefile.common |  45 ++++--
 x86/Makefile.x86_64 |  43 +++---
 x86/cstart64.S      |  78 ++++++++++
 x86/efi.lds         |  67 +++++++++
 x86/efi_main.c      | 167 ++++++++++++++++++++++
 x86/vmexit.c        |   7 +
 10 files changed, 741 insertions(+), 32 deletions(-)
 create mode 100644 lib/linux/uefi.h
 create mode 100644 x86/efi.lds
 create mode 100644 x86/efi_main.c

Comments

Andrew Jones July 12, 2021, 4:29 p.m. UTC | #1
On Fri, Jul 02, 2021 at 01:48:14PM +0200, Varad Gautam wrote:
> This series brings EFI support to a reduced subset of kvm-unit-tests
> on x86_64. I'm sending it out for early review since it covers enough
> ground to allow adding KVM testcases for EFI-only environments.
> 
> EFI support works by changing the test entrypoint to a stub entry
> point for the EFI loader to jump to in long mode, where the test binary
> exits EFI boot services, performs the remaining CPU bootstrapping,
> and then calls the testcase main().
> 
> Since the EFI loader only understands PE objects, the first commit
> introduces a `configure --efi` mode which builds each test as a shared
> lib. This shared lib is repackaged into a PE via objdump.
> 
> Commit 2-4 take a trip from the asm entrypoint to C to exit EFI and
> relocate ELF .dynamic contents.
> 
> Commit 5 adds post-EFI long mode x86_64 setup and calls the testcase.
> 
> Commit 6 patches out some broken tests for EFI. Testcases that refuse
> to build as shared libs are also left disabled, these need some massaging.
> 
> git tree: https://github.com/varadgautam/kvm-unit-tests/commits/efi-stub

Hi Varad,

Thanks for this. I haven't reviewed it in detail yet (I just got back from
vacation), but this looks like the right approach. In fact, I had started
going down the efi stub approach for AArch64 a while back as well, but the
effort got preempted by other work [again]. I'll try to allocate time to
play with this for x86 and to build on it for AArch64 in the coming weeks.

drew

> 
> Varad Gautam (6):
>   x86: Build tests as PE objects for the EFI loader
>   x86: Call efi_main from _efi_pe_entry
>   x86: efi_main: Get EFI memory map and exit boot services
>   x86: efi_main: Self-relocate ELF .dynamic addresses
>   cstart64.S: x86_64 bootstrapping after exiting EFI
>   x86: Disable some breaking tests for EFI and modify vmexit test
> 
>  .gitignore          |   2 +
>  Makefile            |  16 ++-
>  configure           |  11 ++
>  lib/linux/uefi.h    | 337 ++++++++++++++++++++++++++++++++++++++++++++
>  x86/Makefile.common |  45 ++++--
>  x86/Makefile.x86_64 |  43 +++---
>  x86/cstart64.S      |  78 ++++++++++
>  x86/efi.lds         |  67 +++++++++
>  x86/efi_main.c      | 167 ++++++++++++++++++++++
>  x86/vmexit.c        |   7 +
>  10 files changed, 741 insertions(+), 32 deletions(-)
>  create mode 100644 lib/linux/uefi.h
>  create mode 100644 x86/efi.lds
>  create mode 100644 x86/efi_main.c
> 
> -- 
> 2.30.2
>
Marc Orr Aug. 13, 2021, 6:44 p.m. UTC | #2
On Fri, Jul 2, 2021 at 4:48 AM Varad Gautam <varad.gautam@suse.com> wrote:
>
> This series brings EFI support to a reduced subset of kvm-unit-tests
> on x86_64. I'm sending it out for early review since it covers enough
> ground to allow adding KVM testcases for EFI-only environments.
>
> EFI support works by changing the test entrypoint to a stub entry
> point for the EFI loader to jump to in long mode, where the test binary
> exits EFI boot services, performs the remaining CPU bootstrapping,
> and then calls the testcase main().
>
> Since the EFI loader only understands PE objects, the first commit
> introduces a `configure --efi` mode which builds each test as a shared
> lib. This shared lib is repackaged into a PE via objdump.
>
> Commit 2-4 take a trip from the asm entrypoint to C to exit EFI and
> relocate ELF .dynamic contents.
>
> Commit 5 adds post-EFI long mode x86_64 setup and calls the testcase.
>
> Commit 6 patches out some broken tests for EFI. Testcases that refuse
> to build as shared libs are also left disabled, these need some massaging.
>
> git tree: https://github.com/varadgautam/kvm-unit-tests/commits/efi-stub

Thanks for this patchset. My colleague, Zixuan Wang
<zixuanwang@google.com>, has also been working to extend
kvm-unit-tests to run under UEFI. Our goal is to enable running
kvm-unit-tests under SEV-ES.

Our approach is a bit different. Rather than pull in bits of the EFI
library and Linux EFI ABI, we've elected to build the entire
kvm-unit-tests binaries as an EFI app (similar to the ARM approach).

To date, we have _most_ x86 test cases (39/44) working under UEFI and
we've also got some of the test cases to boot under SEV-ES, using the
UEFI #VC handler.

We will post our patchset as soon as possible (hopefully by Monday) so
that the community can see our approach. We are very eager to see
kvm-unit-tests running under SEV-ES (and SNP) and are happy to work
with you all on either approach, depending on what the community
thinks is the best approach.

Thanks in advance,
Marc
Andrew Jones Aug. 16, 2021, 7:26 a.m. UTC | #3
On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> On Fri, Jul 2, 2021 at 4:48 AM Varad Gautam <varad.gautam@suse.com> wrote:
> >
> > This series brings EFI support to a reduced subset of kvm-unit-tests
> > on x86_64. I'm sending it out for early review since it covers enough
> > ground to allow adding KVM testcases for EFI-only environments.
> >
> > EFI support works by changing the test entrypoint to a stub entry
> > point for the EFI loader to jump to in long mode, where the test binary
> > exits EFI boot services, performs the remaining CPU bootstrapping,
> > and then calls the testcase main().
> >
> > Since the EFI loader only understands PE objects, the first commit
> > introduces a `configure --efi` mode which builds each test as a shared
> > lib. This shared lib is repackaged into a PE via objdump.
> >
> > Commit 2-4 take a trip from the asm entrypoint to C to exit EFI and
> > relocate ELF .dynamic contents.
> >
> > Commit 5 adds post-EFI long mode x86_64 setup and calls the testcase.
> >
> > Commit 6 patches out some broken tests for EFI. Testcases that refuse
> > to build as shared libs are also left disabled, these need some massaging.
> >
> > git tree: https://github.com/varadgautam/kvm-unit-tests/commits/efi-stub
> 
> Thanks for this patchset. My colleague, Zixuan Wang
> <zixuanwang@google.com>, has also been working to extend
> kvm-unit-tests to run under UEFI. Our goal is to enable running
> kvm-unit-tests under SEV-ES.
> 
> Our approach is a bit different. Rather than pull in bits of the EFI
> library and Linux EFI ABI, we've elected to build the entire
> kvm-unit-tests binaries as an EFI app (similar to the ARM approach).
> 
> To date, we have _most_ x86 test cases (39/44) working under UEFI and
> we've also got some of the test cases to boot under SEV-ES, using the
> UEFI #VC handler.
> 
> We will post our patchset as soon as possible (hopefully by Monday) so
> that the community can see our approach. We are very eager to see
> kvm-unit-tests running under SEV-ES (and SNP) and are happy to work
> with you all on either approach, depending on what the community
> thinks is the best approach.
> 
> Thanks in advance,
> Marc
>

Hi Marc,

I'm definitely eager to see your approach. I was actually working on
a second version of EFI support for ARM using the stub approach like
this series before getting perpetually sidetracked. I've been wanted
to experiment with Varad's code to continue that, but haven't been
able to find the time. I'm curious if you considered the stub approach
as well, but then opted for the app approach in the end. I was
leaning towards the stub approach to avoid the gnu-efi dependency.

Thanks,
drew
Marc Orr Aug. 17, 2021, 3:41 a.m. UTC | #4
On Mon, Aug 16, 2021 at 12:26 AM Andrew Jones <drjones@redhat.com> wrote:
>
> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> > On Fri, Jul 2, 2021 at 4:48 AM Varad Gautam <varad.gautam@suse.com> wrote:
> > >
> > > This series brings EFI support to a reduced subset of kvm-unit-tests
> > > on x86_64. I'm sending it out for early review since it covers enough
> > > ground to allow adding KVM testcases for EFI-only environments.
> > >
> > > EFI support works by changing the test entrypoint to a stub entry
> > > point for the EFI loader to jump to in long mode, where the test binary
> > > exits EFI boot services, performs the remaining CPU bootstrapping,
> > > and then calls the testcase main().
> > >
> > > Since the EFI loader only understands PE objects, the first commit
> > > introduces a `configure --efi` mode which builds each test as a shared
> > > lib. This shared lib is repackaged into a PE via objdump.
> > >
> > > Commit 2-4 take a trip from the asm entrypoint to C to exit EFI and
> > > relocate ELF .dynamic contents.
> > >
> > > Commit 5 adds post-EFI long mode x86_64 setup and calls the testcase.
> > >
> > > Commit 6 patches out some broken tests for EFI. Testcases that refuse
> > > to build as shared libs are also left disabled, these need some massaging.
> > >
> > > git tree: https://github.com/varadgautam/kvm-unit-tests/commits/efi-stub
> >
> > Thanks for this patchset. My colleague, Zixuan Wang
> > <zixuanwang@google.com>, has also been working to extend
> > kvm-unit-tests to run under UEFI. Our goal is to enable running
> > kvm-unit-tests under SEV-ES.
> >
> > Our approach is a bit different. Rather than pull in bits of the EFI
> > library and Linux EFI ABI, we've elected to build the entire
> > kvm-unit-tests binaries as an EFI app (similar to the ARM approach).
> >
> > To date, we have _most_ x86 test cases (39/44) working under UEFI and
> > we've also got some of the test cases to boot under SEV-ES, using the
> > UEFI #VC handler.
> >
> > We will post our patchset as soon as possible (hopefully by Monday) so
> > that the community can see our approach. We are very eager to see
> > kvm-unit-tests running under SEV-ES (and SNP) and are happy to work
> > with you all on either approach, depending on what the community
> > thinks is the best approach.
> >
> > Thanks in advance,
> > Marc
> >
>
> Hi Marc,
>
> I'm definitely eager to see your approach. I was actually working on
> a second version of EFI support for ARM using the stub approach like
> this series before getting perpetually sidetracked. I've been wanted
> to experiment with Varad's code to continue that, but haven't been
> able to find the time. I'm curious if you considered the stub approach
> as well, but then opted for the app approach in the end. I was
> leaning towards the stub approach to avoid the gnu-efi dependency.

Ack. We never seriously contemplated the stub approach. In hindsight,
I think we were probably biased towards the EFI app approach because
we have some test cases running as UEFI EFI apps internally with great
success (not using the kvm-unit-tests framework though). That being
said, I agree that avoiding the gnu-efi dependency is a win. I also
asked Zixuan, who wrote our patches, if he had an opinion on this. He
said that GNU-EFI provides useful set up code and library functions,
so we do not have to re-implement and debug them in KVM-Unit-Tests.

In any case, we have the patchset ready to post. However, Zixuan ran
into some permission issues when he tried to post the patches because
they are outside of our corporate domain. We'll try to get these
permissions issues fixed up and post the patches tomorrow. If we
cannot get them fixed up, I can always post the patches on his behalf.

Also, we will spend some more time reading Varad's patches this week,
so we can better contrast the stub approach taken by Varad vs. the EFI
app approach that we've taken.

Thanks,
Marc
Joerg Roedel Aug. 17, 2021, 10:49 a.m. UTC | #5
Hi Marc,

On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> To date, we have _most_ x86 test cases (39/44) working under UEFI and
> we've also got some of the test cases to boot under SEV-ES, using the
> UEFI #VC handler.

While the EFI APP approach simplifies the implementation a lot, I don't
think it is the best path to SEV and TDX testing for a couple of
reasons:

	1) It leaves the details of #VC/#VE handling and the SEV-ES
	   specific communication channels (GHCB) under control of the
	   firmware. So we can't reliably test those interfaces from an
	   EFI APP.

	2) Same for the memory validation/acceptance interface needed
	   for SEV-SNP and TDX. Using an EFI APP leaves those under
	   firmware control and we are not able to reliably test them.

	3) The IDT also stays under control of the firmware in an EFI
	   APP, otherwise the firmware couldn't provide a #VC handler.
	   This makes it unreliable to test anything IDT or IRQ related.

	4) Relying on the firmware #VC hanlder limits the tests to its
	   abilities. Implementing a separate #VC handler routine for
	   kvm-unit-tests is more work, but it makes test development
	   much more flexible.

So it comes down to the fact that and EFI APP leaves control over
SEV/TDX specific hypervisor interfaces in the firmware, making it hard
and unreliable to test these interfaces from kvm-unit-tests. The stub
approach on the other side gives the tests full control over the VM,
allowing to test all aspects of the guest-host interface.

Regards,

	Joerg
Marc Orr Aug. 18, 2021, 1:52 a.m. UTC | #6
On Tue, Aug 17, 2021 at 3:49 AM Joerg Roedel <jroedel@suse.de> wrote:
>
> Hi Marc,
>
> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> > To date, we have _most_ x86 test cases (39/44) working under UEFI and
> > we've also got some of the test cases to boot under SEV-ES, using the
> > UEFI #VC handler.
>
> While the EFI APP approach simplifies the implementation a lot, I don't
> think it is the best path to SEV and TDX testing for a couple of
> reasons:
>
>         1) It leaves the details of #VC/#VE handling and the SEV-ES
>            specific communication channels (GHCB) under control of the
>            firmware. So we can't reliably test those interfaces from an
>            EFI APP.
>
>         2) Same for the memory validation/acceptance interface needed
>            for SEV-SNP and TDX. Using an EFI APP leaves those under
>            firmware control and we are not able to reliably test them.
>
>         3) The IDT also stays under control of the firmware in an EFI
>            APP, otherwise the firmware couldn't provide a #VC handler.
>            This makes it unreliable to test anything IDT or IRQ related.
>
>         4) Relying on the firmware #VC hanlder limits the tests to its
>            abilities. Implementing a separate #VC handler routine for
>            kvm-unit-tests is more work, but it makes test development
>            much more flexible.
>
> So it comes down to the fact that and EFI APP leaves control over
> SEV/TDX specific hypervisor interfaces in the firmware, making it hard
> and unreliable to test these interfaces from kvm-unit-tests. The stub
> approach on the other side gives the tests full control over the VM,
> allowing to test all aspects of the guest-host interface.

I think we might be using terminology differently. (Maybe I mis-used
the term “EFI app”?) With our approach, it is true that all
pre-existing x86_64 test cases work out of the box with the UEFI #VC
handler. However, because kvm-unit-tests calls `ExitBootServices` to
take full control of the system it executes as a “UEFI-stubbed
kernel”. Thus, it should be trivial for test cases to update the IDT
to set up a custom #VC handler for the duration of a test. (Some of
the x86_64 test cases already do something similar where they install
a temporary exception handler and then restore the “default”
kvm-unit-tests exception handler.)

In general, our approach is to set up the test cases to run with the
kvm-unit-tests configuration (e.g., IDT, GDT). The one exception is
the #VC handler. However, all of this state can be overridden within a
test as needed.

Zixuan just posted the patches. So hopefully they make things more clear.

Thanks,
Marc
Varad Gautam Aug. 18, 2021, 8:38 a.m. UTC | #7
Hi Marc, Zixuan,

On 8/18/21 3:52 AM, Marc Orr wrote:
> On Tue, Aug 17, 2021 at 3:49 AM Joerg Roedel <jroedel@suse.de> wrote:
>>
>> Hi Marc,
>>
>> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
>>> To date, we have _most_ x86 test cases (39/44) working under UEFI and
>>> we've also got some of the test cases to boot under SEV-ES, using the
>>> UEFI #VC handler.
>>
>> While the EFI APP approach simplifies the implementation a lot, I don't
>> think it is the best path to SEV and TDX testing for a couple of
>> reasons:
>>
>>         1) It leaves the details of #VC/#VE handling and the SEV-ES
>>            specific communication channels (GHCB) under control of the
>>            firmware. So we can't reliably test those interfaces from an
>>            EFI APP.
>>
>>         2) Same for the memory validation/acceptance interface needed
>>            for SEV-SNP and TDX. Using an EFI APP leaves those under
>>            firmware control and we are not able to reliably test them.
>>
>>         3) The IDT also stays under control of the firmware in an EFI
>>            APP, otherwise the firmware couldn't provide a #VC handler.
>>            This makes it unreliable to test anything IDT or IRQ related.
>>
>>         4) Relying on the firmware #VC hanlder limits the tests to its
>>            abilities. Implementing a separate #VC handler routine for
>>            kvm-unit-tests is more work, but it makes test development
>>            much more flexible.
>>
>> So it comes down to the fact that and EFI APP leaves control over
>> SEV/TDX specific hypervisor interfaces in the firmware, making it hard
>> and unreliable to test these interfaces from kvm-unit-tests. The stub
>> approach on the other side gives the tests full control over the VM,
>> allowing to test all aspects of the guest-host interface.
> 
> I think we might be using terminology differently. (Maybe I mis-used
> the term “EFI app”?) With our approach, it is true that all
> pre-existing x86_64 test cases work out of the box with the UEFI #VC
> handler. However, because kvm-unit-tests calls `ExitBootServices` to
> take full control of the system it executes as a “UEFI-stubbed
> kernel”. Thus, it should be trivial for test cases to update the IDT
> to set up a custom #VC handler for the duration of a test. (Some of
> the x86_64 test cases already do something similar where they install
> a temporary exception handler and then restore the “default”
> kvm-unit-tests exception handler.)
> 
> In general, our approach is to set up the test cases to run with the
> kvm-unit-tests configuration (e.g., IDT, GDT). The one exception is
> the #VC handler. However, all of this state can be overridden within a
> test as needed.
> 
> Zixuan just posted the patches. So hopefully they make things more clear.
> 

Nomenclature aside, I believe Zixuan's patchset [1] takes the same approach
as I posted here. In the end, we need to:
- build the testcases as ELF shared objs and link them to look like a PE
- switch away from UEFI GDT/IDT/pagetable states on early boot to what
  kvm-unit-tests needs
- modify the testcases that contain non-PIC asm stubs to allow building
  them as shared objs

I went with avoiding to bring in gnu-efi objects into kvm-unit-tests
for EFI helpers, and disabling the non-PIC testcases for the RFC's sake.

I'll try out "x86 UEFI: Convert x86 test cases to PIC" [2] from Zixuan's
patchset with my series and see what breaks. I think we can combine
the two patchsets.

[1] https://lore.kernel.org/r/20210818000905.1111226-1-zixuanwang@google.com/
[2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/

Thanks,
Varad

> Thanks,
> Marc
>
Marc Orr Aug. 19, 2021, 1:32 a.m. UTC | #8
On Wed, Aug 18, 2021 at 1:38 AM Varad Gautam <varad.gautam@suse.com> wrote:
>
> Hi Marc, Zixuan,
>
> On 8/18/21 3:52 AM, Marc Orr wrote:
> > On Tue, Aug 17, 2021 at 3:49 AM Joerg Roedel <jroedel@suse.de> wrote:
> >>
> >> Hi Marc,
> >>
> >> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> >>> To date, we have _most_ x86 test cases (39/44) working under UEFI and
> >>> we've also got some of the test cases to boot under SEV-ES, using the
> >>> UEFI #VC handler.
> >>
> >> While the EFI APP approach simplifies the implementation a lot, I don't
> >> think it is the best path to SEV and TDX testing for a couple of
> >> reasons:
> >>
> >>         1) It leaves the details of #VC/#VE handling and the SEV-ES
> >>            specific communication channels (GHCB) under control of the
> >>            firmware. So we can't reliably test those interfaces from an
> >>            EFI APP.
> >>
> >>         2) Same for the memory validation/acceptance interface needed
> >>            for SEV-SNP and TDX. Using an EFI APP leaves those under
> >>            firmware control and we are not able to reliably test them.
> >>
> >>         3) The IDT also stays under control of the firmware in an EFI
> >>            APP, otherwise the firmware couldn't provide a #VC handler.
> >>            This makes it unreliable to test anything IDT or IRQ related.
> >>
> >>         4) Relying on the firmware #VC hanlder limits the tests to its
> >>            abilities. Implementing a separate #VC handler routine for
> >>            kvm-unit-tests is more work, but it makes test development
> >>            much more flexible.
> >>
> >> So it comes down to the fact that and EFI APP leaves control over
> >> SEV/TDX specific hypervisor interfaces in the firmware, making it hard
> >> and unreliable to test these interfaces from kvm-unit-tests. The stub
> >> approach on the other side gives the tests full control over the VM,
> >> allowing to test all aspects of the guest-host interface.
> >
> > I think we might be using terminology differently. (Maybe I mis-used
> > the term “EFI app”?) With our approach, it is true that all
> > pre-existing x86_64 test cases work out of the box with the UEFI #VC
> > handler. However, because kvm-unit-tests calls `ExitBootServices` to
> > take full control of the system it executes as a “UEFI-stubbed
> > kernel”. Thus, it should be trivial for test cases to update the IDT
> > to set up a custom #VC handler for the duration of a test. (Some of
> > the x86_64 test cases already do something similar where they install
> > a temporary exception handler and then restore the “default”
> > kvm-unit-tests exception handler.)
> >
> > In general, our approach is to set up the test cases to run with the
> > kvm-unit-tests configuration (e.g., IDT, GDT). The one exception is
> > the #VC handler. However, all of this state can be overridden within a
> > test as needed.
> >
> > Zixuan just posted the patches. So hopefully they make things more clear.
> >
>
> Nomenclature aside, I believe Zixuan's patchset [1] takes the same approach
> as I posted here. In the end, we need to:
> - build the testcases as ELF shared objs and link them to look like a PE
> - switch away from UEFI GDT/IDT/pagetable states on early boot to what
>   kvm-unit-tests needs
> - modify the testcases that contain non-PIC asm stubs to allow building
>   them as shared objs
>
> I went with avoiding to bring in gnu-efi objects into kvm-unit-tests
> for EFI helpers, and disabling the non-PIC testcases for the RFC's sake.
>
> I'll try out "x86 UEFI: Convert x86 test cases to PIC" [2] from Zixuan's
> patchset with my series and see what breaks. I think we can combine
> the two patchsets.
>
> [1] https://lore.kernel.org/r/20210818000905.1111226-1-zixuanwang@google.com/
> [2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/

This sounds great to us. We will also experiment with combining the
two patchsets and report back when we have some experience with this.
Though, please do also report back if you have an update on this
before we do.

Thanks,
Marc
Nadav Amit Aug. 19, 2021, 1:42 a.m. UTC | #9
> On Aug 18, 2021, at 6:32 PM, Marc Orr <marcorr@google.com> wrote:
> 
> This sounds great to us. We will also experiment with combining the
> two patchsets and report back when we have some experience with this.
> Though, please do also report back if you have an update on this
> before we do.

Just wondering (aka, my standard question): does it work on
bare-metal (putting aside tests that rely on test-devices)?
Zixuan Wang Aug. 19, 2021, 1:54 a.m. UTC | #10
On Wed, Aug 18, 2021 at 6:42 PM Nadav Amit <nadav.amit@gmail.com> wrote:
>
> Just wondering (aka, my standard question): does it work on
> bare-metal (putting aside tests that rely on test-devices)?
>

Hi Nadav,

We tested our patches in VMs, and have not tested on bare-metal machines.

Regards,
Zixuan Wang
Varad Gautam Aug. 19, 2021, 11:36 a.m. UTC | #11
On 8/19/21 3:32 AM, Marc Orr wrote:
> On Wed, Aug 18, 2021 at 1:38 AM Varad Gautam <varad.gautam@suse.com> wrote:
>>
>> Hi Marc, Zixuan,
>>
>> On 8/18/21 3:52 AM, Marc Orr wrote:
>>> On Tue, Aug 17, 2021 at 3:49 AM Joerg Roedel <jroedel@suse.de> wrote:
>>>>
>>>> Hi Marc,
>>>>
>>>> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
>>>>> To date, we have _most_ x86 test cases (39/44) working under UEFI and
>>>>> we've also got some of the test cases to boot under SEV-ES, using the
>>>>> UEFI #VC handler.
>>>>
>>>> While the EFI APP approach simplifies the implementation a lot, I don't
>>>> think it is the best path to SEV and TDX testing for a couple of
>>>> reasons:
>>>>
>>>>         1) It leaves the details of #VC/#VE handling and the SEV-ES
>>>>            specific communication channels (GHCB) under control of the
>>>>            firmware. So we can't reliably test those interfaces from an
>>>>            EFI APP.
>>>>
>>>>         2) Same for the memory validation/acceptance interface needed
>>>>            for SEV-SNP and TDX. Using an EFI APP leaves those under
>>>>            firmware control and we are not able to reliably test them.
>>>>
>>>>         3) The IDT also stays under control of the firmware in an EFI
>>>>            APP, otherwise the firmware couldn't provide a #VC handler.
>>>>            This makes it unreliable to test anything IDT or IRQ related.
>>>>
>>>>         4) Relying on the firmware #VC hanlder limits the tests to its
>>>>            abilities. Implementing a separate #VC handler routine for
>>>>            kvm-unit-tests is more work, but it makes test development
>>>>            much more flexible.
>>>>
>>>> So it comes down to the fact that and EFI APP leaves control over
>>>> SEV/TDX specific hypervisor interfaces in the firmware, making it hard
>>>> and unreliable to test these interfaces from kvm-unit-tests. The stub
>>>> approach on the other side gives the tests full control over the VM,
>>>> allowing to test all aspects of the guest-host interface.
>>>
>>> I think we might be using terminology differently. (Maybe I mis-used
>>> the term “EFI app”?) With our approach, it is true that all
>>> pre-existing x86_64 test cases work out of the box with the UEFI #VC
>>> handler. However, because kvm-unit-tests calls `ExitBootServices` to
>>> take full control of the system it executes as a “UEFI-stubbed
>>> kernel”. Thus, it should be trivial for test cases to update the IDT
>>> to set up a custom #VC handler for the duration of a test. (Some of
>>> the x86_64 test cases already do something similar where they install
>>> a temporary exception handler and then restore the “default”
>>> kvm-unit-tests exception handler.)
>>>
>>> In general, our approach is to set up the test cases to run with the
>>> kvm-unit-tests configuration (e.g., IDT, GDT). The one exception is
>>> the #VC handler. However, all of this state can be overridden within a
>>> test as needed.
>>>
>>> Zixuan just posted the patches. So hopefully they make things more clear.
>>>
>>
>> Nomenclature aside, I believe Zixuan's patchset [1] takes the same approach
>> as I posted here. In the end, we need to:
>> - build the testcases as ELF shared objs and link them to look like a PE
>> - switch away from UEFI GDT/IDT/pagetable states on early boot to what
>>   kvm-unit-tests needs
>> - modify the testcases that contain non-PIC asm stubs to allow building
>>   them as shared objs
>>
>> I went with avoiding to bring in gnu-efi objects into kvm-unit-tests
>> for EFI helpers, and disabling the non-PIC testcases for the RFC's sake.
>>
>> I'll try out "x86 UEFI: Convert x86 test cases to PIC" [2] from Zixuan's
>> patchset with my series and see what breaks. I think we can combine
>> the two patchsets.
>>
>> [1] https://lore.kernel.org/r/20210818000905.1111226-1-zixuanwang@google.com/
>> [2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/
> 
> This sounds great to us. We will also experiment with combining the
> two patchsets and report back when we have some experience with this.
> Though, please do also report back if you have an update on this
> before we do.
> 

I sent out a v2 [1] with Zixuan's "x86 UEFI: Convert x86 test cases to PIC" [2]
pulled in, PTAL.

[1] https://lore.kernel.org/r/20210819113400.26516-1-varad.gautam@suse.com/
[2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/

Thanks,
Varad

> Thanks,
> Marc
>
Marc Orr Aug. 20, 2021, 5:29 p.m. UTC | #12
On Thu, Aug 19, 2021 at 4:36 AM Varad Gautam <varad.gautam@suse.com> wrote:
>
> On 8/19/21 3:32 AM, Marc Orr wrote:
> > On Wed, Aug 18, 2021 at 1:38 AM Varad Gautam <varad.gautam@suse.com> wrote:
> >>
> >> Hi Marc, Zixuan,
> >>
> >> On 8/18/21 3:52 AM, Marc Orr wrote:
> >>> On Tue, Aug 17, 2021 at 3:49 AM Joerg Roedel <jroedel@suse.de> wrote:
> >>>>
> >>>> Hi Marc,
> >>>>
> >>>> On Fri, Aug 13, 2021 at 11:44:39AM -0700, Marc Orr wrote:
> >>>>> To date, we have _most_ x86 test cases (39/44) working under UEFI and
> >>>>> we've also got some of the test cases to boot under SEV-ES, using the
> >>>>> UEFI #VC handler.
> >>>>
> >>>> While the EFI APP approach simplifies the implementation a lot, I don't
> >>>> think it is the best path to SEV and TDX testing for a couple of
> >>>> reasons:
> >>>>
> >>>>         1) It leaves the details of #VC/#VE handling and the SEV-ES
> >>>>            specific communication channels (GHCB) under control of the
> >>>>            firmware. So we can't reliably test those interfaces from an
> >>>>            EFI APP.
> >>>>
> >>>>         2) Same for the memory validation/acceptance interface needed
> >>>>            for SEV-SNP and TDX. Using an EFI APP leaves those under
> >>>>            firmware control and we are not able to reliably test them.
> >>>>
> >>>>         3) The IDT also stays under control of the firmware in an EFI
> >>>>            APP, otherwise the firmware couldn't provide a #VC handler.
> >>>>            This makes it unreliable to test anything IDT or IRQ related.
> >>>>
> >>>>         4) Relying on the firmware #VC hanlder limits the tests to its
> >>>>            abilities. Implementing a separate #VC handler routine for
> >>>>            kvm-unit-tests is more work, but it makes test development
> >>>>            much more flexible.
> >>>>
> >>>> So it comes down to the fact that and EFI APP leaves control over
> >>>> SEV/TDX specific hypervisor interfaces in the firmware, making it hard
> >>>> and unreliable to test these interfaces from kvm-unit-tests. The stub
> >>>> approach on the other side gives the tests full control over the VM,
> >>>> allowing to test all aspects of the guest-host interface.
> >>>
> >>> I think we might be using terminology differently. (Maybe I mis-used
> >>> the term “EFI app”?) With our approach, it is true that all
> >>> pre-existing x86_64 test cases work out of the box with the UEFI #VC
> >>> handler. However, because kvm-unit-tests calls `ExitBootServices` to
> >>> take full control of the system it executes as a “UEFI-stubbed
> >>> kernel”. Thus, it should be trivial for test cases to update the IDT
> >>> to set up a custom #VC handler for the duration of a test. (Some of
> >>> the x86_64 test cases already do something similar where they install
> >>> a temporary exception handler and then restore the “default”
> >>> kvm-unit-tests exception handler.)
> >>>
> >>> In general, our approach is to set up the test cases to run with the
> >>> kvm-unit-tests configuration (e.g., IDT, GDT). The one exception is
> >>> the #VC handler. However, all of this state can be overridden within a
> >>> test as needed.
> >>>
> >>> Zixuan just posted the patches. So hopefully they make things more clear.
> >>>
> >>
> >> Nomenclature aside, I believe Zixuan's patchset [1] takes the same approach
> >> as I posted here. In the end, we need to:
> >> - build the testcases as ELF shared objs and link them to look like a PE
> >> - switch away from UEFI GDT/IDT/pagetable states on early boot to what
> >>   kvm-unit-tests needs
> >> - modify the testcases that contain non-PIC asm stubs to allow building
> >>   them as shared objs
> >>
> >> I went with avoiding to bring in gnu-efi objects into kvm-unit-tests
> >> for EFI helpers, and disabling the non-PIC testcases for the RFC's sake.
> >>
> >> I'll try out "x86 UEFI: Convert x86 test cases to PIC" [2] from Zixuan's
> >> patchset with my series and see what breaks. I think we can combine
> >> the two patchsets.
> >>
> >> [1] https://lore.kernel.org/r/20210818000905.1111226-1-zixuanwang@google.com/
> >> [2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/
> >
> > This sounds great to us. We will also experiment with combining the
> > two patchsets and report back when we have some experience with this.
> > Though, please do also report back if you have an update on this
> > before we do.
> >
>
> I sent out a v2 [1] with Zixuan's "x86 UEFI: Convert x86 test cases to PIC" [2]
> pulled in, PTAL.
>
> [1] https://lore.kernel.org/r/20210819113400.26516-1-varad.gautam@suse.com/
> [2] https://lore.kernel.org/r/20210818000905.1111226-10-zixuanwang@google.com/

Thanks. This is a good step. However, after reviewing this new patch
set I think we should go a step further and completely combine the two
patch sets. This way, we get the benefit of Varad’s patches, which is
that we don’t need to link the gnu-efi library. And we get the
benefits of Zixuan’s patches which are twofold: (1) the vast majority
of the x86_64 test cases work under UEFI (optionally with SEV-ES) and
(2) much of the assembly code is refactored into C, making it more
readable/maintainable.

We’ve started experimenting with using Varad’s patch set as the
foundation for Zixuan's patch set. Initial testing on this
Frankenstein patch set is encouraging. I’d like to see Zixuan finish
this effort. Please give us a few days to further test and organize
the combined patch set. We’ll post it as soon as we can. Likely early
next week.

Thanks,
Marc