On Wed, Sep 29, 2021 at 6:30 PM Ricardo Koller <ricarkol@xxxxxxxxxx> wrote:
>
> On Wed, Sep 29, 2021 at 05:36:49PM -0700, Jim Mattson wrote:
> > Though gcc conveniently compiles a simple memset to "rep stos," clang
> > prefers to call the libc version of memset. If a test is dynamically
> > linked, the libc memset isn't available in L1 (nor is the PLT or the
> > GOT, for that matter). Even if the test is statically linked, the libc
> > memset may choose to use some CPU features, like AVX, which may not be
> > enabled in L1. Note that __builtin_memset doesn't solve the problem,
> > because (a) the compiler is free to call memset anyway, and (b)
> > __builtin_memset may also choose to use features like AVX, which may
> > not be available in L1.
> >
> > To avoid a myriad of problems, use an explicit "rep stos" to clear the
> > VMCB in generic_svm_setup(), which is called both from L0 and L1.
> >
> > Reported-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
> > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > Fixes: 20ba262f8631a ("selftests: KVM: AMD Nested test infrastructure")
> > ---
> > tools/testing/selftests/kvm/lib/x86_64/svm.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> > index 2ac98d70d02b..161eba7cd128 100644
> > --- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
> > +++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
> > @@ -54,6 +54,18 @@ static void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
> > seg->base = base;
> > }
> >
> > +/*
> > + * Avoid using memset to clear the vmcb, since libc may not be
> > + * available in L1 (and, even if it is, features that libc memset may
> > + * want to use, like AVX, may not be enabled).
> > + */
> > +static void clear_vmcb(struct vmcb *vmcb)
> > +{
> > + int n = sizeof(*vmcb) / sizeof(u32);
> > +
> > + asm volatile ("rep stosl" : "+c"(n), "+D"(vmcb) : "a"(0) : "memory");
> > +}
> > +
> > void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_rsp)
> > {
> > struct vmcb *vmcb = svm->vmcb;
> > @@ -70,7 +82,7 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip, void *guest_r
> > wrmsr(MSR_EFER, efer | EFER_SVME);
> > wrmsr(MSR_VM_HSAVE_PA, svm->save_area_gpa);
> >
> > - memset(vmcb, 0, sizeof(*vmcb));
> > + clear_vmcb(vmcb);
> > asm volatile ("vmsave %0\n\t" : : "a" (vmcb_gpa) : "memory");
> > vmcb_set_seg(&save->es, get_es(), 0, -1U, data_seg_attr);
> > vmcb_set_seg(&save->cs, get_cs(), 0, -1U, code_seg_attr);
> > --
> > 2.33.0.685.g46640cef36-goog
> >
>
> Reviewed-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
Ping!
