From 0df37a33f602fdce8a2697655c318a702f79ba28 Mon Sep 17 00:00:00 2001 From: Giacomo Travaglini Date: Wed, 23 Nov 2022 08:35:50 +0000 Subject: [PATCH] arch-arm: Setup TC/ISA at construction time 2nd attempt This partly reverts commit ec75787aef56665e893d70293bf3a0f93c33bb6a by fixing the original problem noted by Bobby (long regressions): setupThreadContext has to be implemented otherswise the GICv3 cpu interface will end up holding old references when switching TC/ISAs. This new implementation is still setting up the cpu interface reference in the ISA only when it is required, but it is storing the TC/ISA reference within the interface every time the ISA::setupThreadContext gets called. Change-Id: I2f54f95761d63655162c253e887b872f3718c764 Signed-off-by: Giacomo Travaglini Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/65931 Reviewed-by: Jason Lowe-Power Tested-by: kokoro Maintainer: Bobby Bruce --- src/arch/arm/isa.cc | 33 ++++++++++++++++++++--------- src/arch/arm/isa.hh | 1 + src/dev/arm/gic_v3.cc | 2 +- src/dev/arm/gic_v3_cpu_interface.cc | 8 +++++-- src/dev/arm/gic_v3_cpu_interface.hh | 6 +++--- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc index a30fd94596..543e0eba7b 100644 --- a/src/arch/arm/isa.cc +++ b/src/arch/arm/isa.cc @@ -524,15 +524,10 @@ ISA::setupThreadContext() selfDebug->init(tc); - Gicv3 *gicv3 = dynamic_cast(system->getGIC()); - if (!gicv3) - return; - - if (!gicv3CpuInterface) - gicv3CpuInterface.reset(gicv3->getCPUInterface(tc->contextId())); - - gicv3CpuInterface->setISA(this); - gicv3CpuInterface->setThreadContext(tc); + if (auto gicv3_ifc = getGICv3CPUInterface(tc); gicv3_ifc) { + gicv3_ifc->setISA(this); + gicv3_ifc->setThreadContext(tc); + } } void @@ -2008,10 +2003,28 @@ ISA::getGenericTimer() BaseISADevice & ISA::getGICv3CPUInterface() { - panic_if(!gicv3CpuInterface, "GICV3 cpu interface is not registered!"); + if (gicv3CpuInterface) + return *gicv3CpuInterface.get(); + + auto gicv3_ifc = getGICv3CPUInterface(tc); + panic_if(!gicv3_ifc, "The system does not have a GICv3 irq controller\n"); + gicv3CpuInterface.reset(gicv3_ifc); + return *gicv3CpuInterface.get(); } +BaseISADevice* +ISA::getGICv3CPUInterface(ThreadContext *tc) +{ + assert(system); + Gicv3 *gicv3 = dynamic_cast(system->getGIC()); + if (gicv3) { + return gicv3->getCPUInterface(tc->contextId()); + } else { + return nullptr; + } +} + bool ISA::inSecureState() const { diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh index 1f7a7561a7..9e1afa714b 100644 --- a/src/arch/arm/isa.hh +++ b/src/arch/arm/isa.hh @@ -116,6 +116,7 @@ namespace ArmISA BaseISADevice &getGenericTimer(); BaseISADevice &getGICv3CPUInterface(); + BaseISADevice *getGICv3CPUInterface(ThreadContext *tc); RegVal miscRegs[NUM_MISCREGS]; const RegId *intRegMap; diff --git a/src/dev/arm/gic_v3.cc b/src/dev/arm/gic_v3.cc index dde3818b07..e14d1f2bef 100644 --- a/src/dev/arm/gic_v3.cc +++ b/src/dev/arm/gic_v3.cc @@ -147,7 +147,7 @@ Gicv3::init() for (int i = 0; i < threads; i++) { redistributors[i] = new Gicv3Redistributor(this, i); - cpuInterfaces[i] = new Gicv3CPUInterface(this, i); + cpuInterfaces[i] = new Gicv3CPUInterface(this, sys->threads[i]); } distRange = RangeSize(params().dist_addr, diff --git a/src/dev/arm/gic_v3_cpu_interface.cc b/src/dev/arm/gic_v3_cpu_interface.cc index 0e1dbaa04b..28a173943d 100644 --- a/src/dev/arm/gic_v3_cpu_interface.cc +++ b/src/dev/arm/gic_v3_cpu_interface.cc @@ -55,15 +55,19 @@ using namespace ArmISA; const uint8_t Gicv3CPUInterface::GIC_MIN_BPR; const uint8_t Gicv3CPUInterface::GIC_MIN_BPR_NS; -Gicv3CPUInterface::Gicv3CPUInterface(Gicv3 * gic, uint32_t cpu_id) +Gicv3CPUInterface::Gicv3CPUInterface(Gicv3 * gic, ThreadContext *_tc) : BaseISADevice(), gic(gic), redistributor(nullptr), distributor(nullptr), - cpuId(cpu_id) + tc(_tc), + maintenanceInterrupt(gic->params().maint_int->get(tc)), + cpuId(tc->contextId()) { hppi.prio = 0xff; hppi.intid = Gicv3::INTID_SPURIOUS; + + setISA(static_cast(tc->getIsaPtr())); } void diff --git a/src/dev/arm/gic_v3_cpu_interface.hh b/src/dev/arm/gic_v3_cpu_interface.hh index e860373fb5..ff476bc3c6 100644 --- a/src/dev/arm/gic_v3_cpu_interface.hh +++ b/src/dev/arm/gic_v3_cpu_interface.hh @@ -68,10 +68,10 @@ class Gicv3CPUInterface : public ArmISA::BaseISADevice, public Serializable Gicv3 * gic; Gicv3Redistributor * redistributor; Gicv3Distributor * distributor; - uint32_t cpuId; - ArmInterruptPin *maintenanceInterrupt; ThreadContext *tc; + ArmInterruptPin *maintenanceInterrupt; + uint32_t cpuId; BitUnion64(ICC_CTLR_EL1) Bitfield<63, 20> res0_3; @@ -359,7 +359,7 @@ class Gicv3CPUInterface : public ArmISA::BaseISADevice, public Serializable void setBankedMiscReg(ArmISA::MiscRegIndex misc_reg, RegVal val) const; public: - Gicv3CPUInterface(Gicv3 * gic, uint32_t cpu_id); + Gicv3CPUInterface(Gicv3 * gic, ThreadContext *tc); void init();