[0/8] Add printf and formatted asserts in the guest

Message

Aaron Lewis March 1, 2023, 5:34 a.m. UTC

Extend the ucall framework to offer GUEST_PRINTF and GUEST_ASSERT_FMT
in selftests.  This allows the guest to use string formating for easier
debugging and richer assert messages.

I was originally going to send this series out as an RFC to demonstrate
two different ways of implementing prints in the guest (both through the
same ucall interface).  I ultimately decided against it because the other
approach had enough cons to convince me that this one is better.

As a quick overview on the approach I abondoned, it involved adding enough
support to the guest to allow it to call the LIBC version of vsprintf()
directly.  In order to do that a simple TLS segment had to be added to the
guest, AVX-512 needed to be enabled, and the loadable ELF segments needed
to be copied from the host.  This all seemed very intrusive to the guest.
I tried reducing this burden by getting the string functions to not
use AVX-512 instructions, unfortunately the complier flags I tried didn't
work.  Also, the approach used in this series is far less intrusive to
the guest anyway, so I abondoned it.

That exercise informed how I set up the selftest.  The selftest, aside
from using GUEST_PRINTF and GUEST_ASSERT_FMT, also checks XCR0 to show
that the prints work without AVX-512 being enabled.  Two things I
learned LIBC loves to do when using string functions is use the TLS and
call AVX-512 instructions.  Either of which will cause the test to either
crash or (unintentionally) assert.

I say unintentionally assert because the test ends with a formatted
assert firing.  This is intentional, and is meant to demonstrate the
formatted assert.

That is one reason I don't really expect the selftest to be accepted with
this series.  The other reason is it doesn't test anything in the kernel.
And if the selftest is not accepted then the first two patches can be
omitted too.  The core of the series are patches 3-7.

Patches 1-2:
 - Adds XGETBV/XSETBV and xfeature flags to common code.
 - Needed for the selftest added in the final commit.
 - Also included in:
     https://lore.kernel.org/kvm/[e-mail]/
 - Can be omitted from the final series along with the selftest.

Patches 3-5:
 - Adds a local version of vsprintf to the selftests for the guest.

Patches 6-7:
 - Adds GUEST_PRINTF and GUEST_ASSERT_FMT to the ucall framework.

Patch 8:
 - Adds a selftest to demonstrate the usage of prints and formatted
   asserts in the guest.
 - This test is a demo and doesn't have to be merged with this series,
   which also means patches 1 and 2 don't have to be merged either.

Aaron Lewis (8):
  KVM: selftests: Hoist XGETBV and XSETBV to make them more accessible
  KVM: selftests: Add XFEATURE masks to common code
  KVM: selftests: Add strnlen() to the string overrides
  KVM: selftests: Copy printf.c to KVM selftests
  KVM: selftests: Add vsprintf() to KVM selftests
  KVM: selftests: Add additional pages to the guest to accommodate ucall
  KVM: selftests: Add string formatting options to ucall
  KVM: selftests: Add a selftest for guest prints and formatted asserts

 tools/testing/selftests/kvm/Makefile          |   3 +
 .../testing/selftests/kvm/include/test_util.h |   2 +
 .../selftests/kvm/include/ucall_common.h      |  20 ++
 .../selftests/kvm/include/x86_64/processor.h  |  36 +++
 tools/testing/selftests/kvm/lib/kvm_util.c    |   4 +
 tools/testing/selftests/kvm/lib/printf.c      | 286 ++++++++++++++++++
 .../selftests/kvm/lib/string_override.c       |   9 +
 .../testing/selftests/kvm/lib/ucall_common.c  |  24 ++
 tools/testing/selftests/kvm/x86_64/amx_test.c |  46 +--
 .../selftests/kvm/x86_64/guest_print_test.c   | 100 ++++++
 10 files changed, 495 insertions(+), 35 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/printf.c
 create mode 100644 tools/testing/selftests/kvm/x86_64/guest_print_test.c

Comments

Sean Christopherson March 23, 2023, 8:57 p.m. UTC | #1

On Wed, Mar 01, 2023, Aaron Lewis wrote:
> I say unintentionally assert because the test ends with a formatted
> assert firing.  This is intentional, and is meant to demonstrate the
> formatted assert.
> 
> That is one reason I don't really expect the selftest to be accepted with
> this series.  The other reason is it doesn't test anything in the kernel.

I don't have any objection to a selftest that tests selftests.  But it should
actually be a proper test and not something the user has to manually verify.
One thought would be to have the host side of the test pass in params to the
guest, and then have the the guest assert (or not) with a hardcoded format string.

Then on the host side, don't treat UCALL_ABORT as a failure and instead verify
that it fired when expected, and also provided the correct string, e.g. with a
strcmp() or whatever.  And do the same for GUEST_PRINTF/UCALL_PRINTF.

And it should be arch-agnostic, because at a galnce, the actual guts in patches 3-7
don't have an arch specific enabling.

E.g. something like this, and then use PRINTF_STRING and ASSERT_STRING in the
host to generate and verify the string.

#define PRINTF_STRING "Got params a = '0x%lx' and b = '0x%lx instead'"
#define ASSERT_STRING "Expected 0x%lx, got 0x%lx instead"

static void guest_code(uint64_t a, uint64_t b)
{
	GUEST_PRINTF(PRINTF_STRING, a, b);
	GUEST_ASSERT_FMT(a == b, ASSERT_FMT, a, b);
	GUEST_DONE();
}

> And if the selftest is not accepted then the first two patches can be
> omitted too.  The core of the series are patches 3-7.

As above, the first two patches should be omitted anyways, because guest_print_test.c
shouldn't be x86-specific.