Re: [Qemu-devel] [PATCH 31/35] cpu: Move icount_decr to CPUNegativeOffsetState

[Richard Henderson]

On 3/25/19 10:23 AM, Paolo Bonzini wrote:
>> RFC: I had problems with the build system moving qom/cpu.c
>> from common-obj-y to obj-y.  Then I tried just splitting out
>> a piece of it, and ran into odder build system issues.  Then
>> I hacked Makefile.target in a way that doesn't look right,
>> but works.
>>
>> Question: Should I bother splitting qom/cpu.c at all?  Just
>> move the whole thing to obj-y (with non-broken build changes,
>> obviously, whatever that might be).
> 
> I think you should, because moving it to obj-y would be a major headache
> if we ever were to resurrect the multiarch patches by Peter C.
> 
> That said, does cpu_neg really need env_neg and thus ArchCPU?

At one point point during development I had a pointer in CPUState to the
icount_decr.  But I didn't like having to copy this initialization into
target/*/ as with the setting of env_ptr.


> 1) define CPUTLB in terms of MAX_MMU_MODES (12) instead of NB_MMU_MODES

Plausible, now that unused mmu modes consume < 100 bytes
instead of many kB.

> 
> 2) assert that
> 
>    offsetof(ArchCPU, env) - offsetof(ArchCPU, neg) ==
>       sizeof(CPUNegativeOffsetState)

This requires that alignof(neg) == alignof(env), lest there be extra padding in
between the two structures.  But perhaps that's just a matter of adjusting the
declaration like so:

    CPUNegativeOffsetState neg QEMU_ALIGNED(__alignof(CPUArchState));

In any case the _Static_assert would let us know.

> 
> 3) use this in cpu_neg to go from cpu->env_ptr to the
> CPUNegativeOffsetState.
> 
> This lets you implement cpu_neg in a target-independent manner.

Plausible.  Thanks for the feedback.


r~