RFC: Why dont we move to newer capstone?

RFC: Why dont we move to newer capstone?

[Lucien Murray-Pitts]

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

Hi folks,

Whilst working on a m68k patch I noticed that the capstone in use today
(3.0) doesnt support the M68K and thus a hand turned disasm function is
used.

The newer capstone (5.0) appears to support a few more CPU, inc. m68k.

Why we move to this newer capstone?

Furthermore, if making a move why not move to something with wider cpu
support like libopcodes  ?

Cheers,
Luc

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

RFC: Why dont we move to newer capstone?

[Lucien Murray-Pitts]

Hi folks,

Whilst working on a m68k patch I noticed that the capstone in use
today (3.0) doesnt support the M68K and thus a hand turned disasm
function is used.

The newer capstone (5.0) appears to support a few more CPU, inc. m68k.

Why we move to this newer capstone?

Furthermore, if making a move why not move to something with wider cpu
support like libopcodes  ?

Cheers,
Luc

Re: RFC: Why dont we move to newer capstone?

[Peter Maydell]

On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
<lucienmp.qemu@gmail.com> wrote:
> Whilst working on a m68k patch I noticed that the capstone in use
> today (3.0) doesnt support the M68K and thus a hand turned disasm
> function is used.
>
> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
>
> Why we move to this newer capstone?

Moving to a newer capstone sounds like a good idea. The only
reason we haven't moved forward as far as I'm aware is that
nobody has done the work to send a patch to do that move
forward to the newer version. Richard Henderson would
probably know if there was any other blocker.

> Furthermore, if making a move why not move to something with wider cpu
> support like libopcodes  ?

Unfortunately as far as I know libopcodes is GPLv3, which is not
compatible with the GPLv2-only code in QEMU. Otherwise it
would be an obvious choice.

thanks
-- PMM

Re: RFC: Why dont we move to newer capstone?

[Richard Henderson]

On 10/5/19 3:11 AM, Lucien Murray-Pitts wrote:
> Hi folks,
> 
> Whilst working on a m68k patch I noticed that the capstone in use today (3.0)
> doesnt support the M68K and thus a hand turned disasm function is used.  
> 
> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.

I don't see a capstone v5.  The latest I see is v4.1.

> Why we move to this newer capstone?

I had some patches for that floating about in May, but
it missed the merge window for qemu 4.1.

Thanks for reminding me about them.


r~

Re: RFC: Why dont we move to newer capstone?

[Daniel P. Berrangé]

On Sat, Oct 05, 2019 at 02:33:34PM +0100, Peter Maydell wrote:
> On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
> <lucienmp.qemu@gmail.com> wrote:
> > Whilst working on a m68k patch I noticed that the capstone in use
> > today (3.0) doesnt support the M68K and thus a hand turned disasm
> > function is used.
> >
> > The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
> >
> > Why we move to this newer capstone?
> 
> Moving to a newer capstone sounds like a good idea. The only
> reason we haven't moved forward as far as I'm aware is that
> nobody has done the work to send a patch to do that move
> forward to the newer version. Richard Henderson would
> probably know if there was any other blocker.

Bearing in mind our distro support policy, we need to continue to
support 3.0 series of capstone for a while yet based on what I
see in various distros. eg Ubuntu 18.04 LTS has 3.0.4, as does
Fedora 29.  Version 4.0 is only in a few very new distros:

   https://repology.org/project/capstone/versions

We can of course use features from newer capstone, *provided* we correctly
do conditional compilation so that we can still build against 3.0 series
on distros that have that version.

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: RFC: Why dont we move to newer capstone?

[Thomas Huth]

On 15/10/2019 10.27, Daniel P. Berrangé wrote:
> On Sat, Oct 05, 2019 at 02:33:34PM +0100, Peter Maydell wrote:
>> On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
>> <lucienmp.qemu@gmail.com> wrote:
>>> Whilst working on a m68k patch I noticed that the capstone in use
>>> today (3.0) doesnt support the M68K and thus a hand turned disasm
>>> function is used.
>>>
>>> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
>>>
>>> Why we move to this newer capstone?
>>
>> Moving to a newer capstone sounds like a good idea. The only
>> reason we haven't moved forward as far as I'm aware is that
>> nobody has done the work to send a patch to do that move
>> forward to the newer version. Richard Henderson would
>> probably know if there was any other blocker.
> 
> Bearing in mind our distro support policy, we need to continue to
> support 3.0 series of capstone for a while yet based on what I
> see in various distros. eg Ubuntu 18.04 LTS has 3.0.4, as does
> Fedora 29.  Version 4.0 is only in a few very new distros:
> 
>    https://repology.org/project/capstone/versions
> 
> We can of course use features from newer capstone, *provided* we correctly
> do conditional compilation so that we can still build against 3.0 series
> on distros that have that version.

We're embedding the capstone submodule in the release tarballs, so I
think we're independent from the distro release, aren't we? So this
should not be an issue, as far as I can see.

 Thomas

Re: RFC: Why dont we move to newer capstone?

[Daniel P. Berrangé]

