mbox series

[RESEND,v5,00/28] target/arm: Allow CONFIG_TCG=n builds

Message ID 20230213202927.28992-1-farosas@suse.de (mailing list archive)
Headers show
Series target/arm: Allow CONFIG_TCG=n builds | expand

Message

Fabiano Rosas Feb. 13, 2023, 8:28 p.m. UTC
Hi, could someone take this?

I rebased and put the two series together to simplify. I'm keeping it
up-to-date and tested while it doesn't go in.

CI run here: https://gitlab.com/farosas/qemu/-/pipelines/776103500

Based on master.
Supersedes "target/arm: CONFIG_TCG=n part 1".

Thanks!

v5:
https://lore.kernel.org/r/20230120184825.31626-1-farosas@suse.de

This series makes the necessary changes to allow the use of
--disable-tcg for arm.

- Used "max" as the default CPU for KVM-only builds. This allows me to
  drop all the clunky qtest changes and it keeps disabling TCG
  separate from changing cpu defaults.

  I'm neutral towards removing the defaults for arm. We can do that in a
  separate series. It would be nice to make the TCG default equal to the
  non-TCG one. Otherwise we're bound to get reports that "this command
  line used to work" if users switch from: 'CONFIG_TCG=n -accel kvm' to
  'CONFIG_TCG=y -accel kvm' (the latter would try to use the cortex-a15
  as default).

- Move the ifdef around valid_cpus into the patches that move the
  respective cpus. Patches 1 & 2.

v5 was based on "target/arm: CONFIG_TCG=n part 1":
https://lore.kernel.org/r/20230118193518.26433-1-farosas@suse.de

v4:
https://lore.kernel.org/r/20230119135424.5417-1-farosas@suse.de

v3:
https://lore.kernel.org/r/20230113140419.4013-1-farosas@suse.de

v2:
https://lore.kernel.org/r/20230109224232.11661-1-farosas@suse.de

v1:
https://lore.kernel.org/r/20230104215835.24692-1-farosas@suse.de

Claudio Fontana (6):
  target/arm: rename handle_semihosting to tcg_handle_semihosting
  target/arm: wrap psci call with tcg_enabled
  target/arm: wrap call to aarch64_sve_change_el in tcg_enabled()
  target/arm: move helpers to tcg/
  target/arm: Move psci.c into the tcg directory
  target/arm: move cpu_tcg to tcg/cpu32.c

Fabiano Rosas (21):
  target/arm: Move PC alignment check
  target/arm: Move cpregs code out of cpu.h
  target/arm: Move cpregs code into cpregs.c
  target/arm: Move define_debug_regs() to cpregs.c
  target/arm: Wrap breakpoint/watchpoint updates with tcg_enabled
  target/arm: move translate modules to tcg/
  target/arm: Wrap arm_rebuild_hflags calls with tcg_enabled
  target/arm: Move hflags code into the tcg directory
  target/arm: Move regime_using_lpae_format into internal.h
  target/arm: Don't access TCG code when debugging with KVM
  cpu-defs.h: Expose CPUTLBEntryFull to non-TCG code
  target/arm: Move cortex sysregs into cpregs.c
  tests/avocado: Skip tests that require a missing accelerator
  tests/avocado: Tag TCG tests with accel:tcg
  target/arm: Move 64-bit TCG CPUs into tcg/
  target/arm: Use "max" as default cpu for the virt machine with KVM
  tests/qtest: arm-cpu-features: Match tests to required accelerators
  tests/qtest: Restrict tpm-tis-devices-{swtpm}-test to CONFIG_TCG
  target/avocado: Pass parameters to migration test on aarch64
  arm/Kconfig: Always select SEMIHOSTING when TCG is present
  arm/Kconfig: Do not build TCG-only boards on a KVM-only build

