[PATCH 0/3] enable PPC in test-meson-builds on ubuntu

[PATCH 0/3] enable PPC in test-meson-builds on ubuntu

[Bruce Richardson]

To help developers catch errors as soon as possible, we want to ensure
that as wide a variety of builds are done by test-meson-builds as
possible. Unfortunately, for those using Ubuntu, the shipped version
of GCC compiler for PowerPC on that system is not correctly detected by
the script. We fix this by detecting if Ubuntu is in use, and pointing
to the correct cross-file in that case.

On enabling those PPC builds on my system, a couple of other build
issues we encountered. These are fixed by the set before enabling the
build in the script.

Bruce Richardson (3):
  vhost: fix build for powerpc
  build: fix failures due to incompatible IPSec lib
  devtools: enable testing ppc builds on ubuntu

 devtools/test-meson-builds.sh       | 3 +++
 drivers/crypto/ipsec_mb/meson.build | 5 +++++
 lib/vhost/vduse.c                   | 9 ++++++---
 3 files changed, 14 insertions(+), 3 deletions(-)

--
2.39.2

[PATCH 1/3] vhost: fix build for powerpc

[Bruce Richardson]

When building on Ubuntu using the packaged powerpc compiler[1], a
warning is issued about the print format of the __u64 values.

../../lib/vhost/vduse.c: In function ‘vduse_vring_setup’:
../../lib/vhost/vhost.h:676:17: error: format ‘%llx’ expects argument of
type ‘long long unsigned int’, but argument 5 has type ‘__u64’ {aka
‘long unsigned int’} [-Werror=format=]
  676 |                 "VHOST_CONFIG: (%s) " fmt, prefix, ##args)
      |                 ^~~~~~~~~~~~~~~~~~~~~

Changing the format specifier to %lx, or to use PRIx64 breaks other
builds, so the safest solution is to explicitly typecast the printed
values to match the format string.

[1] powerpc64le-linux-gnu-gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0

Fixes: a9120db8b98b ("vhost: add VDUSE device startup")
Cc: maxime.coquelin@redhat.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/vhost/vduse.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
index 73ed424232..e2b6d35d37 100644
--- a/lib/vhost/vduse.c
+++ b/lib/vhost/vduse.c
@@ -162,9 +162,12 @@ vduse_vring_setup(struct virtio_net *dev, unsigned int index)
 
 	VHOST_LOG_CONFIG(dev->ifname, INFO, "VQ %u info:\n", index);
 	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tnum: %u\n", vq_info.num);
-	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n", vq_info.desc_addr);
-	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n", vq_info.driver_addr);
-	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n", vq_info.device_addr);
+	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n",
+			(unsigned long long)vq_info.desc_addr);
+	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n",
+			(unsigned long long)vq_info.driver_addr);
+	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n",
+			(unsigned long long)vq_info.device_addr);
 	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tavail_idx: %u\n", vq_info.split.avail_index);
 	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tready: %u\n", vq_info.ready);
 
-- 
2.39.2

Re: [PATCH 1/3] vhost: fix build for powerpc

[Bruce Richardson]

+PPC maintainer

On Thu, Aug 31, 2023 at 01:10:56PM +0100, Bruce Richardson wrote:
> When building on Ubuntu using the packaged powerpc compiler[1], a
> warning is issued about the print format of the __u64 values.
> 
> ../../lib/vhost/vduse.c: In function ‘vduse_vring_setup’:
> ../../lib/vhost/vhost.h:676:17: error: format ‘%llx’ expects argument of
> type ‘long long unsigned int’, but argument 5 has type ‘__u64’ {aka
> ‘long unsigned int’} [-Werror=format=]
>   676 |                 "VHOST_CONFIG: (%s) " fmt, prefix, ##args)
>       |                 ^~~~~~~~~~~~~~~~~~~~~
> 
> Changing the format specifier to %lx, or to use PRIx64 breaks other
> builds, so the safest solution is to explicitly typecast the printed
> values to match the format string.
> 
> [1] powerpc64le-linux-gnu-gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0
> 
> Fixes: a9120db8b98b ("vhost: add VDUSE device startup")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/vhost/vduse.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index 73ed424232..e2b6d35d37 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -162,9 +162,12 @@ vduse_vring_setup(struct virtio_net *dev, unsigned int index)
>  
>  	VHOST_LOG_CONFIG(dev->ifname, INFO, "VQ %u info:\n", index);
>  	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tnum: %u\n", vq_info.num);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n", vq_info.desc_addr);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n", vq_info.driver_addr);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n", vq_info.device_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n",
> +			(unsigned long long)vq_info.desc_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n",
> +			(unsigned long long)vq_info.driver_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n",
> +			(unsigned long long)vq_info.device_addr);
>  	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tavail_idx: %u\n", vq_info.split.avail_index);
>  	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tready: %u\n", vq_info.ready);
>  
> -- 
> 2.39.2
> 

