On Wed, Dec 4, 2013 at 7:50 PM, Fedorov Sergey <s.fedo...@samsung.com> wrote:
>
> On 12/03/2013 04:15 PM, Peter Crosthwaite wrote:
>>
>> On Tue, Dec 3, 2013 at 6:48 PM, Sergey Fedorov <s.fedo...@samsung.com>
>> wrote:
>>>
>>> TTBCR has additional fields PD0 and PD1 when using Short-descriptor
>>> translation table format on a CPU with TrustZone feature support.
>>>
>>> Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com>
>>> ---
>>> target-arm/helper.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>>> index a247ca0..6642e53 100644
>>> --- a/target-arm/helper.c
>>> +++ b/target-arm/helper.c
>>> @@ -1159,8 +1159,10 @@ static int vmsa_ttbcr_raw_write(CPUARMState *env,
>>> const ARMCPRegInfo *ri,
>>> {
>>> int maskshift = extract32(value, 0, 3);
>>>
>>> - if (arm_feature(env, ARM_FEATURE_LPAE)) {
>>> + if (arm_feature(env, ARM_FEATURE_LPAE) && (value & (1 << 31))) {
>>
>> This appears to be changing more than just trustzone dependent
>> behavior. That is, if we take just this hunk and ignore the one below
>> you see a change in the non-tz behaviour. Is the hunk legitimate
>> irrespective of trustzone support?
>
>
> Yes, current implementation is not accurate according to ARMv7-AR reference
> manual. See "B4.1.153 TTBCR, Translation Table Base Control Register, VMSA |
> TTBCR format when using the Long-descriptor translation table format". When
> LPAE feature is supported, EAE, bit[31] selects translation descriptor
> format and, therefore, TTBCR format.
>
Ok,
You should probably split it off as a separate patch. And you could
send probably send it immediately. What you just wrote is a nice
commit message.
>
>>
>>> value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
>>> + } else if (arm_feature(env, ARM_FEATURE_TRUSTZONE)) {
>>> + value &= 0x37;
>>> } else {
>>> value &= 7;
>>> }
>>
>> There are a few magic numbers in the patch probably worth macrofiying.
>
>
> As I can see, magic numbers are widely used through all of this file to
> represent CP register fields and other things. Maybe the macrofying should
> be done separately from this patch series?
>
So target-arm is riddled with legacy style. Converting all of
target-arm to self-doccing macros is a big job, but if we keep
expanding without doing it that job only gets bigger. I'll defer to
PMM on what we want to policy to be going forward. - any thoughts?
Regards,
Peter
>>
>> Regards,
>> Peter
>>
>>> --
>>> 1.7.9.5
>>>
>>>
>>
>
> Best regards,
> Sergey Fedorov
>
