[PATCH 0/3] dtc: Fixes for the fdt check and update submodule to 1.6.1

[PATCH 0/3] dtc: Fixes for the fdt check and update submodule to 1.6.1

[Thomas Huth]

There are some issues in the checks for libfdt in meson.build which
get fixed with the first two patches.

And while we're at it, also update the dtc submodule to a proper release
version (in the third patch).

Thomas Huth (3):
  meson.build: Fix the check for a usable libfdt
  meson.build: Don't use internal libfdt if the user requested the
    system libfdt
  dtc: Update to version 1.6.1

 dtc         | 2 +-
 meson.build | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.27.0

[PATCH 1/3] meson.build: Fix the check for a usable libfdt

[Thomas Huth]

The check for libfdt currently has a flaw: If there is a system libfdt, the
meson.build code initialized the fdt variable with fdt = cc.find_library(...).
However, if this libfdt is too old and there is no internal dtc module
available, it continues with "fdt" pointing to the old and unusable version.
The check later in the file that tries to detect whether libfdt is necessary
then fails to trigger:

 if not fdt.found() and fdt_required.length() > 0
  error('fdt not available but required by targets ' + ', '.join(fdt_required))
 endif

The build fails then during compilation instead, which is of course bad
since this is quite confusing and already wasted quite some time of the user.
Thus if libfdt is not usable, we should unset the "fdt" variable immediately
again, so that the build already fails during the configuration phase.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/255
Signed-off-by: Thomas Huth <thuth@redhat.com>

If the system has
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index 0b555197c3..ee156ff0a5 100644
--- a/meson.build
+++ b/meson.build
@@ -1916,6 +1916,7 @@ if have_system
       fdt_opt = 'internal'
     else
       fdt_opt = 'disabled'
+      fdt = not_found
     endif
   endif
   if fdt_opt == 'internal'
-- 
2.27.0

[PATCH 2/3] meson.build: Don't use internal libfdt if the user requested the system libfdt

[Thomas Huth]

