Re: [PATCH 4/8] KVM: selftests: Copy printf.c to KVM selftests

[Sean Christopherson <seanjc@google.com>]

On Tue, Apr 18, 2023, Andrew Jones wrote:
> On Tue, Apr 18, 2023 at 08:03:53AM -0700, Sean Christopherson wrote:
> > On Mon, Apr 17, 2023, Aaron Lewis wrote:
> > > On Thu, Mar 23, 2023, Sean Christopherson wrote:
> > > > > +static char *number(char *str, long num, int base, int size, int precision,
> > > > > +		    int type)
> > > > 
> > > > Do we actually need a custom number()?  I.e. can we sub in a libc equivalent?
> > > > That would reduce the craziness of this file by more than a few degrees.
> > > 
> > > Yeah, I think we need it.  One of the biggest problems I'm trying to avoid
> > > here is the use of LIBC in a guest.  Using it always seems to end poorly
> > > because guests generally don't set up AVX-512 or a TLS segmet, nor should
> > > they have to.  Calling into LIBC seems to require both of them too often,
> > > so it seems like it's better to just avoid it.
> > 
> > True, we'd probably end up in a world of hurt.
> > 
> > I was going to suggest copy+pasting from a different source, e.g. musl, in the
> > hopes of reducing the crazy by a degree, but after looking at the musl source,
> > that's a terrible idea :-)
> > 
> > And copying from the kernel has the advantage of keeping any bugs/quirks that
> > users may be expecting and/or relying on.
> 
> What about trying to use tools/include/nolibc/? Maybe we could provide our
> own tools/include/nolibc/arch-*.h files where the my_syscall* macros get
> implemented with ucalls, and then the ucalls would implement the syscalls,
> possibly just forwarding the parameters to real syscalls. We can implement
> copy_from/to_guest() functions to deal with pointer parameters.

Hmm, I was going to say that pulling in nolibc would conflict with the host side's
need for an actual libc, but I think we could solve that conundrum by putting
ucall_fmt() in a dedicated file and compiling it separately, a la string_override.c.

However, I don't think we'd want to override my_syscall to do a ucall.  If I'm
reading the code correctly, that would trigger a ucall after every escape sequence,
which isn't what we want, expecially for a GUEST_ASSERT.

That's solvable by having my_syscall3() be a memcpy() to the buffer provided by
KVM's guest-side vsprintf(), but that still leaves the question of whether or not
taking a dependency on nolibc.h would be a net positive.

E.g. pulling in nolibc.h as-is would well and truly put ucall_fmt.c (or whatever
it's called) into its own world, e.g. it would end up with different typedefs for
all basic types.  Those shouldn't cause problems, but it'd be a weird setup.  And
I don't think we can rule out the possibility of the nolibc dependency causing
subtle problems, e.g. what will happen when linking due to both nolibc and
string_override.c defining globally visible mem{cmp,cpy,set}() functions.

Another minor issue is that nolibc's vfprintf() handles a subset of escapes compared
to the kernel's vsprintf().  That probably won't be a big deal in practice, but it's
again a potential maintenance concern for us in the future.

I'm definitely torn.  As much as I dislike the idea of copy+pasting mode code into
KVM selftests, I think pulling in nolibc would bring its own set of problems.

My vote is probably to copy+paste, at least for the initial implementation.  Being
able to do the equivalent of printf() in the guest would be a huge improvement for
debugging and triaging selftests, i.e. is something I would like to see landed
fairly quickly.  Copy+pasting seems like it gives us the fastest path forward,
e.g. doesn't risk getting bogged down with weird linker errors and whatnot.