Re: [PATCH 1/3] vhost: fix build for powerpc

[David Christensen]

On 9/1/23 7:59 AM, Bruce Richardson wrote:
> +PPC maintainer
> 
> On Thu, Aug 31, 2023 at 01:10:56PM +0100, Bruce Richardson wrote:
>> When building on Ubuntu using the packaged powerpc compiler[1], a
>> warning is issued about the print format of the __u64 values.
>>
>> ../../lib/vhost/vduse.c: In function ‘vduse_vring_setup’:
>> ../../lib/vhost/vhost.h:676:17: error: format ‘%llx’ expects argument of
>> type ‘long long unsigned int’, but argument 5 has type ‘__u64’ {aka
>> ‘long unsigned int’} [-Werror=format=]
>>    676 |                 "VHOST_CONFIG: (%s) " fmt, prefix, ##args)
>>        |                 ^~~~~~~~~~~~~~~~~~~~~
>>
>> Changing the format specifier to %lx, or to use PRIx64 breaks other
>> builds, so the safest solution is to explicitly typecast the printed
>> values to match the format string.
>>
>> [1] powerpc64le-linux-gnu-gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0
>>
>> Fixes: a9120db8b98b ("vhost: add VDUSE device startup")
>> Cc: maxime.coquelin@redhat.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---

Tested-by: David Christensen <drc@linux.vnet.ibm.com>

Re: [PATCH 1/3] vhost: fix build for powerpc

[Maxime Coquelin]

On 8/31/23 14:10, Bruce Richardson wrote:
> When building on Ubuntu using the packaged powerpc compiler[1], a
> warning is issued about the print format of the __u64 values.
> 
> ../../lib/vhost/vduse.c: In function ‘vduse_vring_setup’:
> ../../lib/vhost/vhost.h:676:17: error: format ‘%llx’ expects argument of
> type ‘long long unsigned int’, but argument 5 has type ‘__u64’ {aka
> ‘long unsigned int’} [-Werror=format=]
>    676 |                 "VHOST_CONFIG: (%s) " fmt, prefix, ##args)
>        |                 ^~~~~~~~~~~~~~~~~~~~~
> 
> Changing the format specifier to %lx, or to use PRIx64 breaks other
> builds, so the safest solution is to explicitly typecast the printed
> values to match the format string.
> 
> [1] powerpc64le-linux-gnu-gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0
> 
> Fixes: a9120db8b98b ("vhost: add VDUSE device startup")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/vhost/vduse.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/vhost/vduse.c b/lib/vhost/vduse.c
> index 73ed424232..e2b6d35d37 100644
> --- a/lib/vhost/vduse.c
> +++ b/lib/vhost/vduse.c
> @@ -162,9 +162,12 @@ vduse_vring_setup(struct virtio_net *dev, unsigned int index)
>   
>   	VHOST_LOG_CONFIG(dev->ifname, INFO, "VQ %u info:\n", index);
>   	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tnum: %u\n", vq_info.num);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n", vq_info.desc_addr);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n", vq_info.driver_addr);
> -	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n", vq_info.device_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdesc_addr: %llx\n",
> +			(unsigned long long)vq_info.desc_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdriver_addr: %llx\n",
> +			(unsigned long long)vq_info.driver_addr);
> +	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tdevice_addr: %llx\n",
> +			(unsigned long long)vq_info.device_addr);
>   	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tavail_idx: %u\n", vq_info.split.avail_index);
>   	VHOST_LOG_CONFIG(dev->ifname, INFO, "\tready: %u\n", vq_info.ready);
>   

It is surprising PRIx64 does not work on other architectures.
I don't see a better solution, so:

Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[PATCH 2/3] crypto/ipsec_mb: fix build failures due to incompatible lib

[Bruce Richardson]

