On 4 April 2014 06:32, Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
>> + if (arm_feature(env, ARM_FEATURE_AARCH64)) {
>> + /* 32 bit view is [31:18] 0...0 [43:32]. */
>> + uint32_t cbar32 = cpu->reset_cbar
>
> Should you extract64 on the lower order bits as well to avoid weird |
> results on a misaligned reset_cbar (or perhaps its worth an assert?).
Can't assert, it's a QOM property; we could perhaps validate
earlier on in init, but that might be a bit painful to find a suitable
place to put it. extracting the low bits too seems a reasonable
compromise.
>> + | extract64(cpu->reset_cbar, 32, 12);
>> + ARMCPRegInfo cbar_reginfo[] = {
>> + { .name = "CBAR",
>> + .type = ARM_CP_CONST,
>> + .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
>> + .access = PL1_R, .resetvalue = cpu->reset_cbar },
>> + { .name = "CBAR_EL1", .state = ARM_CP_STATE_AA64,
>> + .type = ARM_CP_CONST,
>> + .opc0 = 3, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 0,
>> + .access = PL1_R, .resetvalue = cbar32 },
>> + REGINFO_SENTINEL
>> + };
>> + /* We don't implement a r/w 64 bit CBAR currently */
>
> Is it even valid? Writable CBAR seems like a bug to me that's just
> fixed in the V8 version bump.
This is all IMPDEF anyway (and as you'll see from the next patch
A15's CBAR is RO too). R/W CBAR isn't an inherently dumb idea:
you could use it to make system designs where setting CBAR is
the responsibility of board-specific bootrom or firmware before
handoff to tho OS, for instance. Having the h/w hardwire it is
probably more robust though.
thanks
-- PMM
