On Fri, Oct 23, 2015 at 04:29:35PM +0100, Peter Maydell wrote:
> On 14 October 2015 at 23:55, Edgar E. Iglesias <edgar.igles...@gmail.com>
> wrote:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Add support for AArch32 S2 negative t0sz. In preparation for
> > using 40bit IPAs on AArch32.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
> > ---
> > target-arm/helper.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 4e19838..a8a46db 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -6475,6 +6475,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > target_ulong address,
> > if (va_size == 64) {
> > t0sz = MIN(t0sz, 39);
> > t0sz = MAX(t0sz, 16);
> > + } else {
> > + bool sext = extract32(t0sz, 4, 1);
> > + bool sign = extract32(t0sz, 3, 1);
> > + t0sz = sextract32(t0sz, 0, 4);
> > +
> > + /* If the sign-extend bit is not the same as t0sz[3], the result
> > + * is unpredictable. Flag this as a guest error. */
> > + if (sign != sext) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n");
> > + }
>
> Shouldn't this be guarded by a check on whether this is an s2
> translation, since the 4-bit signed T0SZ and the S bit are only for
> the VTCR, not for the normal TTBCRs ?
Yes, sounds good. I've changed the patch to the following:
@@ -6521,8 +6521,24 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
*/
int32_t t0sz = extract32(tcr->raw_tcr, 0, 6);
if (va_size == 64) {
+ /* AArch64 translation. */
t0sz = MIN(t0sz, 39);
t0sz = MAX(t0sz, 16);
+ } else if (mmu_idx != ARMMMUIdx_S2NS) {
+ /* AArch32 stage 1 translation. */
+ t0sz = extract32(t0sz, 0, 3);
+ } else {
+ /* AArch32 stage 2 translation. */
+ bool sext = extract32(t0sz, 4, 1);
+ bool sign = extract32(t0sz, 3, 1);
+ t0sz = sextract32(t0sz, 0, 4);
+
+ /* If the sign-extend bit is not the same as t0sz[3], the result
+ * is unpredictable. Flag this as a guest error. */
+ if (sign != sext) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n");
+ }
}
We can also remove the error log and add more complete checks in future
patches if you prefer...
Cheers,
Edgar
>
> That is, we have 3 cases here for determining t0sz:
> * AArch64 6-bit unsigned field
> * AArch32 stage 1 3-bit unsigned field
> * AArch32 stage 2 4-bit signed field
> so we need more than just a single if/else.
>
> It's true that bits 3 and 4 are RES0 for TTBCR, but if we're
> going to actually start logging guest errors here maybe we
> should actually report the real problem (RES0 bits being set)
> for that case.
>
> thanks
> -- PMM
