diff mbox

[v1,07/10] target-arm: Supress the use of TTBR1 for S2 translations

Message ID 1441311266-8644-8-git-send-email-edgar.iglesias@gmail.com
State New
Headers show

Commit Message

Edgar E. Iglesias Sept. 3, 2015, 8:14 p.m. UTC
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Stage-2 MMU translations do not use TTBR1.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Peter Maydell Sept. 8, 2015, 2:32 p.m. UTC | #1
On 3 September 2015 at 21:14, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Stage-2 MMU translations do not use TTBR1.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/helper.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 9ea9719..66b3fed 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          }
>      }
>
> +    /* Stage2 translations do not use TTBR1.  */
> +    if (mmu_idx == ARMMMUIdx_S2NS) {
> +        ttbr1_valid = false;
> +    }
> +

I think this is unnecessary, because we've already set ttbr1_valid
to false in the previous chunk of code for the case where el == 2
(as it is for stage 2 translations).

thanks
-- PMM
Edgar E. Iglesias Sept. 8, 2015, 2:42 p.m. UTC | #2
On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
> On 3 September 2015 at 21:14, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Stage-2 MMU translations do not use TTBR1.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/helper.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 9ea9719..66b3fed 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> >          }
> >      }
> >
> > +    /* Stage2 translations do not use TTBR1.  */
> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
> > +        ttbr1_valid = false;
> > +    }
> > +
> 
> I think this is unnecessary, because we've already set ttbr1_valid
> to false in the previous chunk of code for the case where el == 2
> (as it is for stage 2 translations).

I think we may be confused here.

Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.

Maybe I should have waited with this stuff until I've posted a more
complete S2 implementation but basically what will happen is that
when HCR.VM is set, we'll do a S2 translation after S1 for NS EL0 and 1.
I don't have it all complete yet though, so I started with these smaller
chunks...

Cheers,
Edgar
Peter Maydell Sept. 8, 2015, 2:50 p.m. UTC | #3
On 8 September 2015 at 15:42, Edgar E. Iglesias
<edgar.iglesias@xilinx.com> wrote:
> On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
>> On 3 September 2015 at 21:14, Edgar E. Iglesias
>> <edgar.iglesias@gmail.com> wrote:
>> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>> >
>> > Stage-2 MMU translations do not use TTBR1.
>> >
>> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> > ---
>> >  target-arm/helper.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/target-arm/helper.c b/target-arm/helper.c
>> > index 9ea9719..66b3fed 100644
>> > --- a/target-arm/helper.c
>> > +++ b/target-arm/helper.c
>> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>> >          }
>> >      }
>> >
>> > +    /* Stage2 translations do not use TTBR1.  */
>> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
>> > +        ttbr1_valid = false;
>> > +    }
>> > +
>>
>> I think this is unnecessary, because we've already set ttbr1_valid
>> to false in the previous chunk of code for the case where el == 2
>> (as it is for stage 2 translations).
>
> I think we may be confused here.
>
> Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.

Yep. el is the result of regime_el(), which returns what the ARM ARM
calls "the EL that the translation regime is controlled from".
In particular, we do things this way because it's the register width
of the controlling EL that determines whether the translation
regime is 64 bit, whether the TCR/TTBR/etc registers are the 64-bit
forms or not, etc.

thanks
-- PMM
Edgar E. Iglesias Sept. 8, 2015, 2:57 p.m. UTC | #4
On Tue, Sep 08, 2015 at 03:50:34PM +0100, Peter Maydell wrote:
> On 8 September 2015 at 15:42, Edgar E. Iglesias
> <edgar.iglesias@xilinx.com> wrote:
> > On Tue, Sep 08, 2015 at 03:32:36PM +0100, Peter Maydell wrote:
> >> On 3 September 2015 at 21:14, Edgar E. Iglesias
> >> <edgar.iglesias@gmail.com> wrote:
> >> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >> >
> >> > Stage-2 MMU translations do not use TTBR1.
> >> >
> >> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >> > ---
> >> >  target-arm/helper.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> >> > index 9ea9719..66b3fed 100644
> >> > --- a/target-arm/helper.c
> >> > +++ b/target-arm/helper.c
> >> > @@ -6372,6 +6372,11 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
> >> >          }
> >> >      }
> >> >
> >> > +    /* Stage2 translations do not use TTBR1.  */
> >> > +    if (mmu_idx == ARMMMUIdx_S2NS) {
> >> > +        ttbr1_valid = false;
> >> > +    }
> >> > +
> >>
> >> I think this is unnecessary, because we've already set ttbr1_valid
> >> to false in the previous chunk of code for the case where el == 2
> >> (as it is for stage 2 translations).
> >
> > I think we may be confused here.
> >
> > Note S2NS translations are controlled by EL2 but apply to NS EL0 and EL1.
> 
> Yep. el is the result of regime_el(), which returns what the ARM ARM
> calls "the EL that the translation regime is controlled from".
> In particular, we do things this way because it's the register width
> of the controlling EL that determines whether the translation
> regime is 64 bit, whether the TCR/TTBR/etc registers are the 64-bit
> forms or not, etc.

OK, I see. I'll have another look at this...

Thanks!
Edgar
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 9ea9719..66b3fed 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6372,6 +6372,11 @@  static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
         }
     }
 
+    /* Stage2 translations do not use TTBR1.  */
+    if (mmu_idx == ARMMMUIdx_S2NS) {
+        ttbr1_valid = false;
+    }
+
     /* Determine whether this address is in the region controlled by
      * TTBR0 or TTBR1 (or if it is in neither region and should fault).
      * This is a Non-secure PL0/1 stage 1 translation, so controlled by