On Tue, Oct 15, 2019 at 10:36:40AM +0200, Thomas Huth wrote:
> On 15/10/2019 10.27, Daniel P. Berrangé wrote:
> > On Sat, Oct 05, 2019 at 02:33:34PM +0100, Peter Maydell wrote:
> >> On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
> >> <lucienmp.qemu@gmail.com> wrote:
> >>> Whilst working on a m68k patch I noticed that the capstone in use
> >>> today (3.0) doesnt support the M68K and thus a hand turned disasm
> >>> function is used.
> >>>
> >>> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
> >>>
> >>> Why we move to this newer capstone?
> >>
> >> Moving to a newer capstone sounds like a good idea. The only
> >> reason we haven't moved forward as far as I'm aware is that
> >> nobody has done the work to send a patch to do that move
> >> forward to the newer version. Richard Henderson would
> >> probably know if there was any other blocker.
> > 
> > Bearing in mind our distro support policy, we need to continue to
> > support 3.0 series of capstone for a while yet based on what I
> > see in various distros. eg Ubuntu 18.04 LTS has 3.0.4, as does
> > Fedora 29.  Version 4.0 is only in a few very new distros:
> > 
> >    https://repology.org/project/capstone/versions
> > 
> > We can of course use features from newer capstone, *provided* we correctly
> > do conditional compilation so that we can still build against 3.0 series
> > on distros that have that version.
> 
> We're embedding the capstone submodule in the release tarballs, so I
> think we're independent from the distro release, aren't we? So this
> should not be an issue, as far as I can see.

It is an issue for people/distros who don't want to building with bundled
3rd party code.

I'd suggest it is probably time we could drop the capstone git submodule.
We originally added it because capstone wasn't widely present in distros
we care about. AFAICT, it is now present in all the distros, so could be
treated the same way as any other 3rd party library dep we have.

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: RFC: Why dont we move to newer capstone?

[Marc-André Lureau]

Hi

On Tue, Oct 15, 2019 at 10:48 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Oct 15, 2019 at 10:36:40AM +0200, Thomas Huth wrote:
> > On 15/10/2019 10.27, Daniel P. Berrangé wrote:
> > > On Sat, Oct 05, 2019 at 02:33:34PM +0100, Peter Maydell wrote:
> > >> On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
> > >> <lucienmp.qemu@gmail.com> wrote:
> > >>> Whilst working on a m68k patch I noticed that the capstone in use
> > >>> today (3.0) doesnt support the M68K and thus a hand turned disasm
> > >>> function is used.
> > >>>
> > >>> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
> > >>>
> > >>> Why we move to this newer capstone?
> > >>
> > >> Moving to a newer capstone sounds like a good idea. The only
> > >> reason we haven't moved forward as far as I'm aware is that
> > >> nobody has done the work to send a patch to do that move
> > >> forward to the newer version. Richard Henderson would
> > >> probably know if there was any other blocker.
> > >
> > > Bearing in mind our distro support policy, we need to continue to
> > > support 3.0 series of capstone for a while yet based on what I
> > > see in various distros. eg Ubuntu 18.04 LTS has 3.0.4, as does
> > > Fedora 29.  Version 4.0 is only in a few very new distros:
> > >
> > >    https://repology.org/project/capstone/versions
> > >
> > > We can of course use features from newer capstone, *provided* we correctly
> > > do conditional compilation so that we can still build against 3.0 series
> > > on distros that have that version.
> >
> > We're embedding the capstone submodule in the release tarballs, so I
> > think we're independent from the distro release, aren't we? So this
> > should not be an issue, as far as I can see.
>
> It is an issue for people/distros who don't want to building with bundled
> 3rd party code.
>
> I'd suggest it is probably time we could drop the capstone git submodule.
> We originally added it because capstone wasn't widely present in distros
> we care about. AFAICT, it is now present in all the distros, so could be
> treated the same way as any other 3rd party library dep we have.

I suppose the same applies to dtc (1.4.2 required by qemu, but xenial
has 1.4.0... so we have to wait until April 26, 2020? 18.04 LTS
release date + 2y).

libslirp will take even longer.

Re: RFC: Why dont we move to newer capstone?

[Daniel P. Berrangé]

