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.
