st microops.

Gabe Black (Gerrit) via gem5-dev Tue, 22 Feb 2022 02:18:37 -0800

Gabe Black has uploaded this change for review. (https://gem5-review.googlesource.com/c/public/gem5/+/57009 )


Change subject: arch-x86: Move segmentation checks to ld/st microops.
......................................................................

arch-x86: Move segmentation checks to ld/st microops.

These microops were applying the segment base, but not checking limits,
segmentation level access permissions, whether the segment is usable,
etc. Then in the TLB, the original offset and other information had to
be recovered from the post segmentation address, and the request flags.

Instead, we can perform those checks in the microops which already have
all the information they need. This simplifies the plumbing to the TLB,
and avoids having to reverse segmentation which is error prone.

Change-Id: I1fe70ad650bf4f221660e608c49b70e081223e3f
---
M src/arch/amdgpu/common/tlb.cc
M src/arch/x86/SConscript
A src/arch/x86/insts/microldstop.cc
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/isa/microops/ldstop.isa
M src/arch/x86/ldstflags.hh
M src/arch/x86/tlb.cc
M src/arch/x86/ucasmlib/arch/x86/microops/ldstop.py
8 files changed, 235 insertions(+), 187 deletions(-)



diff --git a/src/arch/amdgpu/common/tlb.cc b/src/arch/amdgpu/common/tlb.cc
index 698570b..f0f806a 100644
--- a/src/arch/amdgpu/common/tlb.cc
+++ b/src/arch/amdgpu/common/tlb.cc
@@ -438,53 +438,25 @@
         // If protected mode has been enabled...
         if (m5Reg.prot) {
             DPRINTF(GPUTLB, "In protected mode.\n");
-            // If we're not in 64-bit mode, do protection/limit checks
-            if (m5Reg.mode != LongMode) {
-                DPRINTF(GPUTLB, "Not in long mode. Checking segment "
-                        "protection.\n");

+ // If we're not in 64-bit mode and doing a fetch, check CSlimits.

+            if (m5Reg.mode != LongMode && mode == BaseMMU::Execute) {
+                DPRINTF(GPUTLB, "Not in long mode. Checking CS limits.\n");

-                // Check for a null segment selector.

- if (!(seg == SEGMENT_REG_TSG || seg ==SYS_SEGMENT_REG_IDTR ||

-                    seg == SEGMENT_REG_HS || seg == SEGMENT_REG_LS)
-                    && !tc->readMiscRegNoEffect(MISCREG_SEG_SEL(seg))) {

+ const SegAttr attr =tc->readMiscRegNoEffect(MISCREG_CS_ATTR);

+                // Check for an unusable segment.
+                if (attr.unusable) {
+                    DPRINTF(GPUTLB, "Unusable segment.\n");
                     return std::make_shared<GeneralProtection>(0);
                 }
-
-                bool expandDown = false;

- SegAttr attr =tc->readMiscRegNoEffect(MISCREG_SEG_ATTR(seg));

-
-                if (seg >= SEGMENT_REG_ES && seg <= SEGMENT_REG_HS) {
-                    if (!attr.writable && (mode == BaseMMU::Write ||
-                        storeCheck))
-                        return std::make_shared<GeneralProtection>(0);
-
-                    if (!attr.readable && mode == BaseMMU::Read)
-                        return std::make_shared<GeneralProtection>(0);
-
-                    expandDown = attr.expandDown;
-
-                }
-
-                Addr base = tc->readMiscRegNoEffect(MISCREG_SEG_BASE(seg));

- Addr limit =tc->readMiscRegNoEffect(MISCREG_SEG_LIMIT(seg));- Addr logSize = (flags >> AddrSizeFlagShift) &AddrSizeFlagMask;

-                int size = 8 << logSize;
-
-                Addr offset = (vaddr - base) & mask(size);
-                Addr endOffset = offset + req->getSize() - 1;
-
-                if (expandDown) {
-                    DPRINTF(GPUTLB, "Checking an expand down segment.\n");
-                    warn_once("Expand down segments are untested.\n");
-
-                    if (offset <= limit || endOffset <= limit)
-                        return std::make_shared<GeneralProtection>(0);
-                } else {
-                    if (offset > limit || endOffset > limit)
-                        return std::make_shared<GeneralProtection>(0);

+ const Addr base =tc->readMiscRegNoEffect(MISCREG_CS_EFF_BASE);+ const Addr limit =tc->readMiscRegNoEffect(MISCREG_CS_LIMIT);

+                const Addr offset = vaddr - base;
+                if (offset + req->getSize() - 1 > limit) {
+                    DPRINTF(GPUTLB, "Segment limit check failed, "
+                            "offset = %#x limit = %#x.\n", offset, limit);
+                    return std::make_shared<GeneralProtection>(0);
                 }
             }
-
             // If paging is enabled, do the translation.
             if (m5Reg.paging) {
                 DPRINTF(GPUTLB, "Paging enabled.\n");
diff --git a/src/arch/x86/SConscript b/src/arch/x86/SConscript
index 5c80532..c592248 100644
--- a/src/arch/x86/SConscript
+++ b/src/arch/x86/SConscript
@@ -48,6 +48,7 @@
 Source('fs_workload.cc', tags='x86 isa')
 Source('insts/badmicroop.cc', tags='x86 isa')
 Source('insts/microop.cc', tags='x86 isa')
+Source('insts/microldstop.cc', tags='x86 isa')
 Source('insts/microregop.cc', tags='x86 isa')
 Source('insts/static_inst.cc', tags='x86 isa')
 Source('interrupts.cc', tags='x86 isa')

diff --git a/src/arch/x86/insts/microldstop.ccb/src/arch/x86/insts/microldstop.cc

new file mode 100644
index 0000000..0d603d9
--- /dev/null
+++ b/src/arch/x86/insts/microldstop.cc
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2007-2008 The Hewlett-Packard Development Company
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "arch/x86/insts/microldstop.hh"
+
+#include "arch/x86/faults.hh"
+#include "arch/x86/regs/misc.hh"
+#include "arch/x86/regs/segment.hh"
+#include "base/logging.hh"
+#include "debug/TLB.hh"
+
+namespace gem5
+{
+
+namespace X86ISA
+{
+
+Fault
+MemOp::segmentationCheck(Addr offset, SegmentRegIndex segment,
+        Addr base, Addr size, bool is_load, ThreadContext *tc) const
+{

+ const SegAttr attr =tc->readMiscRegNoEffect(MISCREG_SEG_ATTR(segment));

+    const bool store_check = memFlags & Request::READ_MODIFY_WRITE;
+
+    // Check for an unusable segment.
+    if (attr.unusable) {
+        DPRINTF(TLB, "Unusable segment.\n");
+        return std::make_shared<GeneralProtection>(0);
+    }
+
+    // Check read/write permissions.
+    bool expand_down = false;
+    if (segment >= SEGMENT_REG_ES && segment <= SEGMENT_REG_HS) {
+        if (!attr.writable && (!is_load || store_check)) {
+            DPRINTF(TLB, "Tried to write to unwritable segment.\n");
+            return std::make_shared<GeneralProtection>(0);
+        }
+        if (!attr.readable && is_load) {
+            DPRINTF(TLB, "Tried to read from unreadble segment.\n");
+            return std::make_shared<GeneralProtection>(0);
+        }
+        // Only these segments can be expand down.
+        expand_down = attr.expandDown;
+    }
+
+    // Check the limit.
+    const Addr limit = tc->readMiscRegNoEffect(MISCREG_SEG_LIMIT(segment));
+    if (expand_down) {
+        DPRINTF(TLB, "Checking an expand down segment.\n");
+        warn_once("Expand down segments are untested.\n");
+        if (offset <= limit)
+            return std::make_shared<GeneralProtection>(0);
+    } else {
+        if (offset + size - 1 > limit) {
+            DPRINTF(TLB, "Segment limit check failed, "
+                    "offset = %#x limit = %#x.\n", offset, limit);
+            return std::make_shared<GeneralProtection>(0);
+        }
+    }
+
+    // No problems found!
+    return NoFault;
+}
+
+} // namespace X86ISA
+} // namespace gem5

diff --git a/src/arch/x86/insts/microldstop.hhb/src/arch/x86/insts/microldstop.hh

index e38341b..90b8058 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -74,6 +74,9 @@

foldABit((addressSize == 1 && !mach_inst.rex.present) ? 1 <<6 : 0)

     {}

+    Fault segmentationCheck(Addr offset, SegmentRegIndex segment,
+            Addr base, Addr size, bool is_load, ThreadContext *tc) const;
+
   public:
     const uint8_t dataSize;
     const uint8_t addressSize;

diff --git a/src/arch/x86/isa/microops/ldstop.isab/src/arch/x86/isa/microops/ldstop.isa

index 2e15b71..2806f1e 100644
--- a/src/arch/x86/isa/microops/ldstop.isa
+++ b/src/arch/x86/isa/microops/ldstop.isa
@@ -49,20 +49,13 @@
     Fault %(class_name)s::execute(ExecContext *xc,
           Trace::InstRecord *traceData) const
     {
-        Fault fault = NoFault;
-        Addr EA;
-
         %(op_decl)s;
         %(op_rd)s;
-        %(ea_code)s;