[PATCH kvmtool v2 0/2] Fix compilation with musl-libc based toolchains

[PATCH kvmtool v2 0/2] Fix compilation with musl-libc based toolchains

[J. Neuschäfer]

This patchset enables kvmtool to build on musl-libc.
I have also tested that it still builds on glibc.

Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
Changes in v2:
- Be explicit about GNU vs. POSIX versions of basename
- Link to v1: https://lore.kernel.org/r/20240727-musl-v1-0-35013d2f97a0@gmx.net

---
J. Neuschäfer (2):
      Switch to POSIX version of basename()
      Get __WORDSIZE from <sys/reg.h> for musl compat

 include/linux/bitops.h | 2 +-
 vfio/core.c            | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
---
base-commit: ca31abf5d9c3453c852b263ccb451751b29b944b
change-id: 20240727-musl-853c66deb931

Best regards,
--
J. Neuschäfer <j.neuschaefer@gmx.net>

[PATCH kvmtool v2 1/2] Switch to POSIX version of basename()

[J. Neuschäfer]

There are two versions of the basename function: The POSIX version is
defined in <libgen.h>, and glibc additionally provides a GNU-specific
version in <string.h>. musl-libc only provides the POSIX version,
resulting in a compilation failure:

vfio/core.c:538:22: error: implicit declaration of function 'basename' [-Werror=implicit-function-declaration]
  538 |         group_name = basename(group_path);
      |                      ^~~~~~~~

Reviewed-by: Alyssa Ross <hi@alyssa.is>
Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
 vfio/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vfio/core.c b/vfio/core.c
index 3ff2c0b..8f88489 100644
--- a/vfio/core.c
+++ b/vfio/core.c
@@ -3,6 +3,7 @@
 #include "kvm/ioport.h"

 #include <linux/list.h>
+#include <libgen.h>

 #define VFIO_DEV_DIR		"/dev/vfio"
 #define VFIO_DEV_NODE		VFIO_DEV_DIR "/vfio"

--
2.43.0

Re: [PATCH kvmtool v2 1/2] Switch to POSIX version of basename()

[Andre Przywara]

On Sat, 27 Jul 2024 19:11:02 +0200
J. Neuschäfer <j.neuschaefer@gmx.net> wrote:

Hi,

> There are two versions of the basename function: The POSIX version is
> defined in <libgen.h>, and glibc additionally provides a GNU-specific
> version in <string.h>.

That's right, the Linux manpage confirms that. It seems like on GLIBC
Linux we get the GNU version, since we implicitly include string.h, and
define _GNU_SOURCE. The manpage talks about the differences between
the two: the POSIX version can modify the string, and it differs when the
last character is a '/'. Both cases do not apply to us here, so we can use
either version:

> musl-libc only provides the POSIX version,
> resulting in a compilation failure:
> 
> vfio/core.c:538:22: error: implicit declaration of function 'basename' [-Werror=implicit-function-declaration]
>   538 |         group_name = basename(group_path);
>       |                      ^~~~~~~~
> 
> Reviewed-by: Alyssa Ross <hi@alyssa.is>
> Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  vfio/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/vfio/core.c b/vfio/core.c
> index 3ff2c0b..8f88489 100644
> --- a/vfio/core.c
> +++ b/vfio/core.c
> @@ -3,6 +3,7 @@
>  #include "kvm/ioport.h"
> 
>  #include <linux/list.h>
> +#include <libgen.h>
> 
>  #define VFIO_DEV_DIR		"/dev/vfio"
>  #define VFIO_DEV_NODE		VFIO_DEV_DIR "/vfio"
> 
> --
> 2.43.0
> 

[PATCH kvmtool v2 2/2] Get __WORDSIZE from <sys/reg.h> for musl compat

[J. Neuschäfer]

musl-libc doesn't provide <bits/wordsize.h>, but it defines __WORDSIZE
in <sys/reg.h> and <sys/user.h>.

Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
 include/linux/bitops.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index ae33922..4f133ba 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -1,7 +1,7 @@
 #ifndef _KVM_LINUX_BITOPS_H_
 #define _KVM_LINUX_BITOPS_H_

-#include <bits/wordsize.h>
+#include <sys/reg.h>

 #include <linux/kernel.h>
 #include <linux/compiler.h>

--
2.43.0

Re: [PATCH kvmtool v2 2/2] Get __WORDSIZE from <sys/reg.h> for musl compat

[Alexandru Elisei]

Hi,

CC'ing the maintainers (can be found in README).

On Sat, Jul 27, 2024 at 07:11:03PM +0200, J. Neuschäfer wrote:
> musl-libc doesn't provide <bits/wordsize.h>, but it defines __WORDSIZE
> in <sys/reg.h> and <sys/user.h>.
> 
> Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
> ---
>  include/linux/bitops.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index ae33922..4f133ba 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -1,7 +1,7 @@
>  #ifndef _KVM_LINUX_BITOPS_H_
>  #define _KVM_LINUX_BITOPS_H_
> 
> -#include <bits/wordsize.h>
> +#include <sys/reg.h>

