[PATCH v2 0/6] Add printf and formatted asserts in the guest

[PATCH v2 0/6] Add printf and formatted asserts in the guest

[Aaron Lewis]

Extend the ucall framework to offer GUEST_PRINTF() and GUEST_ASSERT_FMT()
in selftests.  This will allow for better and easier guest debugging.

v1 -> v2:
 - Added a proper selftest [Sean]
 - Added support for snprintf [Shaoqin]
 - Added ucall_nr_pages_required() [Sean]
 - Added ucall_fmt2() for GUEST_ASSERT_FMT()
 - Dropped the original version of printf.c [Sean]
 - Dropped patches 1-2 and 8 [Sean]

Aaron Lewis (6):
  KVM: selftests: Add strnlen() to the string overrides
  KVM: selftests: Add kvm_snprintf() 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 ucall_fmt2()
  KVM: selftests: Add a selftest for guest prints and formatted asserts

 tools/testing/selftests/kvm/Makefile          |   3 +
 .../testing/selftests/kvm/guest_print_test.c  | 207 ++++++++++++
 .../testing/selftests/kvm/include/test_util.h |   3 +
 .../selftests/kvm/include/ucall_common.h      |  20 ++
 tools/testing/selftests/kvm/lib/kvm_sprintf.c | 313 ++++++++++++++++++
 tools/testing/selftests/kvm/lib/kvm_util.c    |   4 +
 .../selftests/kvm/lib/string_override.c       |   9 +
 .../testing/selftests/kvm/lib/ucall_common.c  |  47 +++
 8 files changed, 606 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/guest_print_test.c
 create mode 100644 tools/testing/selftests/kvm/lib/kvm_sprintf.c

-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH v2 1/6] KVM: selftests: Add strnlen() to the string overrides

[Aaron Lewis]

Add strnlen() to the string overrides to allow it to be called in the
guest.

The implementation for strnlen() was taken from the kernel's generic
version, lib/string.c.

This will be needed when printf() is introduced.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 tools/testing/selftests/kvm/Makefile              | 1 +
 tools/testing/selftests/kvm/lib/string_override.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 18cadc669798..d93bee00c72a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -198,6 +198,7 @@ endif
 CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
 	-Wno-gnu-variable-sized-type-not-at-end \
 	-fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \
+	-fno-builtin-strnlen \
 	-fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \
 	-I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \
 	-I$(<D) -Iinclude/$(ARCH_DIR) -I ../rseq -I.. $(EXTRA_CFLAGS) \
diff --git a/tools/testing/selftests/kvm/lib/string_override.c b/tools/testing/selftests/kvm/lib/string_override.c
index 632398adc229..5d1c87277c49 100644
--- a/tools/testing/selftests/kvm/lib/string_override.c
+++ b/tools/testing/selftests/kvm/lib/string_override.c
@@ -37,3 +37,12 @@ void *memset(void *s, int c, size_t count)
 		*xs++ = c;
 	return s;
 }