Philippe Mathieu-Daudé (1):
  gitlab-ci: Check building KVM-only aarch64 target

 .gitlab-ci.d/crossbuilds.yml                  |    11 +
 .../custom-runners/ubuntu-22.04-aarch64.yml   |     4 -
 MAINTAINERS                                   |     1 +
 configs/devices/aarch64-softmmu/default.mak   |     4 -
 configs/devices/arm-softmmu/default.mak       |    39 -
 hw/arm/Kconfig                                |    43 +-
 hw/arm/boot.c                                 |     6 +-
 hw/arm/virt.c                                 |    10 +-
 hw/intc/armv7m_nvic.c                         |    20 +-
 include/exec/cpu-defs.h                       |     6 +
 target/arm/Kconfig                            |     7 +
 target/arm/arm-powerctl.c                     |     7 +-
 target/arm/cpregs.c                           |  9825 +++++++++++++++
 target/arm/cpregs.h                           |   104 +
 target/arm/cpu.c                              |     9 +-
 target/arm/cpu.h                              |    91 -
 target/arm/cpu64.c                            |   634 +-
 target/arm/helper.c                           | 10224 +---------------
 target/arm/internals.h                        |    42 +-
 target/arm/machine.c                          |    30 +-
 target/arm/meson.build                        |    49 +-
 target/arm/ptw.c                              |     4 +
 target/arm/tcg-stubs.c                        |    27 +
 target/arm/{ => tcg}/a32-uncond.decode        |     0
 target/arm/{ => tcg}/a32.decode               |     0
 target/arm/{cpu_tcg.c => tcg/cpu32.c}         |    72 +-
 target/arm/tcg/cpu64.c                        |   655 +
 target/arm/{ => tcg}/crypto_helper.c          |     0
 target/arm/{ => tcg}/debug_helper.c           |   459 -
 target/arm/{ => tcg}/helper-a64.c             |     0
 target/arm/tcg/hflags.c                       |   403 +
 target/arm/{ => tcg}/iwmmxt_helper.c          |     0
 target/arm/{ => tcg}/m-nocp.decode            |     0
 target/arm/{ => tcg}/m_helper.c               |     0
 target/arm/tcg/meson.build                    |    53 +
 target/arm/{ => tcg}/mte_helper.c             |     0
 target/arm/{ => tcg}/mve.decode               |     0
 target/arm/{ => tcg}/mve_helper.c             |     0
 target/arm/{ => tcg}/neon-dp.decode           |     0
 target/arm/{ => tcg}/neon-ls.decode           |     0
 target/arm/{ => tcg}/neon-shared.decode       |     0
 target/arm/{ => tcg}/neon_helper.c            |     0
 target/arm/{ => tcg}/op_helper.c              |     0
 target/arm/{ => tcg}/pauth_helper.c           |     0
 target/arm/{ => tcg}/psci.c                   |     0
 target/arm/{ => tcg}/sme-fa64.decode          |     0
 target/arm/{ => tcg}/sme.decode               |     0
 target/arm/{ => tcg}/sme_helper.c             |     0
 target/arm/{ => tcg}/sve.decode               |     0
 target/arm/{ => tcg}/sve_helper.c             |     0
 target/arm/{ => tcg}/t16.decode               |     0
 target/arm/{ => tcg}/t32.decode               |     0
 target/arm/{ => tcg}/tlb_helper.c             |    18 -
 target/arm/{ => tcg}/translate-a64.c          |     0
 target/arm/{ => tcg}/translate-a64.h          |     0
 target/arm/{ => tcg}/translate-m-nocp.c       |     0
 target/arm/{ => tcg}/translate-mve.c          |     0
 target/arm/{ => tcg}/translate-neon.c         |     0
 target/arm/{ => tcg}/translate-sme.c          |     0
 target/arm/{ => tcg}/translate-sve.c          |     0
 target/arm/{ => tcg}/translate-vfp.c          |     0
 target/arm/{ => tcg}/translate.c              |     0
 target/arm/{ => tcg}/translate.h              |     0
 target/arm/{ => tcg}/vec_helper.c             |     0
 target/arm/{ => tcg}/vec_internal.h           |     0
 target/arm/{ => tcg}/vfp-uncond.decode        |     0
 target/arm/{ => tcg}/vfp.decode               |     0
 target/arm/trace-events                       |     2 +-
 tests/avocado/avocado_qemu/__init__.py        |     4 +
 tests/avocado/boot_linux_console.py           |     1 +
 tests/avocado/migration.py                    |    10 +
 tests/avocado/reverse_debugging.py            |     8 +
 tests/qtest/arm-cpu-features.c                |    34 +-
 tests/qtest/meson.build                       |     4 +-
 74 files changed, 11576 insertions(+), 11344 deletions(-)
 create mode 100644 target/arm/cpregs.c
 create mode 100644 target/arm/tcg-stubs.c
 rename target/arm/{ => tcg}/a32-uncond.decode (100%)
 rename target/arm/{ => tcg}/a32.decode (100%)
 rename target/arm/{cpu_tcg.c => tcg/cpu32.c} (93%)
 create mode 100644 target/arm/tcg/cpu64.c
 rename target/arm/{ => tcg}/crypto_helper.c (100%)
 rename target/arm/{ => tcg}/debug_helper.c (57%)
 rename target/arm/{ => tcg}/helper-a64.c (100%)
 create mode 100644 target/arm/tcg/hflags.c
 rename target/arm/{ => tcg}/iwmmxt_helper.c (100%)
 rename target/arm/{ => tcg}/m-nocp.decode (100%)
 rename target/arm/{ => tcg}/m_helper.c (100%)
 create mode 100644 target/arm/tcg/meson.build
 rename target/arm/{ => tcg}/mte_helper.c (100%)
 rename target/arm/{ => tcg}/mve.decode (100%)
 rename target/arm/{ => tcg}/mve_helper.c (100%)
 rename target/arm/{ => tcg}/neon-dp.decode (100%)
 rename target/arm/{ => tcg}/neon-ls.decode (100%)
 rename target/arm/{ => tcg}/neon-shared.decode (100%)
 rename target/arm/{ => tcg}/neon_helper.c (100%)
 rename target/arm/{ => tcg}/op_helper.c (100%)
 rename target/arm/{ => tcg}/pauth_helper.c (100%)
 rename target/arm/{ => tcg}/psci.c (100%)
 rename target/arm/{ => tcg}/sme-fa64.decode (100%)
 rename target/arm/{ => tcg}/sme.decode (100%)
 rename target/arm/{ => tcg}/sme_helper.c (100%)
 rename target/arm/{ => tcg}/sve.decode (100%)
 rename target/arm/{ => tcg}/sve_helper.c (100%)
 rename target/arm/{ => tcg}/t16.decode (100%)
 rename target/arm/{ => tcg}/t32.decode (100%)
 rename target/arm/{ => tcg}/tlb_helper.c (94%)
 rename target/arm/{ => tcg}/translate-a64.c (100%)
 rename target/arm/{ => tcg}/translate-a64.h (100%)
 rename target/arm/{ => tcg}/translate-m-nocp.c (100%)
 rename target/arm/{ => tcg}/translate-mve.c (100%)
 rename target/arm/{ => tcg}/translate-neon.c (100%)
 rename target/arm/{ => tcg}/translate-sme.c (100%)
 rename target/arm/{ => tcg}/translate-sve.c (100%)
 rename target/arm/{ => tcg}/translate-vfp.c (100%)
 rename target/arm/{ => tcg}/translate.c (100%)
 rename target/arm/{ => tcg}/translate.h (100%)
 rename target/arm/{ => tcg}/vec_helper.c (100%)
 rename target/arm/{ => tcg}/vec_internal.h (100%)
 rename target/arm/{ => tcg}/vfp-uncond.decode (100%)
 rename target/arm/{ => tcg}/vfp.decode (100%)

