Bug 35 - Does not compile with musl libc: drivers/bus/pci/linux/pci_uio.c
Summary: Does not compile with musl libc: drivers/bus/pci/linux/pci_uio.c
Status: RESOLVED FIXED
Alias: None
Product: DPDK
Classification: Unclassified
Component: ethdev (show other bugs)
Version: 18.05
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Ferruh YIGIT
URL:
Depends on:
Blocks:
 
Reported: 2018-05-03 12:33 CEST by Raph
Modified: 2021-04-02 01:44 CEST (History)
3 users (show)



Attachments
0001-bus-pci-add-fallback-for-out-lwb-_p-for-non-GNU-libc.patch (2.34 KB, patch)
2019-03-11 12:35 CET, ncopa
Details | Diff

Description Raph 2018-05-03 12:33:14 CEST
This does not compile on X86 because it assumes the presence of the glibc-only functions `outl_p`, `outw_p` and `outb_p`. This can be fixed reliably for all c libraries and operating systems by simply importing our own definitions, viz:-

```
#if defined(RTE_ARCH_X86)
#include <sys/io.h>

// Used in pci_uio_ioport_write, these functions are present in glibc
// in <sys/io.h> (without the pci_uio_ prefix) but not in the
// musl lib c.

static __inline void
pci_uio_outl_p(unsigned int value, unsigned short int port)
{
  __asm__ __volatile__ ("outl %0,%w1\noutb %%al,$0x80": :"a" (value),
			"Nd" (port));
}

static __inline void
pci_uio_outw_p(unsigned short int value, unsigned short int port)
{
  __asm__ __volatile__ ("outw %w0,%w1\noutb %%al,$0x80": :"a" (value),
			"Nd" (port));
}

static __inline void
pci_uio_outb_p(unsigned char value, unsigned short int port)
{
  __asm__ __volatile__ ("outb %b0,%w1\noutb %%al,$0x80": :"a" (value),
			"Nd" (port));
}
```

And then prefixing the usages of the glibc-only functions with `pci_uio_`, eg `outl_p` becomes `pci_uio_outlw_p`.
Comment 1 Ajit Khaparde 2018-08-02 22:44:20 CEST
Ferruh,
Looks like an old bug. Can you please take a look?

Thanks
Ajit
Comment 2 Ferruh YIGIT 2018-10-03 16:54:33 CEST
Is musl support in our scope? Who is using or testing it?

If there are people want to add that support or send fixes for it, it is great, but I am not sure if we officially support it.

What about closing this defect as "won't fix"
Comment 3 ncopa 2019-03-11 12:34:38 CET
(In reply to Ferruh YIGIT from comment #2)
> Is musl support in our scope? Who is using or testing it?

Alpine Linux uses musl libc and is fairly popular among docker users. I found this issue because I am trying to make ceph build on Alpine Linux.
 
> If there are people want to add that support or send fixes for it, it is
> great, but I am not sure if we officially support it.

The only thing required for officially supporting musl is that you officially support POSIX rather than GNU libc only. This will improve the over-all portability.

For example, I had a few build errors due to use of O_RDWR but without any #include <fcntl.h>. This currently works with GNU libc beacause something else happens to pull in fcntl.h, but there is no guarantee that this will work in future versions of GNU libc.

> What about closing this defect as "won't fix"

I can provide patches for you if that helps.
Comment 4 ncopa 2019-03-11 12:35:47 CET
Created attachment 31 [details]
0001-bus-pci-add-fallback-for-out-lwb-_p-for-non-GNU-libc.patch

Suggested patch for adding fallback for non-GNU libcs.
Comment 5 ncopa 2019-03-11 12:36:52 CET
We could also add macros for non x86 to get rid of some ifdefs:

diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 6058cd9f8..45759ca64 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -40,6 +40,11 @@ pci_uio_outb_p(unsigned char value, unsigned short int port)
                        "Nd" (port));
 }
 #endif
+#else /* RTE_ARCH_X86 */
+#define pci_uio_outl_p(s, reg) *(volatile uint32_t *)reg = s
+#define pci_uio_outw_p(s, reg) *(volatile uint16_t *)reg = s
+#define pci_uio_outb_p(s, reg) *(volatile uint8_t *)reg = s
+
 #endif
 
 #include <rte_log.h>