+
+size_t strnlen(const char *s, size_t count)
+{
+	const char *sc;
+
+	for (sc = s; count-- && *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
-- 
2.40.0.634.g4ca3ef3211-goog

[PATCH v2 2/6] KVM: selftests: Add kvm_snprintf() to KVM selftests

[Aaron Lewis]

Add a local version of kvm_snprintf() for use in the guest.

Having a local copy allows the guest access to string formatting
options without dependencies on LIBC.  LIBC is problematic because
it heavily relies on both AVX-512 instructions and a TLS, neither of
which are guaranteed to be set up in the guest.

The file kvm_sprintf.c was lifted from arch/x86/boot/printf.c and
adapted to work in the guest, including the addition of buffer length.
I.e. s/sprintf/snprintf/

The functions where prefixed with "kvm_" to allow guests to explicitly
call them.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../testing/selftests/kvm/include/test_util.h |   3 +
 tools/testing/selftests/kvm/lib/kvm_sprintf.c | 313 ++++++++++++++++++
 3 files changed, 317 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/kvm_sprintf.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index d93bee00c72a..84b126398729 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -23,6 +23,7 @@ LIBKVM += lib/guest_modes.c
 LIBKVM += lib/io.c
 LIBKVM += lib/kvm_util.c
 LIBKVM += lib/memstress.c
+LIBKVM += lib/kvm_sprintf.c
 LIBKVM += lib/rbtree.c
 LIBKVM += lib/sparsebit.c
 LIBKVM += lib/test_util.c
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index a6e9f215ce70..45cb0dd41412 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -186,4 +186,7 @@ static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
 	return num;
 }
 
+int kvm_vsnprintf(char *buf, int n, const char *fmt, va_list args);
+int kvm_snprintf(char *buf, int n, const char *fmt, ...);
+
 #endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/kvm_sprintf.c b/tools/testing/selftests/kvm/lib/kvm_sprintf.c
new file mode 100644
index 000000000000..db369e00a6fc
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/kvm_sprintf.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include "test_util.h"
+
+#define ASSIGN_AND_INC_SAFE(str, end, v) \
+({					 \
+	if (str < end)			 \
+		*str = (v);		 \
+	str++;				 \
+})
+
+static int isdigit(int ch)
+{
+	return (ch >= '0') && (ch <= '9');
+}
+
+static int skip_atoi(const char **s)
+{
+	int i = 0;
+
+	while (isdigit(**s))
+		i = i * 10 + *((*s)++) - '0';
+	return i;
+}
+
+#define ZEROPAD	1		/* pad with zero */
+#define SIGN	2		/* unsigned/signed long */
+#define PLUS	4		/* show plus */
+#define SPACE	8		/* space if plus */
+#define LEFT	16		/* left justified */
+#define SMALL	32		/* Must be 32 == 0x20 */
+#define SPECIAL	64		/* 0x */
+
+#define __do_div(n, base)				\
+({							\
+	int __res;					\
+							\
+	__res = ((uint64_t) n) % (uint32_t) base;	\
+	n = ((uint64_t) n) / (uint32_t) base;		\
+	__res;						\
+})
+
+static char *number(char *str, const char *end, long num, int base, int size,
+		    int precision, int type)
+{
+	/* we are called with base 8, 10 or 16, only, thus don't need "G..."  */
+	static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
+
+	char tmp[66];
+	char c, sign, locase;
+	int i;
+
+	/*
+	 * locase = 0 or 0x20. ORing digits or letters with 'locase'
+	 * produces same digits or (maybe lowercased) letters
+	 */
+	locase = (type & SMALL);
+	if (type & LEFT)
+		type &= ~ZEROPAD;
+	if (base < 2 || base > 16)
+		return NULL;
+	c = (type & ZEROPAD) ? '0' : ' ';
+	sign = 0;
+	if (type & SIGN) {
+		if (num < 0) {
+			sign = '-';
+			num = -num;
+			size--;
+		} else if (type & PLUS) {
+			sign = '+';
+			size--;
+		} else if (type & SPACE) {
+			sign = ' ';
+			size--;
+		}
+	}
+	if (type & SPECIAL) {
+		if (base == 16)
+			size -= 2;
+		else if (base == 8)
+			size--;
+	}
+	i = 0;
+	if (num == 0)
+		tmp[i++] = '0';
+	else
+		while (num != 0)
+			tmp[i++] = (digits[__do_div(num, base)] | locase);
+	if (i > precision)
+		precision = i;
+	size -= precision;
+	if (!(type & (ZEROPAD + LEFT)))
+		while (size-- > 0)
+			ASSIGN_AND_INC_SAFE(str, end, ' ');
+	if (sign)
+		ASSIGN_AND_INC_SAFE(str, end, sign);
+	if (type & SPECIAL) {
+		if (base == 8)
+			ASSIGN_AND_INC_SAFE(str, end, '0');
+		else if (base == 16) {
+			ASSIGN_AND_INC_SAFE(str, end, '0');
+			ASSIGN_AND_INC_SAFE(str, end, 'x');
+		}
+	}
+	if (!(type & LEFT))
+		while (size-- > 0)
+			ASSIGN_AND_INC_SAFE(str, end, c);
+	while (i < precision--)
+		ASSIGN_AND_INC_SAFE(str, end, '0');
+	while (i-- > 0)
+		ASSIGN_AND_INC_SAFE(str, end, tmp[i]);
+	while (size-- > 0)
+		ASSIGN_AND_INC_SAFE(str, end, ' ');
+
+	return str;
+}
+
+int kvm_vsnprintf(char *buf, int n, const char *fmt, va_list args)
+{
+	char *str, *end;
+	const char *s;
+	uint64_t num;
+	int i, base;
+	int len;
+
+	int flags;		/* flags to number() */
+
+	int field_width;	/* width of output field */
+	int precision;		/*
+				 * min. # of digits for integers; max
+				 * number of chars for from string
+				 */
+	int qualifier;		/* 'h', 'l', or 'L' for integer fields */
+
+	end = buf + n;
+	/* Make sure end is always >= buf */
+	if (end < buf) {
+		end = ((void *)-1);
+		n = end - buf;
+	}
+
+	for (str = buf; *fmt; ++fmt) {
+		if (*fmt != '%') {
+			ASSIGN_AND_INC_SAFE(str, end, *fmt);
+			continue;
+		}
+
+		/* process flags */
+		flags = 0;
+repeat:
+		++fmt;		/* this also skips first '%' */
+		switch (*fmt) {
+		case '-':
+			flags |= LEFT;
+			goto repeat;
+		case '+':
+			flags |= PLUS;
+			goto repeat;
+		case ' ':
+			flags |= SPACE;
+			goto repeat;
+		case '#':
+			flags |= SPECIAL;
+			goto repeat;
+		case '0':
+			flags |= ZEROPAD;
+			goto repeat;
+		}
+
+		/* get field width */
+		field_width = -1;
+		if (isdigit(*fmt))
+			field_width = skip_atoi(&fmt);
+		else if (*fmt == '*') {
+			++fmt;
+			/* it's the next argument */
+			field_width = va_arg(args, int);
+			if (field_width < 0) {
+				field_width = -field_width;
+				flags |= LEFT;
+			}
+		}
+
+		/* get the precision */
+		precision = -1;
+		if (*fmt == '.') {
+			++fmt;
+			if (isdigit(*fmt))
+				precision = skip_atoi(&fmt);
+			else if (*fmt == '*') {
+				++fmt;
+				/* it's the next argument */
+				precision = va_arg(args, int);
+			}
+			if (precision < 0)
+				precision = 0;
+		}
+
+		/* get the conversion qualifier */
+		qualifier = -1;
+		if (*fmt == 'h' || *fmt == 'l' || *fmt == 'L') {
+			qualifier = *fmt;
+			++fmt;
+		}
+
+		/* default base */
+		base = 10;
+
+		switch (*fmt) {
+		case 'c':
+			if (!(flags & LEFT))
+				while (--field_width > 0)
+					ASSIGN_AND_INC_SAFE(str, end, ' ');
+			ASSIGN_AND_INC_SAFE(str, end,
+					    (uint8_t)va_arg(args, int));
+			while (--field_width > 0)
+				ASSIGN_AND_INC_SAFE(str, end, ' ');
+			continue;
+
+		case 's':
+			s = va_arg(args, char *);
+			len = strnlen(s, precision);
+
+			if (!(flags & LEFT))
+				while (len < field_width--)
+					ASSIGN_AND_INC_SAFE(str, end, ' ');
+			for (i = 0; i < len; ++i)
+				ASSIGN_AND_INC_SAFE(str, end, *s++);
+			while (len < field_width--)
+				ASSIGN_AND_INC_SAFE(str, end, ' ');
+			continue;
+
+		case 'p':
+			if (field_width == -1) {
+				field_width = 2 * sizeof(void *);
+				flags |= SPECIAL | SMALL | ZEROPAD;
+			}
+			str = number(str, end,
+				     (uint64_t)va_arg(args, void *), 16,
+				     field_width, precision, flags);
+			continue;
+
+		case 'n':
+			if (qualifier == 'l') {
+				long *ip = va_arg(args, long *);
+				*ip = (str - buf);
+			} else {
+				int *ip = va_arg(args, int *);
+				*ip = (str - buf);
+			}
+			continue;
+
+		case '%':
+			ASSIGN_AND_INC_SAFE(str, end, '%');
+			continue;
+
+		/* integer number formats - set up the flags and "break" */
+		case 'o':
+			base = 8;
+			break;
+
+		case 'x':
+			flags |= SMALL;
+		case 'X':
+			base = 16;
+			break;
+
+		case 'd':
+		case 'i':
+			flags |= SIGN;
+		case 'u':
+			break;
+
+		default:
+			ASSIGN_AND_INC_SAFE(str, end, '%');
+			if (*fmt)
+				ASSIGN_AND_INC_SAFE(str, end, *fmt);
+			else
+				--fmt;
+			continue;
+		}
+		if (qualifier == 'l')
+			num = va_arg(args, uint64_t);
+		else if (qualifier == 'h') {
+			num = (uint16_t)va_arg(args, int);
+			if (flags & SIGN)
+				num = (int16_t)num;
+		} else if (flags & SIGN)
+			num = va_arg(args, int);
+		else
+			num = va_arg(args, uint32_t);
+		str = number(str, end, num, base, field_width, precision, flags);
+	}
+
+	if (n > 0) {
+		if (str < end)
+			*str = '\0';
+		else
+			end[-1] = '\0';
+	}
+	return str - buf;
+}
+
+int kvm_snprintf(char *buf, int n, const char *fmt, ...)
+{
+	va_list va;
+	int len;
+
+	va_start(va, fmt);
+	len = kvm_vsnprintf(buf, n, fmt, va);
+	va_end(va);
+
+	return len;
+}
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH v2 2/6] KVM: selftests: Add kvm_snprintf() to KVM selftests

