Re: [Qemu-devel] [PATCH v5 34/37] target-arm: Implement CBAR for Cortex-A57

[Peter Maydell]

On 4 April 2014 06:32, Peter Crosthwaite <address@hidden> 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