v25 -> v26:
* i386: separate fpu_helper into user and sysemu parts
 - removed the splitting of the user mode into their own user/fpu_helper.c,
   seems not worth it.

v24 -> v25:

* i386: separate fpu_helper into user and sysemu parts

 - added 2 preliminary patches to the series (from Richard)
 - rebased on those

* i386: move TCG btp_helper into sysemu/

 - fixed title typo (Alex)
 - nested #ifdef more easily (Richard)

v23 -> v24:

* i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu                                                                                     
 - remove additional #ifdef TARGET_X86_64
 - split in two patches, so it is easier to understand.

v22 -> v23:

* i386: move TCG btp_helper into sysemu/
 - extended the #ifndef CONFIG_USER_ONLY to entire else of
   if (cpl != 0).

* i386: split misc helper into user and sysemu parts
 - added g_assert_not_reached() and changed user file name to -stubs.

* i386: separate fpu_helper into user and sysemu parts
 - removed unused return value
 - added comment abut issues with current cpu_x86_fsave.

* i386: split off sysemu part of cpu.c
 - rename cpu-softmmu.c to cpu-sysemu.c
 - fixed two mispelled comments, and add two comments
   in the headers of cpu.c and cpu-sysemu.c to describe them

* i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu
 - defined some aux functions to reduce repeated code

* i386: make cpu_load_efer sysemu-only
 - move the function to helper.c, remove "inline"

v21 -> v22: replace "softmmu" with "sysemu"

v20 -> v21:

* meson: add target_user_arch
  - add hexagon

v19 -> v20:

* add new patch to make gdbstub only write certain registers for softmmu.
  In particular, CR0, CR2, CR3 and EFER should not be changed under
  CONFIG_USER_ONLY. (Paolo)

* add new patch to make cpu_load_efer softmmu-only (Paolo)

* i386: split svm_helper into softmmu and stub-only user

  - fixed commit message spelling (Eric)

  - mention in commit message that this reproduces the existing stubs,
    but really everything that requires s->cpl == 0 should be impossible
    to trigger from user-mode, and we could find a way to assert that
    more consistently.

v18 -> v19:

* i386: split smm helper (softmmu)
  - add g_assert_not_reached and cpu_abort for invalid states in
    CONFIG_USER_ONLY (Paolo)

* i386: move TCG btp_helper into softmmu/
  - for CONFIG_USER_ONLY, assert that the hidden IOBPT flags are not set
    while attempting to generate io_bpt helpers.
    Theory to verify (Paolo)

* i386: slit svm_helper into softmmu and stub-only user
  - added XXX in the commit message to highlight the question about
    whether the same check should be done controlling access to
    cpu_load_efer() and state of the hidden SVME flag. (Paolo)

v17 -> v18:

* meson: add target_user_arch

 - add target_user_arch to all targets which build user.
   Otherwise meson complains about missing key for archs without it.
   (Paolo)

* wrap a few gen_helper_ calls around ifndef CONFIG_USER_ONLY.
  This would need a look from someone like Alex or Richard I think,
  as potentially we could remove even more code I think around the
  gen_helper_ calls for CONFIG_USER_ONLY.

  In the current master code, we have empty helpers for user mode,
  but still we generate the preamble code, temporary variables etc,
  just to then call a helper_() function that does nothing.

  In particular I am referring to patches:

  i386: split tcg btp_helper into softmmu and user parts
        DEF_HELPER_FLAGS_3(set_dr, TCG_CALL_NO_WG, void, env, int, tl)
        DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
        gen_bpt_io
        gen_helper_set_dr(cpu_env, s->tmp2_i32, s->T0);

  i386: split smm helper (softmmu)
        DEF_HELPER_1(rsm, void, env)
	gen_helper_rsm(cpu_env);

  (Alex, Richard?)

* removed suffixes from user/ and softmmu/ modules
  (Alex, Philippe).
  Where possible, removed user stubs entirely.
  Renamed the leftover svm_helper stubs to user/svm_stubs.c

* cleaned up lefover unnecessary header files and squashed them.
 

v16 -> v17: changed to RFC

* tcg_ops are already in master, removed from the series

* i386: split cpu accelerators from cpu.c, using AccelCPUClass:
  removed spurious ; and added spacing before/after functions (Richard)

* added new patches as RFC for the next steps, introducing target-specific
  user-mode specific meson variables, and applied to i386/tcg as an
  example, in order to gather feedback.

v15 -> v16:

* cpu: Move synchronize_from_tb() to tcg_ops:
  - adjusted comments (Alex)

* cpu: tcg_ops: move to tcg-cpu-ops.h, keep a pointer in CPUClass:
  - remove forward decl. of AccelCPUClass, should be in a later patch. (Alex)
  - simplified comment about tcg_ops in struct CPUClass (Alex)
  - remove obsolete comment about ARM blocking TCGCPUOps from being const.
    (Alex)

* accel: replace struct CpusAccel with AccelOpsClass:
  - reworded commit message to be clearer about the objective (Alex)

* accel: introduce AccelCPUClass extending CPUClass
  - reworded commit message to be clearer about the objective (Alex)

* hw/core/cpu: call qemu_init_vcpu in cpu_common_realizefn:
  - dropped this patch (Alex, Philippe)

  will try again later, also in the context of:
  https://www.mail-archive.com/qemu-devel@nongnu.org/msg686480.html

* accel: introduce new accessor functions
  - squashed comments in previous patch introducing accel-cpu.h. (Philippe)

* accel-cpu: make cpu_realizefn return a bool
  - split in two patches, separating the change to the phys_bits check
    (Philippe)

v14 -> v15:

* change the TcgCpuOperations so that all fields of the struct are
  defined unconditionally, as per original patch by Eduardo,
  before moving them to a separate struct referenced by a pointer
  (Richard, Eduardo).

* changed (c) year to 2021

* added a patch to make accel_cpu->cpu_realizefn return bool, and
  adapt host_cpu, kvm_cpu, hvf_cpu and tcg_cpu in i386 accordingly.
  Ultimately, consistently moving to a pattern of realize functions
  returning bool will require a rework of all devices.

v13 -> v14: rebased on latest master.
v12 -> v13: rebased on latest master.

v11 -> v12: reordered patches and improved tcg_ops

* reordered all TcgCpuOperations stuff so it is at the beginning

* added patches for ARM-specific tcg ops
  debug_check_watchpoint and adjust_watchpoint_address

* added a patch that puts a forward declared pointer in the struct,
  so as to reduce the change of misuse between common_ss and specific_ss code,
  and tidy up as a consequence all targets, by defining dedicated structs.

v10 -> v11: split off PART 2,

no further changes to PART 2 other than the split.

v9 -> v10: minor tweaks and fixes

* in "i386: split cpu accelerators from cpu.c",

use kvm/kvm-cpu.c, hvf/hvf-cpu.c, tcg/tcg-cpu.c.
Easier to understand compared to editing multiple cpu.c files,
and matches the header files if needed (kvm-cpu.h).

* in "accel: replace struct CpusAccel with AccelOpsClass",

make it a bit more consistent, by naming the files defining
the AccelOpsClass types "...-accel-ops.c" instead of the old
naming "...-cpus.c".

* in "cpu: move cc->transaction_failed to tcg_ops",

protect with CONFIG_TCG the use of tcg_ops for hw/misc/jazz.c,

 #include "exec/memattrs.h" (Philippe, Eduardo)

* in "cpu: Move synchronize_from_tb() to tcg_ops",

 #include "hw/core/cpu.h" (Philippe, Eduardo)

do not remove the comment about struct TcgCpuOperations (Philippe)

* in "accel/tcg: split TCG-only code from cpu_exec_realizefn",

invert tcg_target_initialized set order (Alex)

* in "i386: move TCG cpu class initialization out of helper.c",

extract helper-tcg.h, tcg-cpu.c, and tcg-cpu.h directly into
tcg/, avoiding the extra move later to tcg/ (Alex)



v8 -> v9: move additional methods to CPUClass->tcg_ops

do_unaligned_access, transaction_failed and do_interrupt.

do_interrupt is a bit tricky, as the same code is reused
(albeit not usually directly) for KVM under certain odd conditions.

Change arm, as the only user of do_interrupt callback for KVM,
to instead call the target function directly arm_do_interrupt.

v7 -> v8: add missing CONFIG_TCGs, fix bugs

* add the prerequisite patches for "3 tcg" at the beginning of the
  series for convenience (already reviewed, queued by RH).

* add CONFIG_TCG to TCGCpuOperations and tcg_ops variable use

* reduce the scope of the realizefn refactoring, do not
  introduce a separate cpu_accel_realize, and instead use the
  existing cpu_exec_realizefn, there is not enough benefit
  to introduce a new function.

* fix bugs in user mode due to attempt to move the tcg_region_init()
  early, so it could be done just once in tcg_init() for both
  softmmu and user mode. Unfortunately it needs to remain deferred
  for user mode, as it needs to be done after prologue init and
  after the GUEST_BASE has been set.

v6 -> v7: integrate TCGCpuOperations, refactored cpu_exec_realizefn

* integrate TCGCpuOperations (Eduardo)

Taken some refactoring from Eduardo for Tcg-only operations on
CPUClass.

* refactored cpu_exec_realizefn

The other main change is a refactoring of cpu_exec_realizefn,
directly linked to the effort of making many cpu_exec operations
TCG-only (Eduardo series above):

cpu_exec_realizefn is actually a TCG-only thing, with the
exception of a couple things that can be done in base cpu code.

This changes all targets realizefn, so I guess I have to Cc:
the Multiverse? (Universe was already CCed for all accelerators).


v5 -> v6: remove MODULE_INIT_ACCEL_CPU


instead, use a call to accel_init_interfaces().

* The class lookups are now general and performed in accel/

  new AccelCPUClass for new archs are supported as new
  ones appear in the class hierarchy, no need for stubs.

* Split the code a bit better


v4 -> v5: centralized and simplified initializations

I put in Cc: Emilio G. Cota, specifically because in patch 8
I (re)moved for user-mode the call to tcg_regions_init().

The call happens now inside the tcg AccelClass machine_init,
(so earlier). This seems to work fine, but thought to get the
author opinion on this.

Rebased on "tcg-cpus: split into 3 tcg variants" series
(queued by Richard), to avoid some code churn:


https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg04356.html


* Extended AccelClass to user-mode.

user-mode now does not call tcg_exec_init directly,
instead it uses the tcg accel class, and its init_machine method.

Since user-mode does not define or use a machine state,
the machine is just passed as NULL.

The immediate advantage is that now we can call current_accel()
from both user mode and softmmu, so we can work out the correct
class to use for accelerator initializations.

* QOMification of CpusAccelOps

simple QOMification of CpusAccelOps abstract class.

* Centralized all accel_cpu_init, so only one per cpu-arch,
  plus one for all accels will remain.

  So we can expect accel_cpu_init() to be limited to:
  
  softmmu/cpus.c - initializes the chosen softmmu accel ops for the cpus module.
  target/ARCH/cpu.c - initializes the chosen arch-specific cpu accelerator.
  
These changes are meant to address concerns/issues (Paolo):

1) the use of if (tcg_enabled()) and similar in the module_init call path

