On Thu, Apr 16, 2015 at 1:03 PM, Peter Maydell <peter.mayd...@linaro.org>
wrote:

> On 27 March 2015 at 19:10, Greg Bellows <greg.bell...@linaro.org> wrote:
> > Add a utility function for choosing the correct TTBR system register
> based on
> > the specified MMU index. Add use of function on physical address lookup.
>
> > @@ -5376,20 +5390,26 @@ static int get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> >      int32_t tbi = 0;
> >      TCR *tcr = regime_tcr(env, mmu_idx);
> >      int ap, ns, xn, pxn;
> > +    uint32_t el = regime_el(env, mmu_idx);
> >
> >      /* TODO:
> >       * This code assumes we're either a 64-bit EL1 or a 32-bit PL1;
> > -     * it doesn't handle the different format TCR for TCR_EL2, TCR_EL3,
> > -     * and VTCR_EL2, or the fact that those regimes don't have a split
> > +     * it doesn't handle the different format TCR for and VTCR_EL2,
> > +     * or the fact that those regimes don't have a split
> >       * TTBR0/TTBR1. Attribute and permission bit handling should also
> >       * be checked when adding support for those page table walks.
> >       */
> > -    if (arm_el_is_aa64(env, regime_el(env, mmu_idx))) {
> > +    if (arm_el_is_aa64(env, el)) {
> >          va_size = 64;
> > -        if (extract64(address, 55, 1))
> > -            tbi = extract64(tcr->raw_tcr, 38, 1);
> > -        else
> > -            tbi = extract64(tcr->raw_tcr, 37, 1);
> > +        if (el == 3 || el == 2) {
> > +            tbi = extract64(tcr->raw_tcr, 20, 1);
> > +        } else {
> > +            if (extract64(address, 55, 1)) {
> > +                tbi = extract64(tcr->raw_tcr, 38, 1);
> > +            } else {
> > +                tbi = extract64(tcr->raw_tcr, 37, 1);
> > +            }
> > +        }
> >          tbi *= 8;
> >      }
>
> The TTBR related parts of this patch are fine, but this section
> feels a bit incomplete. The location of the TBI bit is not the
> only thing about the TCR format that's different for TCR_EL2
> and TCR_EL3. I think it would be better to move this hunk into
> a separate patch that also fixes the other points the TODO comment
> mentions (including the other parts of the function that access
> raw_tcr bitfields).
>
> ​I broke up the TTBR and TCR changes into separate patches.

I'll add support for the lack of TTBR1 in EL2/3 but the rest (VTCR2 and
certain attributes (shareability & physical addr size) should be left as
separate patches as they are unrelated to EL3 and already unsupported.




> -- PMM
>

Reply via email to