On Fri, Aug 20, 2021 at 4:50 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Wed, Aug 18, 2021, Zixuan Wang wrote: > > AMD SEV-ES introduces a new #VC exception that handles the communication > > between guest and host. UEFI already implements a #VC handler so there > > is no need to re-implement it in KVM-Unit-Tests. To reuse this #VC > > handler, this commit reads UEFI's IDT, copy the #VC IDT entry into > > KVM-Unit-Tests' IDT. > > > > In this commit, load_idt() can work and now guest crashes in > > setup_page_table(), which will be fixed by follow-up commits. > > As a stop gap to get SEV testing of any kind enabled, I think piggybacking the > vBIOS #VC handler is a great idea. But long term, kvm-unit-tests absolutely > needs to have its own #VC handler. > > In addition to the downsides Joerg pointed out[*], relying on an external #VC > introduces the possibility of test failures that are tied to the vBIOS being > used. Such dependencies already exist to some extent, e.g. using a buggy QEMU or > SeaBIOS could certainly introduce failures, but those components are far more > mature and less likely to break in weird ways unique to a specific test. > > Another potential issue is that it's possible vBIOS will be enlightened to the > point where it _never_ expects a #VC, e.g. does #VMGEXIT directly, and thus panics > on any #VC instead of requesting the necessary emulation. > > Fixing the vBIOS image in the repo would mostly solve those problems, but it > wouldn't solve the lack of flexibility for the #VC handler, and debugging a third > party #VC handler would likely be far more difficult to debug when failures > inevitably occur. > > So, if these shenanigans give us test coverage now instead of a few months from > now, than I say go for it. But, we need clear line of sight to a "native" #VC > handler, confidence that it will actually get written in a timely manner, and an > easily reverted set of commits to unwind all of the UEFI stuff. > > [*] https://lkml.kernel.org/r/YRuURERGp8CQ1jAX@xxxxxxx Understood. I must admit, we didn’t have this long term perspective when drafting these patches. But after reading this feedback, we see your point. Luckily, unwinding the code to install the UEFI #VC handler is trivial. Also, we do believe that completing and submitting this patch series such that it uses the UEFI’s #VC handler is the best next step, even with the understanding that it’s not where we want to be six months to one year from now. The reason is that adding a new #VC handler is non-trivial. It seems like a separate patch set. At the same time, using the UEFI #VC handler unlocks a lot of testing (that’s totally non-existent now) and it should be trivial to plumb in the (to be written) kvm-unit-tests #VC handler. In other words, with this patch the community can start using and adding to the tests that are unlocked by the UEFI #VC handler while someone (in parallel) works on a follow-on patch set to add a #VC handler to kvm-unit-tests.
