buildsys: Only build capstone if softmmu/user mode is enabled

[RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Philippe Mathieu-Daudé 3 years, 9 months ago

At least one of softmmu or user mode has to be enabled to use
capstone. If not, don't clone/built it.

This save CI time for the tools/documentation-only build jobs.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 configure | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configure b/configure
index 4bd80ed507..bc5757159a 100755
--- a/configure
+++ b/configure
@@ -5381,6 +5381,10 @@ fi
 ##########################################
 # capstone
 
+if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
+  capstone="no"
+fi
+
 case "$capstone" in
   "" | yes)
     if $pkg_config capstone; then
-- 
2.21.3

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Thomas Huth 3 years, 9 months ago

On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
> At least one of softmmu or user mode has to be enabled to use
> capstone. If not, don't clone/built it.
> 
> This save CI time for the tools/documentation-only build jobs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  configure | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configure b/configure
> index 4bd80ed507..bc5757159a 100755
> --- a/configure
> +++ b/configure
> @@ -5381,6 +5381,10 @@ fi
>  ##########################################
>  # capstone
>  
> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
> +  capstone="no"
> +fi

I don't think this is right. You could have a KVM-only build where you
still want to use the disassembler for the human monitor.

But maybe it could be disabled if softmmu="no", linux_user="no" and
bsd_user="no" ?

 Thomas

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Philippe Mathieu-Daudé 3 years, 9 months ago

On 7/24/20 9:56 AM, Thomas Huth wrote:
> On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
>> At least one of softmmu or user mode has to be enabled to use
>> capstone. If not, don't clone/built it.
>>
>> This save CI time for the tools/documentation-only build jobs.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  configure | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 4bd80ed507..bc5757159a 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5381,6 +5381,10 @@ fi
>>  ##########################################
>>  # capstone
>>  
>> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
>> +  capstone="no"
>> +fi
> 
> I don't think this is right. You could have a KVM-only build where you
> still want to use the disassembler for the human monitor.

I had the same question with KVM, I agree this is unclear, this is why
I added RFC.

Don't we have !softmmu implies !kvm?

> 
> But maybe it could be disabled if softmmu="no", linux_user="no" and
> bsd_user="no" ?
> 
>  Thomas
> 
>

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Thomas Huth 3 years, 9 months ago

On 24/07/2020 11.38, Philippe Mathieu-Daudé wrote:
> On 7/24/20 9:56 AM, Thomas Huth wrote:
>> On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
>>> At least one of softmmu or user mode has to be enabled to use
>>> capstone. If not, don't clone/built it.
>>>
>>> This save CI time for the tools/documentation-only build jobs.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  configure | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/configure b/configure
>>> index 4bd80ed507..bc5757159a 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -5381,6 +5381,10 @@ fi
>>>  ##########################################
>>>  # capstone
>>>  
>>> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
>>> +  capstone="no"
>>> +fi
>>
>> I don't think this is right. You could have a KVM-only build where you
>> still want to use the disassembler for the human monitor.
> 
> I had the same question with KVM, I agree this is unclear, this is why
> I added RFC.
> 
> Don't we have !softmmu implies !kvm?

Yes. You can not use KVM with linux-user builds.

But I think it's wrong to test for accelerators here. You could still
build a QEMU binary with HVF or HAX and still want to have disassembler
support in there.

 Thomas

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Philippe Mathieu-Daudé 3 years, 9 months ago

On 7/24/20 11:38 AM, Philippe Mathieu-Daudé wrote:
> On 7/24/20 9:56 AM, Thomas Huth wrote:
>> On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
>>> At least one of softmmu or user mode has to be enabled to use
>>> capstone. If not, don't clone/built it.
>>>
>>> This save CI time for the tools/documentation-only build jobs.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  configure | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/configure b/configure
>>> index 4bd80ed507..bc5757159a 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -5381,6 +5381,10 @@ fi
>>>  ##########################################
>>>  # capstone
>>>  
>>> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
>>> +  capstone="no"
>>> +fi
>>
>> I don't think this is right. You could have a KVM-only build where you
>> still want to use the disassembler for the human monitor.
> 
> I had the same question with KVM, I agree this is unclear, this is why
> I added RFC.
> 
> Don't we have !softmmu implies !kvm?