Comments

Peter Maydell Feb. 16, 2023, 4:19 p.m. UTC | #1
On Mon, 13 Feb 2023 at 20:31, Fabiano Rosas <farosas@suse.de> wrote:
>
> Hi, could someone take this?
>
> I rebased and put the two series together to simplify. I'm keeping it
> up-to-date and tested while it doesn't go in.
>
> CI run here: https://gitlab.com/farosas/qemu/-/pipelines/776103500
>
> Based on master.
> Supersedes "target/arm: CONFIG_TCG=n part 1".

I've taken patches 1-5 into target-arm.next.
Patch 6 doesn't apply on top of my tree, which is unsurprising
given that it's trying to move 9000+ lines of code in a single
patch. That's so much I couldn't even figure out how to recreate
the code movement locally. The diff is not very legible either.

I think that patch needs to be restructured somehow so that
it's a bit clearer that it's only doing code movement and a
bit more likely that it might be possible to apply it without
massive conflicts.

thanks
-- PMM
Peter Maydell Feb. 16, 2023, 4:30 p.m. UTC | #2
On Thu, 16 Feb 2023 at 16:19, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 13 Feb 2023 at 20:31, Fabiano Rosas <farosas@suse.de> wrote:
> >
> > Hi, could someone take this?
> >
> > I rebased and put the two series together to simplify. I'm keeping it
> > up-to-date and tested while it doesn't go in.
> >
> > CI run here: https://gitlab.com/farosas/qemu/-/pipelines/776103500
> >
> > Based on master.
> > Supersedes "target/arm: CONFIG_TCG=n part 1".
>
> I've taken patches 1-5 into target-arm.next.

