[PATCH v4 00/22] target/ppc: Remove hidden usages of *env

Víctor Colombo posted 22 patches 1 year, 11 months ago
Failed in applying to current master (apply log)
hw/ppc/pegasos2.c        |   2 +-
hw/ppc/spapr.c           |   2 +-
target/ppc/cpu.c         |   2 +-
target/ppc/cpu.h         | 220 ++++++++++++++++++---------------------
target/ppc/cpu_init.c    |  23 ++--
target/ppc/excp_helper.c |  54 +++++-----
target/ppc/fpu_helper.c  |  28 ++---
target/ppc/gdbstub.c     |   2 +-
target/ppc/helper_regs.c |  15 ++-
target/ppc/kvm.c         |   7 +-
target/ppc/machine.c     |   2 +-
target/ppc/mem_helper.c  |  23 ++--
target/ppc/misc_helper.c |   2 +-
target/ppc/mmu-radix64.c |  11 +-
target/ppc/mmu_common.c  |  40 +++----
target/ppc/mmu_helper.c  |   6 +-
16 files changed, 220 insertions(+), 219 deletions(-)
[PATCH v4 00/22] target/ppc: Remove hidden usages of *env
Posted by Víctor Colombo 1 year, 11 months ago
By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'`
we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't
take *env as a parameter.

Richard Henderson said [1] that these macros hiding the usage of *env "are evil".
This patch series remove them and substitute with an explicit usage of *env by
using registerfields API.

Patch 20 (target/ppc: Add unused msr bits FIELDs) declares unused FIELDs, the
same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I
did that to keep the changes consistent with what was already present before.

Patch 21 (target/ppc: Change MSR_* to follow POWER ISA numbering convention)
changes the MSR_* bit number to match POWER ISA by adding a new macro to
'invert' the ordering. (added in v2)

[1]: https://lists.gnu.org/archive/html/qemu-ppc/2021-11/msg00280.html

Sending a v4 because another patch queued in ppc-next added an usage of
msr_de

v2:
- Abandon the ideia to add an M_MSR_* macro
- Instead, use registerfields API as suggested by Richard
- Add patch 21 to invert MSR_* values to match ISA ordering

v3:
- Add macro to extract both FE0 and FE1. Use it to simplify the
  conditionals in patch 17
- Fix the checks that should be a xor
- Fix incorrect parameter in FIELD_EX64 (was env->msr should be value)
  in patch 16
- Fix patch 13 title

v4:
- Rebase on top of target/ppc: Fix BookE debug interrupt generation
- Add patch target/ppc: Remove msr_de macro. msr_de was not being used
  anywhere before, but an usage was added in the patch
  target/ppc: Fix BookE debug interrupt generation
- Necessary changes in other patches:
  dont remove msr_de in patch 2
  context change in other patches


Víctor Colombo (22):
  target/ppc: Remove fpscr_* macros from cpu.h
  target/ppc: Remove unused msr_* macros
  target/ppc: Remove msr_pr macro
  target/ppc: Remove msr_le macro
  target/ppc: Remove msr_ds macro
  target/ppc: Remove msr_ile macro
  target/ppc: Remove msr_ee macro
  target/ppc: Remove msr_ce macro
  target/ppc: Remove msr_pow macro
  target/ppc: Remove msr_me macro
  target/ppc: Remove msr_gs macro
  target/ppc: Remove msr_fp macro
  target/ppc: Remove msr_cm macro
  target/ppc: Remove msr_ir macro
  target/ppc: Remove msr_dr macro
  target/ppc: Remove msr_ep macro
  target/ppc: Remove msr_fe0 and msr_fe1 macros
  target/ppc: Remove msr_ts macro
  target/ppc: Remove msr_hv macro
  target/ppc: Remove msr_de macro
  target/ppc: Add unused msr bits FIELDs
  target/ppc: Change MSR_* to follow POWER ISA numbering convention

 hw/ppc/pegasos2.c        |   2 +-
 hw/ppc/spapr.c           |   2 +-
 target/ppc/cpu.c         |   2 +-
 target/ppc/cpu.h         | 220 ++++++++++++++++++---------------------
 target/ppc/cpu_init.c    |  23 ++--
 target/ppc/excp_helper.c |  54 +++++-----
 target/ppc/fpu_helper.c  |  28 ++---
 target/ppc/gdbstub.c     |   2 +-
 target/ppc/helper_regs.c |  15 ++-
 target/ppc/kvm.c         |   7 +-
 target/ppc/machine.c     |   2 +-
 target/ppc/mem_helper.c  |  23 ++--
 target/ppc/misc_helper.c |   2 +-
 target/ppc/mmu-radix64.c |  11 +-
 target/ppc/mmu_common.c  |  40 +++----
 target/ppc/mmu_helper.c  |   6 +-
 16 files changed, 220 insertions(+), 219 deletions(-)

