On Thu, Apr 20, 2023 at 10:50:07AM -0700, Sean Christopherson wrote:
> 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.
Fully agree.
> 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.
Works for me.
Thanks,
drew