I've also taken patches 18, 19, 22, 23, 24, which seemed like
straightforward test fixes.

thanks
-- PMM
Fabiano Rosas Feb. 16, 2023, 4:47 p.m. UTC | #3
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 13 Feb 2023 at 20:31, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> Hi, could someone take this?
>>
>> I rebased and put the two series together to simplify. I'm keeping it
>> up-to-date and tested while it doesn't go in.
>>
>> CI run here: https://gitlab.com/farosas/qemu/-/pipelines/776103500
>>
>> Based on master.
>> Supersedes "target/arm: CONFIG_TCG=n part 1".
>
> I've taken patches 1-5 into target-arm.next.
> Patch 6 doesn't apply on top of my tree, which is unsurprising
> given that it's trying to move 9000+ lines of code in a single
> patch. That's so much I couldn't even figure out how to recreate
> the code movement locally. The diff is not very legible either.

Ugh, sorry about that. I forgot to apply --patience on this last resend.
That diff should be just a sequence of 9k '-' lines and 9k '+' lines.

I'll try to split it up, let's see how it goes, the ARMCPRegInfo arrays
are huge, they might end up pulling everything with them.
Peter Maydell Feb. 16, 2023, 5:22 p.m. UTC | #4
On Thu, 16 Feb 2023 at 16:47, Fabiano Rosas <farosas@suse.de> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 13 Feb 2023 at 20:31, Fabiano Rosas <farosas@suse.de> wrote:
> >>
> >> Hi, could someone take this?
> >>
> >> I rebased and put the two series together to simplify. I'm keeping it
> >> up-to-date and tested while it doesn't go in.
> >>
> >> CI run here: https://gitlab.com/farosas/qemu/-/pipelines/776103500
> >>
> >> Based on master.
> >> Supersedes "target/arm: CONFIG_TCG=n part 1".
> >
> > I've taken patches 1-5 into target-arm.next.
> > Patch 6 doesn't apply on top of my tree, which is unsurprising
> > given that it's trying to move 9000+ lines of code in a single
> > patch. That's so much I couldn't even figure out how to recreate
> > the code movement locally. The diff is not very legible either.
>
> Ugh, sorry about that. I forgot to apply --patience on this last resend.
> That diff should be just a sequence of 9k '-' lines and 9k '+' lines.
>
> I'll try to split it up, let's see how it goes, the ARMCPRegInfo arrays
> are huge, they might end up pulling everything with them.

If there are opportunities here to split some of the different
categories of cpreg into their own file, the way we've done
already with the debug registers, that might be a way to help
structure it. For instance the PMU registers could probably
all live in their own file, and the generic-timer regs similarly.

thanks
-- PMM
Richard Henderson Feb. 16, 2023, 7:21 p.m. UTC | #5
On 2/16/23 06:47, Fabiano Rosas wrote:
> I'll try to split it up, let's see how it goes, the ARMCPRegInfo arrays
> are huge, they might end up pulling everything with them.

Feel free to make static functions non-static in e.g. internals.h or cpregs.h as necessary 
to allow an array to be moved without all of the helper functions at the same time.


r~