qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 09/11] target-arm: Use mmu_idx in get_phys_ad


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2 09/11] target-arm: Use mmu_idx in get_phys_addr()
Date: Fri, 30 Jan 2015 10:24:11 +0000

On 30 January 2015 at 02:03, Edgar E. Iglesias <address@hidden> wrote:
> On Thu, Jan 29, 2015 at 06:55:15PM +0000, Peter Maydell wrote:
>> Now we have the mmu_idx in get_phys_addr(), use it correctly to
>> determine the behaviour of virtual to physical address translations,
>> rather than using just an is_user flag and the current CPU state.
>>
>> Some TODO comments have been added to indicate where changes will
>> need to be made to add EL2 and 64-bit EL3 support.
>> -    /* This will go away when we handle mmu_idx properly here */
>> -    int is_user = (mmu_idx == ARMMMUIdx_S12NSE0 ||
>> -                   mmu_idx == ARMMMUIdx_S1SE0 ||
>> -                   mmu_idx == ARMMMUIdx_S1NSE0);
>> +    if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
>> +        /* TODO: when we support EL2 we should here call ourselves 
>> recursively
>> +         * to do the stage 1 and then stage 2 translations. The ldl_phys
>> +         * calls for stage 1 will also need changing.
>> +         * For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
>> +         */
>> +        assert(!arm_feature(env, ARM_FEATURE_EL2));
>> +        mmu_idx += ARMMMUIdx_S1NSE0;
>
> I'm not sure I understand this. Did you mean the following?
> mmu_idx = ARMMMUIdx_S1NSE0;

No. This code is handling "we asked for a stage1+2 EL0 or EL1
lookup but we don't have EL2". In this case these degrade
to the equivalent stage-1-only lookups:
 S12NSE0 -> S1NSE0
 S12NSE1 -> S1NSE1
We're relying on S12NSE0 being zero and the E0/E1 indexes
being consecutive on both sides.

> Maybe you can relax the assert to check for FEATURE_EL2 and hcr_el2 & HCR_VM ?
> And not change the mmu_idx.

The assert is here to say "if you want to implement EL2 there
is work to do here". For EL2, this is going to look
something like:
    if (arm_feature(env, ARM_FEATURE_EL2 && (hcr_el2 & HCR_VM)) {
        /* stage 2 exists and is enabled */
        hwaddr ipa;
        get_phys_addr(env, addr, &ipa, ..., stage 1 mmuidx, ...);
        handle stage 1 faults;
        get_phys_addr(env, ipa, &physaddr, ...., stage 2 mmuidx, ...);
        handle stage 2 faults;
        combine protection etc info from stage 1 and stage 2;
        return final physaddr for combined lookup;
    }

That's quite a bit of extra code, so it's deferred til we
actually implement EL2, and in the meantime we assert as a
marker for "if you hit this you need to implement all that".

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]