2) the excessive number of accel_cpu_init() to hunt down in the codebase.


* Fixed wrong use of host_cpu_class_init (Eduardo)


v3 -> v4: QOMification of X86CPUAccelClass


In this version I basically QOMified X86CPUAccel, taking the
suggestions from Eduardo as the starting point,
but stopping just short of making it an actual QOM interface,
using a plain abstract class, and then subclasses for the
actual objects.

Initialization is still using the existing qemu initialization
framework (module_call_init), which is I still think is better
than the alternatives proposed, in the current state.

Possibly some improvements could be developed in the future here.
In this case, effort should be put in keeping things extendible,
in order not to be blocked once accelerators also become modules.

Motivation and higher level steps:

https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html

Looking forward to your comments on this proposal,

Ciao,

Claudio


Claudio Fontana (18):
  i386: split cpu accelerators from cpu.c, using AccelCPUClass
  cpu: call AccelCPUClass::cpu_realizefn in cpu_exec_realizefn
  accel: introduce new accessor functions
  target/i386: fix host_cpu_adjust_phys_bits error handling
  accel-cpu: make cpu_realizefn return a bool
  meson: add target_user_arch
  i386: split off sysemu-only functionality in tcg-cpu
  i386: split smm helper (sysemu)
  i386: split tcg excp_helper into sysemu and user parts
  i386: move TCG bpt_helper into sysemu/
  i386: split misc helper user stubs and sysemu part
  i386: separate fpu_helper sysemu-only parts
  i386: split svm_helper into sysemu and stub-only user
  i386: split seg_helper into user-only and sysemu parts
  i386: split off sysemu part of cpu.c
  target/i386: gdbstub: introduce aux functions to read/write CS64 regs
  target/i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu
  i386: make cpu_load_efer sysemu-only

Richard Henderson (2):
  target/i386: Rename helper_fldt, helper_fstt
  target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor

 meson.build                               |   5 +
 include/hw/core/accel-cpu.h               |   2 +-
 include/qemu/accel.h                      |  13 +
 target/i386/cpu-internal.h                |  70 ++
 target/i386/cpu.h                         |  43 +-
 target/i386/helper.h                      |  11 +
 target/i386/host-cpu.h                    |  19 +
 target/i386/kvm/kvm-cpu.h                 |  41 ++
 target/i386/tcg/helper-tcg.h              |   8 +
 target/i386/tcg/seg_helper.h              |  66 ++
 target/i386/tcg/tcg-cpu.h                 |  21 +-
 accel/accel-common.c                      |  19 +
 cpu.c                                     |   5 +-
 hw/i386/pc_piix.c                         |   1 +
 target/i386/cpu-sysemu.c                  | 352 ++++++++++
 target/i386/cpu.c                         | 782 ++--------------------
 target/i386/gdbstub.c                     | 165 ++---
 target/i386/helper.c                      |  13 +
 target/i386/host-cpu.c                    | 204 ++++++
 target/i386/hvf/hvf-cpu.c                 |  68 ++
 target/i386/kvm/kvm-cpu.c                 | 151 +++++
 target/i386/kvm/kvm.c                     |   3 +-
 target/i386/tcg/bpt_helper.c              | 276 --------
 target/i386/tcg/excp_helper.c             | 572 ----------------
 target/i386/tcg/fpu_helper.c              | 122 ++--
 target/i386/tcg/misc_helper.c             | 463 -------------
 target/i386/tcg/seg_helper.c              | 237 +------
 target/i386/tcg/sysemu/bpt_helper.c       | 293 ++++++++
 target/i386/tcg/sysemu/excp_helper.c      | 581 ++++++++++++++++
 target/i386/tcg/sysemu/fpu_helper.c       |  57 ++
 target/i386/tcg/sysemu/misc_helper.c      | 438 ++++++++++++
 target/i386/tcg/sysemu/seg_helper.c       | 125 ++++
 target/i386/tcg/{ => sysemu}/smm_helper.c |  19 +-
 target/i386/tcg/{ => sysemu}/svm_helper.c |  62 +-
 target/i386/tcg/sysemu/tcg-cpu.c          |  83 +++
 target/i386/tcg/tcg-cpu.c                 |  50 +-
 target/i386/tcg/translate.c               |  13 +-
 target/i386/tcg/user/excp_helper.c        |  39 ++
 target/i386/tcg/user/misc_stubs.c         |  75 +++
 target/i386/tcg/user/seg_helper.c         | 109 +++
 target/i386/tcg/user/svm_stubs.c          |  76 +++
 MAINTAINERS                               |   2 +-
 target/alpha/meson.build                  |   3 +
 target/arm/meson.build                    |   2 +
 target/cris/meson.build                   |   3 +
 target/hexagon/meson.build                |   3 +
 target/hppa/meson.build                   |   3 +
 target/i386/hvf/meson.build               |   1 +
 target/i386/kvm/meson.build               |   7 +-
 target/i386/meson.build                   |   9 +-
 target/i386/tcg/meson.build               |   5 +-
 target/i386/tcg/sysemu/meson.build        |  10 +
 target/i386/tcg/user/meson.build          |   6 +
 target/m68k/meson.build                   |   3 +
 target/microblaze/meson.build             |   3 +
 target/mips/meson.build                   |   3 +
 target/nios2/meson.build                  |   3 +
 target/openrisc/meson.build               |   3 +
 target/ppc/meson.build                    |   3 +
 target/riscv/meson.build                  |   3 +
 target/s390x/meson.build                  |   3 +
 target/sh4/meson.build                    |   3 +
 target/sparc/meson.build                  |   3 +
 target/tilegx/meson.build                 |   3 +
 target/tricore/meson.build                |   3 +
 target/xtensa/meson.build                 |   3 +
 66 files changed, 3266 insertions(+), 2579 deletions(-)
 create mode 100644 target/i386/cpu-internal.h
 create mode 100644 target/i386/host-cpu.h
 create mode 100644 target/i386/kvm/kvm-cpu.h
 create mode 100644 target/i386/tcg/seg_helper.h
 create mode 100644 target/i386/cpu-sysemu.c
 create mode 100644 target/i386/host-cpu.c
 create mode 100644 target/i386/hvf/hvf-cpu.c
 create mode 100644 target/i386/kvm/kvm-cpu.c
 create mode 100644 target/i386/tcg/sysemu/bpt_helper.c
 create mode 100644 target/i386/tcg/sysemu/excp_helper.c
 create mode 100644 target/i386/tcg/sysemu/fpu_helper.c
 create mode 100644 target/i386/tcg/sysemu/misc_helper.c
 create mode 100644 target/i386/tcg/sysemu/seg_helper.c
 rename target/i386/tcg/{ => sysemu}/smm_helper.c (98%)
 rename target/i386/tcg/{ => sysemu}/svm_helper.c (96%)
 create mode 100644 target/i386/tcg/sysemu/tcg-cpu.c
 create mode 100644 target/i386/tcg/user/excp_helper.c
 create mode 100644 target/i386/tcg/user/misc_stubs.c
 create mode 100644 target/i386/tcg/user/seg_helper.c
 create mode 100644 target/i386/tcg/user/svm_stubs.c
 create mode 100644 target/i386/tcg/sysemu/meson.build
 create mode 100644 target/i386/tcg/user/meson.build

-- 
2.26.2

Patchew URL: https://patchew.org/QEMU/20210301085450.1732-1-cfontana@suse.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210301085450.1732-1-cfontana@suse.de
Subject: [PATCH v26 00/20] i386 cleanup PART 2

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210301085450.1732-1-cfontana@suse.de -> patchew/20210301085450.1732-1-cfontana@suse.de
Switched to a new branch 'test'
00a5877 i386: make cpu_load_efer sysemu-only
bc37797 target/i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu
2b17816 target/i386: gdbstub: introduce aux functions to read/write CS64 regs
3a208cf i386: split off sysemu part of cpu.c
2c7e080 i386: split seg_helper into user-only and sysemu parts
d83014b i386: split svm_helper into sysemu and stub-only user
b6d69a4 i386: separate fpu_helper sysemu-only parts
93cb687 i386: split misc helper user stubs and sysemu part
d8352df i386: move TCG bpt_helper into sysemu/
8fbb05f i386: split tcg excp_helper into sysemu and user parts
0a91873 i386: split smm helper (sysemu)
86ad6ce i386: split off sysemu-only functionality in tcg-cpu
ecd60a2 meson: add target_user_arch
eec9c82 accel-cpu: make cpu_realizefn return a bool
007716a target/i386: fix host_cpu_adjust_phys_bits error handling
48530bb accel: introduce new accessor functions
cedc6f6 cpu: call AccelCPUClass::cpu_realizefn in cpu_exec_realizefn
ffbf00d i386: split cpu accelerators from cpu.c, using AccelCPUClass
d8033c6 target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
d516237 target/i386: Rename helper_fldt, helper_fstt

=== OUTPUT BEGIN ===
1/20 Checking commit d5162371be89 (target/i386: Rename helper_fldt, helper_fstt)
2/20 Checking commit d8033c640ed4 (target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor)
3/20 Checking commit ffbf00db2eb3 (i386: split cpu accelerators from cpu.c, using AccelCPUClass)
WARNING: line over 80 characters
#1337: FILE: target/i386/tcg/tcg-cpu.c:125:
+    memory_region_add_subregion_overlap(cpu->cpu_as_root, 0, cpu->cpu_as_mem, 0);

total: 0 errors, 1 warnings, 1256 lines checked

Patch 3/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/20 Checking commit cedc6f6e4c18 (cpu: call AccelCPUClass::cpu_realizefn in cpu_exec_realizefn)
5/20 Checking commit 48530bbeb4c2 (accel: introduce new accessor functions)
6/20 Checking commit 007716a87edf (target/i386: fix host_cpu_adjust_phys_bits error handling)
7/20 Checking commit eec9c82ab85c (accel-cpu: make cpu_realizefn return a bool)
8/20 Checking commit ecd60a2189f5 (meson: add target_user_arch)
9/20 Checking commit 86ad6ce0eb6a (i386: split off sysemu-only functionality in tcg-cpu)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100644

WARNING: line over 80 characters
#116: FILE: target/i386/tcg/sysemu/tcg-cpu.c:72:
+    memory_region_add_subregion_overlap(cpu->cpu_as_root, 0, cpu->cpu_as_mem, 0);

total: 0 errors, 2 warnings, 212 lines checked

Patch 9/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/20 Checking commit 0a9187311234 (i386: split smm helper (sysemu))
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#24: 
 target/i386/tcg/{ => sysemu}/smm_helper.c | 19 ++-----------------

total: 0 errors, 1 warnings, 84 lines checked