-- 
2.25.1


Re: [PATCH v4 00/22] target/ppc: Remove hidden usages of *env
Posted by Daniel Henrique Barboza 1 year, 11 months ago
Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 5/4/22 18:05, Víctor Colombo wrote:
> By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'`
> we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't
> take *env as a parameter.
> 
> Richard Henderson said [1] that these macros hiding the usage of *env "are evil".
> This patch series remove them and substitute with an explicit usage of *env by
> using registerfields API.
> 
> Patch 20 (target/ppc: Add unused msr bits FIELDs) declares unused FIELDs, the
> same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I
> did that to keep the changes consistent with what was already present before.
> 
> Patch 21 (target/ppc: Change MSR_* to follow POWER ISA numbering convention)
> changes the MSR_* bit number to match POWER ISA by adding a new macro to
> 'invert' the ordering. (added in v2)
> 
> [1]: https://lists.gnu.org/archive/html/qemu-ppc/2021-11/msg00280.html
> 
> Sending a v4 because another patch queued in ppc-next added an usage of
> msr_de
> 
> v2:
> - Abandon the ideia to add an M_MSR_* macro
> - Instead, use registerfields API as suggested by Richard
> - Add patch 21 to invert MSR_* values to match ISA ordering
> 
> v3:
> - Add macro to extract both FE0 and FE1. Use it to simplify the
>    conditionals in patch 17
> - Fix the checks that should be a xor
> - Fix incorrect parameter in FIELD_EX64 (was env->msr should be value)
>    in patch 16
> - Fix patch 13 title
> 
> v4:
> - Rebase on top of target/ppc: Fix BookE debug interrupt generation
> - Add patch target/ppc: Remove msr_de macro. msr_de was not being used
>    anywhere before, but an usage was added in the patch
>    target/ppc: Fix BookE debug interrupt generation
> - Necessary changes in other patches:
>    dont remove msr_de in patch 2
>    context change in other patches
> 
> 
> Víctor Colombo (22):
>    target/ppc: Remove fpscr_* macros from cpu.h
>    target/ppc: Remove unused msr_* macros
>    target/ppc: Remove msr_pr macro
>    target/ppc: Remove msr_le macro
>    target/ppc: Remove msr_ds macro
>    target/ppc: Remove msr_ile macro
>    target/ppc: Remove msr_ee macro
>    target/ppc: Remove msr_ce macro
>    target/ppc: Remove msr_pow macro
>    target/ppc: Remove msr_me macro
>    target/ppc: Remove msr_gs macro
>    target/ppc: Remove msr_fp macro
>    target/ppc: Remove msr_cm macro
>    target/ppc: Remove msr_ir macro
>    target/ppc: Remove msr_dr macro
>    target/ppc: Remove msr_ep macro
>    target/ppc: Remove msr_fe0 and msr_fe1 macros
>    target/ppc: Remove msr_ts macro
>    target/ppc: Remove msr_hv macro
>    target/ppc: Remove msr_de macro
>    target/ppc: Add unused msr bits FIELDs
>    target/ppc: Change MSR_* to follow POWER ISA numbering convention
> 
>   hw/ppc/pegasos2.c        |   2 +-
>   hw/ppc/spapr.c           |   2 +-
>   target/ppc/cpu.c         |   2 +-
>   target/ppc/cpu.h         | 220 ++++++++++++++++++---------------------
>   target/ppc/cpu_init.c    |  23 ++--
>   target/ppc/excp_helper.c |  54 +++++-----
>   target/ppc/fpu_helper.c  |  28 ++---
>   target/ppc/gdbstub.c     |   2 +-
>   target/ppc/helper_regs.c |  15 ++-
>   target/ppc/kvm.c         |   7 +-
>   target/ppc/machine.c     |   2 +-
>   target/ppc/mem_helper.c  |  23 ++--
>   target/ppc/misc_helper.c |   2 +-
>   target/ppc/mmu-radix64.c |  11 +-
>   target/ppc/mmu_common.c  |  40 +++----
>   target/ppc/mmu_helper.c  |   6 +-
>   16 files changed, 220 insertions(+), 219 deletions(-)
>