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

[Peter Crosthwaite]

On Fri, Apr 4, 2014 at 6:25 PM, Peter Maydell <address@hidden> wrote:
> 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,

Is realize allowed to fail due to bad property values? Thinking more
about it, perhaps the ideal solution is to populate the Error **
passed to realize and bail out and let the realize() caller deal with
it.

but that might be a bit painful to find a suitable
> place to put it. extracting the low bits too seems a reasonable
> compromise.
>

Ok,

Regards,
Peter

>>> +                | 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
>