Patch 10/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/20 Checking commit 8fbb05f77818 (i386: split tcg excp_helper into sysemu and user parts)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#599: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#756: FILE: target/i386/tcg/sysemu/excp_helper.c:153:
+            /* Bits 20-13 provide bits 39-32 of the address, bit 21 is reserved.

WARNING: Block comments use a leading /* on a separate line
#830: FILE: target/i386/tcg/sysemu/excp_helper.c:227:
+/* return value:

WARNING: line over 80 characters
#931: FILE: target/i386/tcg/sysemu/excp_helper.c:328:
+            pdpe_addr = ((pml4e & PG_ADDRESS_MASK) + (((addr >> 30) & 0x1ff) << 3)) &

WARNING: Block comments use a leading /* on a separate line
#1025: FILE: target/i386/tcg/sysemu/excp_helper.c:422:
+            /* Bits 20-13 provide bits 39-32 of the address, bit 21 is reserved.

WARNING: Block comments use a leading /* on a separate line
#1120: FILE: target/i386/tcg/sysemu/excp_helper.c:517:
+        /* only set write access if already dirty... otherwise wait

WARNING: Block comments use * on subsequent lines
#1121: FILE: target/i386/tcg/sysemu/excp_helper.c:518:
+        /* only set write access if already dirty... otherwise wait
+           for dirty access */

WARNING: Block comments use a trailing */ on a separate line
#1121: FILE: target/i386/tcg/sysemu/excp_helper.c:518:
+           for dirty access */

WARNING: Block comments use a leading /* on a separate line
#1134: FILE: target/i386/tcg/sysemu/excp_helper.c:531:
+    /* Even if 4MB pages, we map only one 4KB page in the cache to

WARNING: Block comments use * on subsequent lines
#1135: FILE: target/i386/tcg/sysemu/excp_helper.c:532:
+    /* Even if 4MB pages, we map only one 4KB page in the cache to
+       avoid filling it too fast */

WARNING: Block comments use a trailing */ on a separate line
#1135: FILE: target/i386/tcg/sysemu/excp_helper.c:532:
+       avoid filling it too fast */

ERROR: braces {} are necessary for all arms of this statement
#1149: FILE: target/i386/tcg/sysemu/excp_helper.c:546:
+    if (is_user)
[...]

total: 1 errors, 11 warnings, 631 lines checked

Patch 11/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

12/20 Checking commit d8352df00ca5 (i386: move TCG bpt_helper into sysemu/)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#364: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#500: FILE: target/i386/tcg/sysemu/bpt_helper.c:132:
+    /* If nothing is changing except the global/local enable bits,

WARNING: Block comments use * on subsequent lines
#501: FILE: target/i386/tcg/sysemu/bpt_helper.c:133:
+    /* If nothing is changing except the global/local enable bits,
+       then we can make the change more efficient.  */

WARNING: Block comments use a trailing */ on a separate line
#501: FILE: target/i386/tcg/sysemu/bpt_helper.c:133:
+       then we can make the change more efficient.  */

WARNING: Block comments use a leading /* on a separate line
#503: FILE: target/i386/tcg/sysemu/bpt_helper.c:135:
+        /* Fold the global and local enable bits together into the

WARNING: Block comments use * on subsequent lines
#504: FILE: target/i386/tcg/sysemu/bpt_helper.c:136:
+        /* Fold the global and local enable bits together into the
+           global fields, then xor to show which registers have

WARNING: Block comments use a trailing */ on a separate line
#505: FILE: target/i386/tcg/sysemu/bpt_helper.c:137:
+           changed collective enable state.  */

total: 0 errors, 7 warnings, 628 lines checked

Patch 12/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/20 Checking commit 93cb687b7b8c (i386: split misc helper user stubs and sysemu part)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#527: 
new file mode 100644

