On 11/15/18 7:36 PM, Peter Maydell wrote: > On 29 October 2018 at 15:53, Richard Henderson > <richard.hender...@linaro.org> wrote: >> Although we can't do much with ASIDs except remember them, this >> will allow cleanups within target/ that should make things clearer. >> >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> >> --- >> include/exec/cpu-defs.h | 2 ++ >> include/exec/exec-all.h | 19 +++++++++++++++++++ >> accel/tcg/cputlb.c | 23 +++++++++++++++++++++++ >> 3 files changed, 44 insertions(+) >> >> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h >> index 6a60f94a41..8fbfe8c8e2 100644 >> --- a/include/exec/cpu-defs.h >> +++ b/include/exec/cpu-defs.h >> @@ -152,6 +152,8 @@ typedef struct CPUTLBDesc { >> target_ulong large_page_mask; >> /* The next index to use in the tlb victim table. */ >> size_t vindex; >> + /* The current ASID for this tlb. */ >> + uint32_t asid; >> } CPUTLBDesc; >> >> /* >> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >> index 815e5b1e83..478f488704 100644 >> --- a/include/exec/exec-all.h >> +++ b/include/exec/exec-all.h >> @@ -226,6 +226,21 @@ void tlb_flush_by_mmuidx_all_cpus(CPUState *cpu, >> uint16_t idxmap); >> * depend on when the guests translation ends the TB. >> */ >> void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, uint16_t idxmap); >> +/** >> + * tlb_set_asid_for_mmuidx: >> + * @cpu: Originating cpu >> + * @asid: Address Space Identifier >> + * @idxmap: bitmap of MMU indicies to set to @asid > > "indices", but it looks like in this header we consistently > use "MMU indexes", so better to stick with that. (Similarly below.) > >> + * @depmap: bitmap of dependent MMU indicies >> + * >> + * Set an ASID for all of @idxmap. If any previous ASID was different, >> + * then we may flush the mmu idx. If a flush is required, then also flush > > presumably "will flush", rather than "may flush" ? > >> + * all dependent mmu indicies in @depmap. This latter is typically used >> + * for secondary page resolution, for implementing virtualization within >> + * the guest. >> + */ >> +void tlb_set_asid_for_mmuidx(CPUState *cpu, uint32_t asid, >> + uint16_t idxmap, uint16_t dep_idxmap); >> /** >> * tlb_set_page_with_attrs: >> * @cpu: CPU to add this TLB entry for >> @@ -311,6 +326,10 @@ static inline void >> tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, >> uint16_t idxmap) >> { >> } >> +static inline void tlb_set_asid_for_mmuidx(CPUState *cpu, uint32_t asid, >> + uint16_t idxmap, uint16_t depmap) >> +{ >> +} >> #endif >> >> #define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache >> line */ >> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c >> index af6bd8ccf9..60b3dc2de3 100644 >> --- a/accel/tcg/cputlb.c >> +++ b/accel/tcg/cputlb.c >> @@ -360,6 +360,29 @@ void tlb_flush_page_all_cpus_synced(CPUState *src, >> target_ulong addr) >> tlb_flush_page_by_mmuidx_all_cpus_synced(src, addr, ALL_MMUIDX_BITS); >> } >> >> +void tlb_set_asid_for_mmuidx(CPUState *cpu, uint32_t asid, uint16_t idxmap, >> + uint16_t depmap) >> +{ >> + CPUArchState *env = cpu->env_ptr; >> + uint16_t work, to_flush = 0; > > The other functions that work on the tlb defer their > actual operation to an async-work type function or > do a run-on-cpu if the passed-in CPU is not the current > CPU. Do we need to do that here too?
I don't *think* so. I would expect an ASID to be set in response to some cpu-local change, like setting TTBR0_EL1. Something that cannot be done across cpus. I could assert_cpu_is_self, if you like. What I *should* be adding as well is cross-cpu asid-specific tlb flushing, a-la TLBI ASID. That would need the run-on-cpu stuff. > So this will flush all the passed in indexes in idxmap > if any one of them was previously the wrong ASID. Is that > necessary, or could we just flush only the ones which > were wrong and not flush any that were already the correct ASID ? It probably wouldn't be necessary. But I also sort of expect the set of indexes to always have the same ASID -- e.g. kernel vs user views of the same address space, e.g. S12NSE0 + S12NSE1. I don't really know what to do when this presumed invariant is violated. r~