Re: [Qemu-devel] [PATCH 1/4] cputlb: Add tlb_set_asid_for_mmuidx

[Peter Maydell]

On 15 November 2018 at 18:51, Richard Henderson
<address@hidden> wrote:
> On 11/15/18 7:36 PM, Peter Maydell wrote:
>> On 29 October 2018 at 15:53, Richard Henderson
>> <address@hidden> 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 <address@hidden>

>>> +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.

Yes, if the function expects to be called only for the
current CPU we should assert and document this.

> 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.

We should either document the invariant at the cputlb.c level
(and assert it, if not too tedious to do), or say that the cputlb
code doesn't care, and just only flush the indexes which have the
wrong ASID currently, I think.

thanks
-- PMM