@@ -552,25 +557,13 @@ pci_uio_ioport_write(struct rte_pci_ioport *p,
        for (s = data; len > 0; s += size, reg += size, len -= size) {
                if (len >= 4) {
                        size = 4;
-#if defined(RTE_ARCH_X86)
                        pci_uio_outl_p(*(const uint32_t *)s, reg);
-#else
-                       *(volatile uint32_t *)reg = *(const uint32_t *)s;
-#endif
                } else if (len >= 2) {
                        size = 2;
-#if defined(RTE_ARCH_X86)
                        pci_uio_outw_p(*(const uint16_t *)s, reg);
-#else
-                       *(volatile uint16_t *)reg = *(const uint16_t *)s;
-#endif
                } else {
                        size = 1;
-#if defined(RTE_ARCH_X86)
                        pci_uio_outb_p(*s, reg);
-#else
-                       *(volatile uint8_t *)reg = *s;
-#endif
                }
        }
 }
Comment 6 Ferruh YIGIT 2019-03-11 12:42:50 CET
(In reply to ncopa from comment #5)
> We could also add macros for non x86 to get rid of some ifdefs:
> 
> diff --git a/drivers/bus/pci/linux/pci_uio.c
> b/drivers/bus/pci/linux/pci_uio.c
> index 6058cd9f8..45759ca64 100644
> --- a/drivers/bus/pci/linux/pci_uio.c
> +++ b/drivers/bus/pci/linux/pci_uio.c
> @@ -40,6 +40,11 @@ pci_uio_outb_p(unsigned char value, unsigned short int
> port)
>                         "Nd" (port));
>  }
>  #endif
> +#else /* RTE_ARCH_X86 */
> +#define pci_uio_outl_p(s, reg) *(volatile uint32_t *)reg = s
> +#define pci_uio_outw_p(s, reg) *(volatile uint16_t *)reg = s
> +#define pci_uio_outb_p(s, reg) *(volatile uint8_t *)reg = s
> +
>  #endif
>  
>  #include <rte_log.h>
> @@ -552,25 +557,13 @@ pci_uio_ioport_write(struct rte_pci_ioport *p,
>         for (s = data; len > 0; s += size, reg += size, len -= size) {
>                 if (len >= 4) {
>                         size = 4;
> -#if defined(RTE_ARCH_X86)
>                         pci_uio_outl_p(*(const uint32_t *)s, reg);
> -#else
> -                       *(volatile uint32_t *)reg = *(const uint32_t *)s;
> -#endif
>                 } else if (len >= 2) {
>                         size = 2;
> -#if defined(RTE_ARCH_X86)
>                         pci_uio_outw_p(*(const uint16_t *)s, reg);
> -#else
> -                       *(volatile uint16_t *)reg = *(const uint16_t *)s;
> -#endif
>                 } else {
>                         size = 1;
> -#if defined(RTE_ARCH_X86)
>                         pci_uio_outb_p(*s, reg);
> -#else
> -                       *(volatile uint8_t *)reg = *s;
> -#endif
>                 }
>         }
>  }

+1 to get rid of #ifdef via macros
Comment 7 Ferruh YIGIT 2019-03-11 12:49:45 CET
(In reply to ncopa from comment #4)
> Created attachment 31 [details]
> 0001-bus-pci-add-fallback-for-out-lwb-_p-for-non-GNU-libc.patch
> 
> Suggested patch for adding fallback for non-GNU libcs.

Can you please send the suggested patch and an additional for macros to the mail list? We can continue from there?

Thanks,
Comment 8 Ferruh YIGIT 2019-06-26 10:20:39 CEST
Hi @Raph, Can you please update us what is the latest status of musl support of DPDK?
Comment 9 Ferruh YIGIT 2021-02-09 15:52:25 CET
There is an existing patchset, it should be solving this problem too,

if you are still around, can you please check?
https://patches.dpdk.org/project/dpdk/list/?series=15024&state=*
Comment 10 Thomas Monjalon 2021-04-02 01:44:21 CEST
Resolved in http://git.dpdk.org/dpdk/commit/?id=204a7f44bc
Comment 11 Thomas Monjalon 2021-04-02 01:44:24 CEST
Resolved in http://git.dpdk.org/dpdk/commit/?id=e0473c6d5b

Note You need to log in before you can comment on or make changes to this bug.