This command is one of two that should be implemented by ATAPI devices.
An ATAPI device are essentially optical devices which use SCSI commands
which are transported over ATA using two special commands, a PACKET
command which actually sends the SCSI commands, and an IDENTIFY command
which is basically the same as the ATA IDENTIFY command but which is
only implemented on ATAPI devices. In order to determine if the device
connected to an IDE controller is an optical drive or a regular ATA hard
drive, software can send the ATAPI_IDENTIFY_DEVICE command and see if
gets an appropriate response.
In gem5, this command was originally not implemented by the IDE disk
model. If a driver attempted to send it, the gem5 model would panic and
kill the simulation. To fix that problem, that command was added to the
list of supported commands and just made a synonym for the ATA IDENTIFY
command since they have essentially the same response.
Unfortunately, this makes all ATA devices look like ATAPI devices, which
is not what we have implemented.
Instead, when we get this command, what we *should* do, as far as I can
tell by reading this:
http://users.utcluj.ro/~baruch/media/siee/labor/ATA-Interface.pdf
is to set the ERR bit in the status register, and then set the ABRT bit
in the error register to indicate that the command was not implemented.
I've attempted to implement that into the model with this change by
setting those bits as described, and then setting the "action" member to
be ACT_CMD_ERROR. I think that action is there primarily to support
cancelled transfers, but it seems like it has the desired effect(?).
Since the error bits are not really explicitly set or managed by the
model in most cases, this change also adds a little bit of code at the
top of startCommand which clears them to zero. These bits are supposed
to "contain the status of the last command executed", and if we're
starting a new command, the error bits no longer apply.
I'm confident that conceptually this is how the ATAPI_IDENTIFY_DEVICE
command should behave in our model, at least unless we decide to
implement real ATAPI models which actually accept SCSI commands, etc.
I'm less confident that I've modified the model to actually implement
that behavior, but as far as I can tell it doesn't seem to have broken
anything, and now SeaBIOS no longer things our disk model is a CDROM
drive.
Change-Id: I2c0840e279e9caa89c21a4e7cbdbcaf6bccd92ac
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55523
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
The stack size is something that applies to addresses when performing
accesses as part of some instructions. This was handled inconsistently
or incompletely or simply incorrectly in a few ways.
First, when pushing or popping from the stack, the *address size* should
be set to the stack size. The data size is generally the operand size.
When the stack pointer is incremented/decremented, it should be changed
by the data size. When a stack pointer is manipulated, the data size
for those calculations should be the stack size. Importantly that does
not change the value of the increment/decrement, which is the operand
size still. This usage has been fixed throughout.
The TLB generally needs to know what the address size was in order to
figure out what segment offset was used so that it can do limit checks.
There is some inherent inaccuracy in doing things in reverse like this,
but that's how it works currently. To find that size, the TLB tried to
start from first principles to figure out what the default address size
was, and then whether there was an override was passed in through the
request flags.
This is *very* inaccurate for a few reasons. First, the override doesn't
always apply. Second, the address size used by a particular instruction
doesn't have to be based on any particular size, whether that is the
default or alternate address size, the stack size, etc.
Instead, the instructions now pass the actual size being used in as a 2
bit value (0 -> 1 byte, 1 -> 2 bytes, 2 -> 4 bytes, 3 -> 8 bytes),
avoiding most of the inaccuracy and approximation.
Because the CPU won't embed any size information into fetches, we'll
just assume those have no wrap around within the address size.
Finally, there were microops that had been added which overrode the
address size to be the stack size internally, and try to help the TLB
figure out what to do to figure out the address size. Because both of
those things are now handled in a different way, those microops are no
longer needed or used and have been deleted.
Change-Id: I2b1bdf1acf1540bf643fac6d49fe1a5a576ba5c1
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55443
Tested-by: kokoro <noreply+kokoro@google.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
If TARGET_GPU_ISA is not set, even if the GPU ISA namespace isn't used
by anything, the logic which figures out what to set it to will fail.
This checks for that condition and sets it to something invalid, but
doesn't crash. If that namespace is actually used, then the build will
still fail.
Change-Id: Iec44255cccbafa4aceaa68bdd8b6a835dc0637a0
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56895
Reviewed-by: Matthew Poremba <matthew.poremba@amd.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
The Request::NO_ACCESS flag instructs the cpu model to not issue
the request to the memory port.
While Atomic and Timing CPU models properly implement it [1], [2],
* MinorCPU is not looking at the flag
* O3CPU is looking at the flag only in case of a nested transaction
start/commit
This patch is extending NO_ACCESS support to all memory instructions.
This is achieved by using the localAccess callback in the Request object.
Handling of nested hardware transactions in the O3 LSQUnit is moved within
the local accessor callback
[1]: https://github.com/gem5/gem5/blob/v21.1.0.2/\
src/cpu/simple/timing.cc#L318
[2]: https://github.com/gem5/gem5/blob/v21.1.0.2/\
src/cpu/simple/atomic.cc#L396
Change-Id: Ifd5b388c53ead4fe358aa35d2197c12f1c5bb4f2
Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56591
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: ZHENGRONG WANG <seanyukigeek@gmail.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
The MutableSet class used to be part of the collections module directly,
but in 3.3 was moved to collections.abc. Apparently there was still a
version in collections, since we had been importing it from that old
location and it had been working up until now. After a recent update,
this stopped working for me, and may be tied to an update to the local
version of python on my machine.
This change imports MutableSet from collections.abc instead of
collections directly. I found only one place that this class was used in
src or ext, so I don't think it needs to be fixed anywhere else.
Change-Id: I8b2e82160fd433d57af4a7008ec282ee8ad8a422
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56849
Maintainer: Bobby Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabeblack@google.com>
This patch updates the canAllocate function to account both for
the number of regions of registers that need to be allocated,
and for the fact that the registers aren't one continuous chunk.
The patch also consolidates the registers as much as possible when
a register chunk is freed. This prevents fragmentation from making
it impossible to allocate enough registers
Change-Id: Ic95cfe614d247add475f7139d3703991042f8149
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56909
Reviewed-by: Matt Sinclair <mattdsinclair@gmail.com>
Maintainer: Matt Sinclair <mattdsinclair@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Matthew Poremba <matthew.poremba@amd.com>
If the size of the address range is smaller than the maximum number
of outstanding requests allowed downstream, the tester will get stuck
trying to find a unique address. This patch adds a check for this
condition and forces the tester to wait for responses before
trying to generate another request.
Change-Id: Ie894a074cc4f8c7ad3d875dc21e8eb4f04562d72
Signed-off-by: Tiago Mück <tiago.muck@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56811
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
The tester currently assumes that one token per lane is needed when
checking if an action is ready to be issued. When actually issuing
requests, it is possible that a memory location is not valid for various
reasons. This was not being considered when checking for tokens causing
the tester to acquire more tokens than requests sent. Since tokens are
returned when requests are coalesced, this was causing some tokens never
to be returned, eventually depleting the token pool and causing a
deadlock.
Add a new method which determines the number of tokens needed for an
action. This is called by both the ready check method and the method to
issue the action to ensure they are aligned.
Change-Id: Ic1af72085c3b077539eb3fd129331e776ebdffbc
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56450
Reviewed-by: Matt Sinclair <mattdsinclair@gmail.com>
Maintainer: Matt Sinclair <mattdsinclair@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
This protocol is using an old style where read/writes to memory were
being done by writing to a DataBlock in a DirectoryMemory entry. This
results in having multiple copies of memory, leads to stale copies in at
least one memory (usually DRAM), and require --access-backing-store in
most cases to work properly. This changeset removes all references to
getDirectoryEntry(...).DataBlk and instead forwards those reads and
writes to DRAM always.
This results in new transient states BL_WM, BDW_WM, and B_WM which are
blocked states waiting on memory acks indicating a write request is
complete. The appropriate transitions are updates to move to these new
states and stall states are updated to include them. DMA write ACK is
also moved to when the request is sent to memory, rather than when the
request is received.
Change-Id: Ic5bd6a8a8881d7df782e0f7eed8be9d873610e04
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56446
Reviewed-by: Matt Sinclair <mattdsinclair@gmail.com>
Maintainer: Matt Sinclair <mattdsinclair@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
The directory has an assert that this is at least one destination for a
probe when sending an invalidation or shared probe to coherence end
points in the protocol (TCC, LLC). This is not necessarily request and
for certain configurations there will be no probes required and none
will be sent. One such configuration is the GPU protocol tester which
would not require a probe to the CPU if it does not exist.
To fix this we first collect the probe destinations. Then we check if
any destinations exist. If so, we send the probe message. Otherwise we
immediately enqueue a probe complete message to the trigger queue. This
reorganization prevents messages with no destinations from being
enqueued, meeting the criteria for the assertion.
Change-Id: If016f457cb8c9e0277a910ac2c3f315c25b50ce8
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55543
Reviewed-by: Matt Sinclair <mattdsinclair@gmail.com>
Maintainer: Matt Sinclair <mattdsinclair@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
JIRA: https://gem5.atlassian.net/browse/GEM5-1185
Fixed an issue in which a CleanUnique responder would incorrectly
deallocate the cache block when handling an stale CU when the state
is UD_RU or UC_RU (thus incorrectly transitioning to RU).
The fix is to handle stale CUs similarly to stale WBs where we
override the dataValid TBE field to prevent the wrong state
transition.
This patch moves the stale code path to a separate transition
(similarly to stale WBs/Evicts) and moves the dataValid override to
Initiate_Request_Stale so it applies to all stale request types.
Notice now the stale field is also set on stale Comp_UC responses.
Additional minor change: CheckUpgrade_FromRU is the same as
CheckUpgrade_FromStore so it was removed.
Change-Id: I0a2cedcfde1dc30d67aa2c16d71b7470369c2b6e
Signed-off-by: Tiago Mück <tiago.muck@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56810
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Meatboy 106 <garbage2collector@gmail.com>
The recent change to add an "exclude" pattern to Glob in SCons also
seems to have triggered a bug where SCons has decided directories that
don't exist are files, and then gets upset later when we try to treat
them as directories.
To avoid that bug, and to also make recursive searching for isa parser
.py files work, we can replace the call to Glob with a loop based on
os.walk.
Also, tell the microcode assembler not to generate the parsetab.py file
in the first place. This comes with a minor performance overhead, but
shouldn't matter for us since there are *much* bigger overheads when
processing ISA descriptions.
Change-Id: Ia84e97dab72723ad3f4350798ad70178e231144c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56749
Maintainer: Bobby Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
When instructions perform accesses, they embed the segment being used
into the request flags. When the CPU creates a request instead, for
instance when fetching an instruction, it doesn't know to do that.
This change adds a check in the TLB when makes sure CS is used when
checking a fetch, even if the flags didn't say to.
Change-Id: Ie9da3afc0b10eeb96247353150c64f1829cea41b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55247
Reviewed-by: Matthew Poremba <matthew.poremba@amd.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
When using fast forwarding, createThreads() is not
called upon FutureCPUs. This causes segment fault
as the decoder is initialized in createThreads() and
needed when instantiating CPUs.
This commit basically fixes this by invoking
createThreads() on FutureCPUs after they are created.
Change-Id: I812d18f06878f9fc3fa2183a2c8a64d316413398
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56812
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Luming Wang <wlm199558@126.com>
Tested-by: kokoro <noreply+kokoro@google.com>
When dealing with segmentation in x86, it is *usually* illegal to
attempt to access a segment which has a null selector when in protected
mode and not in 64 bit mode. While this is *almost* true, it is not
actually technically true.
What actually *is* true is that if you *set up* a segment using a null
selector in those circumstances, that segment becomes unusable, and then
tryint to use it causes a fault.
When in real mode, it is perfectly legal to use a null selector to
access memory, since that is just a selector with numerical value 0.
When you then transition into protected mode, the selector would still
be 0 (a null selector), but the segment itself would still be set up
properly and usuable using the base value, limit, and other attributes
it carried over from real mode.
Rather than check if a segment has a null selector while handling
segmentation, it's more correct for us to keep track of whether the
segment is currently usable and check that in the TLB.
Change-Id: Ic2c09e1cfa05afcb03900213b72733545c8f0f4c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55245
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bobby Bruce <bbruce@ucdavis.edu>
These were loading the immediate into a temporary microcode register
which would then be used to calculate the address to actually send to
the memory system. Unfortunately this was using a data size equal to the
address size, which would mean that the immediate would be merged into
that temporary, leaving previously set bits intact. The data size
*should* have been set to 8, and was already in other similar
instructions. That forces the limm microop to overwrite the temporary
entirely.
Change-Id: I87c82b4677db768ccb6401a3dbda61317c014152
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55286
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bobby Bruce <bbruce@ucdavis.edu>
