1. Patchew
  2. QEMU
  3. View series

[PATCH] RFC: build-sys: drop dtc submodule

marcandre.lureau@redhat.com posted 1 patch 2 years, 8 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210825124309.223622-1-marcandre.lureau@redhat.com
configure         | 22 +-------------------
meson.build       | 53 ++++++++---------------------------------------
.gitmodules       |  3 ---
dtc               |  1 -
meson_options.txt |  3 ---
5 files changed, 10 insertions(+), 72 deletions(-)
delete mode 160000 dtc
[PATCH] RFC: build-sys: drop dtc submodule
Posted by marcandre.lureau@redhat.com 2 years, 8 months ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

DTC is widely available, we could consider to stop bundling it.

curl -s 'https://repology.org/api/v1/project/dtc' |  \
   jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
   egrep -i 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'

centos_8: ["1.6.0","1.6.0"]
debian_oldstable: ["1.4.7"]
freebsd: ["1.6.0"]
homebrew: ["1.6.1"]
openbsd: ["1.6.0"]
opensuse_leap_15_2: ["1.5.1","1.5.1"]
pkgsrc_current: ["1.4.7"]
ubuntu_18_04: ["1.4.5"]

MinGW package on Fedora pending review.
(https://bugzilla.redhat.com/show_bug.cgi?id=1997511)

Debian is lacking the MinGW package.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure         | 22 +-------------------
 meson.build       | 53 ++++++++---------------------------------------
 .gitmodules       |  3 ---
 dtc               |  1 -
 meson_options.txt |  3 ---
 5 files changed, 10 insertions(+), 72 deletions(-)
 delete mode 160000 dtc

diff --git a/configure b/configure
index 9a79a004d7..04050c0426 100755
--- a/configure
+++ b/configure
@@ -296,7 +296,6 @@ curl="auto"
 iconv="auto"
 curses="auto"
 docs="auto"
-fdt="auto"
 netmap="no"
 sdl="auto"
 sdl_image="auto"
@@ -1212,14 +1211,6 @@ for opt do
   ;;
   --enable-curl) curl="enabled"
   ;;
-  --disable-fdt) fdt="disabled"
-  ;;
-  --enable-fdt) fdt="enabled"
-  ;;
-  --enable-fdt=git) fdt="internal"
-  ;;
-  --enable-fdt=system) fdt="system"
-  ;;
   --disable-linux-aio) linux_aio="no"
   ;;
   --enable-linux-aio) linux_aio="yes"
@@ -1890,7 +1881,6 @@ disabled with --disable-FEATURE, default is enabled if available
   brlapi          BrlAPI (Braile)
   curl            curl connectivity
   membarrier      membarrier system call (for Linux 4.14+ or Windows)
-  fdt             fdt device tree
   kvm             KVM acceleration support
   hax             HAX acceleration support
   hvf             Hypervisor.framework acceleration support
@@ -3439,16 +3429,6 @@ if compile_prog "" "" ; then
   iovec=yes
 fi
 
-##########################################
-# fdt probe
-
-case "$fdt" in
-  auto | enabled | internal)
-    # Simpler to always update submodule, even if not needed.
-    git_submodules="${git_submodules} dtc"
-    ;;
-esac
-
 ##########################################
 # opengl probe (for sdl2, gtk)
 
@@ -5199,7 +5179,7 @@ if test "$skip_meson" = no; then
         -Dlibusb=$libusb -Dsmartcard=$smartcard -Dusb_redir=$usb_redir -Dvte=$vte \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
-        -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
+        -Dcapstone=$capstone -Dslirp=$slirp -Dbrlapi=$brlapi \
         -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
         -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse -Dlibxml2=$libxml2 \
diff --git a/meson.build b/meson.build
index b3e7ec0e92..1b30dd7bdc 100644
--- a/meson.build
+++ b/meson.build
@@ -1897,50 +1897,15 @@ if get_option('cfi') and slirp_opt == 'system'
 endif
 
 fdt = not_found