[Sean Christopherson]

On Mon, Apr 24, 2023, Aaron Lewis wrote:
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index a6e9f215ce70..45cb0dd41412 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -186,4 +186,7 @@ static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
>  	return num;
>  }
>  
> +int kvm_vsnprintf(char *buf, int n, const char *fmt, va_list args);
> +int kvm_snprintf(char *buf, int n, const char *fmt, ...);
> +
>  #endif /* SELFTEST_KVM_TEST_UTIL_H */
> diff --git a/tools/testing/selftests/kvm/lib/kvm_sprintf.c b/tools/testing/selftests/kvm/lib/kvm_sprintf.c
> new file mode 100644
> index 000000000000..db369e00a6fc
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/kvm_sprintf.c
> @@ -0,0 +1,313 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include "test_util.h"
> +
> +#define ASSIGN_AND_INC_SAFE(str, end, v) \
> +({					 \
> +	if (str < end)			 \

This should assert instead of hiding the error and forcing the caller to detect
the problem, which may or may not actually happen.  The easiest thing is to just
thie guest only, e.g. name the helpers guest_*snprintf() and then do

#define APPEND_BUFFER_SAFE(str, end, v)	\
do {					\
	GUEST_ASSERT(str < end);	\
	*str++ = (v);			\
} while (0)

I doubt there will be a use case for using the custom snprintf in a helper that
is common to guest and host, and if someone does need/want that functionality,
they can add a global flag to track whether or not selftests is running guest or
host code.

And I would deliberately use GUEST_ASSERT() instead of a GUEST_UCALL_FAILED.
KVM carves out enough space for KVM_MAX_VCPUS concurrent/nested ucalls, so for
the vast majority of tests using GUEST_ASSERT() will succeed and be useful.  And
if some test exhausts the ucall pool, it'll end up with GUEST_UCALL_FAILED anyways.

[PATCH v2 3/6] KVM: selftests: Add additional pages to the guest to accommodate ucall

[Aaron Lewis]

Add additional pages to the guest to account for the number of pages
the ucall framework uses.

This is done in preparation for adding string formatting options to
the guest through ucall helpers.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 1 +
 tools/testing/selftests/kvm/lib/kvm_util.c         | 4 ++++
 tools/testing/selftests/kvm/lib/ucall_common.c     | 5 +++++
 3 files changed, 10 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 1a6aaef5ccae..bcbb362aa77f 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -34,6 +34,7 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu);
 void ucall(uint64_t cmd, int nargs, ...);
 uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa);
+int ucall_nr_pages_required(uint64_t page_size);
 
 /*
  * Perform userspace call without any associated data.  This bare call avoids
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 298c4372fb1a..80b3df2a79e6 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -312,6 +312,7 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
 				     uint32_t nr_runnable_vcpus,
 				     uint64_t extra_mem_pages)
 {
+	uint64_t page_size = vm_guest_mode_params[mode].page_size;
 	uint64_t nr_pages;
 
 	TEST_ASSERT(nr_runnable_vcpus,
@@ -340,6 +341,9 @@ static uint64_t vm_nr_pages_required(enum vm_guest_mode mode,
 	 */
 	nr_pages += (nr_pages + extra_mem_pages) / PTES_PER_MIN_PAGE * 2;
 
+	/* Account for the number of pages needed by ucall. */
+	nr_pages += ucall_nr_pages_required(page_size);
+
 	return vm_adjust_num_guest_pages(mode, nr_pages);
 }
 
diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
index 2f0e2ea941cc..77ada362273d 100644
--- a/tools/testing/selftests/kvm/lib/ucall_common.c
+++ b/tools/testing/selftests/kvm/lib/ucall_common.c
@@ -11,6 +11,11 @@ struct ucall_header {
 	struct ucall ucalls[KVM_MAX_VCPUS];
 };
 
+int ucall_nr_pages_required(uint64_t page_size)
+{
+	return align_up(sizeof(struct ucall_header), page_size) / page_size;
+}
+
 /*
  * ucall_pool holds per-VM values (global data is duplicated by each VM), it
  * must not be accessed from host code.
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH v2 3/6] KVM: selftests: Add additional pages to the guest to accommodate ucall

[Sean Christopherson]

On Mon, Apr 24, 2023, Aaron Lewis wrote:
> Add additional pages to the guest to account for the number of pages
> the ucall framework uses.
> 
> This is done in preparation for adding string formatting options to
> the guest through ucall helpers.

IIUC, this is a bug fix, but things work today because the ucall overhead is small
enough to be covered by the arbitrary 512 page buffer.  If that's correct, please
state that in the changelog.

[PATCH v2 4/6] KVM: selftests: Add string formatting options to ucall

[Aaron Lewis]

Add more flexibility to guest debugging and testing by adding
GUEST_PRINTF() and GUEST_ASSERT_FMT() to the ucall framework.

A buffer to hold the formatted string was added to the ucall struct.
That allows the guest/host to avoid the problem of passing an
arbitrary number of parameters between themselves when resolving the
string.  Instead, the string is resolved in the guest then passed
back to the host to be logged.

The formatted buffer is set to 1024 bytes which increases the size
of the ucall struct.  As a result, this will increase the number of
pages requested for the guest.

The buffer size was chosen to accommodate most use cases, and based on
similar usage.  E.g. printf() uses the same size buffer in
arch/x86/boot/printf.c.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../selftests/kvm/include/ucall_common.h       | 18 ++++++++++++++++++
 tools/testing/selftests/kvm/lib/ucall_common.c | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index bcbb362aa77f..7281a6892779 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -13,15 +13,18 @@ enum {
 	UCALL_NONE,
 	UCALL_SYNC,
 	UCALL_ABORT,
+	UCALL_PRINTF,
 	UCALL_DONE,
 	UCALL_UNHANDLED,
 };
 
 #define UCALL_MAX_ARGS 7
+#define UCALL_BUFFER_LEN 1024
 
 struct ucall {
 	uint64_t cmd;
 	uint64_t args[UCALL_MAX_ARGS];
+	char buffer[UCALL_BUFFER_LEN];
 
 	/* Host virtual address of this struct. */
 	struct ucall *hva;
