ID_MMFR0 has an invalid value on aarch64 cpu (A57, A53)

Bug #1723984 reported by Bug
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Invalid
Undecided
Unassigned

Bug Description

The ID_MMFR0 register, accessed from aarch64 state as an invalid value:
- ARM ARM v8 documentation (D7.2 General system control registers) described bits AuxReg[23:20] to be
  "In ARMv8-A the only permitted value is 0010"
- Cortex A53 and Cortex A57 TRM describe the value to be 0x10201105, so AuxReg[23:20] is 0010 too
- in QEMU target/arm/cpu64.c, the relevant value is
  cpu->id_mmfr0 = 0x10101105;

The 1 should be changed to 2.

Spotted & Tested on the following qemu revision:

commit 48ae1f60d8c9a770e6da64407984d84e25253c69
Merge: 78b62d3 b867eaa
Author: Peter Maydell <email address hidden>
Date: Mon Oct 16 14:28:13 2017 +0100

Revision history for this message
Peter Maydell (pmaydell) wrote :

QEMU's behaviour in this case is matching the hardware. We claim to model an r1p0 (based on the MIDR value we report), and for the r1p0 the A53 and A57 reported the ID_MMFR0 as 0x10101105 -- this is documented in the TRMs for that rev of the CPUs. r1p3 reports the 0x10201105 you describe, but this isn't the rev of the CPU we claim to be.

In theory we could bump the rXpX but I'm not sure there's much point unless it's causing a real problem (we'd need to check what else might have changed between the two revisions).

Revision history for this message
Bug (kewlubuntu) wrote :

Oh I see. I didn't check older TRM since the ARM ARM was quite strict on the value, sorry.
I'll read the MIDR to have a more robust code then. Thank you.

Revision history for this message
Peter Maydell (pmaydell) wrote :

You shouldn't need to read the MIDR at all.

There are two sensible strategies for software I think:

 (1) trust the architectural statement that v8 implies that the AIFSR and ADFSR both exist -- AIUI both QEMU and the hardware implementations that report 0001 in this MMFR0 field do actually implement those registers, so this is safe.

 (2) read and pay attention to the AuxReg field, by handling 0001 as "only Auxiliary Control Register is supported, AIFSR and ADFSR are not supported". This will work fine too -- on implementations that report 0001 you may be not using the AIFSR/ADFSR but that's ok because on those implementations they only RAZ/WI anyhow so you couldn't do anything interesting with them anyway.

If your code is genuinely v8 only then (1) is easiest. If you also need to support ARMv7 then (2) is best, because 0001 is a permitted value in ID_MMFR0 for an ARMv7 implementation, so you need to handle it regardless of the A53/A57 behaviour.

Neither approach requires detecting and special casing A53/A57 revisions via the MIDR.

Revision history for this message
Bug (kewlubuntu) wrote :

I see your point. Thank you for the advice. I'm doing some low-level check to be sure to be on a known platform, so this midr based code is very localized. For the "core" of the kernel, I'm mostly using (1) as access to MMU registers are localized in armv7/armv8 specialized sub-directories.

Revision history for this message
Peter Maydell (pmaydell) wrote :

Thanks for the update -- I'm going to close this bug. (Incidentally, my experience with checks of the "insist we're on a known platform with ID register values we recognize" kind is that they're more trouble than they're worth, especially if you plan running the software in an emulator.)

Changed in qemu:
status: New → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.