ERROR: switch and case should be at the same indent
#820: FILE: target/i386/tcg/sysemu/misc_helper.c:289:
+    switch ((uint32_t)env->regs[R_ECX]) {
[...]
+     case MSR_IA32_UCODE_REV:

total: 1 errors, 1 warnings, 1012 lines checked

Patch 13/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

14/20 Checking commit b6d69a4d70a7 (i386: separate fpu_helper sysemu-only parts)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#108: 
new file mode 100644

total: 0 errors, 1 warnings, 135 lines checked

Patch 14/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/20 Checking commit d83014bb1c38 (i386: split svm_helper into sysemu and stub-only user)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
 target/i386/tcg/{ => sysemu}/svm_helper.c | 62 +------------------------

total: 0 errors, 1 warnings, 169 lines checked

Patch 15/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/20 Checking commit 2c7e080c0171 (i386: split seg_helper into user-only and sysemu parts)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#321: 
new file mode 100644

ERROR: do not use C99 // comments
#348: FILE: target/i386/tcg/seg_helper.h:23:
+//#define DEBUG_PCALL

WARNING: Block comments use a leading /* on a separate line
#621: FILE: target/i386/tcg/user/seg_helper.c:73:
+    /* Since we emulate only user space, we cannot do more than

WARNING: Block comments use * on subsequent lines
#622: FILE: target/i386/tcg/user/seg_helper.c:74:
+    /* Since we emulate only user space, we cannot do more than
+       exiting the emulation with the suitable exception and error

WARNING: Block comments use a trailing */ on a separate line
#623: FILE: target/i386/tcg/user/seg_helper.c:75:
+       code. So update EIP for INT 0x80 and EXCP_SYSCALL. */

WARNING: Block comments use a leading /* on a separate line
#634: FILE: target/i386/tcg/user/seg_helper.c:86:
+    /* if user mode only, we simulate a fake exception

WARNING: Block comments use * on subsequent lines
#635: FILE: target/i386/tcg/user/seg_helper.c:87:
+    /* if user mode only, we simulate a fake exception
+       which will be handled outside the cpu execution

WARNING: Block comments use a trailing */ on a separate line
#636: FILE: target/i386/tcg/user/seg_helper.c:88:
+       loop */

total: 1 errors, 7 warnings, 595 lines checked

Patch 16/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

17/20 Checking commit 3a208cf87826 (i386: split off sysemu part of cpu.c)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#18: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#52: FILE: target/i386/cpu-internal.h:30:
+    /* feature flags names are taken from "Intel Processor Identification and

WARNING: Block comments use a leading /* on a separate line
#135: FILE: target/i386/cpu-sysemu.c:37:
+/* Return a QDict containing keys for all properties that can be included

WARNING: Block comments use a leading /* on a separate line
#188: FILE: target/i386/cpu-sysemu.c:90:
+/* Convert CPU model data from X86CPU object to a property dictionary

WARNING: Block comments use a leading /* on a separate line
#202: FILE: target/i386/cpu-sysemu.c:104:
+/* Convert CPU model data from X86CPU object to a property dictionary

WARNING: Block comments use a leading /* on a separate line
#218: FILE: target/i386/cpu-sysemu.c:120:
+        /* "hotplugged" is the only property that is configurable

WARNING: Block comments use a leading /* on a separate line
#308: FILE: target/i386/cpu-sysemu.c:210:
+        /* As we don't return every single property, full expansion needs

total: 0 errors, 7 warnings, 901 lines checked

Patch 17/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/20 Checking commit 2b1781677162 (target/i386: gdbstub: introduce aux functions to read/write CS64 regs)
WARNING: line over 80 characters
#62: FILE: target/i386/gdbstub.c:163:
+            return gdb_read_reg_cs64(env->hflags, mem_buf, env->segs[R_FS].base);

WARNING: line over 80 characters
#68: FILE: target/i386/gdbstub.c:165:
+            return gdb_read_reg_cs64(env->hflags, mem_buf, env->segs[R_GS].base);

WARNING: line over 80 characters
#156: FILE: target/i386/gdbstub.c:318:
+            return gdb_write_reg_cs64(env->hflags, mem_buf, &env->segs[R_FS].base);

WARNING: line over 80 characters
#165: FILE: target/i386/gdbstub.c:320:
+            return gdb_write_reg_cs64(env->hflags, mem_buf, &env->segs[R_GS].base);

total: 0 errors, 4 warnings, 221 lines checked

Patch 18/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
19/20 Checking commit bc377979cd9a (target/i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu)
20/20 Checking commit 00a5877fc1a3 (i386: make cpu_load_efer sysemu-only)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210301085450.1732-1-cfontana@suse.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Hi,

anything else for me to do here?

The latest rebased state of this series should be always available here:

https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8

When it comes to the ARM cleanup series,
I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?

Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..

Thanks,

Claudio


On 3/1/21 9:54 AM, Claudio Fontana wrote:
> v25 -> v26:
> * i386: separate fpu_helper into user and sysemu parts
>  - removed the splitting of the user mode into their own user/fpu_helper.c,
>    seems not worth it.
> 
> v24 -> v25:
> 
> * i386: separate fpu_helper into user and sysemu parts
> 
>  - added 2 preliminary patches to the series (from Richard)
>  - rebased on those
> 
> * i386: move TCG btp_helper into sysemu/
> 
>  - fixed title typo (Alex)
>  - nested #ifdef more easily (Richard)
> 
> v23 -> v24:
> 
> * i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu                                                                                     
>  - remove additional #ifdef TARGET_X86_64
>  - split in two patches, so it is easier to understand.
> 
> v22 -> v23:
> 
> * i386: move TCG btp_helper into sysemu/
>  - extended the #ifndef CONFIG_USER_ONLY to entire else of
>    if (cpl != 0).
> 
> * i386: split misc helper into user and sysemu parts
>  - added g_assert_not_reached() and changed user file name to -stubs.
> 
> * i386: separate fpu_helper into user and sysemu parts
>  - removed unused return value
>  - added comment abut issues with current cpu_x86_fsave.
> 
> * i386: split off sysemu part of cpu.c
>  - rename cpu-softmmu.c to cpu-sysemu.c
>  - fixed two mispelled comments, and add two comments
>    in the headers of cpu.c and cpu-sysemu.c to describe them
> 
> * i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu
>  - defined some aux functions to reduce repeated code
> 
> * i386: make cpu_load_efer sysemu-only
>  - move the function to helper.c, remove "inline"
> 
> v21 -> v22: replace "softmmu" with "sysemu"
> 
> v20 -> v21:
> 
> * meson: add target_user_arch
>   - add hexagon
> 
> v19 -> v20:
> 
> * add new patch to make gdbstub only write certain registers for softmmu.
>   In particular, CR0, CR2, CR3 and EFER should not be changed under
>   CONFIG_USER_ONLY. (Paolo)
> 
> * add new patch to make cpu_load_efer softmmu-only (Paolo)
> 
> * i386: split svm_helper into softmmu and stub-only user
> 
>   - fixed commit message spelling (Eric)
> 
>   - mention in commit message that this reproduces the existing stubs,
>     but really everything that requires s->cpl == 0 should be impossible
>     to trigger from user-mode, and we could find a way to assert that
>     more consistently.
> 
> v18 -> v19:
> 
> * i386: split smm helper (softmmu)
>   - add g_assert_not_reached and cpu_abort for invalid states in
>     CONFIG_USER_ONLY (Paolo)
> 
> * i386: move TCG btp_helper into softmmu/
>   - for CONFIG_USER_ONLY, assert that the hidden IOBPT flags are not set
>     while attempting to generate io_bpt helpers.
>     Theory to verify (Paolo)
> 
> * i386: slit svm_helper into softmmu and stub-only user
>   - added XXX in the commit message to highlight the question about
>     whether the same check should be done controlling access to
>     cpu_load_efer() and state of the hidden SVME flag. (Paolo)
> 
> v17 -> v18:
> 
> * meson: add target_user_arch
> 
>  - add target_user_arch to all targets which build user.
>    Otherwise meson complains about missing key for archs without it.
>    (Paolo)
> 
> * wrap a few gen_helper_ calls around ifndef CONFIG_USER_ONLY.
>   This would need a look from someone like Alex or Richard I think,
>   as potentially we could remove even more code I think around the
>   gen_helper_ calls for CONFIG_USER_ONLY.
> 
>   In the current master code, we have empty helpers for user mode,
>   but still we generate the preamble code, temporary variables etc,
>   just to then call a helper_() function that does nothing.
> 
>   In particular I am referring to patches:
> 
>   i386: split tcg btp_helper into softmmu and user parts
>         DEF_HELPER_FLAGS_3(set_dr, TCG_CALL_NO_WG, void, env, int, tl)
>         DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
>         gen_bpt_io
>         gen_helper_set_dr(cpu_env, s->tmp2_i32, s->T0);
> 
>   i386: split smm helper (softmmu)
>         DEF_HELPER_1(rsm, void, env)
> 	gen_helper_rsm(cpu_env);
> 
>   (Alex, Richard?)
> 
> * removed suffixes from user/ and softmmu/ modules
>   (Alex, Philippe).
>   Where possible, removed user stubs entirely.
>   Renamed the leftover svm_helper stubs to user/svm_stubs.c
> 
> * cleaned up lefover unnecessary header files and squashed them.
>  
> 
> v16 -> v17: changed to RFC
> 
> * tcg_ops are already in master, removed from the series
> 
> * i386: split cpu accelerators from cpu.c, using AccelCPUClass:
>   removed spurious ; and added spacing before/after functions (Richard)
> 
> * added new patches as RFC for the next steps, introducing target-specific
>   user-mode specific meson variables, and applied to i386/tcg as an
>   example, in order to gather feedback.
> 
> v15 -> v16:
> 
> * cpu: Move synchronize_from_tb() to tcg_ops:
>   - adjusted comments (Alex)
> 
> * cpu: tcg_ops: move to tcg-cpu-ops.h, keep a pointer in CPUClass:
>   - remove forward decl. of AccelCPUClass, should be in a later patch. (Alex)
>   - simplified comment about tcg_ops in struct CPUClass (Alex)
>   - remove obsolete comment about ARM blocking TCGCPUOps from being const.
>     (Alex)
> 
> * accel: replace struct CpusAccel with AccelOpsClass:
>   - reworded commit message to be clearer about the objective (Alex)
> 
> * accel: introduce AccelCPUClass extending CPUClass
>   - reworded commit message to be clearer about the objective (Alex)
> 
> * hw/core/cpu: call qemu_init_vcpu in cpu_common_realizefn:
>   - dropped this patch (Alex, Philippe)
> 
>   will try again later, also in the context of:
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg686480.html
> 
> * accel: introduce new accessor functions
>   - squashed comments in previous patch introducing accel-cpu.h. (Philippe)
> 
> * accel-cpu: make cpu_realizefn return a bool
>   - split in two patches, separating the change to the phys_bits check
>     (Philippe)
> 
> v14 -> v15:
> 
> * change the TcgCpuOperations so that all fields of the struct are
>   defined unconditionally, as per original patch by Eduardo,
>   before moving them to a separate struct referenced by a pointer
>   (Richard, Eduardo).
> 
> * changed (c) year to 2021
> 
> * added a patch to make accel_cpu->cpu_realizefn return bool, and
>   adapt host_cpu, kvm_cpu, hvf_cpu and tcg_cpu in i386 accordingly.
>   Ultimately, consistently moving to a pattern of realize functions
>   returning bool will require a rework of all devices.
> 
> v13 -> v14: rebased on latest master.
> v12 -> v13: rebased on latest master.
> 
> v11 -> v12: reordered patches and improved tcg_ops
> 
> * reordered all TcgCpuOperations stuff so it is at the beginning
> 
> * added patches for ARM-specific tcg ops
>   debug_check_watchpoint and adjust_watchpoint_address
> 
> * added a patch that puts a forward declared pointer in the struct,
>   so as to reduce the change of misuse between common_ss and specific_ss code,
>   and tidy up as a consequence all targets, by defining dedicated structs.
> 
> v10 -> v11: split off PART 2,
> 
> no further changes to PART 2 other than the split.
> 
> v9 -> v10: minor tweaks and fixes
> 
> * in "i386: split cpu accelerators from cpu.c",
> 
> use kvm/kvm-cpu.c, hvf/hvf-cpu.c, tcg/tcg-cpu.c.
> Easier to understand compared to editing multiple cpu.c files,
> and matches the header files if needed (kvm-cpu.h).
> 
> * in "accel: replace struct CpusAccel with AccelOpsClass",
> 
> make it a bit more consistent, by naming the files defining
> the AccelOpsClass types "...-accel-ops.c" instead of the old
> naming "...-cpus.c".
> 
> * in "cpu: move cc->transaction_failed to tcg_ops",
> 
> protect with CONFIG_TCG the use of tcg_ops for hw/misc/jazz.c,
> 
>  #include "exec/memattrs.h" (Philippe, Eduardo)
> 
> * in "cpu: Move synchronize_from_tb() to tcg_ops",
> 
>  #include "hw/core/cpu.h" (Philippe, Eduardo)
> 
> do not remove the comment about struct TcgCpuOperations (Philippe)
> 
> * in "accel/tcg: split TCG-only code from cpu_exec_realizefn",
> 
> invert tcg_target_initialized set order (Alex)
> 
> * in "i386: move TCG cpu class initialization out of helper.c",
> 
> extract helper-tcg.h, tcg-cpu.c, and tcg-cpu.h directly into
> tcg/, avoiding the extra move later to tcg/ (Alex)
> 
> 
> 
> v8 -> v9: move additional methods to CPUClass->tcg_ops
> 
> do_unaligned_access, transaction_failed and do_interrupt.
> 
> do_interrupt is a bit tricky, as the same code is reused
> (albeit not usually directly) for KVM under certain odd conditions.
> 
> Change arm, as the only user of do_interrupt callback for KVM,
> to instead call the target function directly arm_do_interrupt.
> 
> v7 -> v8: add missing CONFIG_TCGs, fix bugs
> 
> * add the prerequisite patches for "3 tcg" at the beginning of the
>   series for convenience (already reviewed, queued by RH).
> 
> * add CONFIG_TCG to TCGCpuOperations and tcg_ops variable use
> 
> * reduce the scope of the realizefn refactoring, do not
>   introduce a separate cpu_accel_realize, and instead use the
>   existing cpu_exec_realizefn, there is not enough benefit
>   to introduce a new function.
> 
> * fix bugs in user mode due to attempt to move the tcg_region_init()
>   early, so it could be done just once in tcg_init() for both
>   softmmu and user mode. Unfortunately it needs to remain deferred
>   for user mode, as it needs to be done after prologue init and
>   after the GUEST_BASE has been set.
> 
> v6 -> v7: integrate TCGCpuOperations, refactored cpu_exec_realizefn
> 
> * integrate TCGCpuOperations (Eduardo)
> 
> Taken some refactoring from Eduardo for Tcg-only operations on
> CPUClass.
> 
> * refactored cpu_exec_realizefn
> 
> The other main change is a refactoring of cpu_exec_realizefn,
> directly linked to the effort of making many cpu_exec operations
> TCG-only (Eduardo series above):
> 
> cpu_exec_realizefn is actually a TCG-only thing, with the
> exception of a couple things that can be done in base cpu code.
> 
> This changes all targets realizefn, so I guess I have to Cc:
> the Multiverse? (Universe was already CCed for all accelerators).
> 
> 
> v5 -> v6: remove MODULE_INIT_ACCEL_CPU
> 
> 
> instead, use a call to accel_init_interfaces().
> 
> * The class lookups are now general and performed in accel/
> 
>   new AccelCPUClass for new archs are supported as new
>   ones appear in the class hierarchy, no need for stubs.
> 
> * Split the code a bit better
> 
> 
> v4 -> v5: centralized and simplified initializations
> 
> I put in Cc: Emilio G. Cota, specifically because in patch 8
> I (re)moved for user-mode the call to tcg_regions_init().
> 
> The call happens now inside the tcg AccelClass machine_init,
> (so earlier). This seems to work fine, but thought to get the
> author opinion on this.
> 
> Rebased on "tcg-cpus: split into 3 tcg variants" series
> (queued by Richard), to avoid some code churn:
> 
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg04356.html
> 
> 
> * Extended AccelClass to user-mode.
> 
> user-mode now does not call tcg_exec_init directly,
> instead it uses the tcg accel class, and its init_machine method.
> 
> Since user-mode does not define or use a machine state,
> the machine is just passed as NULL.
> 
> The immediate advantage is that now we can call current_accel()
> from both user mode and softmmu, so we can work out the correct
> class to use for accelerator initializations.
> 
> * QOMification of CpusAccelOps
> 
> simple QOMification of CpusAccelOps abstract class.
> 
> * Centralized all accel_cpu_init, so only one per cpu-arch,
>   plus one for all accels will remain.
> 
>   So we can expect accel_cpu_init() to be limited to:
>   
>   softmmu/cpus.c - initializes the chosen softmmu accel ops for the cpus module.
>   target/ARCH/cpu.c - initializes the chosen arch-specific cpu accelerator.
>   
> These changes are meant to address concerns/issues (Paolo):
> 
> 1) the use of if (tcg_enabled()) and similar in the module_init call path
> 
> 2) the excessive number of accel_cpu_init() to hunt down in the codebase.
> 
> 
> * Fixed wrong use of host_cpu_class_init (Eduardo)
> 
> 
> v3 -> v4: QOMification of X86CPUAccelClass
> 
> 
> In this version I basically QOMified X86CPUAccel, taking the
> suggestions from Eduardo as the starting point,
> but stopping just short of making it an actual QOM interface,
> using a plain abstract class, and then subclasses for the
> actual objects.
> 
> Initialization is still using the existing qemu initialization
> framework (module_call_init), which is I still think is better
> than the alternatives proposed, in the current state.
> 
> Possibly some improvements could be developed in the future here.
> In this case, effort should be put in keeping things extendible,
> in order not to be blocked once accelerators also become modules.
> 
> Motivation and higher level steps:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html
> 
> Looking forward to your comments on this proposal,
> 
> Ciao,
> 
> Claudio
> 
> 
> Claudio Fontana (18):
>   i386: split cpu accelerators from cpu.c, using AccelCPUClass
>   cpu: call AccelCPUClass::cpu_realizefn in cpu_exec_realizefn
>   accel: introduce new accessor functions
>   target/i386: fix host_cpu_adjust_phys_bits error handling
>   accel-cpu: make cpu_realizefn return a bool
>   meson: add target_user_arch
>   i386: split off sysemu-only functionality in tcg-cpu
>   i386: split smm helper (sysemu)
>   i386: split tcg excp_helper into sysemu and user parts
>   i386: move TCG bpt_helper into sysemu/
>   i386: split misc helper user stubs and sysemu part
>   i386: separate fpu_helper sysemu-only parts
>   i386: split svm_helper into sysemu and stub-only user
>   i386: split seg_helper into user-only and sysemu parts
>   i386: split off sysemu part of cpu.c
>   target/i386: gdbstub: introduce aux functions to read/write CS64 regs
>   target/i386: gdbstub: only write CR0/CR2/CR3/EFER for sysemu
>   i386: make cpu_load_efer sysemu-only
> 
> Richard Henderson (2):
>   target/i386: Rename helper_fldt, helper_fstt
>   target/i386: Split out do_fsave, do_frstor, do_fxsave, do_fxrstor
> 
>  meson.build                               |   5 +
>  include/hw/core/accel-cpu.h               |   2 +-
>  include/qemu/accel.h                      |  13 +
>  target/i386/cpu-internal.h                |  70 ++
>  target/i386/cpu.h                         |  43 +-
>  target/i386/helper.h                      |  11 +
>  target/i386/host-cpu.h                    |  19 +
>  target/i386/kvm/kvm-cpu.h                 |  41 ++
>  target/i386/tcg/helper-tcg.h              |   8 +
>  target/i386/tcg/seg_helper.h              |  66 ++
>  target/i386/tcg/tcg-cpu.h                 |  21 +-
>  accel/accel-common.c                      |  19 +
>  cpu.c                                     |   5 +-
>  hw/i386/pc_piix.c                         |   1 +
>  target/i386/cpu-sysemu.c                  | 352 ++++++++++
>  target/i386/cpu.c                         | 782 ++--------------------
>  target/i386/gdbstub.c                     | 165 ++---
>  target/i386/helper.c                      |  13 +
>  target/i386/host-cpu.c                    | 204 ++++++
>  target/i386/hvf/hvf-cpu.c                 |  68 ++
>  target/i386/kvm/kvm-cpu.c                 | 151 +++++
>  target/i386/kvm/kvm.c                     |   3 +-
>  target/i386/tcg/bpt_helper.c              | 276 --------
>  target/i386/tcg/excp_helper.c             | 572 ----------------
>  target/i386/tcg/fpu_helper.c              | 122 ++--
>  target/i386/tcg/misc_helper.c             | 463 -------------
>  target/i386/tcg/seg_helper.c              | 237 +------
>  target/i386/tcg/sysemu/bpt_helper.c       | 293 ++++++++
>  target/i386/tcg/sysemu/excp_helper.c      | 581 ++++++++++++++++
>  target/i386/tcg/sysemu/fpu_helper.c       |  57 ++
>  target/i386/tcg/sysemu/misc_helper.c      | 438 ++++++++++++
>  target/i386/tcg/sysemu/seg_helper.c       | 125 ++++
>  target/i386/tcg/{ => sysemu}/smm_helper.c |  19 +-
>  target/i386/tcg/{ => sysemu}/svm_helper.c |  62 +-
>  target/i386/tcg/sysemu/tcg-cpu.c          |  83 +++
>  target/i386/tcg/tcg-cpu.c                 |  50 +-
>  target/i386/tcg/translate.c               |  13 +-
>  target/i386/tcg/user/excp_helper.c        |  39 ++
>  target/i386/tcg/user/misc_stubs.c         |  75 +++
>  target/i386/tcg/user/seg_helper.c         | 109 +++
>  target/i386/tcg/user/svm_stubs.c          |  76 +++
>  MAINTAINERS                               |   2 +-
>  target/alpha/meson.build                  |   3 +
>  target/arm/meson.build                    |   2 +
>  target/cris/meson.build                   |   3 +
>  target/hexagon/meson.build                |   3 +
>  target/hppa/meson.build                   |   3 +
>  target/i386/hvf/meson.build               |   1 +
>  target/i386/kvm/meson.build               |   7 +-
>  target/i386/meson.build                   |   9 +-
>  target/i386/tcg/meson.build               |   5 +-
>  target/i386/tcg/sysemu/meson.build        |  10 +
>  target/i386/tcg/user/meson.build          |   6 +
>  target/m68k/meson.build                   |   3 +
>  target/microblaze/meson.build             |   3 +
>  target/mips/meson.build                   |   3 +
>  target/nios2/meson.build                  |   3 +
>  target/openrisc/meson.build               |   3 +
>  target/ppc/meson.build                    |   3 +
>  target/riscv/meson.build                  |   3 +
>  target/s390x/meson.build                  |   3 +
>  target/sh4/meson.build                    |   3 +
>  target/sparc/meson.build                  |   3 +
>  target/tilegx/meson.build                 |   3 +
>  target/tricore/meson.build                |   3 +
>  target/xtensa/meson.build                 |   3 +
>  66 files changed, 3266 insertions(+), 2579 deletions(-)
>  create mode 100644 target/i386/cpu-internal.h
>  create mode 100644 target/i386/host-cpu.h
>  create mode 100644 target/i386/kvm/kvm-cpu.h
>  create mode 100644 target/i386/tcg/seg_helper.h
>  create mode 100644 target/i386/cpu-sysemu.c
>  create mode 100644 target/i386/host-cpu.c
>  create mode 100644 target/i386/hvf/hvf-cpu.c
>  create mode 100644 target/i386/kvm/kvm-cpu.c
>  create mode 100644 target/i386/tcg/sysemu/bpt_helper.c
>  create mode 100644 target/i386/tcg/sysemu/excp_helper.c
>  create mode 100644 target/i386/tcg/sysemu/fpu_helper.c
>  create mode 100644 target/i386/tcg/sysemu/misc_helper.c
>  create mode 100644 target/i386/tcg/sysemu/seg_helper.c
>  rename target/i386/tcg/{ => sysemu}/smm_helper.c (98%)
>  rename target/i386/tcg/{ => sysemu}/svm_helper.c (96%)
>  create mode 100644 target/i386/tcg/sysemu/tcg-cpu.c
>  create mode 100644 target/i386/tcg/user/excp_helper.c
>  create mode 100644 target/i386/tcg/user/misc_stubs.c
>  create mode 100644 target/i386/tcg/user/seg_helper.c
>  create mode 100644 target/i386/tcg/user/svm_stubs.c
>  create mode 100644 target/i386/tcg/sysemu/meson.build
>  create mode 100644 target/i386/tcg/user/meson.build
> 

Hi Claudio,

On 3/8/21 1:57 PM, Claudio Fontana wrote:
> Hi,
> 
> anything else for me to do here?
> 
> The latest rebased state of this series should be always available here:
> 
> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
> 
> When it comes to the ARM cleanup series,
> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
> 
> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..

TBH I wrote these patches during my personal spare time and this
became a real Pandora box that drained too much energy. I prefer
to step back and focus on finishing smaller tasks before burning
out. That said I appreciate your effort and am interested in
following / reviewing your work.

Regards,

Phil.

On 3/8/21 2:27 PM, Philippe Mathieu-Daudé wrote:
> Hi Claudio,
> 
> On 3/8/21 1:57 PM, Claudio Fontana wrote:
>> Hi,
>>
>> anything else for me to do here?
>>
>> The latest rebased state of this series should be always available here:
>>
>> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
>>
>> When it comes to the ARM cleanup series,
>> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
>>
>> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
> 
> TBH I wrote these patches during my personal spare time and this
> became a real Pandora box that drained too much energy. I prefer
> to step back and focus on finishing smaller tasks before burning
> out. That said I appreciate your effort and am interested in
> following / reviewing your work.
> 
> Regards,
> 
> Phil.
> 

Thanks Philippe for sharing this, and I agree completely, it is very draining.

The effort of making tests happy that run in artificial environments in particular often feels to me
as too disconnected from actually ensuring that there is no real run time regression.

qtest_enabled() (implicitly, or explicitly via open-ended else statements) is another painful variable to keep in mind in cpu and machine code, so it is not helpful in my view.

I'll try to push more to get the tests running again, if you have any comment or idea, feel free to just point me in the right direction,
that is very valuable to me, even without working code.

Currently I am struggling with arm-cpu-features and other tests (device-introspect-test, qom-test, test-hmp).

I'll publish arm_cleanup_v5 soon,

Thanks a lot,

Claudio

On 3/8/21 2:52 PM, Claudio Fontana wrote:
> On 3/8/21 2:27 PM, Philippe Mathieu-Daudé wrote:
>> Hi Claudio,
>>
>> On 3/8/21 1:57 PM, Claudio Fontana wrote:
>>> Hi,
>>>
>>> anything else for me to do here?
>>>
>>> The latest rebased state of this series should be always available here:
>>>
>>> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
>>>
>>> When it comes to the ARM cleanup series,
>>> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
>>>
>>> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
>>
>> TBH I wrote these patches during my personal spare time and this
>> became a real Pandora box that drained too much energy. I prefer
>> to step back and focus on finishing smaller tasks before burning
>> out. That said I appreciate your effort and am interested in
>> following / reviewing your work.
>>
>> Regards,
>>
>> Phil.
>>
> 
> Thanks Philippe for sharing this, and I agree completely, it is very draining.
> 
> The effort of making tests happy that run in artificial environments in particular often feels to me
> as too disconnected from actually ensuring that there is no real run time regression.
> 
> qtest_enabled() (implicitly, or explicitly via open-ended else statements) is another painful variable to keep in mind in cpu and machine code, so it is not helpful in my view.
> 
> I'll try to push more to get the tests running again, if you have any comment or idea, feel free to just point me in the right direction,
> that is very valuable to me, even without working code.

Basically I gave up after realizing from Daniel reviews that we need
QMP commands to query QEMU at runtime its built-in features, so we
have build-agnostic tests easier to maintain. I agree this is the
best way to resolve this particular case, but also scale for various
other cases.

On 3/8/21 3:57 PM, Philippe Mathieu-Daudé wrote:
> On 3/8/21 2:52 PM, Claudio Fontana wrote:
>> On 3/8/21 2:27 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Claudio,
>>>
>>> On 3/8/21 1:57 PM, Claudio Fontana wrote:
>>>> Hi,
>>>>
>>>> anything else for me to do here?
>>>>
>>>> The latest rebased state of this series should be always available here:
>>>>
>>>> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
>>>>
>>>> When it comes to the ARM cleanup series,
>>>> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
>>>>
>>>> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
>>>
>>> TBH I wrote these patches during my personal spare time and this
>>> became a real Pandora box that drained too much energy. I prefer
>>> to step back and focus on finishing smaller tasks before burning
>>> out. That said I appreciate your effort and am interested in
>>> following / reviewing your work.
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>
>> Thanks Philippe for sharing this, and I agree completely, it is very draining.
>>
>> The effort of making tests happy that run in artificial environments in particular often feels to me
>> as too disconnected from actually ensuring that there is no real run time regression.
>>
>> qtest_enabled() (implicitly, or explicitly via open-ended else statements) is another painful variable to keep in mind in cpu and machine code, so it is not helpful in my view.
>>
>> I'll try to push more to get the tests running again, if you have any comment or idea, feel free to just point me in the right direction,
>> that is very valuable to me, even without working code.
> 
> Basically I gave up after realizing from Daniel reviews that we need
> QMP commands to query QEMU at runtime its built-in features, so we
> have build-agnostic tests easier to maintain. I agree this is the
> best way to resolve this particular case, but also scale for various
> other cases.
> 

Well, yes, but in order to get things to work, even just a kvm-build fix would be good until we have the perfect solution, no?

We also fixed the tcg tests when doing this for i386, so I think we can fix these issues for arm too.

But this doesn't mean that we don't need the QMP commands to query QEMU at runtime for its "built-in"/module-loaded features.
We need that too, and I suspect this will be more and more needed by libvirt, as QEMU modularizes.

I just think the two things could proceed in parallel..

Ciao,

Claudio

On Mon, Mar 08, 2021 at 03:57:29PM +0100, Philippe Mathieu-Daudé wrote:
> On 3/8/21 2:52 PM, Claudio Fontana wrote:
> > On 3/8/21 2:27 PM, Philippe Mathieu-Daudé wrote:
> >> Hi Claudio,
> >>
> >> On 3/8/21 1:57 PM, Claudio Fontana wrote:
> >>> Hi,
> >>>
> >>> anything else for me to do here?
> >>>
> >>> The latest rebased state of this series should be always available here:
> >>>
> >>> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
> >>>
> >>> When it comes to the ARM cleanup series,
> >>> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
> >>>
> >>> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
> >>
> >> TBH I wrote these patches during my personal spare time and this
> >> became a real Pandora box that drained too much energy. I prefer
> >> to step back and focus on finishing smaller tasks before burning
> >> out. That said I appreciate your effort and am interested in
> >> following / reviewing your work.
> >>
> >> Regards,
> >>
> >> Phil.
> >>
> > 
> > Thanks Philippe for sharing this, and I agree completely, it is very draining.
> > 
> > The effort of making tests happy that run in artificial environments in particular often feels to me
> > as too disconnected from actually ensuring that there is no real run time regression.
> > 
> > qtest_enabled() (implicitly, or explicitly via open-ended else statements) is another painful variable to keep in mind in cpu and machine code, so it is not helpful in my view.
> > 
> > I'll try to push more to get the tests running again, if you have any comment or idea, feel free to just point me in the right direction,
> > that is very valuable to me, even without working code.
> 
> Basically I gave up after realizing from Daniel reviews that we need
> QMP commands to query QEMU at runtime its built-in features, so we
> have build-agnostic tests easier to maintain. I agree this is the
> best way to resolve this particular case, but also scale for various
> other cases.

Which patch / review are you referring to here ?


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

Claudio Fontana <cfontana@suse.de> writes:

> Hi,
>
> anything else for me to do here?

It looks to me that this series is looking pretty good. Every patch has
at least one review so I think it's just waiting on the maintainers to
pick it up.

Paolo/Richard - are you intending to take the series as is or are you
waiting for something else? I'd like to see the patch delta reduced for
the ARM cleanup work which is still ongoing.

>
> The latest rebased state of this series should be always available here:
>
> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8
>
> When it comes to the ARM cleanup series,
> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
>
> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
>
<snip>

-- 
Alex Bennée

On 3/8/21 3:02 PM, Alex Bennée wrote:
> 
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> Hi,
>>
>> anything else for me to do here?
> 
> It looks to me that this series is looking pretty good. Every patch has
> at least one review so I think it's just waiting on the maintainers to
> pick it up.
> 
> Paolo/Richard - are you intending to take the series as is or are you
> waiting for something else? I'd like to see the patch delta reduced for
> the ARM cleanup work which is still ongoing.

I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
Rebasing is starting to become more and more draining;

I am seeing some changes from Phil that redo some of the patches here (like target arch user),
maybe you would like to drive this?

> 
>>
>> The latest rebased state of this series should be always available here:
>>
>> https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_8

https://gitlab.com/hw-claudio/qemu/-/tree/i386_cleanup_9

is the more recent rebase, but now it is out of date again.

Phil could you help rebase this again?
Otherwise I could only get at this in about two weeks.

Thanks,

Claudio

>>
>> When it comes to the ARM cleanup series,
>> I would like to have the tests pass for ARM, before doing even more changes, could you help me there Philippe?
>>
>> Maybe applying some of your changes on top would fix the failures? I tried, for example with the arm-cpu-features ones, but it didn't work for me..
>>
> <snip>
> 

Claudio Fontana <cfontana@suse.de> writes:

> On 3/8/21 3:02 PM, Alex Bennée wrote:
>> 
>> Claudio Fontana <cfontana@suse.de> writes:
>> 
>>> Hi,
>>>
>>> anything else for me to do here?
>> 
>> It looks to me that this series is looking pretty good. Every patch has
>> at least one review so I think it's just waiting on the maintainers to
>> pick it up.
>> 
>> Paolo/Richard - are you intending to take the series as is or are you
>> waiting for something else? I'd like to see the patch delta reduced for
>> the ARM cleanup work which is still ongoing.
>
> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
> Rebasing is starting to become more and more draining;

This is still the latest re-factor right?

  Subject: [PATCH v28 00/23] i386 cleanup PART 2
  Date: Mon, 22 Mar 2021 14:27:36 +0100
  Message-Id: <20210322132800.7470-1-cfontana@suse.de>

> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
> maybe you would like to drive this?

AIUI his changes where to get qtest passing. I've just been chatting to
Paolo on IRC so I think we are almost ready to go.

  > bonzini_: what's currently holding up getting Claudio's re-factoring
      work merged?
  > f4bug: also ^ - I'm a little worried we have splintering
      re-factoring forks
  *** First activity: f4bug joined 2 hours 8 minutes 6 seconds ago.
  <f4bug> stsquad: the qtests series is pending imammedo review then
      could go in
  <f4bug> stsquad: which would simplify a bit Claudio's series (on
      respin he could drop a pair of patches)
  <f4bug> stsquad: my understanding was bonzini_ would merge the x86
      series, then pm215 could merge the arm one on top
  *** First activity: bonzini_ joined 1 hour 17 minutes 25 seconds ago.
  <bonzini_> ok i can queue it in my next PR
  <f4bug> the only blocking part is qtest not passing, but Claudio's
      refactor series is not the culprit
  <bonzini_> ok

-- 
Alex Bennée

On 5/5/21 12:04 PM, Alex Bennée wrote:
> Claudio Fontana <cfontana@suse.de> writes:
>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>
>>>> Hi,
>>>>
>>>> anything else for me to do here?
>>>
>>> It looks to me that this series is looking pretty good. Every patch has
>>> at least one review so I think it's just waiting on the maintainers to
>>> pick it up.
>>>
>>> Paolo/Richard - are you intending to take the series as is or are you
>>> waiting for something else? I'd like to see the patch delta reduced for
>>> the ARM cleanup work which is still ongoing.
>>
>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>> Rebasing is starting to become more and more draining;
> 
> This is still the latest re-factor right?
> 
>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
> 
>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>> maybe you would like to drive this?
> 
> AIUI his changes where to get qtest passing.

I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3

    meson: Introduce meson_user_arch source set for arch-specific user-mode

    Similarly to the 'target_softmmu_arch' source set which allows
    to restrict target-specific sources to system emulation, add
    the equivalent 'target_user_arch' set for user emulation.

The patch only contains 6 lines in 2 hunks, if it introduced a conflict
it should be trivial to resolve (I wasn't expecting it to conflict with
your work when I merged it TBH).

> I've just been chatting to
> Paolo on IRC so I think we are almost ready to go.
> 
>   > bonzini_: what's currently holding up getting Claudio's re-factoring
>       work merged?
>   > f4bug: also ^ - I'm a little worried we have splintering
>       re-factoring forks
>   *** First activity: f4bug joined 2 hours 8 minutes 6 seconds ago.
>   <f4bug> stsquad: the qtests series is pending imammedo review then
>       could go in
>   <f4bug> stsquad: which would simplify a bit Claudio's series (on
>       respin he could drop a pair of patches)
>   <f4bug> stsquad: my understanding was bonzini_ would merge the x86
>       series, then pm215 could merge the arm one on top
>   *** First activity: bonzini_ joined 1 hour 17 minutes 25 seconds ago.
>   <bonzini_> ok i can queue it in my next PR
>   <f4bug> the only blocking part is qtest not passing, but Claudio's
>       refactor series is not the culprit
>   <bonzini_> ok

I was referring to:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg804015.html

Then these aren't required:
- tests: restrict TCG-only arm-cpu-features tests to TCG builds
- tests: device-introspect-test: cope with ARM TCG-only devices

These could be reworked to use qtest_has_accel() instead of the
/* XXX */ comments:
- tests: do not run test-hmp on all machines for ARM KVM-only
- tests: do not run qom-test on all machines for ARM KVM-only

I didn't noticed the following patch had its content changed:
Revert "target/arm: Restrict v8M IDAU to TCG"
So now this is not a full revert, only the TYPE_IDAU_INTERFACE
type is moved back.

While this fixes the build, we still have a QOM design problem.
QOM interfaces can't be Kconfig-selected out. <- Eduardo, Markus?


More generally I think more code should be automatically stripped
out by Kconfig instead. In [1,2] I suggested to tie accel-specific
Kconfig selectors:

  config ARM_V7R
    bool
    depends on TCG && ARM

  config ARM_V7M
    bool
    depends on TCG && ARM
    select PTIMER

  config XLNX_ZYNQMP_ARM
    bool
    default y if TCG && AARCH64

But this can certainly be done on top of Claudio's work.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777710.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777719.html

On 5/5/21 2:15 PM, Philippe Mathieu-Daudé wrote:
> On 5/5/21 12:04 PM, Alex Bennée wrote:
>> Claudio Fontana <cfontana@suse.de> writes:
>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>
>>>>> Hi,
>>>>>
>>>>> anything else for me to do here?
>>>>
>>>> It looks to me that this series is looking pretty good. Every patch has
>>>> at least one review so I think it's just waiting on the maintainers to
>>>> pick it up.
>>>>
>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>> the ARM cleanup work which is still ongoing.
>>>
>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>> Rebasing is starting to become more and more draining;

> I didn't noticed the following patch had its content changed:
> Revert "target/arm: Restrict v8M IDAU to TCG"
> So now this is not a full revert, only the TYPE_IDAU_INTERFACE
> type is moved back.

Correcting myself. This is a plain revert, and I disagree with it
because if TCG is not available, TCG-specific devices/machines
shouldn't be loaded into QEMU. Cortex-M and v8M IDAU *are* TCG
specific and no such object should be built/registered, so the
interface shouldn't be there neither.

Below is a possible way to get there. Reverting the commit and
build Cortex-M devices in a non-TCG build is a kludge / shortcut
which allows Claudio to remove a big chunk of non-TCG code, so
is probably acceptable, and we can keep cleaning later.

> More generally I think more code should be automatically stripped
> out by Kconfig instead. In [1,2] I suggested to tie accel-specific
> Kconfig selectors:
> 
>   config ARM_V7R
>     bool
>     depends on TCG && ARM
> 
>   config ARM_V7M
>     bool
>     depends on TCG && ARM
>     select PTIMER
> 
>   config XLNX_ZYNQMP_ARM
>     bool
>     default y if TCG && AARCH64
> 
> But this can certainly be done on top of Claudio's work.
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777710.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777719.html
> 

On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/5/21 12:04 PM, Alex Bennée wrote:
> > Claudio Fontana <cfontana@suse.de> writes:
> >> On 3/8/21 3:02 PM, Alex Bennée wrote:
> >>> Claudio Fontana <cfontana@suse.de> writes:
> >>>
> >>>> Hi,
> >>>>
> >>>> anything else for me to do here?
> >>>
> >>> It looks to me that this series is looking pretty good. Every patch has
> >>> at least one review so I think it's just waiting on the maintainers to
> >>> pick it up.
> >>>
> >>> Paolo/Richard - are you intending to take the series as is or are you
> >>> waiting for something else? I'd like to see the patch delta reduced for
> >>> the ARM cleanup work which is still ongoing.
> >>
> >> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
> >> Rebasing is starting to become more and more draining;
> > 
> > This is still the latest re-factor right?
> > 
> >   Subject: [PATCH v28 00/23] i386 cleanup PART 2
> >   Date: Mon, 22 Mar 2021 14:27:36 +0100
> >   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
> > 
> >> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
> >> maybe you would like to drive this?
> > 
> > AIUI his changes where to get qtest passing.
> 
> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
> 
>     meson: Introduce meson_user_arch source set for arch-specific user-mode
> 
>     Similarly to the 'target_softmmu_arch' source set which allows
>     to restrict target-specific sources to system emulation, add
>     the equivalent 'target_user_arch' set for user emulation.
> 
> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
> it should be trivial to resolve (I wasn't expecting it to conflict with
> your work when I merged it TBH).
> 
> > I've just been chatting to
> > Paolo on IRC so I think we are almost ready to go.
> > 
> >   > bonzini_: what's currently holding up getting Claudio's re-factoring
> >       work merged?
> >   > f4bug: also ^ - I'm a little worried we have splintering
> >       re-factoring forks
> >   *** First activity: f4bug joined 2 hours 8 minutes 6 seconds ago.
> >   <f4bug> stsquad: the qtests series is pending imammedo review then
> >       could go in
> >   <f4bug> stsquad: which would simplify a bit Claudio's series (on
> >       respin he could drop a pair of patches)
> >   <f4bug> stsquad: my understanding was bonzini_ would merge the x86
> >       series, then pm215 could merge the arm one on top
> >   *** First activity: bonzini_ joined 1 hour 17 minutes 25 seconds ago.
> >   <bonzini_> ok i can queue it in my next PR
> >   <f4bug> the only blocking part is qtest not passing, but Claudio's
> >       refactor series is not the culprit
> >   <bonzini_> ok
> 
> I was referring to:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg804015.html
> 
> Then these aren't required:
> - tests: restrict TCG-only arm-cpu-features tests to TCG builds
> - tests: device-introspect-test: cope with ARM TCG-only devices
> 
> These could be reworked to use qtest_has_accel() instead of the
> /* XXX */ comments:
> - tests: do not run test-hmp on all machines for ARM KVM-only
> - tests: do not run qom-test on all machines for ARM KVM-only
> 
> I didn't noticed the following patch had its content changed:
> Revert "target/arm: Restrict v8M IDAU to TCG"
> So now this is not a full revert, only the TYPE_IDAU_INTERFACE
> type is moved back.
> 
> While this fixes the build, we still have a QOM design problem.
> QOM interfaces can't be Kconfig-selected out. <- Eduardo, Markus?

