On 6/30/23 16:16, Richard Henderson wrote:
On 6/30/23 14:25, Anton Johansson wrote:
@@ -448,6 +448,13 @@ struct CPUState {
/* track IOMMUs whose translations we've cached in the TCG
TLB */
GArray *iommu_notifiers;
+
+ /*
+ * The following fields needs to be within
CPU_MAX_NEGATIVE_ENV_OFFSET of
+ * CPUArchState. As CPUArchState is assumed to follow CPUState
in the
+ * ArchCPU struct these are placed last. This is checked
statically.
+ */
+ CPUTLB tlb;
};
This is what we had before CPUNegativeOffsetState, comment and all,
and over the course of time the comment was ignored and the CPUTLB
crept toward the middle of the structure.
I recall you talking about this earlier. Is there any reason the
drifting of CPUTLB/IcountDecr from env wouldn't be caught by the static
asserts we've added?
I really don't see how this merge helps. There's nothing
target-specific about CPUTLBDescFast or CPUNegativeOffsetState, and
keeping them separate enforces their importance.
There isn't anything target specific about CPUTLBDescFast but its offset
in ArchCPU does depend on the target. This is due to the
TARGET_PAGE_ENTRY_EXTRA macro in CPUTLBEntryFull which is replaced
with a union in the first patch of this patchset.
On second thought, I think you're right and keeping CPUTLB and IcountDecr
together to emphasize their importance makes sense. There would still be
an implicit assumption on the target to keep CPUArchState and CPUState
close together, at least the intent is signaled better by
CPUNegativeOffsetState.
Even so, statically asserting the offset to CPUTLB and IcountDecr would be a
good idea.
The main concern of this patchset is being able to access the tlb and
icount_decr fields in a target-independent way only having access to
CPUState. Merging CPUNegativeOffsetState simply made the accessing part
simpler, but let's have a cpu_tlb(CPUState *) function instead.
I'll pull out the patch for making CPUTLB target independent and include
it in the patchset replacing CPUArchState with CPUState in cputlb.c and
user-exec.c. The static assert patches will be resubmitted separately.
Thanks,
--
Anton Johansson,
rev.ng Labs Srl.