When cross-compiling for PowerPC on Ubuntu, the x86 IPSec_MB library was
getting found by the build system for use in the PPC build. This led to
failures at compile time due to the library not being linkable.

We can avoid these failures by checking the discovered library for
compatibility at configuration time. This needs a version check as it is
supported only from version 0.60 of meson onwards.

Fixes: c75542ae4200 ("crypto/ipsec_mb: introduce IPsec_mb framework")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/crypto/ipsec_mb/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/crypto/ipsec_mb/meson.build b/drivers/crypto/ipsec_mb/meson.build
index 3057e6fd10..87bf965554 100644
--- a/drivers/crypto/ipsec_mb/meson.build
+++ b/drivers/crypto/ipsec_mb/meson.build
@@ -16,6 +16,11 @@ lib = cc.find_library('IPSec_MB', required: false)
 if not lib.found()
     build = false
     reason = 'missing dependency, "libIPSec_MB"'
+# if the lib is found, check it's the right format
+elif meson.version().version_compare('>=0.60') and not cc.links(
+        'int main(void) {return 0;}', dependencies: lib)
+    build = false
+    reason = 'incompatible dependency, "libIPSec_MB"'
 else
     ext_deps += lib
 
-- 
2.39.2

RE: [PATCH 2/3] crypto/ipsec_mb: fix build failures due to incompatible lib

[Power, Ciara]

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Thursday, August 31, 2023 1:11 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; stable@dpdk.org; Ji, Kai
> <kai.ji@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Akhil Goyal <gakhil@marvell.com>; Power, Ciara <ciara.power@intel.com>;
> Ray Kinsella <mdr@ashroe.eu>; Fan Zhang <fanzhang.oss@gmail.com>
> Subject: [PATCH 2/3] crypto/ipsec_mb: fix build failures due to incompatible
> lib
> 
> When cross-compiling for PowerPC on Ubuntu, the x86 IPSec_MB library was
> getting found by the build system for use in the PPC build. This led to failures
> at compile time due to the library not being linkable.
> 
> We can avoid these failures by checking the discovered library for compatibility
> at configuration time. This needs a version check as it is supported only from
> version 0.60 of meson onwards.
> 
> Fixes: c75542ae4200 ("crypto/ipsec_mb: introduce IPsec_mb framework")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Acked-by: Ciara Power <ciara.power@intel.com>

[PATCH 3/3] devtools: enable testing ppc builds on ubuntu

[Bruce Richardson]

When running PPC builds with the packaged Ubuntu compiler, a different
cross-file needs to be used. Adjust the test-meson-builds script to take
account of this, so that developers can easier pick up on errors.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 devtools/test-meson-builds.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index c41659d28b..605a855999 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -282,6 +282,9 @@ build build-loongarch64-generic-gcc $f ABI $use_shared

 # IBM POWER
 f=$srcdir/config/ppc/ppc64le-power8-linux-gcc
+if grep -q 'NAME="Ubuntu"' /etc/os-release ; then
+	f=$f-ubuntu
+fi
 build build-ppc64-power8-gcc $f ABI $use_shared

 # generic RISC-V
--
2.39.2

Re: [PATCH 3/3] devtools: enable testing ppc builds on ubuntu

[Tyler Retzlaff]

On Thu, Aug 31, 2023 at 01:10:58PM +0100, Bruce Richardson wrote:
> When running PPC builds with the packaged Ubuntu compiler, a different
> cross-file needs to be used. Adjust the test-meson-builds script to take
> account of this, so that developers can easier pick up on errors.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Re: [PATCH 0/3] enable PPC in test-meson-builds on ubuntu

[Thomas Monjalon]

31/08/2023 14:10, Bruce Richardson:
> To help developers catch errors as soon as possible, we want to ensure
> that as wide a variety of builds are done by test-meson-builds as
> possible. Unfortunately, for those using Ubuntu, the shipped version
> of GCC compiler for PowerPC on that system is not correctly detected by
> the script. We fix this by detecting if Ubuntu is in use, and pointing
> to the correct cross-file in that case.
> 
> On enabling those PPC builds on my system, a couple of other build
> issues we encountered. These are fixed by the set before enabling the
> build in the script.
> 
> Bruce Richardson (3):
>   vhost: fix build for powerpc
>   build: fix failures due to incompatible IPSec lib
>   devtools: enable testing ppc builds on ubuntu

Applied, thanks.