I don't get it.  Why exactly QOM interfaces can't be
Kconfig-selected out?

Do you want to be able to compile out an interface while not
compiling out types that implement that interface?  Why?


> 
> 
> More generally I think more code should be automatically stripped
> out by Kconfig instead. In [1,2] I suggested to tie accel-specific
> Kconfig selectors:
> 
>   config ARM_V7R
>     bool
>     depends on TCG && ARM
> 
>   config ARM_V7M
>     bool
>     depends on TCG && ARM
>     select PTIMER
> 
>   config XLNX_ZYNQMP_ARM
>     bool
>     default y if TCG && AARCH64
> 
> But this can certainly be done on top of Claudio's work.
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777710.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777719.html
> 

-- 
Eduardo

On 5/5/21 9:31 PM, Eduardo Habkost wrote:
> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> anything else for me to do here?
>>>>>
>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>> pick it up.
>>>>>
>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>> the ARM cleanup work which is still ongoing.
>>>>
>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>> Rebasing is starting to become more and more draining;
>>>
>>> This is still the latest re-factor right?
>>>
>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>
>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>> maybe you would like to drive this?
>>>
>>> AIUI his changes where to get qtest passing.
>>
>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>
>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>
>>     Similarly to the 'target_softmmu_arch' source set which allows
>>     to restrict target-specific sources to system emulation, add
>>     the equivalent 'target_user_arch' set for user emulation.
>>
>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>> it should be trivial to resolve (I wasn't expecting it to conflict with
>> your work when I merged it TBH).
>>
>>> I've just been chatting to
>>> Paolo on IRC so I think we are almost ready to go.
>>>
>>>   > bonzini_: what's currently holding up getting Claudio's re-factoring
>>>       work merged?
>>>   > f4bug: also ^ - I'm a little worried we have splintering
>>>       re-factoring forks
>>>   *** First activity: f4bug joined 2 hours 8 minutes 6 seconds ago.
>>>   <f4bug> stsquad: the qtests series is pending imammedo review then
>>>       could go in
>>>   <f4bug> stsquad: which would simplify a bit Claudio's series (on
>>>       respin he could drop a pair of patches)
>>>   <f4bug> stsquad: my understanding was bonzini_ would merge the x86
>>>       series, then pm215 could merge the arm one on top
>>>   *** First activity: bonzini_ joined 1 hour 17 minutes 25 seconds ago.
>>>   <bonzini_> ok i can queue it in my next PR
>>>   <f4bug> the only blocking part is qtest not passing, but Claudio's
>>>       refactor series is not the culprit
>>>   <bonzini_> ok
>>
>> I was referring to:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg804015.html
>>
>> Then these aren't required:
>> - tests: restrict TCG-only arm-cpu-features tests to TCG builds
>> - tests: device-introspect-test: cope with ARM TCG-only devices
>>
>> These could be reworked to use qtest_has_accel() instead of the
>> /* XXX */ comments:
>> - tests: do not run test-hmp on all machines for ARM KVM-only
>> - tests: do not run qom-test on all machines for ARM KVM-only
>>
>> I didn't noticed the following patch had its content changed:
>> Revert "target/arm: Restrict v8M IDAU to TCG"
>> So now this is not a full revert, only the TYPE_IDAU_INTERFACE
>> type is moved back.
>>
>> While this fixes the build, we still have a QOM design problem.
>> QOM interfaces can't be Kconfig-selected out. <- Eduardo, Markus?
> 
> I don't get it.  Why exactly QOM interfaces can't be
> Kconfig-selected out?
> 
> Do you want to be able to compile out an interface while not
> compiling out types that implement that interface?  Why?


