After a toolchain upgrade (I think), the x86 fix_hypercall_test started
throwing warnings due to -Werror=array-bounds rightly complaining that
the test is generating an out-of-bounds array access.

The "obvious" fix is to replace the memcpy() with a memcmp() and compare
only the exact size of the hypercall instruction.  That worked, until I
fiddled with the code a bit more and suddenly the test started jumping into
the weeds due to gcc generating a call to the external memcmp() through the
PLT, which isn't supported in the selftests.

To fix that mess, which has been a pitfall for quite some time, provide
implementations of memcmp(), memcpy(), and memset() to effectively override
the compiler built-ins.  My thought is to start with the helpers that are
most likely to be used in guest code, and then add more as needed.

Tested on x86 and ARM, compile tested on RISC-V and s390.  Full testing on
RISC-V and s390 would be welcome, the seemingly benign addition of memxxx()
helpers managed to break ARM due to gcc generating an infinite loop for
memset() (see patch 1 for details).

Sean Christopherson (5):
  KVM: selftests: Implement memcmp(), memcpy(), and memset() for guest
    use
  KVM: selftests: Compare insn opcodes directly in fix_hypercall_test
  KVM: selftests: Remove unnecessary register shuffling in
    fix_hypercall_test
  KVM: selftests: Explicitly verify KVM doesn't patch hypercall if
    quirk==off
  KVM: selftests: Dedup subtests of fix_hypercall_test

 tools/testing/selftests/kvm/Makefile          |   8 +-
 .../selftests/kvm/include/kvm_util_base.h     |  10 ++
 tools/testing/selftests/kvm/lib/kvm_string.c  |  33 +++++
 .../selftests/kvm/x86_64/fix_hypercall_test.c | 124 ++++++++----------
 4 files changed, 107 insertions(+), 68 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/kvm_string.c


base-commit: 29250ba51bc1cbe8a87e923f76978b87c3247a8c
-- 
2.37.2.789.g6183377224-goog

Am 09.09.22 um 01:31 schrieb Sean Christopherson:
> After a toolchain upgrade (I think), the x86 fix_hypercall_test started
> throwing warnings due to -Werror=array-bounds rightly complaining that
> the test is generating an out-of-bounds array access.
> 
> The "obvious" fix is to replace the memcpy() with a memcmp() and compare
> only the exact size of the hypercall instruction.  That worked, until I
> fiddled with the code a bit more and suddenly the test started jumping into
> the weeds due to gcc generating a call to the external memcmp() through the
> PLT, which isn't supported in the selftests.
> 
> To fix that mess, which has been a pitfall for quite some time, provide
> implementations of memcmp(), memcpy(), and memset() to effectively override
> the compiler built-ins.  My thought is to start with the helpers that are
> most likely to be used in guest code, and then add more as needed.
> 
> Tested on x86 and ARM, compile tested on RISC-V and s390.  Full testing on
> RISC-V and s390 would be welcome, the seemingly benign addition of memxxx()
> helpers managed to break ARM due to gcc generating an infinite loop for
> memset() (see patch 1 for details).

Seems to run fine on s390.

On Thu, Sep 8, 2022 at 4:34 PM Sean Christopherson <seanjc@google.com> wrote:
>
> After a toolchain upgrade (I think), the x86 fix_hypercall_test started
> throwing warnings due to -Werror=array-bounds rightly complaining that
> the test is generating an out-of-bounds array access.
>
> The "obvious" fix is to replace the memcpy() with a memcmp() and compare
> only the exact size of the hypercall instruction.  That worked, until I
> fiddled with the code a bit more and suddenly the test started jumping into
> the weeds due to gcc generating a call to the external memcmp() through the
> PLT, which isn't supported in the selftests.
>
> To fix that mess, which has been a pitfall for quite some time, provide
> implementations of memcmp(), memcpy(), and memset() to effectively override
> the compiler built-ins.  My thought is to start with the helpers that are
> most likely to be used in guest code, and then add more as needed.

Ah ha! This also fixes an issue I've long since noticed and finally
got around to debugging this morning. userspace_io_test fails for me
when built with Clang but passes with GCC. It turns out Clang
generates a call to <memset@plt>, whereas GCC directly generates rep
stos, to clear @buffer in guest_code().

On Thu, Sep 22, 2022 at 10:20 AM David Matlack <dmatlack@google.com> wrote:
>
> On Thu, Sep 8, 2022 at 4:34 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > After a toolchain upgrade (I think), the x86 fix_hypercall_test started
> > throwing warnings due to -Werror=array-bounds rightly complaining that
> > the test is generating an out-of-bounds array access.
> >
> > The "obvious" fix is to replace the memcpy() with a memcmp() and compare
> > only the exact size of the hypercall instruction.  That worked, until I
> > fiddled with the code a bit more and suddenly the test started jumping into
> > the weeds due to gcc generating a call to the external memcmp() through the
> > PLT, which isn't supported in the selftests.
> >
> > To fix that mess, which has been a pitfall for quite some time, provide
> > implementations of memcmp(), memcpy(), and memset() to effectively override
> > the compiler built-ins.  My thought is to start with the helpers that are
> > most likely to be used in guest code, and then add more as needed.
>
> Ah ha! This also fixes an issue I've long since noticed and finally
> got around to debugging this morning. userspace_io_test fails for me
> when built with Clang but passes with GCC. It turns out Clang
> generates a call to <memset@plt>, whereas GCC directly generates rep
> stos, to clear @buffer in guest_code().

Hey! Did I miss a revert of commit ed290e1c20da ("KVM: selftests: Fix
nested SVM tests when built with clang") in that patch set?

On Thu, Sep 22, 2022, Jim Mattson wrote:
> On Thu, Sep 22, 2022 at 10:20 AM David Matlack <dmatlack@google.com> wrote:
> >
> > On Thu, Sep 8, 2022 at 4:34 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > After a toolchain upgrade (I think), the x86 fix_hypercall_test started
> > > throwing warnings due to -Werror=array-bounds rightly complaining that
> > > the test is generating an out-of-bounds array access.
> > >
> > > The "obvious" fix is to replace the memcpy() with a memcmp() and compare
> > > only the exact size of the hypercall instruction.  That worked, until I
> > > fiddled with the code a bit more and suddenly the test started jumping into
> > > the weeds due to gcc generating a call to the external memcmp() through the
> > > PLT, which isn't supported in the selftests.
> > >
> > > To fix that mess, which has been a pitfall for quite some time, provide
> > > implementations of memcmp(), memcpy(), and memset() to effectively override
> > > the compiler built-ins.  My thought is to start with the helpers that are
> > > most likely to be used in guest code, and then add more as needed.
> >
> > Ah ha! This also fixes an issue I've long since noticed and finally
> > got around to debugging this morning. userspace_io_test fails for me
> > when built with Clang but passes with GCC. It turns out Clang
> > generates a call to <memset@plt>, whereas GCC directly generates rep
> > stos, to clear @buffer in guest_code().
> 
> Hey! Did I miss a revert of commit ed290e1c20da ("KVM: selftests: Fix
> nested SVM tests when built with clang") in that patch set?

LOL, no, no you did not.  I'll do that in v2.