-fdt_opt = get_option('fdt')
-if have_system
-  if fdt_opt in ['enabled', 'auto', 'system']
-    have_internal = fs.exists(meson.current_source_dir() / 'dtc/libfdt/Makefile.libfdt')
-    fdt = cc.find_library('fdt', kwargs: static_kwargs,
-                          required: fdt_opt == 'system' or
-                                    fdt_opt == 'enabled' and not have_internal)
-    if fdt.found() and cc.links('''
-       #include <libfdt.h>
-       #include <libfdt_env.h>
-       int main(void) { fdt_check_full(NULL, 0); return 0; }''',
-         dependencies: fdt)
-      fdt_opt = 'system'
-    elif have_internal
-      fdt_opt = 'internal'
-    else
-      fdt_opt = 'disabled'
+if fdt_required.length() > 0
+    fdt = cc.find_library('fdt', kwargs: static_kwargs)
+    if not fdt.found() or not cc.links('''
+#include <libfdt.h>
+#include <libfdt_env.h>
+int main(void) { fdt_first_subnode(NULL, 0); return 0; }''',
+                                       dependencies: fdt)
+      error('fdt >= 1.4.2 required by targets ' + ', '.join(fdt_required))
     endif
-  endif
-  if fdt_opt == 'internal'
-    fdt_files = files(
-      'dtc/libfdt/fdt.c',
-      'dtc/libfdt/fdt_ro.c',
-      'dtc/libfdt/fdt_wip.c',
-      'dtc/libfdt/fdt_sw.c',
-      'dtc/libfdt/fdt_rw.c',
-      'dtc/libfdt/fdt_strerror.c',
-      'dtc/libfdt/fdt_empty_tree.c',
-      'dtc/libfdt/fdt_addresses.c',
-      'dtc/libfdt/fdt_overlay.c',
-      'dtc/libfdt/fdt_check.c',
-    )
-
-    fdt_inc = include_directories('dtc/libfdt')
-    libfdt = static_library('fdt',
-                            build_by_default: false,
-                            sources: fdt_files,
-                            include_directories: fdt_inc)
-    fdt = declare_dependency(link_with: libfdt,
-                             include_directories: fdt_inc)
-  endif
-endif
-if not fdt.found() and fdt_required.length() > 0
-  error('fdt not available but required by targets ' + ', '.join(fdt_required))
 endif
 
 config_host_data.set('CONFIG_CAPSTONE', capstone.found())
@@ -3069,7 +3034,7 @@ summary_info += {'Linux io_uring support': linux_io_uring.found()}
 summary_info += {'ATTR/XATTR support': libattr.found()}
 summary_info += {'RDMA support':      config_host.has_key('CONFIG_RDMA')}
 summary_info += {'PVRDMA support':    config_host.has_key('CONFIG_PVRDMA')}
-summary_info += {'fdt support':       fdt_opt == 'disabled' ? false : fdt_opt}
+summary_info += {'fdt support':       fdt.found()}
 summary_info += {'libcap-ng support': libcap_ng.found()}
 summary_info += {'bpf support': libbpf.found()}
 # TODO: add back protocol and server version
diff --git a/.gitmodules b/.gitmodules
index 08b1b48a09..582303698b 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -16,9 +16,6 @@
 [submodule "roms/sgabios"]
 	path = roms/sgabios
 	url = https://gitlab.com/qemu-project/sgabios.git
-[submodule "dtc"]
-	path = dtc
-	url = https://gitlab.com/qemu-project/dtc.git
 [submodule "roms/u-boot"]
 	path = roms/u-boot
 	url = https://gitlab.com/qemu-project/u-boot.git
