Re: [PATCH v4 11/12] KVM: selftests: Support running stress save+restore and #PF test in L2

[Yosry Ahmed]

On Tue, Jul 28, 2026 at 7:43 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Jul 27, 2026, Yosry Ahmed wrote:
> > Extend the stress test to allow running the access+#PF code in L2
> > instead of L1 by adding proper L1 guest code to bootstrap L2. By
> > default, the test runs in L2 after running in L1 if nested is supported.
> >
> > Assisted-by: Gemini:gemini-3.1-pro
> > Signed-off-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> > ---
> >  .../kvm/x86/save_restore_pf_stress_test.c     | 61 ++++++++++++++++++-
> >  1 file changed, 58 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
> > index 664ed280b2e76..0e5ddeb5af444 100644
> > --- a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
> > +++ b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
> > @@ -8,10 +8,13 @@
> >  #include <pthread.h>
> >  #include <signal.h>
> >  #include <unistd.h>
> > +#include <getopt.h>
> >
> >  #include "test_util.h"
> >  #include "kvm_util.h"
> >  #include "processor.h"
> > +#include "svm_util.h"
> > +#include "vmx.h"
> >
> >  #define NR_ITERATIONS                500
> >
> > @@ -81,6 +84,36 @@ static void guest_access_memory(void *arg)
> >       }
> >  }
> >
> > +static void l1_svm_code(struct svm_test_data *svm)
> > +{
> > +     generic_svm_setup(svm, guest_access_memory);
> > +     run_guest(svm->vmcb, svm->vmcb_gpa);
> > +     GUEST_ASSERT(false);
> > +}
> > +
> > +static void l1_vmx_code(struct vmx_pages *vmx)
> > +{
> > +     GUEST_ASSERT(prepare_for_vmx_operation(vmx));
> > +     GUEST_ASSERT(load_vmcs(vmx));
> > +     prepare_vmcs(vmx, guest_access_memory);
> > +
> > +     /* Ignore any #PF */
>
> This comment is rather ambiguous, especially when the #UD interception comes
> along, because unless the reader is reading carefully and sees the PFEC_MASK
> and PFEC_MATCH logic below, it's quite easy to read this as "intercept #PF to
> ignore them".
>
> I don't see any reason to write the code this way.  Yeah yeah, it's the SDM's
> suggested way to ignore #PFs, but IMO that's unnecessarily convoluted.  The more
> obvious way is to leave all fields '0'.
>
> In other words, just delete this entire block of code and rely on the default
> VMX allocation logic to disable interception of all exceptions.
>
> Oh, damnit.  Argh.  I see why you need this.  init_vmcs_control_fields() is buggy
> and actually configures #PF for interception.  Apparently Paolo didn't read the
> "If there is inequality, the meaning of that bit is reversed (for example, a VM
> exit occurs if that bit is clear)." part :-D
>
> So, let's fix that in a prep patch (AFAICT, no existing tests cares about #PF
> interception) so that this test doesn't have to carry confusing code, and because
> intercpeting #PF but nothing else by default is bound to cause problems.

Yeah that felt weird, but the comment "Never match" made it feel like
it was intentional. Anyway, I just sent v5 with your diff below as a
patch, thanks!

>
> diff --git tools/testing/selftests/kvm/lib/x86/vmx.c tools/testing/selftests/kvm/lib/x86/vmx.c
> index cd09c9de4485..089e1a8af53f 100644
> --- tools/testing/selftests/kvm/lib/x86/vmx.c
> +++ tools/testing/selftests/kvm/lib/x86/vmx.c
> @@ -232,7 +232,7 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
>
>         vmwrite(EXCEPTION_BITMAP, 0);
>         vmwrite(PAGE_FAULT_ERROR_CODE_MASK, 0);
> -       vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, -1); /* Never match */
> +       vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, 0);
>         vmwrite(CR3_TARGET_COUNT, 0);
>         vmwrite(VM_EXIT_CONTROLS, rdmsr(MSR_IA32_VMX_EXIT_CTLS) |
>                 VM_EXIT_HOST_ADDR_SPACE_SIZE);    /* 64-bit host */
>
> > +     GUEST_ASSERT(!vmwrite(EXCEPTION_BITMAP, BIT(PF_VECTOR)));
> > +     GUEST_ASSERT(!vmwrite(PAGE_FAULT_ERROR_CODE_MASK, 0));
> > +     GUEST_ASSERT(!vmwrite(PAGE_FAULT_ERROR_CODE_MATCH, -1));
> > +
> > +     GUEST_ASSERT(!vmlaunch());
> > +     GUEST_ASSERT(false);
> > +}