On Tue, Oct 15, 2019 at 11:02:43AM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Oct 15, 2019 at 10:48 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Oct 15, 2019 at 10:36:40AM +0200, Thomas Huth wrote:
> > > On 15/10/2019 10.27, Daniel P. Berrangé wrote:
> > > > On Sat, Oct 05, 2019 at 02:33:34PM +0100, Peter Maydell wrote:
> > > >> On Sat, 5 Oct 2019 at 11:21, Lucien Murray-Pitts
> > > >> <lucienmp.qemu@gmail.com> wrote:
> > > >>> Whilst working on a m68k patch I noticed that the capstone in use
> > > >>> today (3.0) doesnt support the M68K and thus a hand turned disasm
> > > >>> function is used.
> > > >>>
> > > >>> The newer capstone (5.0) appears to support a few more CPU, inc. m68k.
> > > >>>
> > > >>> Why we move to this newer capstone?
> > > >>
> > > >> Moving to a newer capstone sounds like a good idea. The only
> > > >> reason we haven't moved forward as far as I'm aware is that
> > > >> nobody has done the work to send a patch to do that move
> > > >> forward to the newer version. Richard Henderson would
> > > >> probably know if there was any other blocker.
> > > >
> > > > Bearing in mind our distro support policy, we need to continue to
> > > > support 3.0 series of capstone for a while yet based on what I
> > > > see in various distros. eg Ubuntu 18.04 LTS has 3.0.4, as does
> > > > Fedora 29.  Version 4.0 is only in a few very new distros:
> > > >
> > > >    https://repology.org/project/capstone/versions
> > > >
> > > > We can of course use features from newer capstone, *provided* we correctly
> > > > do conditional compilation so that we can still build against 3.0 series
> > > > on distros that have that version.
> > >
> > > We're embedding the capstone submodule in the release tarballs, so I
> > > think we're independent from the distro release, aren't we? So this
> > > should not be an issue, as far as I can see.
> >
> > It is an issue for people/distros who don't want to building with bundled
> > 3rd party code.
> >
> > I'd suggest it is probably time we could drop the capstone git submodule.
> > We originally added it because capstone wasn't widely present in distros
> > we care about. AFAICT, it is now present in all the distros, so could be
> > treated the same way as any other 3rd party library dep we have.
> 
> I suppose the same applies to dtc (1.4.2 required by qemu, but xenial
> has 1.4.0... so we have to wait until April 26, 2020? 18.04 LTS
> release date + 2y).

Possibly - depends on scope of changes between 1.4.0 & 1.4.2 - maybe it
is easy to conditionally support 1.4.0 too.

> libslirp will take even longer.

This is reasonable as a git submodule for a while yet, since it only
existed as a separate project very recently, so isn't widely available
across distros / OS.

IMHO the key point is that submodules bundling 3rd party libraries [1]
should be viewed as something with a limited lifetime. A temporary
hack until distros have the library widely available, rather than
something which continues forever.

Regards,
Daniel

[1] We have other types of submodule.

    The keycodemapdb which is not a library, rather a static database
    from which we auto-generate code to statically link in.

    The firmware submodules which developers don't actually build from
    normally. Ideally these would go into a separate dist tarball but
    we seem stalled on this idea despite discussing it many times.
-- 
|: 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: RFC: Why dont we move to newer capstone?

[Peter Maydell]

On Tue, 15 Oct 2019 at 10:14, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Oct 15, 2019 at 11:02:43AM +0200, Marc-André Lureau wrote:
> > I suppose the same applies to dtc (1.4.2 required by qemu, but xenial
> > has 1.4.0... so we have to wait until April 26, 2020? 18.04 LTS
> > release date + 2y).
>
> Possibly - depends on scope of changes between 1.4.0 & 1.4.2 - maybe it
> is easy to conditionally support 1.4.0 too.

We need fdt_first_subnode() and fdt_next_subnode() which only
came in in 1.4.2.

thanks
-- PMM

Re: RFC: Why dont we move to newer capstone?

[Daniel P. Berrangé]

On Tue, Oct 15, 2019 at 10:57:44AM +0100, Peter Maydell wrote:
> On Tue, 15 Oct 2019 at 10:14, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Tue, Oct 15, 2019 at 11:02:43AM +0200, Marc-André Lureau wrote:
> > > I suppose the same applies to dtc (1.4.2 required by qemu, but xenial
> > > has 1.4.0... so we have to wait until April 26, 2020? 18.04 LTS
> > > release date + 2y).
> >
> > Possibly - depends on scope of changes between 1.4.0 & 1.4.2 - maybe it
> > is easy to conditionally support 1.4.0 too.
> 
> We need fdt_first_subnode() and fdt_next_subnode() which only
> came in in 1.4.2.

Looks like those are just shims around fdt_next_node() which existed
in previous releases already, just to make code a little cleaner:

  commit 4e76ec796c90d44d417f82d9db2d67cfe575f8ed
  Author: Simon Glass <sjg@chromium.org>
  Date:   Fri Apr 26 05:43:31 2013 -0700

    libfdt: Add fdt_next_subnode() to permit easy subnode iteration
    
    Iterating through subnodes with libfdt is a little painful to write as we
    need something like this:
    
    for (depth = 0, count = 0,
            offset = fdt_next_node(fdt, parent_offset, &depth);
         (offset >= 0) && (depth > 0);
         offset = fdt_next_node(fdt, offset, &depth)) {
            if (depth == 1) {
                    /* code body */
            }
    }
    
    Using fdt_next_subnode() we can instead write this, which is shorter and
    easier to get right:
    
    for (offset = fdt_first_subnode(fdt, parent_offset);
         offset >= 0;
         offset = fdt_next_subnode(fdt, offset)) {
            /* code body */
    }
    
    Also, it doesn't require two levels of indentation for the loop body.
    

so I think we could indeed do conditional compilation where we provide a
local impl of fdt_first|next_subnode if we see older dtc present.

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