Re: [Qemu-devel] [RFC PATCH 18/21] target-arm: switch banked CP registers

[Peter Crosthwaite]

On Thu, Dec 19, 2013 at 11:39 PM, Fedorov Sergey <address@hidden> wrote:
>
> On 12/19/2013 04:44 PM, Peter Crosthwaite wrote:
>>
>> On Thu, Dec 19, 2013 at 9:38 PM, Peter Maydell <address@hidden>
>> wrote:
>>>
>>> On 19 December 2013 07:27, Fedorov Sergey <address@hidden> wrote:
>>>>
>>>> Yes, this banking scheme makes state changing events quite heavy. But
>>>> maintaining the active copies allows to keep translation table walking
>>>> code
>>>> untouched. I think there is a trade-off between state changing and
>>>> translation table walking overheads.
>>>
>>> We shouldn't be doing tlb walks that often that it makes a
>>> difference whether we do env->ttbr0 or env->ttbr0[env->ns] ...
>>>
>>>> I think the CP banking is the most fragile thing in this patch series
>>>> and
>>>> this should become much better after review :)
>>>
>>> It would probably be a good idea to look at the v8 ARM ARM and
>>> figure out how banked-for-NS/S registers should fit in with the
>>> AArch64 vs AArch32 split.
>>> [if you don't have a copy, it's on the ARM website:
>>>
>>> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0487a.a_errata2/index.html
>>> you'll need to register an account on the website if you don't already
>>> have one but it's a fairly simple "fill in the form" automated process]
>>>
>>> Note in particular that:
>>>   * many of the current uint32_t fields in our CPU state struct are
>>>     likely to widen to uint64_t, so the AArch64 representation is
>>>     canonical, and the AArch32 register accessors access a part
>>>     of that state (typically the lower 32 bits)
>>>   * registers which are banked S/NS in AArch32 are not necessarily
>>>     banked in AArch64
>>>
>> Adding to that, are there any other reasons to bank a register other
>> than sec-extensions? It seems like what you have implemented here
>> is too sec specific for simply calling it "banked" (without further
>> clarification of what you are banking for).
>>
>> Regards,
>> Peter
>
>
> I'm not sure that I understand your question correctly but I try to answer.
> From ARMv7 ARM document section "B3.15.3 Classification of system control
> registers":
>
> "Banked system control registers have two copies, one Secure and one
> Non-secure."
>

Ok fair enough. I will wager though that sooner or later ARM will find
a reason to bank a reg other than sec-ext. But let's stick to their
terminology which by that quote its quite clear that "banked" means
"banked for security" (No change needed).

Regards,
Peter

> I don't know any use of term "CP15 banked registers" other that related to
> Security Extensions.
>
> Best regards,
> Sergey Fedorov
>
>
>>
>>> AArch64 support is likely to land before your TrustZone stuff
>>> does so we need to make the two features work together cleanly.
>>>
>>> thanks
>>> -- PMM
>>>
>
>