I'd suggest to move the discussions about the ARM series to the arm series thread.

I am concerned here about the groundwork and x86 part.

Thanks,

Claudio

> 
> 
>>
>>
>> More generally I think more code should be automatically stripped
>> out by Kconfig instead. In [1,2] I suggested to tie accel-specific
>> Kconfig selectors:
>>
>>   config ARM_V7R
>>     bool
>>     depends on TCG && ARM
>>
>>   config ARM_V7M
>>     bool
>>     depends on TCG && ARM
>>     select PTIMER
>>
>>   config XLNX_ZYNQMP_ARM
>>     bool
>>     default y if TCG && AARCH64
>>
>> But this can certainly be done on top of Claudio's work.
>>
>> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777710.html
>> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg777719.html
>>
> 

On 5/6/21 9:55 AM, Claudio Fontana wrote:
> On 5/5/21 9:31 PM, Eduardo Habkost wrote:
>> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> anything else for me to do here?
>>>>>>
>>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>>> pick it up.
>>>>>>
>>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>>> the ARM cleanup work which is still ongoing.
>>>>>
>>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>>> Rebasing is starting to become more and more draining;
>>>>
>>>> This is still the latest re-factor right?
>>>>
>>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>>
>>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>>> maybe you would like to drive this?
>>>>
>>>> AIUI his changes where to get qtest passing.
>>>
>>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>>
>>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>>
>>>     Similarly to the 'target_softmmu_arch' source set which allows
>>>     to restrict target-specific sources to system emulation, add
>>>     the equivalent 'target_user_arch' set for user emulation.
>>>
>>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>>> it should be trivial to resolve (I wasn't expecting it to conflict with
>>> your work when I merged it TBH).

