[RFC v1 0/2] tcg-cpus: split into 3 tcg variants

Claudio Fontana posted 2 patches 3 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201014073605.6155-1-cfontana@suse.de
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>
There is a newer version of this series
accel/tcg/meson.build       |   9 +-
accel/tcg/tcg-all.c         |  13 +-
accel/tcg/tcg-cpus-icount.c | 145 +++++++++++
accel/tcg/tcg-cpus-icount.h |  20 ++
accel/tcg/tcg-cpus-mttcg.c  | 142 ++++++++++
accel/tcg/tcg-cpus-mttcg.h  |  25 ++
accel/tcg/tcg-cpus-rr.c     | 305 ++++++++++++++++++++++
accel/tcg/tcg-cpus-rr.h     |  26 ++
accel/tcg/tcg-cpus.c        | 500 +-----------------------------------
accel/tcg/tcg-cpus.h        |   9 +-
softmmu/icount.c            |   2 +-
11 files changed, 697 insertions(+), 499 deletions(-)
create mode 100644 accel/tcg/tcg-cpus-icount.c
create mode 100644 accel/tcg/tcg-cpus-icount.h
create mode 100644 accel/tcg/tcg-cpus-mttcg.c
create mode 100644 accel/tcg/tcg-cpus-mttcg.h
create mode 100644 accel/tcg/tcg-cpus-rr.c
create mode 100644 accel/tcg/tcg-cpus-rr.h
[RFC v1 0/2] tcg-cpus: split into 3 tcg variants
Posted by Claudio Fontana 3 years, 7 months ago
The purpose of this series is to split the tcg-cpus into
3 variants:

tcg_cpus_mttcg    (multithreaded tcg vcpus)
tcg_cpus_rr       (single threaded round robin vcpus)
tcg_cpus_icount   (same as RR, but using icount)

Alex, I read the comment in tcg_start_vcpu_thread saying:

    /*
     * Initialize TCG regions--once. Now is a good time, because:
     * (1) TCG's init context, prologue and target globals have been set up.
     * (2) qemu_tcg_mttcg_enabled() works now (TCG init code runs before the
     *     -accel flag is processed, so the check doesn't work then).
     */

Is this actually current?

I tried to refactor this (see patch 2), and it seems to work to do
the init of regions in tcg_init, and it seems that mttcg_enabled is known
already at that point..

Ciao,

Claudio

Claudio Fontana (2):
  accel/tcg: split CpusAccel into three TCG variants
  accel/tcg: split tcg_start_vcpu_thread

 accel/tcg/meson.build       |   9 +-
 accel/tcg/tcg-all.c         |  13 +-
 accel/tcg/tcg-cpus-icount.c | 145 +++++++++++
 accel/tcg/tcg-cpus-icount.h |  20 ++
 accel/tcg/tcg-cpus-mttcg.c  | 142 ++++++++++
 accel/tcg/tcg-cpus-mttcg.h  |  25 ++
 accel/tcg/tcg-cpus-rr.c     | 305 ++++++++++++++++++++++
 accel/tcg/tcg-cpus-rr.h     |  26 ++
 accel/tcg/tcg-cpus.c        | 500 +-----------------------------------
 accel/tcg/tcg-cpus.h        |   9 +-
 softmmu/icount.c            |   2 +-
 11 files changed, 697 insertions(+), 499 deletions(-)
 create mode 100644 accel/tcg/tcg-cpus-icount.c
 create mode 100644 accel/tcg/tcg-cpus-icount.h
 create mode 100644 accel/tcg/tcg-cpus-mttcg.c
 create mode 100644 accel/tcg/tcg-cpus-mttcg.h
 create mode 100644 accel/tcg/tcg-cpus-rr.c
 create mode 100644 accel/tcg/tcg-cpus-rr.h

-- 
2.26.2


Re: [RFC v1 0/2] tcg-cpus: split into 3 tcg variants
Posted by Alex Bennée 3 years, 7 months ago
Claudio Fontana <cfontana@suse.de> writes:

> The purpose of this series is to split the tcg-cpus into
> 3 variants:
>
> tcg_cpus_mttcg    (multithreaded tcg vcpus)
> tcg_cpus_rr       (single threaded round robin vcpus)
> tcg_cpus_icount   (same as RR, but using icount)

I've no objection to the cosmetic clean-up but I assume the 3 modes will
still be available in TCG enabled binaries.

>
> Alex, I read the comment in tcg_start_vcpu_thread saying:
>
>     /*
>      * Initialize TCG regions--once. Now is a good time, because:
>      * (1) TCG's init context, prologue and target globals have been set up.
>      * (2) qemu_tcg_mttcg_enabled() works now (TCG init code runs before the
>      *     -accel flag is processed, so the check doesn't work then).
>      */
>
> Is this actually current?

Hmm probably not. Now everything is tied to the order of class
initialisation and realisation. AIUI all properties set by the command
line should be complete by the time an object realizes and parent
classes should be processed before their children.