It works because it falls back to the old disas.c (if capstone is
here, use it, else fall-back).

Does this means we can directly remove the capstone experiment &
submodule without waiting for the libllvm integration?

> 
>>
>> But maybe it could be disabled if softmmu="no", linux_user="no" and
>> bsd_user="no" ?
>>
>>  Thomas
>>
>>
>

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Peter Maydell 3 years, 9 months ago

On Fri, 24 Jul 2020 at 10:47, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 7/24/20 11:38 AM, Philippe Mathieu-Daudé wrote:
> > On 7/24/20 9:56 AM, Thomas Huth wrote:
> >> On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
> >>> At least one of softmmu or user mode has to be enabled to use
> >>> capstone. If not, don't clone/built it.
> >>>
> >>> This save CI time for the tools/documentation-only build jobs.

> >>> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
> >>> +  capstone="no"
> >>> +fi
> >>
> >> I don't think this is right. You could have a KVM-only build where you
> >> still want to use the disassembler for the human monitor.
> >
> > I had the same question with KVM, I agree this is unclear, this is why
> > I added RFC.
> >
> > Don't we have !softmmu implies !kvm?
>
> It works because it falls back to the old disas.c (if capstone is
> here, use it, else fall-back).
>
> Does this means we can directly remove the capstone experiment &
> submodule without waiting for the libllvm integration?

The theory (at least at the time) was that capstone was better
than the internal disassembler for at least some targets.
If we want to go from libllvm to capstone as our long term
plan that's cool, but until we actually do that I don't think
we should drop capstone.

As far as this patch goes: if you want to disable capstone for
the tools-and-docs-only setup then I think the right condition is
if [ "$bsd_user" = "no" -a "$linux_user" = "no" -a "$softmmu" = "no" ] ; then
  capstone=no
fi

thanks
-- PMM

Re: [RFC PATCH] buildsys: Only build capstone if softmmu/user mode is enabled

Posted by Philippe Mathieu-Daudé 3 years, 9 months ago

On 7/24/20 11:59 AM, Peter Maydell wrote:
> On Fri, 24 Jul 2020 at 10:47, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 7/24/20 11:38 AM, Philippe Mathieu-Daudé wrote:
>>> On 7/24/20 9:56 AM, Thomas Huth wrote:
>>>> On 24/07/2020 09.16, Philippe Mathieu-Daudé wrote:
>>>>> At least one of softmmu or user mode has to be enabled to use
>>>>> capstone. If not, don't clone/built it.
>>>>>
>>>>> This save CI time for the tools/documentation-only build jobs.
> 
>>>>> +if test -z "$capstone" && test $tcg = 'no' ; then # !tcg implies !softmmu
>>>>> +  capstone="no"
>>>>> +fi
>>>>
>>>> I don't think this is right. You could have a KVM-only build where you
>>>> still want to use the disassembler for the human monitor.
>>>
>>> I had the same question with KVM, I agree this is unclear, this is why
>>> I added RFC.
>>>
>>> Don't we have !softmmu implies !kvm?
>>
>> It works because it falls back to the old disas.c (if capstone is
>> here, use it, else fall-back).
>>
>> Does this means we can directly remove the capstone experiment &
>> submodule without waiting for the libllvm integration?
> 
> The theory (at least at the time) was that capstone was better
> than the internal disassembler for at least some targets.
> If we want to go from libllvm to capstone as our long term
> plan that's cool, but until we actually do that I don't think
> we should drop capstone.
> 
> As far as this patch goes: if you want to disable capstone for
> the tools-and-docs-only setup

This is also useful for a job downloading CI prerequisite artifacts.

> then I think the right condition is
> if [ "$bsd_user" = "no" -a "$linux_user" = "no" -a "$softmmu" = "no" ] ; then
>   capstone=no
> fi

OK, thanks!

> 
> thanks
> -- PMM
>