> I'd suggest to move the discussions about the ARM series to the arm series thread.
> 
> I am concerned here about the groundwork and x86 part.

OK sorry, I was explaining the IRC chat.

Is there any issue rebasing the groundwork on top of commit 46369b50ee3?

Maybe my qtest/accel series is irrelevant to your x86 part, TBH I don't
remember.

Regards,

Phil.

On 5/6/21 11:10 AM, Philippe Mathieu-Daudé wrote:
> On 5/6/21 9:55 AM, Claudio Fontana wrote:
>> On 5/5/21 9:31 PM, Eduardo Habkost wrote:
>>> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>>>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> anything else for me to do here?
>>>>>>>
>>>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>>>> pick it up.
>>>>>>>
>>>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>>>> the ARM cleanup work which is still ongoing.
>>>>>>
>>>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>>>> Rebasing is starting to become more and more draining;
>>>>>
>>>>> This is still the latest re-factor right?
>>>>>
>>>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>>>
>>>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>>>> maybe you would like to drive this?
>>>>>
>>>>> AIUI his changes where to get qtest passing.
>>>>
>>>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>>>
>>>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>>>
>>>>     Similarly to the 'target_softmmu_arch' source set which allows
>>>>     to restrict target-specific sources to system emulation, add
>>>>     the equivalent 'target_user_arch' set for user emulation.
>>>>
>>>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>>>> it should be trivial to resolve (I wasn't expecting it to conflict with
>>>> your work when I merged it TBH).
> 
>> I'd suggest to move the discussions about the ARM series to the arm series thread.
>>
>> I am concerned here about the groundwork and x86 part.
> 
> OK sorry, I was explaining the IRC chat.
> 
> Is there any issue rebasing the groundwork on top of commit 46369b50ee3?
> 
> Maybe my qtest/accel series is irrelevant to your x86 part, TBH I don't
> remember.

So far I could rebase this series on top of commit 229a834518b
(Mon Mar 8 15:45:48 2021).