If the users ran configure with --enable-libfdt=system, they likely did
that on purpose. We should not silently fall back to the internal libfdt
if the system libfdt is not usable, but report the problem with a proper
message instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index ee156ff0a5..bc6a52b909 100644
--- a/meson.build
+++ b/meson.build
@@ -1912,6 +1912,8 @@ if have_system
        int main(void) { fdt_check_full(NULL, 0); return 0; }''',
          dependencies: fdt)
       fdt_opt = 'system'
+    elif fdt_opt == 'system'
+       error('system libfdt requested, but it is too old (1.5.1 or newer required)')
     elif have_internal
       fdt_opt = 'internal'
     else
-- 
2.27.0

Re: [PATCH 2/3] meson.build: Don't use internal libfdt if the user requested the system libfdt

[Philippe Mathieu-Daudé]

On 8/27/21 2:09 PM, Thomas Huth wrote:
> If the users ran configure with --enable-libfdt=system, they likely did
> that on purpose. We should not silently fall back to the internal libfdt
> if the system libfdt is not usable, but report the problem with a proper
> message instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 3/3] dtc: Update to version 1.6.1

[Thomas Huth]

The dtc submodule is currently pointing to non-release commit. It's nicer
if submodules point to release versions instead and since dtc 1.6.1 is
available now, let's update to that version.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 dtc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dtc b/dtc
index 85e5d83984..b6910bec11 160000
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
+Subproject commit b6910bec11614980a21e46fbccc35934b671bd81
-- 
2.27.0

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Thomas Huth]

On 27/08/2021 14.09, Thomas Huth wrote:
> The dtc submodule is currently pointing to non-release commit. It's nicer
> if submodules point to release versions instead and since dtc 1.6.1 is
> available now, let's update to that version.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   dtc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/dtc b/dtc
> index 85e5d83984..b6910bec11 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> +Subproject commit b6910bec11614980a21e46fbccc35934b671bd81

Ping!

David, could you please pick up this patch if you don't mind it?

  Thanks,
   Thomas

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Greg Kurz]

On Thu, 30 Sep 2021 09:10:12 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 27/08/2021 14.09, Thomas Huth wrote:
> > The dtc submodule is currently pointing to non-release commit. It's nicer
> > if submodules point to release versions instead and since dtc 1.6.1 is
> > available now, let's update to that version.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---

> >   dtc | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/dtc b/dtc
> > index 85e5d83984..b6910bec11 160000
> > --- a/dtc
> > +++ b/dtc
> > @@ -1 +1 @@
> > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> > +Subproject commit b6910bec11614980a21e46fbccc35934b671bd81
> 
> Ping!
> 
> David, could you please pick up this patch if you don't mind it?
> 
>   Thanks,
>    Thomas
> 

Hi Thomas !

greg@bahia:[main]dtc$ git log --oneline  85e5d839847af54efab170f2b1331b2a6421e647..v1.6.1
b6910bec1161 Bump version to v1.6.1
21d61d18f968 Fix CID 1461557
4c2ef8f4d14c checks: Introduce is_multiple_of()
e59ca36fb70e Make handling of cpp line information more tolerant
0c3fd9b6aceb checks: Drop interrupt_cells_is_cell check
6b3081abc4ac checks: Add check_is_cell() for all phandle+arg properties
2dffc192a77f yamltree: Remove marker ordering dependency
61e513439e40 pylibfdt: Rework "avoid unused variable warning" lines
c8bddd106095 tests: add a positive gpio test case
ad4abfadb687 checks: replace strstr and strrchr with strends
09c6a6e88718 dtc.h: add strends for suffix matching
9bb9b8d0b4a0 checks: tigthen up nr-gpios prop exception
b07b62ee3342 libfdt: Add FDT alignment check to fdt_check_header()
a2def5479950 libfdt: Check that the root-node name is empty
4ca61f84dc21 libfdt: Check that there is only one root node
34d708249a91 dtc: Remove -O dtbo support
8e7ff260f755 libfdt: Fix a possible "unchecked return value" warning
88875268c05c checks: Warn on node-name and property name being the same
9d2279e7e6ee checks: Change node-name check to match devicetree spec
f527c867a8c6 util: limit gnu_printf format attribute to gcc >= 4.4.0
183df9e9c2b9 gitignore: Ignore the swp files
0db6d09584e1 gitignore: Add cscope files
307afa1a7be8 Update Jon Loeliger's email
ca16a723fa9d fdtdump: Fix gcc11 warning
64990a272e8f srcpos: increase MAX_SRCFILE_DEPTH
163f0469bf2e dtc: Allow overlays to have .dtbo extension
3b01518e688d Set last_comp_version correctly in new dtb and fix potential version issues in fdt_open_into
f7e5737f26aa tests: Fix overlay_overlay_nosugar test case
7cd5d5fe43d5 libfdt: Tweak description of assume-aligned load helpers
a7c404099349 libfdt: Internally perform potentially unaligned loads
bab85e48a6f4 meson: increase default timeout for tests
f8b46098824d meson: do not assume python is installed, skip tests
30a56bce4f0b meson: fix -Wall warning
5e735860c478 libfdt: Check for 8-byte address alignment in fdt_ro_probe_()
67849a327927 build-sys: add meson build
05874d08212d pylibfdt: allow build out of tree
3bc3a6b9fe0c dtc: Fix signedness comparisons warnings: Wrap (-1)
e1147b159e92 dtc: Fix signedness comparisons warnings: change types
04cf1fdc0fcf convert-dtsv0: Fix signedness comparisons warning
b30013edb878 libfdt: Fix kernel-doc comments
cbca977ea121 checks: Allow PCI bridge child nodes without an address
73e0f143b73d libfdt: fdt_strerror(): Fix comparison warning
6c2be7d85315 libfdt: fdt_get_string(): Fix sequential write comparison warnings
82525f41d59e libfdt: libfdt_wip: Fix comparison warning
fb1f65f15832 libfdt: fdt_create_with_flags(): Fix comparison warning
f28aa271000b libfdt: fdt_move(): Fix comparison warnings
3d7c6f44195a libfdt: fdt_add_string_(): Fix comparison warning
10f682788c30 libfdt: fdt_node_offset_by_phandle(): Fix comparison warning
07158f4cf2a2 libfdt: overlay: Fix comparison warning
ce9e1f25a7de libfdt: fdt_resize(): Fix comparison warning
faa76fc10bc5 libfdt: fdt_splice_(): Fix comparison warning
54dca0985316 libfdt: fdt_get_string(): Fix comparison warnings
f8e11e61624e libfdt: fdt_grab_space_(): Fix comparison warning
0c43d4d7bf5a libfdt: fdt_mem_rsv(): Fix comparison warnings
442ea3dd1579 libfdt: fdt_offset_ptr(): Fix comparison warnings
ca19c3db2bf6 Makefile: Specify cflags for libyaml
7bb86f1c0956 libfdt: fix fdt_check_node_offset_ w/ VALID_INPUT
3d522abc7571 dtc: Include stdlib.h in util.h
808cdaaf524f dtc: Avoid UB when shifting
3e3138b4a956 libfdt: fix fdt_check_full buffer overrun
9d7888cbf19c dtc: Consider one-character strings as strings
8259d59f59de checks: Improve i2c reg property checking
fdabcf2980a4 checks: Remove warning for I2C_OWN_SLAVE_ADDRESS
2478b1652c8d libfdt: add extern "C" for C++
f68bfc2668b2 libfdt: trivial typo fix
7be250b4d059 libfdt: Correct condition for reordering blocks
81e0919a3e21 checks: Add interrupt provider test

I don't see anything obvious that would preclude from updating
to v1.6.1, but I don't think either David or I will post another
PR anytime soon. Maybe better if this goes through someone else's
tree.

FWIW I could successfully build QEMU using the updated submodule,
so as discussed on IRC :

Acked-by: Greg Kurz <groug@kaod.org>

David will hopefully nack it in time if he's aware of an issue.

Cheers,

--
Greg

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5843 bytes --]

On Thu, Sep 30, 2021 at 01:56:33PM +0200, Greg Kurz wrote:
> On Thu, 30 Sep 2021 09:10:12 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 27/08/2021 14.09, Thomas Huth wrote:
> > > The dtc submodule is currently pointing to non-release commit. It's nicer
> > > if submodules point to release versions instead and since dtc 1.6.1 is
> > > available now, let's update to that version.
> > > 
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> 
> > >   dtc | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/dtc b/dtc
> > > index 85e5d83984..b6910bec11 160000
> > > --- a/dtc
> > > +++ b/dtc
> > > @@ -1 +1 @@
> > > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> > > +Subproject commit b6910bec11614980a21e46fbccc35934b671bd81
> > 
> > Ping!
> > 
> > David, could you please pick up this patch if you don't mind it?
> > 
> >   Thanks,
> >    Thomas
> > 
> 
> Hi Thomas !
> 
> greg@bahia:[main]dtc$ git log --oneline  85e5d839847af54efab170f2b1331b2a6421e647..v1.6.1
> b6910bec1161 Bump version to v1.6.1
> 21d61d18f968 Fix CID 1461557
> 4c2ef8f4d14c checks: Introduce is_multiple_of()
> e59ca36fb70e Make handling of cpp line information more tolerant
> 0c3fd9b6aceb checks: Drop interrupt_cells_is_cell check
> 6b3081abc4ac checks: Add check_is_cell() for all phandle+arg properties
> 2dffc192a77f yamltree: Remove marker ordering dependency
> 61e513439e40 pylibfdt: Rework "avoid unused variable warning" lines
> c8bddd106095 tests: add a positive gpio test case
> ad4abfadb687 checks: replace strstr and strrchr with strends
> 09c6a6e88718 dtc.h: add strends for suffix matching
> 9bb9b8d0b4a0 checks: tigthen up nr-gpios prop exception
> b07b62ee3342 libfdt: Add FDT alignment check to fdt_check_header()
> a2def5479950 libfdt: Check that the root-node name is empty
> 4ca61f84dc21 libfdt: Check that there is only one root node
> 34d708249a91 dtc: Remove -O dtbo support
> 8e7ff260f755 libfdt: Fix a possible "unchecked return value" warning
> 88875268c05c checks: Warn on node-name and property name being the same
> 9d2279e7e6ee checks: Change node-name check to match devicetree spec
> f527c867a8c6 util: limit gnu_printf format attribute to gcc >= 4.4.0
> 183df9e9c2b9 gitignore: Ignore the swp files
> 0db6d09584e1 gitignore: Add cscope files
> 307afa1a7be8 Update Jon Loeliger's email
> ca16a723fa9d fdtdump: Fix gcc11 warning
> 64990a272e8f srcpos: increase MAX_SRCFILE_DEPTH
> 163f0469bf2e dtc: Allow overlays to have .dtbo extension
> 3b01518e688d Set last_comp_version correctly in new dtb and fix potential version issues in fdt_open_into
> f7e5737f26aa tests: Fix overlay_overlay_nosugar test case
> 7cd5d5fe43d5 libfdt: Tweak description of assume-aligned load helpers
> a7c404099349 libfdt: Internally perform potentially unaligned loads
> bab85e48a6f4 meson: increase default timeout for tests
> f8b46098824d meson: do not assume python is installed, skip tests
> 30a56bce4f0b meson: fix -Wall warning
> 5e735860c478 libfdt: Check for 8-byte address alignment in fdt_ro_probe_()
> 67849a327927 build-sys: add meson build
> 05874d08212d pylibfdt: allow build out of tree
> 3bc3a6b9fe0c dtc: Fix signedness comparisons warnings: Wrap (-1)
> e1147b159e92 dtc: Fix signedness comparisons warnings: change types
> 04cf1fdc0fcf convert-dtsv0: Fix signedness comparisons warning
> b30013edb878 libfdt: Fix kernel-doc comments
> cbca977ea121 checks: Allow PCI bridge child nodes without an address
> 73e0f143b73d libfdt: fdt_strerror(): Fix comparison warning
> 6c2be7d85315 libfdt: fdt_get_string(): Fix sequential write comparison warnings
> 82525f41d59e libfdt: libfdt_wip: Fix comparison warning
> fb1f65f15832 libfdt: fdt_create_with_flags(): Fix comparison warning
> f28aa271000b libfdt: fdt_move(): Fix comparison warnings
> 3d7c6f44195a libfdt: fdt_add_string_(): Fix comparison warning
> 10f682788c30 libfdt: fdt_node_offset_by_phandle(): Fix comparison warning
> 07158f4cf2a2 libfdt: overlay: Fix comparison warning
> ce9e1f25a7de libfdt: fdt_resize(): Fix comparison warning
> faa76fc10bc5 libfdt: fdt_splice_(): Fix comparison warning
> 54dca0985316 libfdt: fdt_get_string(): Fix comparison warnings
> f8e11e61624e libfdt: fdt_grab_space_(): Fix comparison warning
> 0c43d4d7bf5a libfdt: fdt_mem_rsv(): Fix comparison warnings
> 442ea3dd1579 libfdt: fdt_offset_ptr(): Fix comparison warnings
> ca19c3db2bf6 Makefile: Specify cflags for libyaml
> 7bb86f1c0956 libfdt: fix fdt_check_node_offset_ w/ VALID_INPUT
> 3d522abc7571 dtc: Include stdlib.h in util.h
> 808cdaaf524f dtc: Avoid UB when shifting
> 3e3138b4a956 libfdt: fix fdt_check_full buffer overrun
> 9d7888cbf19c dtc: Consider one-character strings as strings
> 8259d59f59de checks: Improve i2c reg property checking
> fdabcf2980a4 checks: Remove warning for I2C_OWN_SLAVE_ADDRESS
> 2478b1652c8d libfdt: add extern "C" for C++
> f68bfc2668b2 libfdt: trivial typo fix
> 7be250b4d059 libfdt: Correct condition for reordering blocks
> 81e0919a3e21 checks: Add interrupt provider test
> 
> I don't see anything obvious that would preclude from updating
> to v1.6.1, but I don't think either David or I will post another
> PR anytime soon. Maybe better if this goes through someone else's
> tree.
> 
> FWIW I could successfully build QEMU using the updated submodule,
> so as discussed on IRC :
> 
> Acked-by: Greg Kurz <groug@kaod.org>
> 
> David will hopefully nack it in time if he's aware of an issue.

No, it should be fine.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
> On 27/08/2021 14.09, Thomas Huth wrote:
> > The dtc submodule is currently pointing to non-release commit. It's nicer
> > if submodules point to release versions instead and since dtc 1.6.1 is
> > available now, let's update to that version.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   dtc | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/dtc b/dtc
> > index 85e5d83984..b6910bec11 160000
> > --- a/dtc
> > +++ b/dtc
> > @@ -1 +1 @@
> > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> > +Subproject commit b6910bec11614980a21e46fbccc35934b671bd81
> 
> Ping!
> 
> David, could you please pick up this patch if you don't mind it?

Uhh... I'm dtc upstream maintainer, but I haven't typically been
handling the dtc submodule in qemu.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Daniel P. Berrangé]

On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
> On 27/08/2021 14.09, Thomas Huth wrote:
> > The dtc submodule is currently pointing to non-release commit. It's nicer
> > if submodules point to release versions instead and since dtc 1.6.1 is
> > available now, let's update to that version.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   dtc | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/dtc b/dtc
> > index 85e5d83984..b6910bec11 160000
> > --- a/dtc
> > +++ b/dtc
> > @@ -1 +1 @@
> > -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> > +Subproject commit b6910bec11614980a21e46fbccc35934b671bd81
> 
> Ping!
> 
> David, could you please pick up this patch if you don't mind it?

Most of our supported platforms don't have version 1.6.1 available.

As a general goal IMHO we should be seeking to eliminate bundling of
3rd party modules that are commonly available in distros. We've
carried dtc for a hell of a long time, and if we keep updating our
submodule we'll keep relyin on new features, and never be able to
drop it because it will always be newer than what's in the distros.

So personally I think we should never again update dtc and capstone
modules. If we want to take adbantage of new features, then do that
through conditional compilation, as we do for any of the other 3rd
party libraries consumed.

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 3/3] dtc: Update to version 1.6.1

[Peter Maydell]

On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
> > On 27/08/2021 14.09, Thomas Huth wrote:
> > > The dtc submodule is currently pointing to non-release commit. It's nicer
> > > if submodules point to release versions instead and since dtc 1.6.1 is
> > > available now, let's update to that version.

> Most of our supported platforms don't have version 1.6.1 available.
>
> As a general goal IMHO we should be seeking to eliminate bundling of
> 3rd party modules that are commonly available in distros. We've
> carried dtc for a hell of a long time, and if we keep updating our
> submodule we'll keep relyin on new features, and never be able to
> drop it because it will always be newer than what's in the distros.
>
> So personally I think we should never again update dtc and capstone
> modules. If we want to take adbantage of new features, then do that
> through conditional compilation, as we do for any of the other 3rd
> party libraries consumed.

I agree in general, but (per the commit message here) our dtc
submodule is currently pointing at some random not-a-release
commit in upstream dtc. We should at least move forward to
whatever the next released dtc after that is, before we say
"no more dtc updates".

-- PMM

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Daniel P. Berrangé]

On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
> On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
> > > On 27/08/2021 14.09, Thomas Huth wrote:
> > > > The dtc submodule is currently pointing to non-release commit. It's nicer
> > > > if submodules point to release versions instead and since dtc 1.6.1 is
> > > > available now, let's update to that version.
> 
> > Most of our supported platforms don't have version 1.6.1 available.
> >
> > As a general goal IMHO we should be seeking to eliminate bundling of
> > 3rd party modules that are commonly available in distros. We've
> > carried dtc for a hell of a long time, and if we keep updating our
> > submodule we'll keep relyin on new features, and never be able to
> > drop it because it will always be newer than what's in the distros.
> >
> > So personally I think we should never again update dtc and capstone
> > modules. If we want to take adbantage of new features, then do that
> > through conditional compilation, as we do for any of the other 3rd
> > party libraries consumed.
> 
> I agree in general, but (per the commit message here) our dtc
> submodule is currently pointing at some random not-a-release
> commit in upstream dtc. We should at least move forward to
> whatever the next released dtc after that is, before we say
> "no more dtc updates".

Yep, if we want to fix it onto an official version tag, that's
OK, just not jumping right to very latest version. We might want
to move it backwards to better align with what we're targetting
in the support

Best I can tell the distros currently have these versions:

     - Alpine 3.14 - 1.6.1
     - CentOS 8 - 1.6.0
     - Debian 10 - 1.4.7
     - Fedora 33 - 1.6.0
     - OpenSUSE Leap 15.3 - 1.5.1
     - Ubuntu 18.04 - 1.4.5
     - FreeBSD Ports - 1.6.0
     - OpenBSD Ports - 1.6.0
     - macOS HomeBrew - 1.6.1
     - Windows MSys2 - 1.6.0


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 3/3] dtc: Update to version 1.6.1

[Peter Maydell]

On Fri, 1 Oct 2021 at 10:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
> > I agree in general, but (per the commit message here) our dtc
> > submodule is currently pointing at some random not-a-release
> > commit in upstream dtc. We should at least move forward to
> > whatever the next released dtc after that is, before we say
> > "no more dtc updates".
>
> Yep, if we want to fix it onto an official version tag, that's
> OK, just not jumping right to very latest version. We might want
> to move it backwards to better align with what we're targetting
> in the support

The reason for the last update to the dtc submodule was not
for any specific external API requirement, but in order to
get a change we needed to get it to build more cleanly when
building it as a submodule (QEMU commit 67953a379e). To
not regress that, we need upstream dtc commit 85e5d839847a.
The next published dtc release after that commit is v1.6.1,
which is what this submodule update patch proposes moving to.

-- PMM

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Peter Maydell]

On Fri, 1 Oct 2021 at 10:51, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 1 Oct 2021 at 10:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
> > > I agree in general, but (per the commit message here) our dtc
> > > submodule is currently pointing at some random not-a-release
> > > commit in upstream dtc. We should at least move forward to
> > > whatever the next released dtc after that is, before we say
> > > "no more dtc updates".
> >
> > Yep, if we want to fix it onto an official version tag, that's
> > OK, just not jumping right to very latest version. We might want
> > to move it backwards to better align with what we're targetting
> > in the support
>
> The reason for the last update to the dtc submodule was not
> for any specific external API requirement, but in order to
> get a change we needed to get it to build more cleanly when
> building it as a submodule (QEMU commit 67953a379e). To
> not regress that, we need upstream dtc commit 85e5d839847a.

...having said that, looking at meson.build it looks like it
now ignores dtc's own Makefile and hardcodes how to build
libfdt, including a list of the source files. This seems
pretty fragile to me, but perhaps it means we don't care
any more about whether dtc's Makefile has the commit
85e5d839847a fix...

-- PMM

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Thomas Huth]

On 01/10/2021 11.44, Daniel P. Berrangé wrote:
> On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
>> On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>
>>> On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
>>>> On 27/08/2021 14.09, Thomas Huth wrote:
>>>>> The dtc submodule is currently pointing to non-release commit. It's nicer
>>>>> if submodules point to release versions instead and since dtc 1.6.1 is
>>>>> available now, let's update to that version.
>>
>>> Most of our supported platforms don't have version 1.6.1 available.
>>>
>>> As a general goal IMHO we should be seeking to eliminate bundling of
>>> 3rd party modules that are commonly available in distros. We've
>>> carried dtc for a hell of a long time, and if we keep updating our
>>> submodule we'll keep relyin on new features, and never be able to
>>> drop it because it will always be newer than what's in the distros.
>>>
>>> So personally I think we should never again update dtc and capstone
>>> modules. If we want to take adbantage of new features, then do that
>>> through conditional compilation, as we do for any of the other 3rd
>>> party libraries consumed.

I basically agree, especially for capstone. But for dtc, that also means 
that we cannot compile certain target boards if its not available ... that's 
somewhat more ugly than if there is just a missing backend feature ... but I 
guess it's still ok. Users could always install a recent libfdt first.

>> I agree in general, but (per the commit message here) our dtc
>> submodule is currently pointing at some random not-a-release
>> commit in upstream dtc. We should at least move forward to
>> whatever the next released dtc after that is, before we say
>> "no more dtc updates".
> 
> Yep, if we want to fix it onto an official version tag, that's
> OK, just not jumping right to very latest version.

That was the intention here. Accidentally, the first release tag after the 
commit that we are currently using, is version 1.6.1, which also happens to 
be the latest version, too.

> We might want
> to move it backwards to better align with what we're targetting
> in the support

We shouldn't use an older versions as submodule since there was a problem in 
libfdt on Sparc hosts on older versions. All other hosts should be fine with 
version 1.5.1 though (which is what we're indirectly checking in meson.build)

> Best I can tell the distros currently have these versions:
> 
>       - Alpine 3.14 - 1.6.1
>       - CentOS 8 - 1.6.0
>       - Debian 10 - 1.4.7
>       - Fedora 33 - 1.6.0
>       - OpenSUSE Leap 15.3 - 1.5.1
>       - Ubuntu 18.04 - 1.4.5
>       - FreeBSD Ports - 1.6.0
>       - OpenBSD Ports - 1.6.0
>       - macOS HomeBrew - 1.6.1
>       - Windows MSys2 - 1.6.0

Thanks! I was just about to collect the same information, too.

So I'd suggest: Update the submodule to v1.6.1 now, so that it points to a 
proper release tag.

Then wait until Debian 10 and Ubuntu 18.04 are EOL (sometime in 2022), then 
we can finally get rid of the dtc submodule an rely on the system fdt 
instead (assuming that versions that are older than 1.6.1 will be fixed by 
the distros on Sparc hosts).

  Thomas

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3019 bytes --]

On Fri, Oct 01, 2021 at 01:41:04PM +0200, Thomas Huth wrote:
> On 01/10/2021 11.44, Daniel P. Berrangé wrote:
> > On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
> > > On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > > 
> > > > On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
> > > > > On 27/08/2021 14.09, Thomas Huth wrote:
> > > > > > The dtc submodule is currently pointing to non-release commit. It's nicer
> > > > > > if submodules point to release versions instead and since dtc 1.6.1 is
> > > > > > available now, let's update to that version.
> > > 
> > > > Most of our supported platforms don't have version 1.6.1 available.
> > > > 
> > > > As a general goal IMHO we should be seeking to eliminate bundling of
> > > > 3rd party modules that are commonly available in distros. We've
> > > > carried dtc for a hell of a long time, and if we keep updating our
> > > > submodule we'll keep relyin on new features, and never be able to
> > > > drop it because it will always be newer than what's in the distros.
> > > > 
> > > > So personally I think we should never again update dtc and capstone
> > > > modules. If we want to take adbantage of new features, then do that
> > > > through conditional compilation, as we do for any of the other 3rd
> > > > party libraries consumed.
> 
> I basically agree, especially for capstone. But for dtc, that also means
> that we cannot compile certain target boards if its not available ... that's
> somewhat more ugly than if there is just a missing backend feature ... but I
> guess it's still ok. Users could always install a recent libfdt first.
> 
> > > I agree in general, but (per the commit message here) our dtc
> > > submodule is currently pointing at some random not-a-release
> > > commit in upstream dtc. We should at least move forward to
> > > whatever the next released dtc after that is, before we say
> > > "no more dtc updates".
> > 
> > Yep, if we want to fix it onto an official version tag, that's
> > OK, just not jumping right to very latest version.
> 
> That was the intention here. Accidentally, the first release tag after the
> commit that we are currently using, is version 1.6.1, which also happens to
> be the latest version, too.

Note that while I think this is a good idea, there's no real stability
difference between official releases and any random git commit.  I
tend to make releases when somebody complains that there's a new
feature or fix they want that isn't yet in a numbered release.  They
don't get any additional testing beyond the build-in make check which
I also run on every commit.. and which is generally fine, because the
coverage is pretty good (a rather contrained problem space makes that
relatively easy).

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Brad Smith]

On 10/1/2021 5:44 AM, Daniel P. Berrangé wrote:

> On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
>> On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>> On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
>>>> On 27/08/2021 14.09, Thomas Huth wrote:
>>>>> The dtc submodule is currently pointing to non-release commit. It's nicer
>>>>> if submodules point to release versions instead and since dtc 1.6.1 is
>>>>> available now, let's update to that version.
>>> Most of our supported platforms don't have version 1.6.1 available.
>>>
>>> As a general goal IMHO we should be seeking to eliminate bundling of
>>> 3rd party modules that are commonly available in distros. We've
>>> carried dtc for a hell of a long time, and if we keep updating our
>>> submodule we'll keep relyin on new features, and never be able to
>>> drop it because it will always be newer than what's in the distros.
>>>
>>> So personally I think we should never again update dtc and capstone
>>> modules. If we want to take adbantage of new features, then do that
>>> through conditional compilation, as we do for any of the other 3rd
>>> party libraries consumed.
>> I agree in general, but (per the commit message here) our dtc
>> submodule is currently pointing at some random not-a-release
>> commit in upstream dtc. We should at least move forward to
>> whatever the next released dtc after that is, before we say
>> "no more dtc updates".
> Yep, if we want to fix it onto an official version tag, that's
> OK, just not jumping right to very latest version. We might want
> to move it backwards to better align with what we're targetting
> in the support
>
> Best I can tell the distros currently have these versions:
>
>       - Alpine 3.14 - 1.6.1
>       - CentOS 8 - 1.6.0
>       - Debian 10 - 1.4.7
>       - Fedora 33 - 1.6.0
>       - OpenSUSE Leap 15.3 - 1.5.1
>       - Ubuntu 18.04 - 1.4.5
>       - FreeBSD Ports - 1.6.0
>       - OpenBSD Ports - 1.6.0
I already updated OpenBSD to 1.6.1.
>       - macOS HomeBrew - 1.6.1
>       - Windows MSys2 - 1.6.0
>
>
> Regards,
> Daniel

Re: [PATCH 3/3] dtc: Update to version 1.6.1

[Brad Smith]

On 10/1/2021 1:54 PM, Brad Smith wrote:

> On 10/1/2021 5:44 AM, Daniel P. Berrangé wrote:
>
>> On Fri, Oct 01, 2021 at 10:37:51AM +0100, Peter Maydell wrote:
>>> On Fri, 1 Oct 2021 at 10:10, Daniel P. Berrangé 
>>> <berrange@redhat.com> wrote:
>>>> On Thu, Sep 30, 2021 at 09:10:12AM +0200, Thomas Huth wrote:
>>>>> On 27/08/2021 14.09, Thomas Huth wrote:
>>>>>> The dtc submodule is currently pointing to non-release commit. 
>>>>>> It's nicer
>>>>>> if submodules point to release versions instead and since dtc 
>>>>>> 1.6.1 is
>>>>>> available now, let's update to that version.
>>>> Most of our supported platforms don't have version 1.6.1 available.
>>>>
>>>> As a general goal IMHO we should be seeking to eliminate bundling of
>>>> 3rd party modules that are commonly available in distros. We've
>>>> carried dtc for a hell of a long time, and if we keep updating our
>>>> submodule we'll keep relyin on new features, and never be able to
>>>> drop it because it will always be newer than what's in the distros.
>>>>
>>>> So personally I think we should never again update dtc and capstone
>>>> modules. If we want to take adbantage of new features, then do that
>>>> through conditional compilation, as we do for any of the other 3rd
>>>> party libraries consumed.
>>> I agree in general, but (per the commit message here) our dtc
>>> submodule is currently pointing at some random not-a-release
>>> commit in upstream dtc. We should at least move forward to
>>> whatever the next released dtc after that is, before we say
>>> "no more dtc updates".
>> Yep, if we want to fix it onto an official version tag, that's
>> OK, just not jumping right to very latest version. We might want
>> to move it backwards to better align with what we're targetting
>> in the support
>>
>> Best I can tell the distros currently have these versions:
>>
>>       - Alpine 3.14 - 1.6.1
>>       - CentOS 8 - 1.6.0
>>       - Debian 10 - 1.4.7
>>       - Fedora 33 - 1.6.0
>>       - OpenSUSE Leap 15.3 - 1.5.1
>>       - Ubuntu 18.04 - 1.4.5
>>       - FreeBSD Ports - 1.6.0
>>       - OpenBSD Ports - 1.6.0
> I already updated OpenBSD to 1.6.1.
and NetBSD is at 1.4.7.
>>       - macOS HomeBrew - 1.6.1
>>       - Windows MSys2 - 1.6.0
>>
>> Regards,
>> Daniel

Re: [PATCH 0/3] dtc: Fixes for the fdt check and update submodule to 1.6.1

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 717 bytes --]

On Fri, Aug 27, 2021 at 4:13 PM Thomas Huth <thuth@redhat.com> wrote:

> There are some issues in the checks for libfdt in meson.build which
> get fixed with the first two patches.
>
> And while we're at it, also update the dtc submodule to a proper release
> version (in the third patch).
>
> Thomas Huth (3):
>   meson.build: Fix the check for a usable libfdt
>   meson.build: Don't use internal libfdt if the user requested the
>     system libfdt
>   dtc: Update to version 1.6.1
>
>  dtc         | 2 +-
>  meson.build | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> --
> 2.27.0
>
>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1225 bytes --]

[PATCH 4/3] gitlab-ci: Don't try to use the system libfdt in the debian job

[Thomas Huth]

libfdt in Debian is too old to be usable for QEMU. So far we were
silently falling back to the internal dtc submodule, but since
this is wrong, let's remove the --enable-fdt=system switch here now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Sorry, I just noticed this after sending out the first three patches already

 .gitlab-ci.d/buildtest.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 903ee65f32..f0eb5f6286 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -74,7 +74,6 @@ build-system-debian:
     job: amd64-debian-container
   variables:
     IMAGE: debian-amd64
-    CONFIGURE_ARGS: --enable-fdt=system
     TARGETS: arm-softmmu avr-softmmu i386-softmmu mipsel-softmmu
       riscv64-softmmu sh4eb-softmmu sparc-softmmu xtensaeb-softmmu
     MAKE_CHECK_ARGS: check-build
-- 
2.27.0

[PATCH 4/3] gitlab-ci: Don't try to use the system libfdt in the debian job

[Thomas Huth]

libfdt in Debian is too old to be usable for QEMU. So far we were
silently falling back to the internal dtc submodule, but since
this is wrong, let's remove the --enable-fdt=system switch here now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Sorry, I just noticed this after sending out the first three patches already

 .gitlab-ci.d/buildtest.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 903ee65f32..f0eb5f6286 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -74,7 +74,6 @@ build-system-debian:
     job: amd64-debian-container
   variables:
     IMAGE: debian-amd64
-    CONFIGURE_ARGS: --enable-fdt=system
     TARGETS: arm-softmmu avr-softmmu i386-softmmu mipsel-softmmu
       riscv64-softmmu sh4eb-softmmu sparc-softmmu xtensaeb-softmmu
     MAKE_CHECK_ARGS: check-build
-- 
2.27.0

Re: [PATCH 4/3] gitlab-ci: Don't try to use the system libfdt in the debian job

[Daniel P. Berrangé]

On Fri, Aug 27, 2021 at 05:17:18PM +0200, Thomas Huth wrote:
> libfdt in Debian is too old to be usable for QEMU. So far we were
> silently falling back to the internal dtc submodule, but since
> this is wrong, let's remove the --enable-fdt=system switch here now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  Sorry, I just noticed this after sending out the first three patches already
> 
>  .gitlab-ci.d/buildtest.yml | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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 4/3] gitlab-ci: Don't try to use the system libfdt in the debian job

[Philippe Mathieu-Daudé]

On 8/27/21 5:17 PM, Thomas Huth wrote:
> libfdt in Debian is too old to be usable for QEMU. So far we were
> silently falling back to the internal dtc submodule, but since
> this is wrong, let's remove the --enable-fdt=system switch here now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  Sorry, I just noticed this after sending out the first three patches already
> 
>  .gitlab-ci.d/buildtest.yml | 1 -
>  1 file changed, 1 deletion(-)

Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>