Looks good to me, thanks for all the effort!
Paolo
On 22/03/21 14:27, Claudio Fontana wrote:
> v27 -> v28:
>
> * rebased on latest master;
> I indicated the conflicts for the affected patches in the commit message,
> in case a new review/eye is required.
>
> * added three patches:
> - accel: move call to accel_init_interfaces
>
> This matches more closely the initialization phases definitions (Paolo)
>
> - accel: add init_accel_cpu for adapting accel behavior to cpu type
>
> This in particular is useful for ARM, that needs different TCG behavior
> depending on the CPU subclass.
>
> - XXX RFC accel: add cpu_reset
>
> This adds an accel cpu behavior to execute after CPU reset.
> This can be used on x86, arm, s390x, mips for KVM and TCG.
>
> The RFC nature of this has to do with the fact that cpu_reset() remains
> in hw/core/cpu.c , which is common_ss,
> and cpu_reset() calls accel_cpu_reset() which is in specific_ss.
>
> So it seems weird that this builds fine, and all tests seem to pass,
> without using a specific_ss call.
>
>
> v26 -> v27: rebased on latest master
>
>
> 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 (21):
> 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
> accel: move call to accel_init_interfaces
> accel: add init_accel_cpu for adapting accel behavior to CPU type
> XXX RFC accel: add cpu_reset
>
> 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 | 4 +-
> include/hw/core/cpu.h | 6 +
> include/qemu/accel.h | 19 +
> 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 | 41 ++
> bsd-user/main.c | 2 +-
> cpu.c | 5 +-
> hw/core/cpu.c | 3 +-
> hw/core/machine.c | 1 +
> hw/i386/pc_piix.c | 1 +
> linux-user/main.c | 2 +-
> softmmu/vl.c | 1 -
> target/i386/cpu-sysemu.c | 352 ++++++++++
> target/i386/cpu.c | 779 ++--------------------
> 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 | 157 +++++
> target/i386/kvm/kvm.c | 3 +-
> target/i386/tcg/bpt_helper.c | 276 --------
> target/i386/tcg/excp_helper.c | 573 ----------------
> target/i386/tcg/fpu_helper.c | 122 ++--
> target/i386/tcg/misc_helper.c | 467 -------------
> target/i386/tcg/seg_helper.c | 237 +------
> target/i386/tcg/sysemu/bpt_helper.c | 293 ++++++++
> target/i386/tcg/sysemu/excp_helper.c | 582 ++++++++++++++++
> target/i386/tcg/sysemu/fpu_helper.c | 57 ++
> target/i386/tcg/sysemu/misc_helper.c | 442 ++++++++++++
> 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 | 56 +-
> 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 | 2 +
> 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/tricore/meson.build | 3 +
> target/xtensa/meson.build | 3 +
> 71 files changed, 3321 insertions(+), 2584 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
>