First trivial conflict with 2cc1a90166b ("Remove deprecated
target tilegx"), removing target/tilegx/meson.build resolves it.

Then apparently commit b8184135835 ("target/i386: allow modifying
TCG phys-addr-bits") which is not trivial (to me).

Finally "meson: add target_user_arch" indeed clashes with commit
46369b50ee3 ("meson: Introduce meson_user_arch source set for
arch-specific user-mode"). I am sorry I didn't notice your patch,
but am glad we both sent a similar patch :) Mine allows optional
sourceset, so you can simplify your patch, only keeping the x86
part:

-- >8 --
diff --git a/target/i386/meson.build b/target/i386/meson.build
index fd24479590..cac26a4581 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -19,6 +19,7 @@ i386_softmmu_ss.add(files(
   'machine.c',
   'monitor.c',
 ))
+i386_user_ss = ss.source_set()

 subdir('kvm')
 subdir('hax')
@@ -28,3 +29,4 @@ subdir('tcg')

 target_arch += {'i386': i386_ss}
 target_softmmu_arch += {'i386': i386_softmmu_ss}
+target_user_arch += {'i386': i386_user_ss}
---

Regards,

Phil.

On 5/6/21 11:33 AM, Philippe Mathieu-Daudé wrote:
> On 5/6/21 11:10 AM, Philippe Mathieu-Daudé wrote:
>> On 5/6/21 9:55 AM, Claudio Fontana wrote:
>>> On 5/5/21 9:31 PM, Eduardo Habkost wrote:
>>>> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> anything else for me to do here?
>>>>>>>>
>>>>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>>>>> pick it up.
>>>>>>>>
>>>>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>>>>> the ARM cleanup work which is still ongoing.
>>>>>>>
>>>>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>>>>> Rebasing is starting to become more and more draining;
>>>>>>
>>>>>> This is still the latest re-factor right?
>>>>>>
>>>>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>>>>
>>>>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>>>>> maybe you would like to drive this?
>>>>>>
>>>>>> AIUI his changes where to get qtest passing.
>>>>>
>>>>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>>>>
>>>>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>>>>
>>>>>     Similarly to the 'target_softmmu_arch' source set which allows
>>>>>     to restrict target-specific sources to system emulation, add
>>>>>     the equivalent 'target_user_arch' set for user emulation.
>>>>>
>>>>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>>>>> it should be trivial to resolve (I wasn't expecting it to conflict with
>>>>> your work when I merged it TBH).
>>
>>> I'd suggest to move the discussions about the ARM series to the arm series thread.
>>>
>>> I am concerned here about the groundwork and x86 part.
>>
>> OK sorry, I was explaining the IRC chat.
>>
>> Is there any issue rebasing the groundwork on top of commit 46369b50ee3?
>>
>> Maybe my qtest/accel series is irrelevant to your x86 part, TBH I don't
>> remember.
> 
> So far I could rebase this series on top of commit 229a834518b
> (Mon Mar 8 15:45:48 2021).

Pfff I realized late you asked on the v26 series, but v28 is
available... Let's try again.

> First trivial conflict with 2cc1a90166b ("Remove deprecated
> target tilegx"), removing target/tilegx/meson.build resolves it.
> 
> Then apparently commit b8184135835 ("target/i386: allow modifying
> TCG phys-addr-bits") which is not trivial (to me).
> 
> Finally "meson: add target_user_arch" indeed clashes with commit
> 46369b50ee3 ("meson: Introduce meson_user_arch source set for
> arch-specific user-mode"). I am sorry I didn't notice your patch,
> but am glad we both sent a similar patch :) Mine allows optional
> sourceset, so you can simplify your patch, only keeping the x86
> part:
> 
> -- >8 --
> diff --git a/target/i386/meson.build b/target/i386/meson.build
> index fd24479590..cac26a4581 100644
> --- a/target/i386/meson.build
> +++ b/target/i386/meson.build
> @@ -19,6 +19,7 @@ i386_softmmu_ss.add(files(
>    'machine.c',
>    'monitor.c',
>  ))
> +i386_user_ss = ss.source_set()
> 
>  subdir('kvm')
>  subdir('hax')
> @@ -28,3 +29,4 @@ subdir('tcg')
> 
>  target_arch += {'i386': i386_ss}
>  target_softmmu_arch += {'i386': i386_softmmu_ss}
> +target_user_arch += {'i386': i386_user_ss}
> ---
> 
> Regards,
> 
> Phil.
> 

On 5/12/21 9:17 AM, Philippe Mathieu-Daudé wrote:
> On 5/6/21 11:33 AM, Philippe Mathieu-Daudé wrote:
>> On 5/6/21 11:10 AM, Philippe Mathieu-Daudé wrote:
>>> On 5/6/21 9:55 AM, Claudio Fontana wrote:
>>>> On 5/5/21 9:31 PM, Eduardo Habkost wrote:
>>>>> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>>>>>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> anything else for me to do here?
>>>>>>>>>
>>>>>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>>>>>> pick it up.
>>>>>>>>>
>>>>>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>>>>>> the ARM cleanup work which is still ongoing.
>>>>>>>>
>>>>>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>>>>>> Rebasing is starting to become more and more draining;
>>>>>>>
>>>>>>> This is still the latest re-factor right?
>>>>>>>
>>>>>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>>>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>>>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>>>>>
>>>>>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>>>>>> maybe you would like to drive this?
>>>>>>>
>>>>>>> AIUI his changes where to get qtest passing.
>>>>>>
>>>>>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>>>>>
>>>>>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>>>>>
>>>>>>     Similarly to the 'target_softmmu_arch' source set which allows
>>>>>>     to restrict target-specific sources to system emulation, add
>>>>>>     the equivalent 'target_user_arch' set for user emulation.
>>>>>>
>>>>>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>>>>>> it should be trivial to resolve (I wasn't expecting it to conflict with
>>>>>> your work when I merged it TBH).
>>>
>>>> I'd suggest to move the discussions about the ARM series to the arm series thread.
>>>>
>>>> I am concerned here about the groundwork and x86 part.
>>>
>>> OK sorry, I was explaining the IRC chat.
>>>
>>> Is there any issue rebasing the groundwork on top of commit 46369b50ee3?
>>>
>>> Maybe my qtest/accel series is irrelevant to your x86 part, TBH I don't
>>> remember.
>>
>> So far I could rebase this series on top of commit 229a834518b
>> (Mon Mar 8 15:45:48 2021).
> 
> Pfff I realized late you asked on the v26 series, but v28 is
> available... Let's try again.

Hi Philippe,

I think that Paolo picked it up in the end?

Thanks,

Claudio

> 
>> First trivial conflict with 2cc1a90166b ("Remove deprecated
>> target tilegx"), removing target/tilegx/meson.build resolves it.
>>
>> Then apparently commit b8184135835 ("target/i386: allow modifying
>> TCG phys-addr-bits") which is not trivial (to me).
>>
>> Finally "meson: add target_user_arch" indeed clashes with commit
>> 46369b50ee3 ("meson: Introduce meson_user_arch source set for
>> arch-specific user-mode"). I am sorry I didn't notice your patch,
>> but am glad we both sent a similar patch :) Mine allows optional
>> sourceset, so you can simplify your patch, only keeping the x86
>> part:
>>
>> -- >8 --
>> diff --git a/target/i386/meson.build b/target/i386/meson.build
>> index fd24479590..cac26a4581 100644
>> --- a/target/i386/meson.build
>> +++ b/target/i386/meson.build
>> @@ -19,6 +19,7 @@ i386_softmmu_ss.add(files(
>>    'machine.c',
>>    'monitor.c',
>>  ))
>> +i386_user_ss = ss.source_set()
>>
>>  subdir('kvm')
>>  subdir('hax')
>> @@ -28,3 +29,4 @@ subdir('tcg')
>>
>>  target_arch += {'i386': i386_ss}
>>  target_softmmu_arch += {'i386': i386_softmmu_ss}
>> +target_user_arch += {'i386': i386_user_ss}
>> ---
>>
>> Regards,
>>
>> Phil.
>>
> 
> 

On 5/17/21 8:17 AM, Claudio Fontana wrote:
> On 5/12/21 9:17 AM, Philippe Mathieu-Daudé wrote:
>> On 5/6/21 11:33 AM, Philippe Mathieu-Daudé wrote:
>>> On 5/6/21 11:10 AM, Philippe Mathieu-Daudé wrote:
>>>> On 5/6/21 9:55 AM, Claudio Fontana wrote:
>>>>> On 5/5/21 9:31 PM, Eduardo Habkost wrote:
>>>>>> On Wed, May 05, 2021 at 02:15:29PM +0200, Philippe Mathieu-Daudé wrote:
>>>>>>> On 5/5/21 12:04 PM, Alex Bennée wrote:
>>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>>> On 3/8/21 3:02 PM, Alex Bennée wrote:
>>>>>>>>>> Claudio Fontana <cfontana@suse.de> writes:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> anything else for me to do here?
>>>>>>>>>>
>>>>>>>>>> It looks to me that this series is looking pretty good. Every patch has
>>>>>>>>>> at least one review so I think it's just waiting on the maintainers to
>>>>>>>>>> pick it up.
>>>>>>>>>>
>>>>>>>>>> Paolo/Richard - are you intending to take the series as is or are you
>>>>>>>>>> waiting for something else? I'd like to see the patch delta reduced for
>>>>>>>>>> the ARM cleanup work which is still ongoing.
>>>>>>>>>
>>>>>>>>> I am a bit at a loss here, as this has been reviewed for a while, but nothing is happening.
>>>>>>>>> Rebasing is starting to become more and more draining;
>>>>>>>>
>>>>>>>> This is still the latest re-factor right?
>>>>>>>>
>>>>>>>>   Subject: [PATCH v28 00/23] i386 cleanup PART 2
>>>>>>>>   Date: Mon, 22 Mar 2021 14:27:36 +0100
>>>>>>>>   Message-Id: <20210322132800.7470-1-cfontana@suse.de>
>>>>>>>>
>>>>>>>>> I am seeing some changes from Phil that redo some of the patches here (like target arch user),
>>>>>>>>> maybe you would like to drive this?
>>>>>>>>
>>>>>>>> AIUI his changes where to get qtest passing.
>>>>>>>
>>>>>>> I hadn't read Claudio's mail, I think he's mentioning commit 46369b50ee3
>>>>>>>
>>>>>>>     meson: Introduce meson_user_arch source set for arch-specific user-mode
>>>>>>>
>>>>>>>     Similarly to the 'target_softmmu_arch' source set which allows
>>>>>>>     to restrict target-specific sources to system emulation, add
>>>>>>>     the equivalent 'target_user_arch' set for user emulation.
>>>>>>>
>>>>>>> The patch only contains 6 lines in 2 hunks, if it introduced a conflict
>>>>>>> it should be trivial to resolve (I wasn't expecting it to conflict with
>>>>>>> your work when I merged it TBH).
>>>>
>>>>> I'd suggest to move the discussions about the ARM series to the arm series thread.
>>>>>
>>>>> I am concerned here about the groundwork and x86 part.
>>>>
>>>> OK sorry, I was explaining the IRC chat.
>>>>
>>>> Is there any issue rebasing the groundwork on top of commit 46369b50ee3?
>>>>
>>>> Maybe my qtest/accel series is irrelevant to your x86 part, TBH I don't
>>>> remember.
>>>
>>> So far I could rebase this series on top of commit 229a834518b
>>> (Mon Mar 8 15:45:48 2021).
>>
>> Pfff I realized late you asked on the v26 series, but v28 is
>> available... Let's try again.
> 
> Hi Philippe,
> 
> I think that Paolo picked it up in the end?

Fortunately yes (for you; unfortunately for me... I thought you
pinged the latest series and tried to rebase it, and Paolo comment
on your v28 was "Looks good to me, thanks for all the effort!"
which was not obvious to me the series was already queued).

Anyway we are done with that part for good now, thanks.
Yes,