With some values of rotations and datasizes, is it possible to call
"bits" with first being smaller than seccond. To prevent it, rcl,
similar to other rotations had an if to check if the value to rotate
is bigger than 1, however, rcl was checking for 'shiftAmt' instead
of 'realShiftAmt'.
This was not detected before because:
1 - The assert triggered in "bits" is from a recent commit
2 - The result is correct.
Change-Id: I669f4e064e12b035538e5dd0cc61eb4546603512
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52803
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
There is a function for this purpose in RegId called flatIndex(), which
I had attempted to use with PhysRegId which inherits from RegId.
Unfortunately, PhysRegId redefines the flatIndex() method and makes it
do something completely different, which is to turn map the index into a
linearization of all registers in the CPU.
Instead of using the decoy wrong method, and because the one we actually
want is not accessible, we can just manually compute the flattened index
in the two places we use it.
Change-Id: I8bd02f0be0f4fb3742da48b2955e9e75ec57245b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52603
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
NMI is platform dependent according to riscv spec, so we intentionally
propose a very minimal design that doesn't give user any new accessible
register to interact with the NMI mechanism.
In current design, the NMI reset vector is fixed to 0x0 and always set
mcause to zero. mret from NMI handler is still possible, but it's up to
the user to detect whether a M-mode trap handler is interrupted and to
recover from it (if at all possible).
1. Add new fault type to represent NMI fault
2. Add non-standard registers to save the status of NMI
a. nmivec[63:2] = NMI reset vector address
b. nmie[0:0] = is NMI enabled = not in NMI handler
c. nmip[0:0] = is NMI pending
3. Add new function in RiscvInterrupts to raise/clear NMI
Bug: 200169094
Test: None
Change-Id: Ia81e1c9589bc02f0690d094fff5f13412846acbe
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52363
Reviewed-by: Yu-hsin Wang <yuhsingw@google.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Two tests, one in test_kvm_fork_run.py and another in
test_kvm_cpu_switch.py, were trying to run the MI_Example protocol as
part of the quick/kokoro run. MI_Example requires the building of X86,
though we try to use GCN3_X86 exclusively to avoid compiling an
additional target. As such, these MI_Example tests have been moved to
the long/nightly run.
Change-Id: I3c196e7e336148a1b7a124b5810348a2e587fe24
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52563
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
When calling a method in a superclass, you can/should use the super()
method to get a reference to that class. The python 2 version of that
method takes two parameters, the current class name, and the "self"
instance. The python 3 version takes no arguments. This is better for a
at least three reasons.
First, this version is less verbose because you don't have to specify
any arguments.
Second, you don't have to remember which argument goes where (I always
have to look it up), and you can't accidentally use the wrong class
name, or forget to update it if you copy code from a different class.
Third, this version will work correctly if you use a class decorator.
I don't know exactly how the mechanics of this work, but it is referred
to in a comment on this stackoverflow question:
https://stackoverflow.com/questions/681953/how-to-decorate-a-class
Change-Id: I427737c8f767e80da86cd245642e3b057121bc3b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52224
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
This enables an SE mode execution of binary if added a Board's
superclass.
This has been abstracted from the SimpleBoard.
The 'set_workload' function has been renamed 'set_se_binary_workload'.
This is clearer on its purpose and doesn't overlap with other
'set_workload' functions.
Change-Id: I714425a3b6b98b91e762ad076eba583bc5953ddd
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52186
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
This patch moves 'connect_things' to the AbstractBoard constructor,
thereby meaning it does not need to be called directly in gem5
configuration scripts. This method has been changed to private as a
result.
As boards that inherit from AbstractBoard require certain things to be
setup prior to `connect_things` being called, a new abstract function,
`_setup_board` has been created. This is called in the AbstractBoard
constructor before `connect_things` and can be overridden by boards to
setup board properties as required.
Change-Id: I558a4321b850a6b19e20b7d56d0bcae5805114b6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52184
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
The override parameter in the constructor has been renamed to to
'download_md5_mismatch'. This makes the purpose of this parameter
clearer.
The default value has been changed from False to True. We found in most
cases we want to re-download files if the md5 values have changes. Not
wanting to do so is the corner case. This allows us to remove a lot of
parameters from test and example scripts, included in this patch.
Change-Id: I99fc7743f5adf78bf6f4f8efc6222e6df83ac6da
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52086
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
There are some cases where default downloading to `~/.cache/gem5` will
not work (for example, running gem5 in a Docker container, an error
observed here:
https://gem5-review.googlesource.com/c/public/gem5/+/51950).
To fix this, the `_get_default_resource_dir` has been altered to iterate
through a list of default resource directory targets. This change will
mean if `~/.cache/gem5` is not available then the resource is downloaded
to the current working directory of gem5.
Change-Id: I84e523f3adc182e140959243ff9335510d6b7185
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52423
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
The purpose of this class is to:
* Create a standard way to set a typical kernel/disk image workload.
* Cleans up the manner in which readfile values/contents are set.
* Allows a user to specify their own kernel arguments, but still sets
sensible defaults.
As of this commit, this interface has been added to the X86Board and the
RISCVBoard.
Change-Id: I34f4c2b829f1ae5c1cae12039436cbb345a89d09
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51949
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Previously SimpleBoard inherited from AbstractBoard and X86Board and
RiscvBoard inherited from the SimpleBoard. This has been shown to be a
needless level of abstraction. As such, this commit refactors the code
to have X86Board and RiscvBoard inherit directly from AbstractBoard.
Code common to the SimpleBoard, X86Board, and RiscvBoard has been moved
to the AbstractBoard.
Change-Id: I5a2c7404efeb4f8ddcb5d8006e3c163d10b88b2c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51790
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
This will let callers create a separate copy of a PCState class
instance. This makes it more explicit when creating copies of a PCState
to make sure the programmer is more aware, and avoids having to know
what the actual type is to make a copy.
Change-Id: I728a278afdb55b800c753a5b7f65f62f4a80c043
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52035
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
These helpers will make it easier to cast a PCStatePtr into an ISA
specific class with less syntactic fluff. They are currently implemented
with a static_cast for performance reasons, but could be implemented
with a dynamic_cast and an assert for extra debugging if you were
willing to pay the performance overhead. In the future this might be
switched/enabled as an extra debugging mode, like how locking can have
extra checks enabled in the Linux kernel.
Change-Id: Ibc2443c6b991ebc2e5d0240a88436849cb6de2b9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52033
Tested-by: kokoro <noreply+kokoro@google.com>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
This class has a lot of common functionality which all PCState classes
use, but in order to make it a true base class which provides the
complete interface for PCState-s throughout gem5, all its methods would
need to become virtual. That doesn't have to be the case today because
we use the literal full ISA specific PC class directly, but we need to
move away from that.
This change leaves PCStateBase empty, since we don't know what will need
to be accessible in base classes through a common/virtual interface.
Also, move methods which do not depend on the InstWidth template
parameter out of SimplePCState and into PCStateCommon. This avoids
having duplicate methods with the same contents which don't depend on
InstWidth.
Change-Id: I31309c4f35e897db1bc8318439fae1567a82b35e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52031
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Earl Ou <shunhsingou@google.com>
Maintainer: Daniel Carvalho <odanrc@yahoo.com.br>
Tested-by: kokoro <noreply+kokoro@google.com>
The setup() method of ScalarPrint, inherited from BasePrint, is not
called by its constructor, and is not called on the ScalarPrint "print"
in DistPrint::operator(). This sets most of the values in BasePrint.
Instead, this method sets these values itself based on the values of the
DistPrint itself.
Unfortunately it looks like this method forgot to set enableUnits, which
ends up otherwise uninitialized when the printUnits method is called.
This change fixes that.
Change-Id: Ib1ea78796539f6d9222d19dad597a4e64aa69808
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52483
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Daniel Carvalho <odanrc@yahoo.com.br>
Tested-by: kokoro <noreply+kokoro@google.com>
The constructor only initialized two fields, data and _tickInserted. The
print() method at least accesses the coherence status bits, which
valgrind determined were being accessed without being initialized.
This change adds a default initializer to all fields to prevent any
value from flapping around uninitialized.
Change-Id: Ie4c839504d49f9a131d8e3c3e8be02ff22f453a6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52404
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
The BaseCache::handleFill function would generate an "old_state" string
unconditionally, just in case it would need to print it out later on in
the function if the Cache debug variable was set. This is very wasteful.
We should only generate that string if we are actually going to use it
later on.
Change-Id: I4a570d1cd2814e5a089eac1233dedd1801d68975
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52405
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
When allocating memory with operator new(size_t), we should also delete
that memory with operator delete(). Note that this is a generic form of
new and delete which do not construct an object in the allocated space,
or delete the object when freeing the space.
There were a number of places where we were over-allocating a structure
so that there would be room after it for other data, at least sometimes
to allocate C structures which would have a trailing array of some other
structure with an undefined size. Those structures were then being
stored in a std::unique_ptr with the default deleter, which just calls
full blown delete and not operator delete.
It seems that this is often ok, and I was not able to find anything that
spelled out in bold letters that it isn't. I did find this sentence:
"If the pointer passed to the standard library deallocation function was
not obtained from the corresponding standard library allocation function,
the behavior is undefined."
On this webpage:
https://en.cppreference.com/w/cpp/memory/new/operator_delete
This is a *little* vague, since they might mean you can't mix malloc and
delete, or new and free. Strictly interpretting it though, it could mean
you can't mix operator new with regular delete, or any other mismatched
combination.
I also found that exactly how this causes problems depends on what heap
allocator you're using. When I used tcmalloc, gem5 would segfault within
that library. When I disabled tcmalloc to run valgrind, the segfault
went away. I think this may be because sometimes you get lucky and
undefined behavior is what you actually wanted, and sometimes you don't.
To fix this problem, this change overrides the deleter on all of these
unique_ptr-s so that they use operator delete. Also, it refactors some
code in arch/x86/kvm/x86_cpu.cc so that the function that allocates
memory with operator new returns a std::unique_ptr instead of a raw
pointer. This raw pointer was always immediately put into a unique_ptr
anyway, and, in addition to tidying up the call sights slightly, also
avoids having to define a custom deleter in each of those locations
instead of once in the allocation function.
Change-Id: I9ebff430996cf603051f5baa8708424819ed8465
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52383
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Gabe Black <gabe.black@gmail.com>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
There has been some confusion about usage of 'Optional'. In some areas
of the codebase it was assumed this specifies an optional parameter
(i.e., one which may or may not set, as it has a default value). This is
incorrect. 'Optional[<type>]' is shorthand for 'Union[<type>, None]',
i.e., it is used to state the value may be 'None'. This patch corrects
this throughout the gem5 codebase.
Change-Id: I77a6708dee448e8480870d073e128aed3d6ae904
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52143
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
There are two kinds of stats in the system. The old one requires an
unique name, while the new one requires an local name. The setName
function has a flag to specify the difference. In the constructor of
InfoAccess, it sets correct flag to the setName function. However, if
you set the name later with the setter, it wouldn't set the flag for
you. This leads the name conflict in new style stats with same local
name. We should also pass the correct flag in the name setter.
Change-Id: I0fcaad3cca65d0f2859c5f6cb28a00813a026a0c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52323
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
