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

[Sean Christopherson <seanjc@google.com>]

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.