When cross-compiling on x86 for arm64, as well as when compiling natively for
arm64 I get this error:

In file included from include/linux/bitmap.h:7,
                 from util/find.c:4:
include/linux/bitops.h:5:10: fatal error: sys/reg.h: No such file or directory
    5 | #include <sys/reg.h>
      |          ^~~~~~~~~~~
compilation terminated.
make: *** [Makefile:510: util/find.o] Error 1
make: *** Waiting for unfinished jobs....
In file included from include/linux/bitmap.h:7,
                 from util/bitmap.c:9:
include/linux/bitops.h:5:10: fatal error: sys/reg.h: No such file or directory
    5 | #include <sys/reg.h>
      |          ^~~~~~~~~~~
compilation terminated.
make: *** [Makefile:510: util/bitmap.o] Error 1

Also, grep finds __WORDSIZE only in bits/wordsize.h on an x86 and arm64 machine:

$ grep -r "define __WORDSIZE" /usr/include/
/usr/include/bits/wordsize.h:# define __WORDSIZE			64
/usr/include/bits/wordsize.h:# define __WORDSIZE			32
/usr/include/bits/wordsize.h:# define __WORDSIZE32_SIZE_ULONG	1
/usr/include/bits/wordsize.h:# define __WORDSIZE32_PTRDIFF_LONG	1
/usr/include/bits/wordsize.h:#define __WORDSIZE_TIME64_COMPAT32	0


Thanks,
Alex

> 
>  #include <linux/kernel.h>
>  #include <linux/compiler.h>
> 
> --
> 2.43.0
> 
> 

Re: [PATCH kvmtool v2 2/2] Get __WORDSIZE from <sys/reg.h> for musl compat

[Andre Przywara]

On Mon, 29 Jul 2024 17:24:24 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> Hi,
> 
> CC'ing the maintainers (can be found in README).
> 
> On Sat, Jul 27, 2024 at 07:11:03PM +0200, J. Neuschäfer wrote:
> > musl-libc doesn't provide <bits/wordsize.h>, but it defines __WORDSIZE
> > in <sys/reg.h> and <sys/user.h>.
> > 
> > Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
> > ---
> >  include/linux/bitops.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index ae33922..4f133ba 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -1,7 +1,7 @@
> >  #ifndef _KVM_LINUX_BITOPS_H_
> >  #define _KVM_LINUX_BITOPS_H_
> > 
> > -#include <bits/wordsize.h>
> > +#include <sys/reg.h>  
> 
> When cross-compiling on x86 for arm64, as well as when compiling natively for
> arm64 I get this error:
> 
> In file included from include/linux/bitmap.h:7,
>                  from util/find.c:4:
> include/linux/bitops.h:5:10: fatal error: sys/reg.h: No such file or directory
>     5 | #include <sys/reg.h>
>       |          ^~~~~~~~~~~
> compilation terminated.
> make: *** [Makefile:510: util/find.o] Error 1
> make: *** Waiting for unfinished jobs....
> In file included from include/linux/bitmap.h:7,
>                  from util/bitmap.c:9:
> include/linux/bitops.h:5:10: fatal error: sys/reg.h: No such file or directory
>     5 | #include <sys/reg.h>
>       |          ^~~~~~~~~~~
> compilation terminated.
> make: *** [Makefile:510: util/bitmap.o] Error 1
> 
> Also, grep finds __WORDSIZE only in bits/wordsize.h on an x86 and arm64 machine:

I wonder if __WORDSIZE (with two leading underscores!) is something
portable enough to be used in userland tools in the first place. The
kernel seems to use it, and that's where we inherited it from, I guess.
But since we only use it in one place (to define BITS_PER_LONG), can't we
just replace this there, with something based on "sizeof(long)"?
Looks cleaner anyway, since "word" is an overloaded term.

Cheers,
Andre

> 
> $ grep -r "define __WORDSIZE" /usr/include/
> /usr/include/bits/wordsize.h:# define __WORDSIZE			64
> /usr/include/bits/wordsize.h:# define __WORDSIZE			32
> /usr/include/bits/wordsize.h:# define __WORDSIZE32_SIZE_ULONG	1
> /usr/include/bits/wordsize.h:# define __WORDSIZE32_PTRDIFF_LONG	1
> /usr/include/bits/wordsize.h:#define __WORDSIZE_TIME64_COMPAT32	0
> 
> 
> Thanks,
> Alex
> 
> > 
> >  #include <linux/kernel.h>
> >  #include <linux/compiler.h>
> > 
> > --
> > 2.43.0
> > 
> >   
>