Message ID | 1481046379-32632-3-git-send-email-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
On Tue, Dec 06, 2016 at 05:46:18PM +0000, Peter Maydell wrote: > In the ARMCPRegInfo definitions for the GICv3 CPU interface > registers, we were trying to use .fieldoffset to specify > the locations of data fields within the GICv3CPUState struct. > This is completely broken, because .fieldoffset is for offsets > into the CPUARMState struct. We didn't notice because we > were only using this for reads to BPR0, AP0R<n>, IGRPEN0 > and CTLR_EL3, and Linux doesn't use these registers. > > Replace the .fieldoffset uses with explicit read functions. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/intc/arm_gicv3_cpuif.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c > index bca30c4..35e8eb3 100644 > --- a/hw/intc/arm_gicv3_cpuif.c > +++ b/hw/intc/arm_gicv3_cpuif.c > @@ -1118,35 +1118,35 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 3, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_bpr[GICV3_G0]), > + .readfn = icc_bpr_read, > .writefn = icc_bpr_write, > }, > { .name = "ICC_AP0R0_EL1", .state = ARM_CP_STATE_BOTH, > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 4, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][0]), > + .readfn = icc_ap_read, > .writefn = icc_ap_write, > }, > { .name = "ICC_AP0R1_EL1", .state = ARM_CP_STATE_BOTH, > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 5, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][1]), > + .readfn = icc_ap_read, > .writefn = icc_ap_write, > }, > { .name = "ICC_AP0R2_EL1", .state = ARM_CP_STATE_BOTH, > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 6, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][2]), > + .readfn = icc_ap_read, > .writefn = icc_ap_write, > }, > { .name = "ICC_AP0R3_EL1", .state = ARM_CP_STATE_BOTH, > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 7, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][3]), > + .readfn = icc_ap_read, > .writefn = icc_ap_write, > }, > /* All the ICC_AP1R*_EL1 registers are banked */ > @@ -1275,7 +1275,7 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { > .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 6, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL1_RW, .accessfn = gicv3_fiq_access, > - .fieldoffset = offsetof(GICv3CPUState, icc_igrpen[GICV3_G0]), > + .readfn = icc_igrpen_read, > .writefn = icc_igrpen_write, > }, > /* This register is banked */ > @@ -1299,7 +1299,6 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { > .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 12, .opc2 = 4, > .type = ARM_CP_IO | ARM_CP_NO_RAW, > .access = PL3_RW, > - .fieldoffset = offsetof(GICv3CPUState, icc_ctlr_el3), > .readfn = icc_ctlr_el3_read, > .writefn = icc_ctlr_el3_write, > }, > -- > 2.7.4 >
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index bca30c4..35e8eb3 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -1118,35 +1118,35 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 3, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_bpr[GICV3_G0]), + .readfn = icc_bpr_read, .writefn = icc_bpr_write, }, { .name = "ICC_AP0R0_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 4, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][0]), + .readfn = icc_ap_read, .writefn = icc_ap_write, }, { .name = "ICC_AP0R1_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 5, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][1]), + .readfn = icc_ap_read, .writefn = icc_ap_write, }, { .name = "ICC_AP0R2_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 6, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][2]), + .readfn = icc_ap_read, .writefn = icc_ap_write, }, { .name = "ICC_AP0R3_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 8, .opc2 = 7, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_apr[GICV3_G0][3]), + .readfn = icc_ap_read, .writefn = icc_ap_write, }, /* All the ICC_AP1R*_EL1 registers are banked */ @@ -1275,7 +1275,7 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 6, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL1_RW, .accessfn = gicv3_fiq_access, - .fieldoffset = offsetof(GICv3CPUState, icc_igrpen[GICV3_G0]), + .readfn = icc_igrpen_read, .writefn = icc_igrpen_write, }, /* This register is banked */ @@ -1299,7 +1299,6 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 12, .opc2 = 4, .type = ARM_CP_IO | ARM_CP_NO_RAW, .access = PL3_RW, - .fieldoffset = offsetof(GICv3CPUState, icc_ctlr_el3), .readfn = icc_ctlr_el3_read, .writefn = icc_ctlr_el3_write, },
In the ARMCPRegInfo definitions for the GICv3 CPU interface registers, we were trying to use .fieldoffset to specify the locations of data fields within the GICv3CPUState struct. This is completely broken, because .fieldoffset is for offsets into the CPUARMState struct. We didn't notice because we were only using this for reads to BPR0, AP0R<n>, IGRPEN0 and CTLR_EL3, and Linux doesn't use these registers. Replace the .fieldoffset uses with explicit read functions. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_cpuif.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)