arch-arm: Fix ArmSystem::_resetAddr evalutation
authorGiacomo Travaglini <giacomo.travaglini@arm.com>
Fri, 13 Mar 2020 11:30:10 +0000 (11:30 +0000)
committerGiacomo Travaglini <giacomo.travaglini@arm.com>
Thu, 19 Mar 2020 22:36:12 +0000 (22:36 +0000)
With:

https://gem5-review.googlesource.com/c/public/gem5/+/26466

The ArmSystem reset address (_resetAddr) is always forced by the
workload:

 _resetAddr = workload->entry

So there is no possibility to manually specify a reset address.

This was not the case before:
The resetAddr was forced only if auto_reset_addr was true or if there
was an associated bootloader to the kernel image. In that case even if
auto_reset_addr was false, the reset address was determined by the
bootloader entry.
This was also not ideal (but it was working)

This patch is cleaning all of this:

If you want to have automatic detection (recommended), you would need to
set auto_reset_addr (now turned to true by default).  This will allow to
keep most fs script untouched.  If you don't want to use automatic
detection, set auto_reset_addr to False and provide your own reset
address.

Change-Id: I5d7a55fd9060b9973c7d5b5542bd199950e1073e
Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26723
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabeblack@google.com>
src/arch/arm/ArmSystem.py
src/arch/arm/fs_workload.cc
src/arch/arm/fs_workload.hh
src/arch/arm/system.cc

index 1b5fc907dd05d827b471c5e9e387c593024672f7..07fdad66302e4c6b033a104ad9c4657aeca36182 100644 (file)
@@ -58,7 +58,7 @@ class ArmSystem(System):
     have_lpae = Param.Bool(True, "True if LPAE is implemented")
     reset_addr = Param.Addr(0x0,
         "Reset address (ARMv8)")
-    auto_reset_addr = Param.Bool(False,
+    auto_reset_addr = Param.Bool(True,
         "Determine reset address from kernel entry point if no boot loader")
     highest_el_is_64 = Param.Bool(False,
         "True if the register width of the highest implemented exception level "
index b3dd6b5b815a789f6405d8174a2010a1b7a599a3..3d81156214cc63b78d8ee5729e743a51c49f1680 100644 (file)
@@ -69,7 +69,9 @@ SkipFunc::returnFromFuncIn(ThreadContext *tc)
     }
 }
 
-FsWorkload::FsWorkload(Params *p) : OsKernel(*p)
+FsWorkload::FsWorkload(Params *p)
+  : OsKernel(*p),
+    kernelEntry((entry & loadAddrMask) + loadAddrOffset)
 {
     bootLoaders.reserve(p->boot_loader.size());
     for (const auto &bl : p->boot_loader) {
@@ -93,7 +95,6 @@ FsWorkload::FsWorkload(Params *p) : OsKernel(*p)
     if (bootldr) {
         bootldr->loadGlobalSymbols(debugSymbolTable);
 
-        entry = bootldr->entryPoint();
         _highestELIs64 = (bootldr->getArch() == ObjectFile::Arm64);
     } else {
         _highestELIs64 = (obj->getArch() == ObjectFile::Arm64);
@@ -116,8 +117,6 @@ FsWorkload::initState()
 
     auto *arm_sys = dynamic_cast<ArmSystem *>(system);
 
-    Addr kernel_entry = (obj->entryPoint() & loadAddrMask) + loadAddrOffset;
-
     if (bootldr) {
         bool is_gic_v2 =
             arm_sys->getGIC()->supportsVersion(BaseGic::GicVersion::GIC_V2);
@@ -136,12 +135,12 @@ FsWorkload::initState()
 
         for (auto tc: arm_sys->threadContexts) {
             if (!arm_sys->highestELIs64())
-                tc->setIntReg(3, kernel_entry);
+                tc->setIntReg(3, kernelEntry);
             if (is_gic_v2)
                 tc->setIntReg(4, arm_sys->params()->gic_cpu_addr);
             tc->setIntReg(5, arm_sys->params()->flags_addr);
         }
-        inform("Using kernel entry physical address at %#x\n", kernel_entry);
+        inform("Using kernel entry physical address at %#x\n", kernelEntry);
     } else {
         // Set the initial PC to be at start of the kernel code
         if (!arm_sys->highestELIs64())
@@ -160,6 +159,16 @@ FsWorkload::getBootLoader(ObjectFile *const obj)
     return nullptr;
 }
 
+Addr
+FsWorkload::resetAddr() const
+{
+    if (bootldr) {
+        return bootldr->entryPoint();
+    } else {
+        return kernelEntry;
+    }
+}
+
 } // namespace ArmISA
 
 ArmISA::FsWorkload *
index 5e97bba7a9918cf70626990df8c39b2c23e397fa..6e1af08ee795a081b3a777918a971a95e58b6e97 100644 (file)
@@ -75,6 +75,13 @@ class FsWorkload : public OsKernel
      */
     bool _highestELIs64 = true;
 
+    /**
+     * This differs from entry since it takes into account where
+     * the kernel is loaded in memory (with loadAddrMask and
+     * loadAddrOffset).
+     */
+    Addr kernelEntry = 0;
+
     /**
      * Get a boot loader that matches the kernel.
      *
@@ -96,6 +103,14 @@ class FsWorkload : public OsKernel
 
     void initState() override;
 
+    /**
+     * Returns the reset address to be used by an ArmSystem.
+     * It the workload is using a bootloader, it will return
+     * the bootloader entry point.
+     * @returns Arm reset address
+     */
+    Addr resetAddr() const;
+
     bool highestELIs64() const { return _highestELIs64; }
 };
 
index 97c3c5f3cc24615e4edd9be4b5cfcb6f1ce5879b..6094121cc72d4794446710c1355523845caab22c 100644 (file)
@@ -73,21 +73,18 @@ ArmSystem::ArmSystem(Params *p)
       semihosting(p->semihosting),
       multiProc(p->multi_proc)
 {
-    if (p->auto_reset_addr) {
-        _resetAddr = (workload->entry & workload->loadAddrMask) +
-            workload->loadAddrOffset;
-    } else {
-        _resetAddr = p->reset_addr;
-    }
-
     auto *arm_workload = dynamic_cast<ArmISA::FsWorkload *>(p->workload);
     panic_if(!arm_workload,
             "Workload was not the expected type (ArmISA::FsWorkload).");
 
-    warn_if(workload->entry != _resetAddr,
-            "Workload entry point %#x overriding reset address %#x",
-            workload->entry, _resetAddr);
-    _resetAddr = workload->entry;
+    if (p->auto_reset_addr) {
+        _resetAddr = arm_workload->resetAddr();
+    } else {
+        _resetAddr = p->reset_addr;
+        warn_if(arm_workload->resetAddr() != _resetAddr,
+                "Workload entry point %#x and reset address %#x are different",
+                arm_workload->resetAddr(), _resetAddr);
+    }
 
     if (arm_workload->highestELIs64() != _highestELIs64) {
         warn("Highest ARM exception-level set to AArch%d but the workload "