>
> I tried to refactor this (see patch 2), and it seems to work to do
> the init of regions in tcg_init, and it seems that mttcg_enabled is known
> already at that point..
>
> Ciao,
>
> Claudio
>
> Claudio Fontana (2):
>   accel/tcg: split CpusAccel into three TCG variants
>   accel/tcg: split tcg_start_vcpu_thread
>
>  accel/tcg/meson.build       |   9 +-
>  accel/tcg/tcg-all.c         |  13 +-
>  accel/tcg/tcg-cpus-icount.c | 145 +++++++++++
>  accel/tcg/tcg-cpus-icount.h |  20 ++
>  accel/tcg/tcg-cpus-mttcg.c  | 142 ++++++++++
>  accel/tcg/tcg-cpus-mttcg.h  |  25 ++
>  accel/tcg/tcg-cpus-rr.c     | 305 ++++++++++++++++++++++
>  accel/tcg/tcg-cpus-rr.h     |  26 ++
>  accel/tcg/tcg-cpus.c        | 500 +-----------------------------------
>  accel/tcg/tcg-cpus.h        |   9 +-
>  softmmu/icount.c            |   2 +-
>  11 files changed, 697 insertions(+), 499 deletions(-)
>  create mode 100644 accel/tcg/tcg-cpus-icount.c
>  create mode 100644 accel/tcg/tcg-cpus-icount.h
>  create mode 100644 accel/tcg/tcg-cpus-mttcg.c
>  create mode 100644 accel/tcg/tcg-cpus-mttcg.h
>  create mode 100644 accel/tcg/tcg-cpus-rr.c
>  create mode 100644 accel/tcg/tcg-cpus-rr.h


-- 
Alex Bennée

Re: [RFC v1 0/2] tcg-cpus: split into 3 tcg variants
Posted by Claudio Fontana 3 years, 7 months ago
On 10/14/20 12:14 PM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> The purpose of this series is to split the tcg-cpus into
>> 3 variants:
>>
>> tcg_cpus_mttcg    (multithreaded tcg vcpus)
>> tcg_cpus_rr       (single threaded round robin vcpus)
>> tcg_cpus_icount   (same as RR, but using icount)
> 
> I've no objection to the cosmetic clean-up but I assume the 3 modes will
> still be available in TCG enabled binaries.

Hi Alex,

yes, all three will be available in the code when enabling TCG.

> 
>>
>> Alex, I read the comment in tcg_start_vcpu_thread saying:
>>
>>     /*
>>      * Initialize TCG regions--once. Now is a good time, because:
>>      * (1) TCG's init context, prologue and target globals have been set up.
>>      * (2) qemu_tcg_mttcg_enabled() works now (TCG init code runs before the
>>      *     -accel flag is processed, so the check doesn't work then).
>>      */
>>
>> Is this actually current?
> 
> Hmm probably not. Now everything is tied to the order of class
> initialisation and realisation. AIUI all properties set by the command
> line should be complete by the time an object realizes and parent
> classes should be processed before their children.

This is great news, as it allows more refactoring as showed on patch 2,

thanks a lot!

Ciao,

Claudio

> 
>>
>> I tried to refactor this (see patch 2), and it seems to work to do
>> the init of regions in tcg_init, and it seems that mttcg_enabled is known
>> already at that point..
>>
>> Ciao,
>>
>> Claudio
>>
>> Claudio Fontana (2):
>>   accel/tcg: split CpusAccel into three TCG variants
>>   accel/tcg: split tcg_start_vcpu_thread
>>
>>  accel/tcg/meson.build       |   9 +-
>>  accel/tcg/tcg-all.c         |  13 +-
>>  accel/tcg/tcg-cpus-icount.c | 145 +++++++++++
>>  accel/tcg/tcg-cpus-icount.h |  20 ++
>>  accel/tcg/tcg-cpus-mttcg.c  | 142 ++++++++++
>>  accel/tcg/tcg-cpus-mttcg.h  |  25 ++
>>  accel/tcg/tcg-cpus-rr.c     | 305 ++++++++++++++++++++++
>>  accel/tcg/tcg-cpus-rr.h     |  26 ++
>>  accel/tcg/tcg-cpus.c        | 500 +-----------------------------------
>>  accel/tcg/tcg-cpus.h        |   9 +-
>>  softmmu/icount.c            |   2 +-
>>  11 files changed, 697 insertions(+), 499 deletions(-)
>>  create mode 100644 accel/tcg/tcg-cpus-icount.c
>>  create mode 100644 accel/tcg/tcg-cpus-icount.h
>>  create mode 100644 accel/tcg/tcg-cpus-mttcg.c
>>  create mode 100644 accel/tcg/tcg-cpus-mttcg.h
>>  create mode 100644 accel/tcg/tcg-cpus-rr.c
>>  create mode 100644 accel/tcg/tcg-cpus-rr.h
> 
> 


Re: [RFC v1 0/2] tcg-cpus: split into 3 tcg variants
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
On 10/14/20 12:14 PM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> The purpose of this series is to split the tcg-cpus into
>> 3 variants:
>>
>> tcg_cpus_mttcg    (multithreaded tcg vcpus)
>> tcg_cpus_rr       (single threaded round robin vcpus)
>> tcg_cpus_icount   (same as RR, but using icount)
> 
> I've no objection to the cosmetic clean-up but I assume the 3 modes will
> still be available in TCG enabled binaries.

Yes, I think so too, no point in disabling some.
However it seems to me it is now easier to review
for a newcomer.

Code easily understandable/reviewable is easier to
maintain and less bug prone :)

Phil.

Re: [RFC v1 0/2] tcg-cpus: split into 3 tcg variants
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
On 10/14/20 9:36 AM, Claudio Fontana wrote:
> The purpose of this series is to split the tcg-cpus into
> 3 variants:
> 
> tcg_cpus_mttcg    (multithreaded tcg vcpus)
> tcg_cpus_rr       (single threaded round robin vcpus)
> tcg_cpus_icount   (same as RR, but using icount)

Good idea!