| Message ID | 1442672127-26223-3-git-send-email-edgar.iglesias@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 19 September 2015 at 07:15, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > The starting level for S2 pagetable walks is computed > differently from the S1 starting level. Implement the S2 > variant. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > target-arm/helper.c | 32 ++++++++++++++++++++------------ > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index b709582..33be8c2 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -6542,18 +6542,26 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > goto do_fault; > } > > - /* The starting level depends on the virtual address size (which can be > - * up to 48 bits) and the translation granule size. It indicates the number > - * of strides (granule_sz bits at a time) needed to consume the bits > - * of the input address. In the pseudocode this is: > - * level = 4 - RoundUp((inputsize - grainsize) / stride) > - * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is > - * our 'granule_sz + 3' and 'stride' is our 'granule_sz'. > - * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying: > - * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz > - * = 4 - (va_size - tsz - 4) / granule_sz; > - */ > - level = 4 - (va_size - tsz - 4) / granule_sz; > + if (mmu_idx == ARMMMUIdx_S2NS) { > + unsigned int startlevel = extract32(tcr->raw_tcr, 6, 2); > + level = 3 - startlevel; > + if (granule_sz == 9) { > + level = 2 - startlevel; > + } I think this is right code-wise but we could make it read a little more nicely: if you make the condition be "if (mmu_idx != ARMMUIdx_S2NS)" then the common case comes first and its long comment works as a description of what we're doing here. Then the else clause can just say /* For stage 2 translations the starting level is specified by the * VCTR_EL2.SL0 field (whose interpretation depends on the page size) */ I was pondering whether writing it as if (granule_sz == 9) { /* 4K pages */ level = 2 - startlevel; } else { /* 16K or 64K pages */ level = 3 - startlevel; } would be slightly better, but it's marginal. Do add a "4K pages" comment in somewhere, though. > + } else { > + /* The starting level depends on the virtual address size (which can > + * be up to 48 bits) and the translation granule size. It indicates > + * the number of strides (granule_sz bits at a time) needed to > + * consume the bits of the input address. In the pseudocode this is: > + * level = 4 - RoundUp((inputsize - grainsize) / stride) > + * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is > + * our 'granule_sz + 3' and 'stride' is our 'granule_sz'. > + * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying: > + * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz > + * = 4 - (va_size - tsz - 4) / granule_sz; > + */ > + level = 4 - (va_size - tsz - 4) / granule_sz; > + } > > /* Clear the vaddr bits which aren't part of the within-region address, > * so that we don't have to special case things when calculating the > -- > 1.9.1 > thanks -- PMM
diff --git a/target-arm/helper.c b/target-arm/helper.c index b709582..33be8c2 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -6542,18 +6542,26 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, goto do_fault; } - /* The starting level depends on the virtual address size (which can be - * up to 48 bits) and the translation granule size. It indicates the number - * of strides (granule_sz bits at a time) needed to consume the bits - * of the input address. In the pseudocode this is: - * level = 4 - RoundUp((inputsize - grainsize) / stride) - * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is - * our 'granule_sz + 3' and 'stride' is our 'granule_sz'. - * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying: - * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz - * = 4 - (va_size - tsz - 4) / granule_sz; - */ - level = 4 - (va_size - tsz - 4) / granule_sz; + if (mmu_idx == ARMMMUIdx_S2NS) { + unsigned int startlevel = extract32(tcr->raw_tcr, 6, 2); + level = 3 - startlevel; + if (granule_sz == 9) { + level = 2 - startlevel; + } + } else { + /* The starting level depends on the virtual address size (which can + * be up to 48 bits) and the translation granule size. It indicates + * the number of strides (granule_sz bits at a time) needed to + * consume the bits of the input address. In the pseudocode this is: + * level = 4 - RoundUp((inputsize - grainsize) / stride) + * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is + * our 'granule_sz + 3' and 'stride' is our 'granule_sz'. + * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying: + * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz + * = 4 - (va_size - tsz - 4) / granule_sz; + */ + level = 4 - (va_size - tsz - 4) / granule_sz; + } /* Clear the vaddr bits which aren't part of the within-region address, * so that we don't have to special case things when calculating the