target/arm/cpu.c | 34 +++- target/arm/cpu.h | 6 + target/arm/cpu_tcg.c | 42 +++++ target/arm/debug_helper.c | 3 + target/arm/helper.c | 336 ++++++++++++++++++++++++++++++++++++-- target/arm/internals.h | 4 + target/arm/machine.c | 28 ++++ target/arm/ptw.c | 136 ++++++++++++--- target/arm/tlb_helper.c | 4 + 9 files changed, 553 insertions(+), 40 deletions(-)
From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> Sorry for the "Reviewed-by" messup. I missed that on the explanation page. Thanks again for the review :) v6: patch 5: - I also changed HPRENR from ARM_CP_ALIAS to ARM_CP_NO_RAW. Its state is also present in the HPRLAR registers, but it doesn't make sense to access it raw. (I think) - I'm freeing the PRBAR/... strings explicitly now since I don't know how to use autofree in this setup correctly. Maybe {} around the part were the string is created/used, such that it is dropped at }? Tobias Röhmel (7): target/arm: Don't add all MIDR aliases for cores that implement PMSA target/arm: Make RVBAR available for all ARMv8 CPUs target/arm: Make stage_2_format for cache attributes optional target/arm: Enable TTBCR_EAE for ARMv8-R AArch32 target/arm: Add PMSAv8r registers target/arm: Add PMSAv8r functionality target/arm: Add ARM Cortex-R52 CPU target/arm/cpu.c | 34 +++- target/arm/cpu.h | 6 + target/arm/cpu_tcg.c | 42 +++++ target/arm/debug_helper.c | 3 + target/arm/helper.c | 336 ++++++++++++++++++++++++++++++++++++-- target/arm/internals.h | 4 + target/arm/machine.c | 28 ++++ target/arm/ptw.c | 136 ++++++++++++--- target/arm/tlb_helper.c | 4 + 9 files changed, 553 insertions(+), 40 deletions(-) -- 2.34.1
On Tue, 6 Dec 2022 at 10:25, <tobias.roehmel@rwth-aachen.de> wrote: > > From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> > > Sorry for the "Reviewed-by" messup. I missed that on the explanation > page. Thanks again for the review :) > > v6: > patch 5: > - I also changed HPRENR from ARM_CP_ALIAS to ARM_CP_NO_RAW. > Its state is also present in the HPRLAR registers, > but it doesn't make sense to access it raw. (I think) > - I'm freeing the PRBAR/... strings explicitly now since > I don't know how to use autofree in this setup correctly. > Maybe {} around the part were the string is created/used, > such that it is dropped at }? > Applied to target-arm.next; thanks for your efforts in getting this patchset through the code review process. -- PMM
Thanks for all the help, I learned a lot! Best regards, Tobias On 19.12.22 18:05, Peter Maydell wrote: > On Tue, 6 Dec 2022 at 10:25, <tobias.roehmel@rwth-aachen.de> wrote: >> From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> >> >> Sorry for the "Reviewed-by" messup. I missed that on the explanation >> page. Thanks again for the review :) >> >> v6: >> patch 5: >> - I also changed HPRENR from ARM_CP_ALIAS to ARM_CP_NO_RAW. >> Its state is also present in the HPRLAR registers, >> but it doesn't make sense to access it raw. (I think) >> - I'm freeing the PRBAR/... strings explicitly now since >> I don't know how to use autofree in this setup correctly. >> Maybe {} around the part were the string is created/used, >> such that it is dropped at }? >> > Applied to target-arm.next; thanks for your efforts in getting > this patchset through the code review process. > > -- PMM
On 6/12/22 11:24, tobias.roehmel@rwth-aachen.de wrote: > From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> > v6: > patch 5: > - I'm freeing the PRBAR/... strings explicitly now since > I don't know how to use autofree in this setup correctly. > Maybe {} around the part were the string is created/used, > such that it is dropped at }? The pointer is declared outside of a for() statement. Then inside this statement you alloc/free twice, using the same pointer. This is correct. If you really want to use g_autofree in such case, you'd need to declare within the same statement, one pointer for each string: for (i = 0; i < MIN(cpu->pmsav7_dregion, 32); ++i) { uint8_t crm = 0b1000 | extract32(i, 1, 3); uint8_t opc1 = extract32(i, 4, 1); uint8_t opc2 = extract32(i, 0, 1) << 2; g_autofree char *prbarn_str = g_strdup_printf("PRBAR%u", i); g_autofree char *prlarn_str = g_strdup_printf("PRLAR%u", i); const ARMCPRegInfo tmp_prbarn_reginfo = { .name = prbarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, .access = PL1_RW, .resetvalue = 0, .accessfn = access_tvm_trvm, .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read }; define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo); opc2 = extract32(i, 0, 1) << 2 | 0x1; const ARMCPRegInfo tmp_prlarn_reginfo = { .name = prlarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, .access = PL1_RW, .resetvalue = 0, .accessfn = access_tvm_trvm, .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read }; define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo); } (Note ARMCPRegInfo can be qualified const). Regards, Phil.
On 06.12.22 11:39, Philippe Mathieu-Daudé wrote: > On 6/12/22 11:24, tobias.roehmel@rwth-aachen.de wrote: >> From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> > >> v6: >> patch 5: >> - I'm freeing the PRBAR/... strings explicitly now since >> I don't know how to use autofree in this setup correctly. >> Maybe {} around the part were the string is created/used, >> such that it is dropped at }? > > The pointer is declared outside of a for() statement. Then > inside this statement you alloc/free twice, using the same > pointer. This is correct. If you really want to use > g_autofree in such case, you'd need to declare within the > same statement, one pointer for each string: > > for (i = 0; i < MIN(cpu->pmsav7_dregion, 32); ++i) { > uint8_t crm = 0b1000 | extract32(i, 1, 3); > uint8_t opc1 = extract32(i, 4, 1); > uint8_t opc2 = extract32(i, 0, 1) << 2; > g_autofree char *prbarn_str = g_strdup_printf("PRBAR%u", i); > g_autofree char *prlarn_str = g_strdup_printf("PRLAR%u", i); > > const ARMCPRegInfo tmp_prbarn_reginfo = { > .name = prbarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, > .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, > .access = PL1_RW, .resetvalue = 0, > .accessfn = access_tvm_trvm, > .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read > }; > define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo); > > opc2 = extract32(i, 0, 1) << 2 | 0x1; > const ARMCPRegInfo tmp_prlarn_reginfo = { > .name = prlarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, > .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, > .access = PL1_RW, .resetvalue = 0, > .accessfn = access_tvm_trvm, > .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read > }; > define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo); > } > > (Note ARMCPRegInfo can be qualified const). > > Regards, > > Phil. Thanks for the explanation! Would that be the preferred way? Best regards, Tobias
On 6/12/22 12:43, Tobias Roehmel wrote: > > On 06.12.22 11:39, Philippe Mathieu-Daudé wrote: >> On 6/12/22 11:24, tobias.roehmel@rwth-aachen.de wrote: >>> From: Tobias Röhmel <tobias.roehmel@rwth-aachen.de> >> >>> v6: >>> patch 5: >>> - I'm freeing the PRBAR/... strings explicitly now since >>> I don't know how to use autofree in this setup correctly. >>> Maybe {} around the part were the string is created/used, >>> such that it is dropped at }? >> >> The pointer is declared outside of a for() statement. Then >> inside this statement you alloc/free twice, using the same >> pointer. This is correct. If you really want to use >> g_autofree in such case, you'd need to declare within the >> same statement, one pointer for each string: >> >> for (i = 0; i < MIN(cpu->pmsav7_dregion, 32); ++i) { >> uint8_t crm = 0b1000 | extract32(i, 1, 3); >> uint8_t opc1 = extract32(i, 4, 1); >> uint8_t opc2 = extract32(i, 0, 1) << 2; >> g_autofree char *prbarn_str = g_strdup_printf("PRBAR%u", i); >> g_autofree char *prlarn_str = g_strdup_printf("PRLAR%u", i); >> >> const ARMCPRegInfo tmp_prbarn_reginfo = { >> .name = prbarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, >> .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, >> .access = PL1_RW, .resetvalue = 0, >> .accessfn = access_tvm_trvm, >> .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read >> }; >> define_one_arm_cp_reg(cpu, &tmp_prbarn_reginfo); >> >> opc2 = extract32(i, 0, 1) << 2 | 0x1; >> const ARMCPRegInfo tmp_prlarn_reginfo = { >> .name = prlarn_str, .type = ARM_CP_ALIAS | ARM_CP_NO_RAW, >> .cp = 15, .opc1 = opc1, .crn = 6, .crm = crm, .opc2 = opc2, >> .access = PL1_RW, .resetvalue = 0, >> .accessfn = access_tvm_trvm, >> .writefn = pmsav8r_regn_write, .readfn = pmsav8r_regn_read >> }; >> define_one_arm_cp_reg(cpu, &tmp_prlarn_reginfo); >> } >> >> (Note ARMCPRegInfo can be qualified const). >> >> Regards, >> >> Phil. > > Thanks for the explanation! Would that be the preferred way? What you posted is good enough IMO :) AFAIK there is no formal style recommendation on using g_autofree.
© 2016 - 2024 Red Hat, Inc.