@@ -32,6 +35,7 @@ void ucall_arch_do_ucall(vm_vaddr_t uc);
 void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu);
 
 void ucall(uint64_t cmd, int nargs, ...);
+void ucall_fmt(uint64_t cmd, const char *fmt, ...);
 uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa);
 int ucall_nr_pages_required(uint64_t page_size);
@@ -47,6 +51,7 @@ int ucall_nr_pages_required(uint64_t page_size);
 #define GUEST_SYNC_ARGS(stage, arg1, arg2, arg3, arg4)	\
 				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
 #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
+#define GUEST_PRINTF(_fmt, _args...) ucall_fmt(UCALL_PRINTF, _fmt, ##_args)
 #define GUEST_DONE()		ucall(UCALL_DONE, 0)
 
 enum guest_assert_builtin_args {
@@ -56,6 +61,17 @@ enum guest_assert_builtin_args {
 	GUEST_ASSERT_BUILTIN_NARGS
 };
 
+#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)		  \
+do {										  \
+	if (!(_condition))							  \
+		ucall_fmt(UCALL_ABORT,						  \
+			  "Failed guest assert: " _condstr " at %s:%ld\n  " _fmt, \
+			  , __FILE__, __LINE__, ##_args);			  \
+} while (0)
+
+#define GUEST_ASSERT_FMT(_condition, _fmt, _args...)	\
+	__GUEST_ASSERT_FMT(_condition, #_condition, _fmt, ##_args)
+
 #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...)		\
 do {									\
 	if (!(_condition))						\
@@ -81,6 +97,8 @@ do {									\
 
 #define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
 
+#define REPORT_GUEST_ASSERT_FMT(_ucall) TEST_FAIL("%s", _ucall.buffer)
+
 #define __REPORT_GUEST_ASSERT(_ucall, fmt, _args...)			\
 	TEST_FAIL("%s at %s:%ld\n" fmt,					\
 		  (const char *)(_ucall).args[GUEST_ERROR_STRING],	\
diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
index 77ada362273d..c09e57c8ef77 100644
--- a/tools/testing/selftests/kvm/lib/ucall_common.c
+++ b/tools/testing/selftests/kvm/lib/ucall_common.c
@@ -55,6 +55,7 @@ static struct ucall *ucall_alloc(void)
 		if (!test_and_set_bit(i, ucall_pool->in_use)) {
 			uc = &ucall_pool->ucalls[i];
 			memset(uc->args, 0, sizeof(uc->args));
+			memset(uc->buffer, 0, sizeof(uc->buffer));
 			return uc;
 		}
 	}
@@ -75,6 +76,23 @@ static void ucall_free(struct ucall *uc)
 	clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use);
 }
 
+void ucall_fmt(uint64_t cmd, const char *fmt, ...)
+{
+	struct ucall *uc;
+	va_list va;
+
+	uc = ucall_alloc();
+	uc->cmd = cmd;
+
+	va_start(va, fmt);
+	kvm_vsnprintf(uc->buffer, UCALL_BUFFER_LEN, fmt, va);
+	va_end(va);
+
+	ucall_arch_do_ucall((vm_vaddr_t)uc->hva);
+
+	ucall_free(uc);
+}
+
 void ucall(uint64_t cmd, int nargs, ...)
 {
 	struct ucall *uc;
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH v2 4/6] KVM: selftests: Add string formatting options to ucall

[Sean Christopherson]

On Mon, Apr 24, 2023, Aaron Lewis wrote:
> Add more flexibility to guest debugging and testing by adding
> GUEST_PRINTF() and GUEST_ASSERT_FMT() to the ucall framework.
> 
> A buffer to hold the formatted string was added to the ucall struct.
> That allows the guest/host to avoid the problem of passing an
> arbitrary number of parameters between themselves when resolving the
> string.  Instead, the string is resolved in the guest then passed
> back to the host to be logged.
> 
> The formatted buffer is set to 1024 bytes which increases the size
> of the ucall struct.  As a result, this will increase the number of
> pages requested for the guest.
> 
> The buffer size was chosen to accommodate most use cases, and based on
> similar usage.  E.g. printf() uses the same size buffer in
> arch/x86/boot/printf.c.

...
>  #define UCALL_MAX_ARGS 7
> +#define UCALL_BUFFER_LEN 1024
>  
>  struct ucall {
>  	uint64_t cmd;
>  	uint64_t args[UCALL_MAX_ARGS];
> +	char buffer[UCALL_BUFFER_LEN];

...

> diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
> index 77ada362273d..c09e57c8ef77 100644
> --- a/tools/testing/selftests/kvm/lib/ucall_common.c
> +++ b/tools/testing/selftests/kvm/lib/ucall_common.c
> @@ -55,6 +55,7 @@ static struct ucall *ucall_alloc(void)
>  		if (!test_and_set_bit(i, ucall_pool->in_use)) {
>  			uc = &ucall_pool->ucalls[i];
>  			memset(uc->args, 0, sizeof(uc->args));
> +			memset(uc->buffer, 0, sizeof(uc->buffer));

The use in boot/printf.c isn't a great reference point.  That "allocation" is
on-stack and effectively free, whereas the use here "requires" zeroing the buffer
during allocation.  I usually tell people to not worry about selftests performance,
but zeroing 1KiB on every ucall seems a bit excessive.

However, that's more of an argument to not zero than it is to try and squeak by
with a smaller size.  The guest really should explicitly tell the host how much
of the buffer.  And with that, there should be no need to zero the buffer because
the host isn't relying on the memory being zeroed.

On a somehwat related topic, this patch should also introduce a macro/helper to
retrieve and print the buffer on the backend.  Partly to reduce copy+paste, partly
to make it easier to review (i.e. show the end-to-end), and partly so that the
ucall code can craft a more explicit contract.

Re: [PATCH v2 4/6] KVM: selftests: Add string formatting options to ucall

[Aaron Lewis]

On Mon, Jun 5, 2023 at 9:44 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Apr 24, 2023, Aaron Lewis wrote:
> > Add more flexibility to guest debugging and testing by adding
> > GUEST_PRINTF() and GUEST_ASSERT_FMT() to the ucall framework.
> >
> > A buffer to hold the formatted string was added to the ucall struct.
> > That allows the guest/host to avoid the problem of passing an
> > arbitrary number of parameters between themselves when resolving the
> > string.  Instead, the string is resolved in the guest then passed
> > back to the host to be logged.
> >
> > The formatted buffer is set to 1024 bytes which increases the size
> > of the ucall struct.  As a result, this will increase the number of
> > pages requested for the guest.
> >
> > The buffer size was chosen to accommodate most use cases, and based on
> > similar usage.  E.g. printf() uses the same size buffer in
> > arch/x86/boot/printf.c.
>
> ...
> >  #define UCALL_MAX_ARGS 7
> > +#define UCALL_BUFFER_LEN 1024
> >
> >  struct ucall {
> >       uint64_t cmd;
> >       uint64_t args[UCALL_MAX_ARGS];
> > +     char buffer[UCALL_BUFFER_LEN];
>
> ...
>
> > diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
> > index 77ada362273d..c09e57c8ef77 100644
> > --- a/tools/testing/selftests/kvm/lib/ucall_common.c
> > +++ b/tools/testing/selftests/kvm/lib/ucall_common.c
> > @@ -55,6 +55,7 @@ static struct ucall *ucall_alloc(void)
> >               if (!test_and_set_bit(i, ucall_pool->in_use)) {
> >                       uc = &ucall_pool->ucalls[i];
> >                       memset(uc->args, 0, sizeof(uc->args));
> > +                     memset(uc->buffer, 0, sizeof(uc->buffer));
>
> The use in boot/printf.c isn't a great reference point.  That "allocation" is
> on-stack and effectively free, whereas the use here "requires" zeroing the buffer
> during allocation.  I usually tell people to not worry about selftests performance,
> but zeroing 1KiB on every ucall seems a bit excessive.
>
> However, that's more of an argument to not zero than it is to try and squeak by
> with a smaller size.  The guest really should explicitly tell the host how much
> of the buffer.  And with that, there should be no need to zero the buffer because
> the host isn't relying on the memory being zeroed.

I don't think zeroing the buffer is actually necessary.  It is more of
a nice-to-have for extra paranoia.  The printf function ensures the
string is NULL terminated, so I think it should be safe to just drop
it and save the cycles.

With the added assert in patch 2, plus a few more I'm planning on
adding, guest_printf() either properly writes a string or dies.  You
brought up a good point in that selftests generally fail hard rather
than hiding errors, so asserting makes sense there.  That also means
there is no real need to pass the length of the string to the host.
The string should be properly written if guest_printf() returns
successfully.

>
> On a somehwat related topic, this patch should also introduce a macro/helper to
> retrieve and print the buffer on the backend.  Partly to reduce copy+paste, partly
> to make it easier to review (i.e. show the end-to-end), and partly so that the
> ucall code can craft a more explicit contract.

If guest_printf() returns successfully, then the expectation is that
the string was correctly written, which makes the contract pretty
simple.  I'm thinking something like this?

#define REPORT_GUEST_PRINTF(_ucall) pr_info("%s", _ucall.buffer)

[PATCH v2 5/6] KVM: selftests: Add ucall_fmt2()

[Aaron Lewis]

Add a second ucall_fmt() function that takes two format strings instead
of one.  This provides more flexibility because the string format in
GUEST_ASSERT_FMT() is no linger limited to only using literals.

This provides better consistency between GUEST_PRINTF() and
GUEST_ASSERT_FMT() as GUEST_PRINTF() is not limited to only using
literals either.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../selftests/kvm/include/ucall_common.h      | 13 +++++-----
 .../testing/selftests/kvm/lib/ucall_common.c  | 24 +++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 7281a6892779..3e8135aaa812 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -36,6 +36,7 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu);
 
 void ucall(uint64_t cmd, int nargs, ...);
 void ucall_fmt(uint64_t cmd, const char *fmt, ...);
+void ucall_fmt2(uint64_t cmd, const char *fmt1, const char *fmt2, ...);
 uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 void ucall_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa);
 int ucall_nr_pages_required(uint64_t page_size);
@@ -61,12 +62,12 @@ enum guest_assert_builtin_args {
 	GUEST_ASSERT_BUILTIN_NARGS
 };
 
-#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)		  \
-do {										  \
-	if (!(_condition))							  \
-		ucall_fmt(UCALL_ABORT,						  \
-			  "Failed guest assert: " _condstr " at %s:%ld\n  " _fmt, \
-			  , __FILE__, __LINE__, ##_args);			  \
+#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)	     \
+do {									     \
+	if (!(_condition))						     \
+		ucall_fmt2(UCALL_ABORT,					     \
+			   "Failed guest assert: " _condstr " at %s:%ld\n  ",\
+			   _fmt, __FILE__, __LINE__, ##_args);		     \
 } while (0)
 
 #define GUEST_ASSERT_FMT(_condition, _fmt, _args...)	\
diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
index c09e57c8ef77..d0f1ad6c0c44 100644
--- a/tools/testing/selftests/kvm/lib/ucall_common.c
+++ b/tools/testing/selftests/kvm/lib/ucall_common.c
@@ -76,6 +76,30 @@ static void ucall_free(struct ucall *uc)
 	clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use);
 }
 
+void ucall_fmt2(uint64_t cmd, const char *fmt1, const char *fmt2, ...)
+{
+	const int fmt_len = 128;
+	char fmt[fmt_len];
+	struct ucall *uc;
+	va_list va;
+	int len;
+
+	len = kvm_snprintf(fmt, fmt_len, "%s%s", fmt1, fmt2);
+	if (len > fmt_len)
+		ucall_arch_do_ucall(GUEST_UCALL_FAILED);
+
+	uc = ucall_alloc();
+	uc->cmd = cmd;
+
+	va_start(va, fmt2);
+	kvm_vsnprintf(uc->buffer, UCALL_BUFFER_LEN, fmt, va);
+	va_end(va);
+
+	ucall_arch_do_ucall((vm_vaddr_t)uc->hva);
+
+	ucall_free(uc);
+}
+
 void ucall_fmt(uint64_t cmd, const char *fmt, ...)
 {
 	struct ucall *uc;
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH v2 5/6] KVM: selftests: Add ucall_fmt2()

[Sean Christopherson]

On Mon, Apr 24, 2023, Aaron Lewis wrote:
> Add a second ucall_fmt() function that takes two format strings instead
> of one.  This provides more flexibility because the string format in
> GUEST_ASSERT_FMT() is no linger limited to only using literals.

...

> -#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)		  \
> -do {										  \
> -	if (!(_condition))							  \
> -		ucall_fmt(UCALL_ABORT,						  \
> -			  "Failed guest assert: " _condstr " at %s:%ld\n  " _fmt, \
> -			  , __FILE__, __LINE__, ##_args);			  \
> +#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)	     \
> +do {									     \
> +	if (!(_condition))						     \
> +		ucall_fmt2(UCALL_ABORT,					     \
> +			   "Failed guest assert: " _condstr " at %s:%ld\n  ",\

I don't see any reason to add ucall_fmt2(), just do the string smushing in
__GUEST_ASSERT_FMT().  I doubt there will be many, if any, uses for this outside
of GUEST_ASSERT_FMT().  Even your test example is contrived, e.g. it would be
just as easy, and arguably more robusted, to #define the expected vs. actual formats
as it is to assign them to global variables.

In other words, this 

#define __GUEST_ASSERT_FMT(_condition, _str, _fmt, _args...)	     		\
do {										\
	char fmt_buffer[512];							\
										\
	if (!(_condition)) {							\
		kvm_snprintf(fmt_buffer, sizeof(fmt_buffer), "%s\n  %s",	\
			     "Failed guest assert: " _str " at %s:%ld", _fmt);	\
		ucall_fmt(UCALL_ABORT, fmt_buffer, __FILE__, __LINE__, ##_args);\
	}									\
} while (0)

is a preferable to copy+pasting an entirely new ucall_fmt2().  (Feel free to use
a different name for the on-stack array, e.g. just "fmt").

> +			   _fmt, __FILE__, __LINE__, ##_args);		     \
>  } while (0)
>  
>  #define GUEST_ASSERT_FMT(_condition, _fmt, _args...)	\
> diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
> index c09e57c8ef77..d0f1ad6c0c44 100644
> --- a/tools/testing/selftests/kvm/lib/ucall_common.c
> +++ b/tools/testing/selftests/kvm/lib/ucall_common.c
> @@ -76,6 +76,30 @@ static void ucall_free(struct ucall *uc)
>  	clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use);
>  }
>  
> +void ucall_fmt2(uint64_t cmd, const char *fmt1, const char *fmt2, ...)
> +{
> +	const int fmt_len = 128;
> +	char fmt[fmt_len];

Just do 

	char fmt[128];

(or whatever size is chosen)

> +	struct ucall *uc;
> +	va_list va;
> +	int len;
> +
> +	len = kvm_snprintf(fmt, fmt_len, "%s%s", fmt1, fmt2);

and then here do sizeof(fmt).  It's self-documenting, and makes it really, really
hard to screw up and use the wrong format.

Regarding the size, can you look into why using 1024 for the buffer fails?  This
really should use the max allowed UCALL buffer size, but I'm seeing shutdowns when
pushing above 512 bytes (I didn't try to precisely find the threshold).  Selftests
are supposed to allocate 5 * 4KiB stacks, so the guest shouldn't be getting anywhere
near a stack overflow.

> +	if (len > fmt_len)

For KVM selftests use case, callers shouldn't need to sanity check, that should be
something kvm_snprintf() itself handles.  I'll follow-up in that patch.

Re: [PATCH v2 5/6] KVM: selftests: Add ucall_fmt2()

[Aaron Lewis]

On Mon, Jun 5, 2023 at 10:41 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Apr 24, 2023, Aaron Lewis wrote:
> > Add a second ucall_fmt() function that takes two format strings instead
> > of one.  This provides more flexibility because the string format in
> > GUEST_ASSERT_FMT() is no linger limited to only using literals.
>
> ...
>
> > -#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)               \
> > -do {                                                                           \
> > -     if (!(_condition))                                                        \
> > -             ucall_fmt(UCALL_ABORT,                                            \
> > -                       "Failed guest assert: " _condstr " at %s:%ld\n  " _fmt, \
> > -                       , __FILE__, __LINE__, ##_args);                         \
> > +#define __GUEST_ASSERT_FMT(_condition, _condstr, _fmt, _args...)          \
> > +do {                                                                      \
> > +     if (!(_condition))                                                   \
> > +             ucall_fmt2(UCALL_ABORT,                                      \
> > +                        "Failed guest assert: " _condstr " at %s:%ld\n  ",\
>
> I don't see any reason to add ucall_fmt2(), just do the string smushing in
> __GUEST_ASSERT_FMT().  I doubt there will be many, if any, uses for this outside
> of GUEST_ASSERT_FMT().  Even your test example is contrived, e.g. it would be
> just as easy, and arguably more robusted, to #define the expected vs. actual formats
> as it is to assign them to global variables.

The way the test was first set up I needed them to be globals, but as
it evolved I realized that requirement no longer held.  I kept them as
globals though to demonstrate they didn't have to be literals.  I
think that gives the API more flexibility and consistency.

>
> In other words, this
>
> #define __GUEST_ASSERT_FMT(_condition, _str, _fmt, _args...)                    \
> do {                                                                            \
>         char fmt_buffer[512];                                                   \
>                                                                                 \
>         if (!(_condition)) {                                                    \
>                 kvm_snprintf(fmt_buffer, sizeof(fmt_buffer), "%s\n  %s",        \
>                              "Failed guest assert: " _str " at %s:%ld", _fmt);  \
>                 ucall_fmt(UCALL_ABORT, fmt_buffer, __FILE__, __LINE__, ##_args);\
>         }                                                                       \
> } while (0)
>
> is a preferable to copy+pasting an entirely new ucall_fmt2().  (Feel free to use
> a different name for the on-stack array, e.g. just "fmt").
>
> > +                        _fmt, __FILE__, __LINE__, ##_args);               \
> >  } while (0)
> >
> >  #define GUEST_ASSERT_FMT(_condition, _fmt, _args...) \
> > diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c
> > index c09e57c8ef77..d0f1ad6c0c44 100644
> > --- a/tools/testing/selftests/kvm/lib/ucall_common.c
> > +++ b/tools/testing/selftests/kvm/lib/ucall_common.c
> > @@ -76,6 +76,30 @@ static void ucall_free(struct ucall *uc)
> >       clear_bit(uc - ucall_pool->ucalls, ucall_pool->in_use);
> >  }
> >
> > +void ucall_fmt2(uint64_t cmd, const char *fmt1, const char *fmt2, ...)
> > +{
> > +     const int fmt_len = 128;
> > +     char fmt[fmt_len];
>
> Just do
>
>         char fmt[128];
>
> (or whatever size is chosen)
>
> > +     struct ucall *uc;
> > +     va_list va;
> > +     int len;
> > +
> > +     len = kvm_snprintf(fmt, fmt_len, "%s%s", fmt1, fmt2);
>
> and then here do sizeof(fmt).  It's self-documenting, and makes it really, really
> hard to screw up and use the wrong format.
>
> Regarding the size, can you look into why using 1024 for the buffer fails?  This
> really should use the max allowed UCALL buffer size, but I'm seeing shutdowns when
> pushing above 512 bytes (I didn't try to precisely find the threshold).  Selftests
> are supposed to allocate 5 * 4KiB stacks, so the guest shouldn't be getting anywhere
> near a stack overflow.

D'oh! This is the result of a bad pattern used in the test.

vcpu_regs_get(vcpu, &regs);
regs.rip = (uintptr_t)guest_code;
vcpu_regs_set(vcpu, &regs);

I set the RIP, but not the RSP.  That makes the stack grow out of
control when called a lot like we do in this test.

It's also x86 specific and this test no longer lives in that directory.

For now I'll change the guest code to run in a loop to restart it, but
I've used this pattern before and it's useful at times.  Also, looping
forces me to sync a global for the guest, and I think I'd rather pass
the parameters directly into the guest code.  Maybe it would make
sense to implement proper support for this in the selftest library at
some point to allow us to use this pattern and have it be less error
prone.

With the fix we are able to set the size to the UCALL buffer size.



>
> > +     if (len > fmt_len)
>
> For KVM selftests use case, callers shouldn't need to sanity check, that should be
> something kvm_snprintf() itself handles.  I'll follow-up in that patch.

[PATCH v2 6/6] KVM: selftests: Add a selftest for guest prints and formatted asserts

[Aaron Lewis]

The purpose of this test is to exercise the various features in KVM's
local snprintf() and compare them to LIBC's snprintf() to ensure they
behave the same.

This is not an exhaustive test.  KVM's local snprintf() does not
implement all the features LIBC does, e.g. KVM's local snprintf() does
not support floats or doubles, so testing for those features were
excluded.

Testing was added for the features that are expected to work to
support a minimal version of printf() in the guest.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../testing/selftests/kvm/guest_print_test.c  | 207 ++++++++++++++++++
 2 files changed, 208 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/guest_print_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 84b126398729..31587a0e2efb 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -121,6 +121,7 @@ TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
 TEST_GEN_PROGS_x86_64 += demand_paging_test
 TEST_GEN_PROGS_x86_64 += dirty_log_test
 TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
+TEST_GEN_PROGS_x86_64 += guest_print_test
 TEST_GEN_PROGS_x86_64 += hardware_disable_test
 TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
 TEST_GEN_PROGS_x86_64 += kvm_page_table_test
diff --git a/tools/testing/selftests/kvm/guest_print_test.c b/tools/testing/selftests/kvm/guest_print_test.c
new file mode 100644
index 000000000000..57489055e5cb
--- /dev/null
+++ b/tools/testing/selftests/kvm/guest_print_test.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A test for GUEST_PRINTF
+ *
+ * Copyright 2022, Google, Inc. and/or its affiliates.
+ */
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+
+/* GUEST_PRINTF()/GUEST_ASSERT_FMT() does not support float or double. */
+#define TYPE_LIST					\
+TYPE(test_type_i64,  I64,  "%ld",   int64_t)		\
+TYPE(test_type_u64,  U64u, "%lu",   uint64_t)		\
+TYPE(test_type_x64,  U64x, "0x%lx", uint64_t)		\
+TYPE(test_type_X64,  U64X, "0x%lX", uint64_t)		\
+TYPE(test_type_u32,  U32u, "%u",    uint32_t)		\
+TYPE(test_type_x32,  U32x, "0x%x",  uint32_t)		\
+TYPE(test_type_X32,  U32X, "0x%X",  uint32_t)		\
+TYPE(test_type_int,  INT,  "%d",    int)		\
+TYPE(test_type_char, CHAR, "%c",    char)		\
+TYPE(test_type_str,  STR,  "'%s'",  const char *)	\
+TYPE(test_type_ptr,  PTR,  "%p",    uintptr_t)
+
+enum args_type {
+#define TYPE(fn, ext, fmt_t, T) TYPE_##ext,
+	TYPE_LIST
+#undef TYPE
+};
+
+static void run_test(struct kvm_vcpu *vcpu, const char *expected_printf,
+		     const char *expected_assert);
+
+#define BUILD_TYPE_STRINGS_AND_HELPER(fn, ext, fmt_t, T)		     \
+const char *PRINTF_FMT_##ext = "Got params a = " fmt_t " and b = " fmt_t;    \
+const char *ASSERT_FMT_##ext = "Expected " fmt_t ", got " fmt_t " instead";  \
+static void fn(struct kvm_vcpu *vcpu, T a, T b)				     \
+{									     \
+	char expected_printf[UCALL_BUFFER_LEN];				     \
+	char expected_assert[UCALL_BUFFER_LEN];				     \
+									     \
+	snprintf(expected_printf, UCALL_BUFFER_LEN, PRINTF_FMT_##ext, a, b); \
+	snprintf(expected_assert, UCALL_BUFFER_LEN, ASSERT_FMT_##ext, a, b); \
+	vcpu_args_set(vcpu, 3, a, b, TYPE_##ext);			     \
+	run_test(vcpu, expected_printf, expected_assert);		     \
+}
+
+#define TYPE(fn, ext, fmt_t, T) \
+		BUILD_TYPE_STRINGS_AND_HELPER(fn, ext, fmt_t, T)
+	TYPE_LIST
+#undef TYPE
+
+static void guest_code(uint64_t a, uint64_t b, uint64_t type)
+{
+	switch (type) {
+#define TYPE(fn, ext, fmt_t, T) case TYPE_##ext:			\
+		GUEST_PRINTF(PRINTF_FMT_##ext, a, b);			\
+		GUEST_ASSERT_FMT(a == b, ASSERT_FMT_##ext, a, b);	\
+		break;
+	TYPE_LIST
+#undef TYPE
+	default:
+		GUEST_SYNC(type);
+	}
+
+	GUEST_DONE();
+}
+
+/*
+ * Unfortunately this gets a little messy because 'assert_msg' doesn't
+ * just contains the matching string, it also contains additional assert
+ * info.  Fortunately the part that matches should be at the very end of
+ * 'assert_msg'.
+ */
+static void ucall_abort(const char *assert_msg, const char *expected_assert_msg)
+{
+	int len_str = strlen(assert_msg);
+	int len_substr = strlen(expected_assert_msg);
+	int offset = len_str - len_substr;
+
+	TEST_ASSERT(len_substr <= len_str,
+		    "Expected to find a substring, len_str: %d, len_substr: %d",
+		    len_str, len_substr);
+
+	TEST_ASSERT(strcmp(&assert_msg[offset], expected_assert_msg) == 0,
+		    "Unexpected mismatch. Expected: '%s', got: '%s'",
+		    expected_assert_msg, &assert_msg[offset]);
+}
+
+static void run_test(struct kvm_vcpu *vcpu, const char *expected_printf,
+		     const char *expected_assert)
+{
+	struct kvm_run *run = vcpu->run;
+	struct kvm_regs regs;
+	struct ucall uc;
+
+	/*
+	 * The guest takes 3 parameters (T val1, T val2, TYPE) which are set
+	 * in the parent call to allow run_tests() to be type-agnostic.
+	 */
+
+	vcpu_regs_get(vcpu, &regs);
+	regs.rip = (uintptr_t)guest_code;
+	vcpu_regs_set(vcpu, &regs);
+
+	while (1) {
+		vcpu_run(vcpu);
+
+		TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
+			    "Unexpected exit reason: %u (%s),\n",
+			    run->exit_reason,
+			    exit_reason_str(run->exit_reason));
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_SYNC:
+			TEST_FAIL("Unknown 'args_type' = %lu", uc.args[1]);
+			break;
+		case UCALL_PRINTF:
+			TEST_ASSERT(strcmp(uc.buffer, expected_printf) == 0,
+				    "Unexpected mismatch. Expected: '%s', got: '%s'",
+				    expected_printf, uc.buffer);
+			break;
+		case UCALL_ABORT:
+			ucall_abort(uc.buffer, expected_assert);
+			break;
+		case UCALL_DONE:
+			return;
+		default:
+			TEST_FAIL("Unknown ucall %lu", uc.cmd);
+		}
+	}
+}
+
+static void test_limits(void)
+{
+	const int buffer_len = UCALL_BUFFER_LEN + 10;
+	char test_str[buffer_len];
+	char test_res[buffer_len];
+	int r;
+
+	memset(test_str, 'a', buffer_len);
+	test_str[buffer_len - 1] = 0;
+
+	r = kvm_snprintf(test_res, UCALL_BUFFER_LEN, "%s", test_str);
+	TEST_ASSERT(r == (buffer_len - 1),
+		    "Unexpected kvm_snprintf() length.  Expected: %d, got: %d",
+		    buffer_len - 1, r);
+
+	r = strlen(test_res);
+	TEST_ASSERT(r == (UCALL_BUFFER_LEN - 1),
+		    "Unexpected strlen() length.  Expected: %d, got: %d",
+		    UCALL_BUFFER_LEN - 1, r);
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	vm = vm_create_with_one_vcpu(&vcpu, NULL);
+
+	test_type_i64(vcpu, -1, -1);
+	test_type_i64(vcpu, -1,  1);
+	test_type_i64(vcpu, 0x1234567890abcdef, 0x1234567890abcdef);
+	test_type_i64(vcpu, 0x1234567890abcdef, 0x1234567890abcdee);
+
+	test_type_u64(vcpu, 0x1234567890abcdef, 0x1234567890abcdef);
+	test_type_u64(vcpu, 0x1234567890abcdef, 0x1234567890abcdee);
+	test_type_x64(vcpu, 0x1234567890abcdef, 0x1234567890abcdef);
+	test_type_x64(vcpu, 0x1234567890abcdef, 0x1234567890abcdee);
+	test_type_X64(vcpu, 0x1234567890abcdef, 0x1234567890abcdef);
+	test_type_X64(vcpu, 0x1234567890abcdef, 0x1234567890abcdee);
+
+	test_type_u32(vcpu, 0x90abcdef, 0x90abcdef);
+	test_type_u32(vcpu, 0x90abcdef, 0x90abcdee);
+	test_type_x32(vcpu, 0x90abcdef, 0x90abcdef);
+	test_type_x32(vcpu, 0x90abcdef, 0x90abcdee);
+	test_type_X32(vcpu, 0x90abcdef, 0x90abcdef);
+	test_type_X32(vcpu, 0x90abcdef, 0x90abcdee);
+
+	test_type_int(vcpu, -1, -1);
+	test_type_int(vcpu, -1,  1);
+	test_type_int(vcpu,  1,  1);
+
+	test_type_char(vcpu, 'a', 'a');
+	test_type_char(vcpu, 'a', 'A');
+	test_type_char(vcpu, 'a', 'b');
+
+	test_type_str(vcpu, "foo", "foo");
+	test_type_str(vcpu, "foo", "bar");
+
+	test_type_ptr(vcpu, 0x1234567890abcdef, 0x1234567890abcdef);
+	test_type_ptr(vcpu, 0x1234567890abcdef, 0x1234567890abcdee);
+
+	kvm_vm_free(vm);
+
+	test_limits();
+
+	return 0;
+}
-- 
2.40.0.634.g4ca3ef3211-goog