diff --git a/dtc b/dtc
deleted file mode 160000
index 85e5d83984..0000000000
--- a/dtc
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
diff --git a/meson_options.txt b/meson_options.txt
index a9a9b8f4c6..8ba5433356 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -152,6 +152,3 @@ option('capstone', type: 'combo', value: 'auto',
 option('slirp', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
        description: 'Whether and how to find the slirp library')
-option('fdt', type: 'combo', value: 'auto',
-       choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
-       description: 'Whether and how to find the libfdt library')
-- 
2.33.0.rc2
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Mark Cave-Ayland 2 years, 8 months ago 
On 25/08/2021 13:43, marcandre.lureau@redhat.com wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> DTC is widely available, we could consider to stop bundling it.
> 
> curl -s 'https://repology.org/api/v1/project/dtc' |  \
>     jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
>     egrep -i 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> 
> centos_8: ["1.6.0","1.6.0"]
> debian_oldstable: ["1.4.7"]
> freebsd: ["1.6.0"]
> homebrew: ["1.6.1"]
> openbsd: ["1.6.0"]
> opensuse_leap_15_2: ["1.5.1","1.5.1"]
> pkgsrc_current: ["1.4.7"]
> ubuntu_18_04: ["1.4.5"]
> 
> MinGW package on Fedora pending review.
> (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> 
> Debian is lacking the MinGW package.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   configure         | 22 +-------------------
>   meson.build       | 53 ++++++++---------------------------------------
>   .gitmodules       |  3 ---
>   dtc               |  1 -
>   meson_options.txt |  3 ---
>   5 files changed, 10 insertions(+), 72 deletions(-)
>   delete mode 160000 dtc
> 
> diff --git a/configure b/configure
> index 9a79a004d7..04050c0426 100755
> --- a/configure
> +++ b/configure
> @@ -296,7 +296,6 @@ curl="auto"
>   iconv="auto"
>   curses="auto"
>   docs="auto"
> -fdt="auto"
>   netmap="no"
>   sdl="auto"
>   sdl_image="auto"
> @@ -1212,14 +1211,6 @@ for opt do
>     ;;
>     --enable-curl) curl="enabled"
>     ;;
> -  --disable-fdt) fdt="disabled"
> -  ;;
> -  --enable-fdt) fdt="enabled"
> -  ;;
> -  --enable-fdt=git) fdt="internal"
> -  ;;
> -  --enable-fdt=system) fdt="system"
> -  ;;
>     --disable-linux-aio) linux_aio="no"
>     ;;
>     --enable-linux-aio) linux_aio="yes"
> @@ -1890,7 +1881,6 @@ disabled with --disable-FEATURE, default is enabled if available
>     brlapi          BrlAPI (Braile)
>     curl            curl connectivity
>     membarrier      membarrier system call (for Linux 4.14+ or Windows)
> -  fdt             fdt device tree
>     kvm             KVM acceleration support
>     hax             HAX acceleration support
>     hvf             Hypervisor.framework acceleration support
> @@ -3439,16 +3429,6 @@ if compile_prog "" "" ; then
>     iovec=yes
>   fi
>   
> -##########################################
> -# fdt probe
> -
> -case "$fdt" in
> -  auto | enabled | internal)
> -    # Simpler to always update submodule, even if not needed.
> -    git_submodules="${git_submodules} dtc"
> -    ;;
> -esac
> -
>   ##########################################
>   # opengl probe (for sdl2, gtk)
>   
> @@ -5199,7 +5179,7 @@ if test "$skip_meson" = no; then
>           -Dlibusb=$libusb -Dsmartcard=$smartcard -Dusb_redir=$usb_redir -Dvte=$vte \
>           -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
>           -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
> -        -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
> +        -Dcapstone=$capstone -Dslirp=$slirp -Dbrlapi=$brlapi \
>           -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
>           -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
>           -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse -Dlibxml2=$libxml2 \
> diff --git a/meson.build b/meson.build
> index b3e7ec0e92..1b30dd7bdc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1897,50 +1897,15 @@ if get_option('cfi') and slirp_opt == 'system'
>   endif
>   
>   fdt = not_found
> -fdt_opt = get_option('fdt')
> -if have_system
> -  if fdt_opt in ['enabled', 'auto', 'system']
> -    have_internal = fs.exists(meson.current_source_dir() / 'dtc/libfdt/Makefile.libfdt')
> -    fdt = cc.find_library('fdt', kwargs: static_kwargs,
> -                          required: fdt_opt == 'system' or
> -                                    fdt_opt == 'enabled' and not have_internal)
> -    if fdt.found() and cc.links('''
> -       #include <libfdt.h>
> -       #include <libfdt_env.h>
> -       int main(void) { fdt_check_full(NULL, 0); return 0; }''',
> -         dependencies: fdt)
> -      fdt_opt = 'system'
> -    elif have_internal
> -      fdt_opt = 'internal'
> -    else
> -      fdt_opt = 'disabled'
> +if fdt_required.length() > 0
> +    fdt = cc.find_library('fdt', kwargs: static_kwargs)
> +    if not fdt.found() or not cc.links('''
> +#include <libfdt.h>
> +#include <libfdt_env.h>
> +int main(void) { fdt_first_subnode(NULL, 0); return 0; }''',
> +                                       dependencies: fdt)
> +      error('fdt >= 1.4.2 required by targets ' + ', '.join(fdt_required))
>       endif
> -  endif
> -  if fdt_opt == 'internal'
> -    fdt_files = files(
> -      'dtc/libfdt/fdt.c',
> -      'dtc/libfdt/fdt_ro.c',
> -      'dtc/libfdt/fdt_wip.c',
> -      'dtc/libfdt/fdt_sw.c',
> -      'dtc/libfdt/fdt_rw.c',
> -      'dtc/libfdt/fdt_strerror.c',
> -      'dtc/libfdt/fdt_empty_tree.c',
> -      'dtc/libfdt/fdt_addresses.c',
> -      'dtc/libfdt/fdt_overlay.c',
> -      'dtc/libfdt/fdt_check.c',
> -    )
> -
> -    fdt_inc = include_directories('dtc/libfdt')
> -    libfdt = static_library('fdt',
> -                            build_by_default: false,
> -                            sources: fdt_files,
> -                            include_directories: fdt_inc)
> -    fdt = declare_dependency(link_with: libfdt,
> -                             include_directories: fdt_inc)
> -  endif
> -endif
> -if not fdt.found() and fdt_required.length() > 0
> -  error('fdt not available but required by targets ' + ', '.join(fdt_required))
>   endif
>   
>   config_host_data.set('CONFIG_CAPSTONE', capstone.found())
> @@ -3069,7 +3034,7 @@ summary_info += {'Linux io_uring support': linux_io_uring.found()}
>   summary_info += {'ATTR/XATTR support': libattr.found()}
>   summary_info += {'RDMA support':      config_host.has_key('CONFIG_RDMA')}
>   summary_info += {'PVRDMA support':    config_host.has_key('CONFIG_PVRDMA')}
> -summary_info += {'fdt support':       fdt_opt == 'disabled' ? false : fdt_opt}
> +summary_info += {'fdt support':       fdt.found()}
>   summary_info += {'libcap-ng support': libcap_ng.found()}
>   summary_info += {'bpf support': libbpf.found()}
>   # TODO: add back protocol and server version
> diff --git a/.gitmodules b/.gitmodules
> index 08b1b48a09..582303698b 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -16,9 +16,6 @@
>   [submodule "roms/sgabios"]
>   	path = roms/sgabios
>   	url = https://gitlab.com/qemu-project/sgabios.git
> -[submodule "dtc"]
> -	path = dtc
> -	url = https://gitlab.com/qemu-project/dtc.git
>   [submodule "roms/u-boot"]
>   	path = roms/u-boot
>   	url = https://gitlab.com/qemu-project/u-boot.git
> diff --git a/dtc b/dtc
> deleted file mode 160000
> index 85e5d83984..0000000000
> --- a/dtc
> +++ /dev/null
> @@ -1 +0,0 @@
> -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> diff --git a/meson_options.txt b/meson_options.txt
> index a9a9b8f4c6..8ba5433356 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -152,6 +152,3 @@ option('capstone', type: 'combo', value: 'auto',
>   option('slirp', type: 'combo', value: 'auto',
>          choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
>          description: 'Whether and how to find the slirp library')
> -option('fdt', type: 'combo', value: 'auto',
> -       choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
> -       description: 'Whether and how to find the libfdt library')

My initial concern here was whether dtc packages are available for native Windows 
builds, although it seems that MSYS2 does carry a dtc package: 
https://packages.msys2.org/search?q=dtc.

Presumably this would be the last part of a larger patchset to include the dtc 
package in the docker images used by the Gitlab CI builds?


ATB,

Mark.
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago 
On 8/25/21 2:43 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> DTC is widely available, we could consider to stop bundling it.
> 
> curl -s 'https://repology.org/api/v1/project/dtc' |  \
>    jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
>    egrep -i 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> 
> centos_8: ["1.6.0","1.6.0"]
> debian_oldstable: ["1.4.7"]
> freebsd: ["1.6.0"]
> homebrew: ["1.6.1"]
> openbsd: ["1.6.0"]
> opensuse_leap_15_2: ["1.5.1","1.5.1"]
> pkgsrc_current: ["1.4.7"]
> ubuntu_18_04: ["1.4.5"]
> 
> MinGW package on Fedora pending review.
> (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> 
> Debian is lacking the MinGW package.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  configure         | 22 +-------------------
>  meson.build       | 53 ++++++++---------------------------------------
>  .gitmodules       |  3 ---
>  dtc               |  1 -
>  meson_options.txt |  3 ---
>  5 files changed, 10 insertions(+), 72 deletions(-)
>  delete mode 160000 dtc

Does this fix https://gitlab.com/qemu-project/qemu/-/issues/255 ?
"Build on sparc64 fails with "undefined reference to `fdt_check_full'"
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Peter Maydell 2 years, 8 months ago 
On Wed, 25 Aug 2021 at 14:28, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 8/25/21 2:43 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > DTC is widely available, we could consider to stop bundling it.
> >
> > curl -s 'https://repology.org/api/v1/project/dtc' |  \
> >    jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
> >    egrep -i 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> >
> > centos_8: ["1.6.0","1.6.0"]
> > debian_oldstable: ["1.4.7"]
> > freebsd: ["1.6.0"]
> > homebrew: ["1.6.1"]
> > openbsd: ["1.6.0"]
> > opensuse_leap_15_2: ["1.5.1","1.5.1"]
> > pkgsrc_current: ["1.4.7"]
> > ubuntu_18_04: ["1.4.5"]
> >
> > MinGW package on Fedora pending review.
> > (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> >
> > Debian is lacking the MinGW package.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  configure         | 22 +-------------------
> >  meson.build       | 53 ++++++++---------------------------------------
> >  .gitmodules       |  3 ---
> >  dtc               |  1 -
> >  meson_options.txt |  3 ---
> >  5 files changed, 10 insertions(+), 72 deletions(-)
> >  delete mode 160000 dtc
>
> Does this fix https://gitlab.com/qemu-project/qemu/-/issues/255 ?
> "Build on sparc64 fails with "undefined reference to `fdt_check_full'"

That bug sounds like it indicates that we can't use this patch:
a comment in the bug says that fdt_check_full() (which we use)
was only added in libfdt 1.5.1. So the libfdt version we need
is still not available in several of the distros we care about.

I suspect the bug you link to may have been fixed -- certainly
meson.build appears to be specifically looking for fdt_check_full()
so we should never try to compile with a libfdt that doesn't have it.

-- PMM
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Daniel P. Berrangé 2 years, 8 months ago 
On Wed, Aug 25, 2021 at 07:12:10PM +0100, Peter Maydell wrote:
> On Wed, 25 Aug 2021 at 14:28, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 8/25/21 2:43 PM, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > DTC is widely available, we could consider to stop bundling it.
> > >
> > > curl -s 'https://repology.org/api/v1/project/dtc' |  \
> > >    jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
> > >    egrep -i 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> > >
> > > centos_8: ["1.6.0","1.6.0"]
> > > debian_oldstable: ["1.4.7"]
> > > freebsd: ["1.6.0"]
> > > homebrew: ["1.6.1"]
> > > openbsd: ["1.6.0"]
> > > opensuse_leap_15_2: ["1.5.1","1.5.1"]
> > > pkgsrc_current: ["1.4.7"]
> > > ubuntu_18_04: ["1.4.5"]
> > >
> > > MinGW package on Fedora pending review.
> > > (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> > >
> > > Debian is lacking the MinGW package.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  configure         | 22 +-------------------
> > >  meson.build       | 53 ++++++++---------------------------------------
> > >  .gitmodules       |  3 ---
> > >  dtc               |  1 -
> > >  meson_options.txt |  3 ---
> > >  5 files changed, 10 insertions(+), 72 deletions(-)
> > >  delete mode 160000 dtc
> >
> > Does this fix https://gitlab.com/qemu-project/qemu/-/issues/255 ?
> > "Build on sparc64 fails with "undefined reference to `fdt_check_full'"
> 
> That bug sounds like it indicates that we can't use this patch:
> a comment in the bug says that fdt_check_full() (which we use)
> was only added in libfdt 1.5.1. So the libfdt version we need
> is still not available in several of the distros we care about.
> 
> I suspect the bug you link to may have been fixed -- certainly
> meson.build appears to be specifically looking for fdt_check_full()
> so we should never try to compile with a libfdt that doesn't have it.

It is slightly more complicated - the fdt_check_full function exists
in all the distros we target AFAICT, but libfdt's ELF symbols file 
never recorded its existance. So if you static link to libfdt you 
get fdt_check_full but if you dynamic link it is inaccessible :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Marc-André Lureau 2 years, 8 months ago 
Hi

On Fri, Aug 27, 2021 at 1:03 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Aug 25, 2021 at 07:12:10PM +0100, Peter Maydell wrote:
> > On Wed, 25 Aug 2021 at 14:28, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
> > >
> > > On 8/25/21 2:43 PM, marcandre.lureau@redhat.com wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > >
> > > > DTC is widely available, we could consider to stop bundling it.
> > > >
> > > > curl -s 'https://repology.org/api/v1/project/dtc' |  \
> > > >    jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"'
> | \
> > > >    egrep -i
> 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> > > >
> > > > centos_8: ["1.6.0","1.6.0"]
> > > > debian_oldstable: ["1.4.7"]
> > > > freebsd: ["1.6.0"]
> > > > homebrew: ["1.6.1"]
> > > > openbsd: ["1.6.0"]
> > > > opensuse_leap_15_2: ["1.5.1","1.5.1"]
> > > > pkgsrc_current: ["1.4.7"]
> > > > ubuntu_18_04: ["1.4.5"]
> > > >
> > > > MinGW package on Fedora pending review.
> > > > (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> > > >
> > > > Debian is lacking the MinGW package.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  configure         | 22 +-------------------
> > > >  meson.build       | 53
> ++++++++---------------------------------------
> > > >  .gitmodules       |  3 ---
> > > >  dtc               |  1 -
> > > >  meson_options.txt |  3 ---
> > > >  5 files changed, 10 insertions(+), 72 deletions(-)
> > > >  delete mode 160000 dtc
> > >
> > > Does this fix https://gitlab.com/qemu-project/qemu/-/issues/255 ?
> > > "Build on sparc64 fails with "undefined reference to `fdt_check_full'"
> >
> > That bug sounds like it indicates that we can't use this patch:
> > a comment in the bug says that fdt_check_full() (which we use)
> > was only added in libfdt 1.5.1. So the libfdt version we need
> > is still not available in several of the distros we care about.
> >
> > I suspect the bug you link to may have been fixed -- certainly
> > meson.build appears to be specifically looking for fdt_check_full()
> > so we should never try to compile with a libfdt that doesn't have it.
>
> It is slightly more complicated - the fdt_check_full function exists
> in all the distros we target AFAICT, but libfdt's ELF symbols file
> never recorded its existance. So if you static link to libfdt you
> get fdt_check_full but if you dynamic link it is inaccessible :-(
>
>
I see, that's where the 1.5.1 version comes from then.

commit eac2ad495b29f15d78daa2a7226653f36515cd7a
Author: David Gibson <david@gibson.dropbear.id.au>
Date:   Mon Mar 25 14:52:47 2019 +1100

    Update version.lds again

git describe --contains eac2ad495b29f15d78daa2a7226653f36515cd7a
v1.5.1~40

Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>
>

-- 
Marc-André Lureau
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Marc-André Lureau 2 years, 8 months ago 
Hi

On Wed, Aug 25, 2021 at 10:13 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 25 Aug 2021 at 14:28, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
> >
> > On 8/25/21 2:43 PM, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > DTC is widely available, we could consider to stop bundling it.
> > >
> > > curl -s 'https://repology.org/api/v1/project/dtc' |  \
> > >    jq -r 'group_by(.repo) | .[] | "\(.[0].repo): \(map(.version))"' | \
> > >    egrep -i
> 'ubuntu_18|debian_old|rhel|centos|bsd|suse_leap_15_2|sles|homebrew|pkgsrc'
> > >
> > > centos_8: ["1.6.0","1.6.0"]
> > > debian_oldstable: ["1.4.7"]
> > > freebsd: ["1.6.0"]
> > > homebrew: ["1.6.1"]
> > > openbsd: ["1.6.0"]
> > > opensuse_leap_15_2: ["1.5.1","1.5.1"]
> > > pkgsrc_current: ["1.4.7"]
> > > ubuntu_18_04: ["1.4.5"]
> > >
> > > MinGW package on Fedora pending review.
> > > (https://bugzilla.redhat.com/show_bug.cgi?id=1997511)
> > >
> > > Debian is lacking the MinGW package.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  configure         | 22 +-------------------
> > >  meson.build       | 53 ++++++++---------------------------------------
> > >  .gitmodules       |  3 ---
> > >  dtc               |  1 -
> > >  meson_options.txt |  3 ---
> > >  5 files changed, 10 insertions(+), 72 deletions(-)
> > >  delete mode 160000 dtc
> >
> > Does this fix https://gitlab.com/qemu-project/qemu/-/issues/255 ?
> > "Build on sparc64 fails with "undefined reference to `fdt_check_full'"
>
> That bug sounds like it indicates that we can't use this patch:
> a comment in the bug says that fdt_check_full() (which we use)
> was only added in libfdt 1.5.1. So the libfdt version we need
> is still not available in several of the distros we care about.
>
> I suspect the bug you link to may have been fixed -- certainly
> meson.build appears to be specifically looking for fdt_check_full()
> so we should never try to compile with a libfdt that doesn't have it.
>
>
Right, the patch is a bit old actually and I didn't update it correctly
(specifically the check and the version requirement error)

fdt_check_full was added in 1.4.7:
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7

Only ubuntu appears to be lagging a bit behind. I wonder if they would
consider an update.

-- 
Marc-André Lureau
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Peter Maydell 2 years, 8 months ago 
On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> fdt_check_full was added in 1.4.7:
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
>
> Only ubuntu appears to be lagging a bit behind. I wonder if they would consider an update.

I doubt it. You would need to wait until that actually falls off
our supported list. You also have a couple of years to wait until
Debian oldstable is no longer on our supported list.

thanks
-- PMM
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Marc-André Lureau 2 years, 8 months ago 
Hi

On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > fdt_check_full was added in 1.4.7:
> > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
> >
> > Only ubuntu appears to be lagging a bit behind. I wonder if they would
> consider an update.
>
> I doubt it. You would need to wait until that actually falls off
> our supported list. You also have a couple of years to wait until
> Debian oldstable is no longer on our supported list.
>
> Maybe, I don't know why debian oldstable would have received a new version
plus fixes, and not ubuntu.

It seems we could have our own fallback copy of fdt_check_full() though..
I'll give that a try.

-- 
Marc-André Lureau
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by David Gibson 2 years, 8 months ago 
On Thu, Aug 26, 2021 at 12:11:17AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> 
> > On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
> > <marcandre.lureau@gmail.com> wrote:
> > > fdt_check_full was added in 1.4.7:
> > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
> > >
> > > Only ubuntu appears to be lagging a bit behind. I wonder if they would
> > consider an update.
> >
> > I doubt it. You would need to wait until that actually falls off
> > our supported list. You also have a couple of years to wait until
> > Debian oldstable is no longer on our supported list.
> >
> > Maybe, I don't know why debian oldstable would have received a new version
> plus fixes, and not ubuntu.
> 
> It seems we could have our own fallback copy of fdt_check_full() though..
> I'll give that a try.

We could, but fdt_check_full() is actually a pretty complex function.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Marc-André Lureau 2 years, 8 months ago 
Hi

On Thu, Aug 26, 2021 at 7:11 AM David Gibson <david@gibson.dropbear.id.au>
wrote:

> On Thu, Aug 26, 2021 at 12:11:17AM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell <peter.maydell@linaro.org
> >
> > wrote:
> >
> > > On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
> > > <marcandre.lureau@gmail.com> wrote:
> > > > fdt_check_full was added in 1.4.7:
> > > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
> > > >
> > > > Only ubuntu appears to be lagging a bit behind. I wonder if they
> would
> > > consider an update.
> > >
> > > I doubt it. You would need to wait until that actually falls off
> > > our supported list. You also have a couple of years to wait until
> > > Debian oldstable is no longer on our supported list.
> > >
> > > Maybe, I don't know why debian oldstable would have received a new
> version
> > plus fixes, and not ubuntu.
> >
> > It seems we could have our own fallback copy of fdt_check_full() though..
> > I'll give that a try.
>
> We could, but fdt_check_full() is actually a pretty complex function.
>
>
Yeah, that would be used for those who don't have >= 1.4.7.

Alternatively we could lower the fdt_check_full to fdt_check_header in this
case? It seems it is used to verify the DT from SLOF. It may be trusted I
suppose, or a malformed DT may only impact the guest?


> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_
> _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>


-- 
Marc-André Lureau
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by Philippe Mathieu-Daudé 2 years, 8 months ago 
On 8/26/21 9:34 AM, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 26, 2021 at 7:11 AM David Gibson
> <david@gibson.dropbear.id.au <mailto:david@gibson.dropbear.id.au>> wrote:
> 
>     On Thu, Aug 26, 2021 at 12:11:17AM +0400, Marc-André Lureau wrote:
>     > Hi
>     >
>     > On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell
>     <peter.maydell@linaro.org <mailto:peter.maydell@linaro.org>>
>     > wrote:
>     >
>     > > On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
>     > > <marcandre.lureau@gmail.com <mailto:marcandre.lureau@gmail.com>>
>     wrote:
>     > > > fdt_check_full was added in 1.4.7:
>     > > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
>     <https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7>
>     > > >
>     > > > Only ubuntu appears to be lagging a bit behind. I wonder if
>     they would
>     > > consider an update.
>     > >
>     > > I doubt it. You would need to wait until that actually falls off
>     > > our supported list. You also have a couple of years to wait until
>     > > Debian oldstable is no longer on our supported list.
>     > >
>     > > Maybe, I don't know why debian oldstable would have received a
>     new version
>     > plus fixes, and not ubuntu.
>     >
>     > It seems we could have our own fallback copy of fdt_check_full()
>     though..
>     > I'll give that a try.
> 
>     We could, but fdt_check_full() is actually a pretty complex function.
> 
> 
> Yeah, that would be used for those who don't have >= 1.4.7.
> 
> Alternatively we could lower the fdt_check_full to fdt_check_header in
> this case? It seems it is used to verify the DT from SLOF. It may be
> trusted I suppose, or a malformed DT may only impact the guest?

Alternative (or complementary?) approach:
https://lore.kernel.org/qemu-devel/20210511155354.3069141-1-philmd@redhat.com/
Re: [PATCH] RFC: build-sys: drop dtc submodule
Posted by David Gibson 2 years, 8 months ago 
On Thu, Aug 26, 2021 at 11:34:59AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 26, 2021 at 7:11 AM David Gibson <david@gibson.dropbear.id.au>
> wrote:
> 
> > On Thu, Aug 26, 2021 at 12:11:17AM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Thu, Aug 26, 2021 at 12:00 AM Peter Maydell <peter.maydell@linaro.org
> > >
> > > wrote:
> > >
> > > > On Wed, 25 Aug 2021 at 20:55, Marc-André Lureau
> > > > <marcandre.lureau@gmail.com> wrote:
> > > > > fdt_check_full was added in 1.4.7:
> > > > > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/tag/?h=v1.4.7
> > > > >
> > > > > Only ubuntu appears to be lagging a bit behind. I wonder if they
> > would
> > > > consider an update.
> > > >
> > > > I doubt it. You would need to wait until that actually falls off
> > > > our supported list. You also have a couple of years to wait until
> > > > Debian oldstable is no longer on our supported list.
> > > >
> > > > Maybe, I don't know why debian oldstable would have received a new
> > version
> > > plus fixes, and not ubuntu.
> > >
> > > It seems we could have our own fallback copy of fdt_check_full() though..
> > > I'll give that a try.
> >
> > We could, but fdt_check_full() is actually a pretty complex function.
> >
> >
> Yeah, that would be used for those who don't have >= 1.4.7.
> 
> Alternatively we could lower the fdt_check_full to fdt_check_header in this
> case? It seems it is used to verify the DT from SLOF. It may be trusted I
> suppose, or a malformed DT may only impact the guest?

No, fdt_check_header() isn't enough.  We can't trust the dt blob from
SLOF, because it's coming from guest context.  We *expect* it to come
from the SLOF iamge we control, but nothing prevents anything else in
the guest from calling the hypercall, or corrupting the in-memory SLOF
image.

And, a bad DT won't just impact the guest - there's a couple of things
we need from it (that's the only reason we need to have SLOF give us
back the DT at all).  Note that the blob might not just have bad
content, but could have bad formatting which will make the functions
qemu uses to access it misbehave.  So, our options are either be
super-careful on every possible DT access after this point, or
pre-check it when it's loaded with fdt_check_full().

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.