On 4 April 2014 06:32, Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx> 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 _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm