Re: [Qemu-devel] [PATCH 0/4] More core code ENV_GET_CPU removals

[Peter Crosthwaite]

On Tue, Jun 23, 2015 at 6:00 AM, Andreas Färber <address@hidden> wrote:
> Hi Peter,
>
> Am 04.06.2015 um 03:10 schrieb Peter Crosthwaite:
>> Was there an outcome?
>
> The conclusion for this series was that I would get a chance to review
> it (which I did later than I wanted to, after my talks from a hotel...)
> and that future patch series should spend a certain minimum time on the
> list, either before or after being applied by a maintainer but before
> being included in a PULL. The idea is to assure a) general review
> opportunity for both affected and non-affected non-maintainers and b)

It's been a month now. The opportunity has been there with replies
from a good number of people.

> opportunity to react to maintainer's oversights before they become facts
> in Git history.
>

That's unusual. If the maintainer and contributor are on the same page
it doesn't make sense for an objection to occur after the ack. If
reviewers want to nack, they should do it regardless of whether the
maintainer has weighed in yet. If you get a nack or a comment, there
is no difference, the contributor can leave it on list for as long as
they like.

I agree that if the maintainer acks it instantly it should be left on
list for a reasonable time if it is a first version or has substantial
change, but should you be ok with v4 of this series (which only
addresses trivial comments) I don't see a need for yet more list time.

> Differences of opinion still exist in what commitment to and extend of
> patch review we expect before patches get applied, and to what degree
> code/commit conventions vary from maintainer to maintainer or subsystem
> to subsystem.
>
> Am I seeing correctly that no respin or follow-up happened yet after I
> reviewed it?
>

No, there have been two versions since this one. I have addresses
Eduardo and Aurelien's comments on v2 and v3.

I have respun everything I have pending for you with "PATCH qom"
subject prefix yesterday/today. All patches were on list well before
soft freeze and almost all patches have RBs. You can safely ignore all
patches from me from before except the QOM-prop-overloading/A9-mpcore
series which is still relevant and not respun.

For a summary here is the review state of the multi-arch patch queue.
I have too many reviewed-but-unapplied patches I have stopped working
on the feature until I get some of this already-reviewed work off my
hands (read bottom up):

d22de8e345     HACK: mb: boot: Disable dtb load in multi-arch
c3202f0920     HACK: mb: boot: Assume using -firmware for mb software
8c27d2cf5a     arm: xilinx_zynq: Add a Microblaze
5f9e0c9c84     arm: boot: Don't assume all CPUs are ARM
3a47db3595     mb: Remove ELF_MACHINE from cpu.h
f886dd4de8     hw: mb: Explicitly include cpu.h for consumers
34b67400b9     arm: Remove ELF_MACHINE from cpu.h
eb628c8ad8     hw: arm: Explicitly include cpu.h for consumers
3981bb283f     core: Introduce multi-arch build
1ac2ff9e6d     arm: enable multi-arch
e7c9f09397     arm: register cpu_list function
69a42b153d     target-arm: Split cp helper API to new C file
a53b4dcedf     arm: cpu: static inline cpu_arm_init
717d43b73e     microblaze: enable multi-arch
1053a79602     target-*: Don't redefine cpu_exec
cf406390fb     core: virtualise CPU interfaces completely
d515ec68c5     HACK: Set tcg_enabled to true
b9397d1c40     cputlb,translate-all: Remove target_ulong from API
a5337ca686     cputlb: Change tlb_set_dirty arg to cpu
e07c23a328     cputlb: move CPU_LOOP for tlb_reset to exec.c
0943c367a6     disas: Defeature print_target_address
95d8a2096a     HACK: monitor: uninclude cpu_ldst
3baa9eeab0     HACK: monitor: Comment out TCG profile ops
105ec2f9ee     cpu-defs: Allow multiple inclusions
a41db438ab     cpu-defs: Move out TB_JMP defines                           R:rth
ccdc103068     translate: move real_host_page setting to -common
2e90cadb43     include/exec: Move tb hash functions out                    R:rth
624c0529d5     include/exec: Move cputlb exec.c defs out                   R:rth
b332fa4c3a     include/exec: Split target_long def to new header
a59e060282     include/exec: Move standard exceptions to cpu-all.h         R:rth
b01e4a1f42     translate-all: Move tcg_handle_interrupt to -common         R:rth
bac799c2fd     exec-all: Move cpu_can_do_io to qom/cpu.h                   R:rth
fa2e6624f9     cpu-common: Define tb_page_addr_t for everyone
326a80bb02     cpus: Listify cpu_list function
f4c00d94c4     translate: Listify tcg_exec_init                            R:rth
8f66b6c191     cpu-exec: Migrate some generic fns to cpus.c                R:rth
0a32363577     Makefile.target: Introduce arch-obj
3f51f6f027     cpu-defs: Move CPU_TEMP_BUF_NLONGS to tcg                   R:rth
5d2163a21b     cpu_defs: Simply CPUTLB padding logic
3a9e19c7f7     vfio: cpu: Use "real" page size API
    T:alexey
edbb1333e9     cpu-all: complete "real" host page size API
    T:alexey
9eee05f488     memory_mapping: Rework cpu related includes
    A:paolo
4561937a3b     cpu-exec: Purge all uses of ENV_GET_CPU()
888b57e00f     cpus: Change exec_init() arg to cpu, not env
    R:af R:aurel
952ac05646     cpus: Change tcg_cpu_exec() arg to cpu, not env
    R:af R:eduardo
860c916334     gdbserver: _fork: Change fn to accept cpu instead of
env    R:af R:eduardo
df44b66c87     translate-all: Change tb_flush() env argument to cpu
    R:eduardo A:eduardo
fb7dc6a221     ppc: Move cpu_exec_init() call to realize function
    R:dgibson R:me_x A:paolo
6405382188     cpus: Convert cpu_index into a bitmap
    R:eduardo R:igor R:dgibson R:me_x A:paolo
a9e1838b66     cpus: Add Error argument to cpu_exec_init()
    R:eduardo R:igor R:dgibson R:me_x A:paolo
ea291ffcde     microblaze: boot: Use cpu_set_pc()                          R:af
11d804948d     arm: boot: Use cpu_set_pc()
    R:pmm R:af
13dd9e90b1     gdbstub: Use cpu_set_pc() helper
07ca0ce6b4     cpu: Add wrapper to the set-pc() hook
4d46aa8725     disas: cris: QOMify target specific disas setup
    R:edgar_x
22bbba694f     disas: cris: Fix 0 buffer length case
    R:edgar_x
8b16aff59c     disas: microblaze: QOMify target specific disas setup
    R:edgar_x
8c08b3d14a     disas: arm: QOMify target specific disas setup              R:pmm
7ca536d824     disas: arm-a64: Make printfer and stream variable
    R:claudio T:claudio R:rth
b316905406     disas: QOMify target specific setup                         R:rth
d10c5b0fc8     disas: Add print_insn to disassemble info                   R:rth

I was hoping to flush out all the reviewed stuff and get those patches
with RTHs RB in for hard freeze but I am stalled on the deps. Then
during hard freeze continue the review of the multi-arch core work for
a merge in 2.5.

Regards,
Peter

> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>