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

Change subject: dev: Rework how PCI BARs are set up in python and C++.
......................................................................

dev: Rework how PCI BARs are set up in python and C++.

In python, the BARs had been configured using three arrays and a scalar
parameter. The arrays tracked the BAR value in the config, whether the
BAR was for a "legacy" IO range, and the size of the BAR, and the
scalar parameter was an offset for the "legacy" IO addresses to map
into the host physical address space. The nature of a BAR was implied
by its raw config space value, with each of the control bits (IO vs.
memory, 64 bit, reserved bits) encoded directly in the value.

Now, the BARs are represented by objects which have different types
depending on what type of BAR they are. There's one for IO, one for
memory, one for the upper 32 bits of a 64 bit BAR (so indices work
out), and one for legacy IO ranges. Each type has parameters which
are appropriate for it, and they're parameters are all grouped together
as a unit instead of being spread across all the previous values.
The legacy IO offset has been removed, since these addresses can be
offset like any other IO address. They can be represented naturally
in the config using their typical IO port numbers, and still be turned
into an address that gem5 will handle correctly in the back end.

Unfortunately, this exposes a problem in the config system where
a VectorParam can't be overwritten successfully one element at a time,
at least when dealing with SimObject classes. It might work with
actual SimObjects in a config, but I haven't tried it. If you were
to do that to, for instance, update the BARs for x86 so that they
used legacy IO ports for the IDE controller, it would complain that
you were trying to instantiate orphaned nodes. Replacing the whole
VectorParam with a new list of BAR objects seems to work, so that's
what's implemented in this change.

On the C++ side, BARs in the config space are treated as flat values
on reads, and are stored in the config structure associated with each
PCI device. On writes, the value is first passed to the BAR object,
and it has a chance to mask any bits which are fixed in hardware and
update its idea of what range it corresponds to in memory.

When sending AddrRanges up to the parent bus to set up routing, the
BARs generate each AddrRange if and only if their type has been
enabled in the config space command register. The BAR object which
represents the upper 32 bits of a 64 bit BAR does not claim to be
IO or memory, and so doesn't contribute a range. It communicates with
the BAR which represents the lower 32 bits, so that that BAR has the
whole base address.

Since the IO or memory BAR enable bits in the command register are now
handled by the PCI device base class, the IDE controller no longer has
to handle that manually. It does still need to keep track of whether
the bus master functionality has been enabled though, which it can
check when those registers are accessed.

There was already a mechanism for decoding addresses based on BARs
in the PCI device base class, but it was overly complicated and not
used consistently across devices. It's been consolidated, and used in
most places where it makes sense.

Finally, a few unnecessary values have been dropped from the base PCI
device's and IDE controller's checkpoint output. These were just local
copies of information already in the BARs, which in turn are already
stored along with the data in the device's config space.

Change-Id: I16d5f8cdf86d7a2d02a6b04d1f9e1b3eb1dd189d
---
M src/dev/arm/RealView.py
M src/dev/net/Ethernet.py
M src/dev/net/sinic.cc
M src/dev/pci/CopyEngine.py
M src/dev/pci/PciDevice.py
M src/dev/pci/device.cc
M src/dev/pci/device.hh
M src/dev/pci/pcireg.h
M src/dev/storage/Ide.py
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_ctrl.hh
M src/dev/virtio/VirtIO.py
M src/dev/virtio/pci.cc
M src/dev/x86/SouthBridge.py
14 files changed, 404 insertions(+), 343 deletions(-)



diff --git a/src/dev/arm/RealView.py b/src/dev/arm/RealView.py
index 9ab0472..9e7ce44 100644
--- a/src/dev/arm/RealView.py
+++ b/src/dev/arm/RealView.py
@@ -62,6 +62,7 @@
 from m5.objects.VirtIOMMIO import MmioVirtIO
 from m5.objects.Display import Display, Display1080p
 from m5.objects.SMMUv3 import SMMUv3
+from m5.objects.PciDevice import PciLegacyIoBar

 # Platforms with KVM support should generally use in-kernel GIC
 # emulation. Use a GIC model that automatically switches between
@@ -717,10 +718,11 @@
     kmi1   = Pl050(pio_addr=0x1c070000, interrupt=ArmSPI(num=45),
                    ps2=PS2TouchKit())
     cf_ctrl = IdeController(disks=[], pci_func=0, pci_dev=0, pci_bus=2,
-                            io_shift = 2, ctrl_offset = 2, Command = 0x1,
-                            BAR0 = 0x1C1A0000, BAR0Size = '256B',
-                            BAR1 = 0x1C1A0100, BAR1Size = '4096B',
-                            BAR0LegacyIO = True, BAR1LegacyIO = True)
+                            io_shift = 2, ctrl_offset = 2, Command = 0x1)
+    cf_ctrl.BARs = (PciLegacyIoBar(addr='0x1C1A0000', size='256B'),
+                    PciLegacyIoBar(addr='0x1C1A0100', size='4096B'),
+                    PciIoBar(size='8B'), PciIoBar(size='4B'),
+                    PciIoBar(size='16B'))

     bootmem        = SimpleMemory(range = AddrRange('64MB'),
                                   conf_table_reported = False)
diff --git a/src/dev/net/Ethernet.py b/src/dev/net/Ethernet.py
index fd16b09..fdb93fd 100644
--- a/src/dev/net/Ethernet.py
+++ b/src/dev/net/Ethernet.py
@@ -40,7 +40,7 @@
 from m5.SimObject import SimObject
 from m5.params import *
 from m5.proxy import *
-from m5.objects.PciDevice import PciDevice
+from m5.objects.PciDevice import PciDevice, PciIoBar, PciMemBar

 ETHERNET_ROLE = 'ETHERNET'
 Port.compat(ETHERNET_ROLE, ETHERNET_ROLE)
@@ -152,17 +152,11 @@
     SubClassCode = 0x00
     ClassCode = 0x02
     ProgIF = 0x00
-    BAR0 = 0x00000000
-    BAR1 = 0x00000000
-    BAR2 = 0x00000000
-    BAR3 = 0x00000000
-    BAR4 = 0x00000000
-    BAR5 = 0x00000000
+    BARs = PciMemBar(size='128kB')
     MaximumLatency = 0x00
     MinimumGrant = 0xff
     InterruptLine = 0x1e
     InterruptPin = 0x01
-    BAR0Size = '128kB'
     wb_delay = Param.Latency('10ns', "delay before desc writeback occurs")
     fetch_delay = Param.Latency('10ns', "delay before desc fetch occurs")
fetch_comp_delay = Param.Latency('10ns', "delay after desc fetch occurs")
@@ -224,18 +218,11 @@
     SubClassCode = 0x00
     ClassCode = 0x02
     ProgIF = 0x00
-    BAR0 = 0x00000001
-    BAR1 = 0x00000000
-    BAR2 = 0x00000000
-    BAR3 = 0x00000000
-    BAR4 = 0x00000000
-    BAR5 = 0x00000000
+    BARs = (PciIoBar(size='256B'), PciMemBar(size='4kB'))
     MaximumLatency = 0x34
     MinimumGrant = 0xb0
     InterruptLine = 0x1e
     InterruptPin = 0x01
-    BAR0Size = '256B'
-    BAR1Size = '4kB'



@@ -265,16 +252,8 @@
     SubClassCode = 0x00
     ClassCode = 0x02
     ProgIF = 0x00
-    BAR0 = 0x00000000
-    BAR1 = 0x00000000
-    BAR2 = 0x00000000
-    BAR3 = 0x00000000
-    BAR4 = 0x00000000
-    BAR5 = 0x00000000
+    BARs = PciMemBar(size='64kB')
     MaximumLatency = 0x34
     MinimumGrant = 0xb0
     InterruptLine = 0x1e
     InterruptPin = 0x01
-    BAR0Size = '64kB'
-
-
diff --git a/src/dev/net/sinic.cc b/src/dev/net/sinic.cc
index bc6fdbd..d586d8e 100644
--- a/src/dev/net/sinic.cc
+++ b/src/dev/net/sinic.cc
@@ -210,10 +210,12 @@
 Device::read(PacketPtr pkt)
 {
     assert(config.command & PCI_CMD_MSE);
-    assert(pkt->getAddr() >= BARAddrs[0] && pkt->getSize() < BARSize[0]);
+
+    Addr daddr = pkt->getAddr();
+    assert(BARs[0]->range().contains(daddr));
+    daddr -= BARs[0]->addr();

     ContextID cpu = pkt->req->contextId();
-    Addr daddr = pkt->getAddr() - BARAddrs[0];
     Addr index = daddr >> Regs::VirtualShift;
     Addr raddr = daddr & Regs::VirtualMask;

@@ -295,10 +297,12 @@
 Device::write(PacketPtr pkt)
 {
     assert(config.command & PCI_CMD_MSE);
-    assert(pkt->getAddr() >= BARAddrs[0] && pkt->getSize() < BARSize[0]);
+
+    Addr daddr = pkt->getAddr();
+    assert(BARs[0]->range().contains(daddr));
+    daddr -= BARs[0]->addr();

     ContextID cpu = pkt->req->contextId();
-    Addr daddr = pkt->getAddr() - BARAddrs[0];
     Addr index = daddr >> Regs::VirtualShift;
     Addr raddr = daddr & Regs::VirtualMask;

diff --git a/src/dev/pci/CopyEngine.py b/src/dev/pci/CopyEngine.py
index 78b7e39..3eba35b 100644
--- a/src/dev/pci/CopyEngine.py
+++ b/src/dev/pci/CopyEngine.py
@@ -28,7 +28,7 @@
 from m5.params import *
 from m5.proxy import *

-from m5.objects.PciDevice import PciDevice
+from m5.objects.PciDevice import PciDevice, PciMemBar

 class CopyEngine(PciDevice):
     type = 'CopyEngine'
@@ -47,12 +47,17 @@
     MinimumGrant = 0xff
     InterruptLine = 0x20
     InterruptPin = 0x01
-    BAR0Size = '1kB'
+
+    BARs = PciMemBar(size='1kB')

     ChanCnt = Param.UInt8(4, "Number of DMA channels that exist on device")
- XferCap = Param.MemorySize('4kB', "Number of bits of transfer size that are supported")
+    XferCap = Param.MemorySize('4kB',
+            "Number of bits of transfer size that are supported")

- latBeforeBegin = Param.Latency('20ns', "Latency after a DMA command is seen before it's proccessed") - latAfterCompletion = Param.Latency('20ns', "Latency after a DMA command is complete before it's reported as such")
+    latBeforeBegin = Param.Latency('20ns',
+            "Latency after a DMA command is seen before it's proccessed")
+    latAfterCompletion = Param.Latency('20ns',
+            "Latency after a DMA command is complete before "
+            "it's reported as such")


diff --git a/src/dev/pci/PciDevice.py b/src/dev/pci/PciDevice.py
index 862bff7..32bb384 100644
--- a/src/dev/pci/PciDevice.py
+++ b/src/dev/pci/PciDevice.py
@@ -42,6 +42,42 @@
 from m5.objects.Device import DmaDevice
 from m5.objects.PciHost import PciHost

+class PciBar(SimObject):
+    type = 'PciBar'
+    cxx_class = 'PciBar'
+    cxx_header = "dev/pci/device.hh"
+    abstract = True
+
+class PciIoBar(PciBar):
+    type = 'PciIoBar'
+    cxx_class = 'PciIoBar'
+    cxx_header = "dev/pci/device.hh"
+
+    size = Param.MemorySize32("IO region size")
+
+class PciLegacyIoBar(PciIoBar):
+    type = 'PciLegacyIoBar'
+    cxx_class = 'PciLegacyIoBar'
+    cxx_header = "dev/pci/device.hh"
+
+    addr = Param.UInt32("Legacy IO address")
+
+# To set up a 64 bit memory BAR, put a PciMemUpperBar immediately after
+# a PciMemBar. The pair will take up the right number of BARs, and will be
+# recognized by the device and turned into a 64 bit BAR when the config is
+# consumed.
+class PciMemBar(PciBar):
+    type = 'PciMemBar'
+    cxx_class = 'PciMemBar'
+    cxx_header = "dev/pci/device.hh"
+
+    size = Param.MemorySize32("Memory region size")
+
+class PciMemUpperBar(PciBar):
+    type = 'PciMemUpperBar'
+    cxx_class = 'PciMemUpperBar'
+    cxx_header = "dev/pci/device.hh"
+
 class PciDevice(DmaDevice):
     type = 'PciDevice'
     cxx_class = 'PciDevice'
@@ -69,25 +105,7 @@
     HeaderType = Param.UInt8(0, "PCI Header Type")
     BIST = Param.UInt8(0, "Built In Self Test")

-    BAR0 = Param.UInt32(0x00, "Base Address Register 0")
-    BAR1 = Param.UInt32(0x00, "Base Address Register 1")
-    BAR2 = Param.UInt32(0x00, "Base Address Register 2")
-    BAR3 = Param.UInt32(0x00, "Base Address Register 3")
-    BAR4 = Param.UInt32(0x00, "Base Address Register 4")
-    BAR5 = Param.UInt32(0x00, "Base Address Register 5")
-    BAR0Size = Param.MemorySize32('0B', "Base Address Register 0 Size")
-    BAR1Size = Param.MemorySize32('0B', "Base Address Register 1 Size")
-    BAR2Size = Param.MemorySize32('0B', "Base Address Register 2 Size")
-    BAR3Size = Param.MemorySize32('0B', "Base Address Register 3 Size")
-    BAR4Size = Param.MemorySize32('0B', "Base Address Register 4 Size")
-    BAR5Size = Param.MemorySize32('0B', "Base Address Register 5 Size")
-    BAR0LegacyIO = Param.Bool(False, "Whether BAR0 is hardwired legacy IO")
-    BAR1LegacyIO = Param.Bool(False, "Whether BAR1 is hardwired legacy IO")
-    BAR2LegacyIO = Param.Bool(False, "Whether BAR2 is hardwired legacy IO")
-    BAR3LegacyIO = Param.Bool(False, "Whether BAR3 is hardwired legacy IO")
-    BAR4LegacyIO = Param.Bool(False, "Whether BAR4 is hardwired legacy IO")
-    BAR5LegacyIO = Param.Bool(False, "Whether BAR5 is hardwired legacy IO")
-    LegacyIOBase = Param.Addr(0x0, "Base Address for Legacy IO")
+ BARs = VectorParam.PciBar([], "Address decoders configured through BARs")

     CardbusCIS = Param.UInt32(0x00, "Cardbus Card Information Structure")
     SubsystemID = Param.UInt16(0x00, "Subsystem ID")
diff --git a/src/dev/pci/device.cc b/src/dev/pci/device.cc
index 1158bc6..d6348bc 100644
--- a/src/dev/pci/device.cc
+++ b/src/dev/pci/device.cc
@@ -82,6 +82,24 @@
     fatal_if(p->InterruptPin >= 5,
              "Invalid PCI interrupt '%i' specified.", p->InterruptPin);

+    int idx = 0;
+    for (auto *bar: p->BARs) {
+        fatal_if(idx >= BARs.size(), "Too many BARs in %s.", name());
+        BARs[idx] = bar;
+        auto *mu = dynamic_cast<PciMemUpperBar *>(bar);
+ // If this is the upper 32 bits of a memory BAR, try to connect it to
+        // the lower 32 bits.
+        if (mu) {
+            fatal_if(idx == 0,
+ "First BAR in %s is upper 32 bits of a memory BAR.", idx);
+            auto *ml = dynamic_cast<PciMemBar *>(BARs[idx - 1]);
+            fatal_if(!ml, "Upper 32 bits of memory BAR in %s doesn't come "
+                    "after the lower 32.");
+            mu->lower(ml);
+        }
+        idx++;
+    }
+
     config.vendor = htole(p->VendorID);
     config.device = htole(p->DeviceID);
     config.command = htole(p->Command);
@@ -95,12 +113,10 @@
     config.headerType = htole(p->HeaderType);
     config.bist = htole(p->BIST);

-    config.baseAddr[0] = htole(p->BAR0);
-    config.baseAddr[1] = htole(p->BAR1);
-    config.baseAddr[2] = htole(p->BAR2);
-    config.baseAddr[3] = htole(p->BAR3);
-    config.baseAddr[4] = htole(p->BAR4);
-    config.baseAddr[5] = htole(p->BAR5);
+    idx = 0;
+    for (auto *bar: BARs)
+        config.baseAddr[idx++] = bar ? bar->write(hostInterface, 0) : 0;
+
     config.cardbusCIS = htole(p->CardbusCIS);
     config.subsystemVendorID = htole(p->SubsystemVendorID);
     config.subsystemID = htole(p->SubsystemID);
@@ -183,33 +199,6 @@
     pxcap.pxls = p->PXCAPLinkStatus;
     pxcap.pxdcap2 = p->PXCAPDevCap2;
     pxcap.pxdc2 = p->PXCAPDevCtrl2;
-
-    BARSize[0] = p->BAR0Size;
-    BARSize[1] = p->BAR1Size;
-    BARSize[2] = p->BAR2Size;
-    BARSize[3] = p->BAR3Size;
-    BARSize[4] = p->BAR4Size;
-    BARSize[5] = p->BAR5Size;
-
-    legacyIO[0] = p->BAR0LegacyIO;
-    legacyIO[1] = p->BAR1LegacyIO;
-    legacyIO[2] = p->BAR2LegacyIO;
-    legacyIO[3] = p->BAR3LegacyIO;
-    legacyIO[4] = p->BAR4LegacyIO;
-    legacyIO[5] = p->BAR5LegacyIO;
-
-    for (int i = 0; i < 6; ++i) {
-        if (legacyIO[i]) {
-            BARAddrs[i] = p->LegacyIOBase + letoh(config.baseAddr[i]);
-            config.baseAddr[i] = 0;
-        } else {
-            BARAddrs[i] = 0;
-            uint32_t barsize = BARSize[i];
-            if (barsize != 0 && !isPowerOf2(barsize)) {
- fatal("BAR %d size %d is not a power of 2\n", i, BARSize[i]);
-            }
-        }
-    }
 }

 Tick
@@ -273,10 +262,15 @@
 PciDevice::getAddrRanges() const
 {
     AddrRangeList ranges;
-    int x = 0;
-    for (x = 0; x < 6; x++)
-        if (BARAddrs[x] != 0)
-            ranges.push_back(RangeSize(BARAddrs[x],BARSize[x]));
+    for (auto *bar: BARs) {
+        if (!bar)
+            continue;
+        PciCommandRegister command = letoh(config.command);
+        if (command.ioSpace && bar->isIo())
+            ranges.push_back(bar->range());
+        if (command.memorySpace && bar->isMem())
+            ranges.push_back(bar->range());
+    }
     return ranges;
 }

@@ -333,6 +327,8 @@
         switch (offset) {
           case PCI_COMMAND:
             config.command = pkt->getLE<uint8_t>();
+            // IO or memory space may have been enabled/disabled.
+            pioPort.sendRangeChange();
             break;
           case PCI_STATUS:
             config.status = pkt->getLE<uint8_t>();
@@ -357,54 +353,13 @@
           case PCI0_BASE_ADDR4:
           case PCI0_BASE_ADDR5:
             {
-                int barnum = BAR_NUMBER(offset);
-
-                if (!legacyIO[barnum]) {
-                    // convert BAR values to host endianness
-                    uint32_t he_old_bar = letoh(config.baseAddr[barnum]);
-                    uint32_t he_new_bar = letoh(pkt->getLE<uint32_t>());
-
-                    uint32_t bar_mask =
- BAR_IO_SPACE(he_old_bar) ? BAR_IO_MASK : BAR_MEM_MASK;
-
- // Writing 0xffffffff to a BAR tells the card to set the
-                    // value of the bar to a bitmask indicating the size of
-                    // memory it needs
-                    if (he_new_bar == 0xffffffff) {
-                        he_new_bar = ~(BARSize[barnum] - 1);
-                    } else {
- // does it mean something special to write 0 to a BAR?
-                        he_new_bar &= ~bar_mask;
-                        if (he_new_bar) {
-                            if (isLargeBAR(barnum)) {
-                                if (BAR_IO_SPACE(he_old_bar))
- warn("IO BARs can't be set as large BAR");
-                                uint64_t he_large_bar =
- letoh(config.baseAddr[barnum + 1]);
-                                he_large_bar = he_large_bar << 32;
-                                he_large_bar += he_new_bar;
-                                BARAddrs[barnum] =
- hostInterface.memAddr(he_large_bar);
-                            } else if (isLargeBAR(barnum - 1)) {
-                                BARAddrs[barnum] = 0;
-                                uint64_t he_large_bar = he_new_bar;
-                                he_large_bar = he_large_bar << 32;
-                                // We need to apply mask to lower bits
-                                he_large_bar +=
-                                         letoh(config.baseAddr[barnum - 1]
-                                         & ~bar_mask);
-                                BARAddrs[barnum - 1] =
- hostInterface.memAddr(he_large_bar);
-                           } else {
- BARAddrs[barnum] = BAR_IO_SPACE(he_old_bar) ?
-                                    hostInterface.pioAddr(he_new_bar) :
-                                    hostInterface.memAddr(he_new_bar);
-                            }
-                            pioPort.sendRangeChange();
-                        }
-                    }
- config.baseAddr[barnum] = htole((he_new_bar & ~bar_mask) | - (he_old_bar & bar_mask));
+                int num = BAR_NUMBER(offset);
+                auto *bar = BARs[num];
+                if (bar) {
+                    config.baseAddr[num] =
+                        htole(bar->write(hostInterface,
+                                         pkt->getLE<uint32_t>()));
+                    pioPort.sendRangeChange();
                 }
             }
             break;
@@ -421,6 +376,8 @@
             // register. However they should never get set, so lets ignore
             // it for now
             config.command = pkt->getLE<uint32_t>();
+            // IO or memory space may have been enabled/disabled.
+            pioPort.sendRangeChange();
             break;

           default:
@@ -441,8 +398,6 @@
 void
 PciDevice::serialize(CheckpointOut &cp) const
 {
-    SERIALIZE_ARRAY(BARSize, sizeof(BARSize) / sizeof(BARSize[0]));
-    SERIALIZE_ARRAY(BARAddrs, sizeof(BARAddrs) / sizeof(BARAddrs[0]));
SERIALIZE_ARRAY(config.data, sizeof(config.data) / sizeof(config.data[0]));

     // serialize the capability list registers
@@ -506,11 +461,14 @@
 void
 PciDevice::unserialize(CheckpointIn &cp)
 {
-    UNSERIALIZE_ARRAY(BARSize, sizeof(BARSize) / sizeof(BARSize[0]));
-    UNSERIALIZE_ARRAY(BARAddrs, sizeof(BARAddrs) / sizeof(BARAddrs[0]));
     UNSERIALIZE_ARRAY(config.data,
                       sizeof(config.data) / sizeof(config.data[0]));

+    for (int idx = 0; idx < BARs.size(); idx++) {
+        if (BARs[idx])
+            BARs[idx]->write(hostInterface, config.baseAddr[idx]);
+    }
+
     // unserialize the capability list registers
     uint16_t tmp16;
     uint32_t tmp32;
@@ -595,3 +553,26 @@
     pioPort.sendRangeChange();
 }

+PciIoBar *
+PciIoBarParams::create()
+{
+    return new PciIoBar(*this);
+}
+
+PciLegacyIoBar *
+PciLegacyIoBarParams::create()
+{
+    return new PciLegacyIoBar(*this);
+}
+
+PciMemBar *
+PciMemBarParams::create()
+{
+    return new PciMemBar(*this);
+}
+
+PciMemUpperBar *
+PciMemUpperBarParams::create()
+{
+    return new PciMemUpperBar(*this);
+}
diff --git a/src/dev/pci/device.hh b/src/dev/pci/device.hh
index 6f28376..095a009 100644
--- a/src/dev/pci/device.hh
+++ b/src/dev/pci/device.hh
@@ -45,21 +45,203 @@
 #ifndef __DEV_PCI_DEVICE_HH__
 #define __DEV_PCI_DEVICE_HH__

+#include <array>
 #include <cstring>
 #include <vector>

 #include "dev/dma_device.hh"
 #include "dev/pci/host.hh"
 #include "dev/pci/pcireg.h"
+#include "params/PciBar.hh"
 #include "params/PciDevice.hh"
+#include "params/PciIoBar.hh"
+#include "params/PciLegacyIoBar.hh"
+#include "params/PciMemBar.hh"
+#include "params/PciMemUpperBar.hh"
 #include "sim/byteswap.hh"

-#define BAR_IO_MASK 0x3
-#define BAR_MEM_MASK 0xF
-#define BAR_IO_SPACE_BIT 0x1
-#define BAR_IO_SPACE(x) ((x) & BAR_IO_SPACE_BIT)
 #define BAR_NUMBER(x) (((x) - PCI0_BASE_ADDR0) >> 0x2);

+class PciBar : public SimObject
+{
+  protected:
+    // The address and size of the region this decoder recognizes.
+    Addr _addr = 0;
+    Addr _size = 0;
+
+  public:
+    PciBar(const PciBarParams &p) : SimObject(&p) {}
+
+    virtual bool isMem() const { return false; }
+    virtual bool isIo() const { return false; }
+
+ // Accepts a value written to config space, consumes it, and returns what
+    // value config space should actually be set to. Both should be in host
+    // endian format.
+    virtual uint32_t write(const PciHost::DeviceInterface &host,
+                           uint32_t val) = 0;
+
+    AddrRange range() const { return AddrRange(_addr, _addr + _size); }
+    Addr addr() const { return _addr; }
+    Addr size() const { return _size; }
+
+    // Hack for devices that don't know their BAR sizes ahead of time :-o.
+    // Don't use unless you have to, since this may not propogate properly
+    // outside of a small window.
+    void size(Addr value) { _size = value; }
+};
+
+class PciIoBar : public PciBar
+{
+  protected:
+    BitUnion32(Bar)
+        Bitfield<31, 2> addr;
+        Bitfield<1> reserved;
+        Bitfield<0> io;
+    EndBitUnion(Bar)
+
+  public:
+    PciIoBar(const PciIoBarParams &p, bool legacy=false) : PciBar(p)
+    {
+        _size = p.size;
+        if (!legacy) {
+            Bar bar = _size;
+ fatal_if(!_size || !isPowerOf2(_size) || bar.io || bar.reserved,
+                    "Illegal size %d for bar %s.", _size, name());
+        }
+    }
+
+    bool isIo() const override { return true; }
+
+    uint32_t
+    write(const PciHost::DeviceInterface &host, uint32_t val) override
+    {
+        // Mask away the bits fixed by hardware.
+        Bar bar = val & ~(_size - 1);
+        // Set the fixed bits to their correct values.
+        bar.reserved = 0;
+        bar.io = 1;
+
+        // Update our address.
+        _addr = host.pioAddr(bar.addr << 2);
+
+        // Return what should go into config space.
+        return bar;
+    }
+};
+
+class PciLegacyIoBar : public PciIoBar
+{
+  protected:
+    Addr fixedAddr;
+
+  public:
+    PciLegacyIoBar(const PciLegacyIoBarParams &p) : PciIoBar(p, true)
+    {
+        // Save the address until we get a host to translate it.
+        fixedAddr = p.addr;
+    }
+
+    uint32_t
+    write(const PciHost::DeviceInterface &host, uint32_t val) override
+    {
+        // Update the address now that we have a host to translate it.
+        _addr = host.pioAddr(fixedAddr);
+        // Ignore writes.
+        return 0;
+    }
+};
+
+class PciMemBar : public PciBar
+{
+  private:
+    BitUnion32(Bar)
+        Bitfield<31, 3> addr;
+        SubBitUnion(type, 2, 1)
+            Bitfield<2> wide;
+            Bitfield<1> reserved;
+        EndSubBitUnion(type)
+        Bitfield<0> io;
+    EndBitUnion(Bar)
+
+    bool _wide = false;
+    uint64_t _lower = 0;
+    uint64_t _upper = 0;
+
+  public:
+    PciMemBar(const PciMemBarParams &p) : PciBar(p)
+    {
+        _size = p.size;
+        Bar bar = _size;
+        fatal_if(!_size || !isPowerOf2(_size) || bar.io || bar.type,
+                "Illegal size %d for bar %s.", _size, name());
+    }
+
+    bool isMem() const override { return true; }
+
+    uint32_t
+    write(const PciHost::DeviceInterface &host, uint32_t val) override
+    {
+        // Mask away the bits fixed by hardware.
+        Bar bar = val & ~(_size - 1);
+        // Set the fixed bits to their correct values.
+        bar.type.wide = wide() ? 1 : 0;
+        bar.type.reserved = 0;
+        bar.io = 0;
+
+        // Keep track of our lower 32 bits.
+        _lower = bar.addr << 3;
+
+        // Update our address.
+        _addr = host.memAddr(upper() + lower());
+
+        // Return what should go into config space.
+        return bar;
+    }
+
+    bool wide() const { return _wide; }
+    void wide(bool val) { _wide = val; }
+
+    uint64_t upper() const { return _upper; }
+    void
+    upper(const PciHost::DeviceInterface &host, uint32_t val)
+    {
+        _upper = (uint64_t)val << 32;
+
+        // Update our address.
+        _addr = host.memAddr(upper() + lower());
+    }
+
+    uint64_t lower() const { return _lower; }
+};
+
+class PciMemUpperBar : public PciBar
+{
+  private:
+    PciMemBar *_lower = nullptr;
+
+  public:
+    PciMemUpperBar(const PciMemUpperBarParams &p) : PciBar(p)
+    {}
+
+    void
+    lower(PciMemBar *val)
+    {
+        _lower = val;
+        // Let our lower half know we're up here.
+        _lower->wide(true);
+    }
+
+    uint32_t
+    write(const PciHost::DeviceInterface &host, uint32_t val) override
+    {
+        // Let our lower half know about the update.
+        assert(_lower);
+        _lower->upper(host, val);
+        return val;
+    }
+};
+
 /**
  * PCI device, base implementation is only config space.
  */
@@ -102,68 +284,29 @@
     std::vector<MSIXTable> msix_table;
     std::vector<MSIXPbaEntry> msix_pba;

-    /** The size of the BARs */
-    uint32_t BARSize[6];
-
-    /** The current address mapping of the BARs */
-    Addr BARAddrs[6];
-
-    /** Whether the BARs are really hardwired legacy IO locations. */
-    bool legacyIO[6];
-
-    /**
-     * Does the given BAR represent 32 lower bits of a 64-bit address?
-     */
-    bool
-    isLargeBAR(int bar) const
-    {
-        return bits(config.baseAddr[bar], 2, 1) == 0x2;
-    }
-
-    /**
-     * Does the given address lie within the space mapped by the given
-     * base address register?
-     */
-    bool
-    isBAR(Addr addr, int bar) const
-    {
-        assert(bar >= 0 && bar < 6);
- return BARAddrs[bar] <= addr && addr < BARAddrs[bar] + BARSize[bar];
-    }
-
-    /**
-     * Which base address register (if any) maps the given address?
-     * @return The BAR number (0-5 inclusive), or -1 if none.
-     */
-    int
-    getBAR(Addr addr)
-    {
-        for (int i = 0; i <= 5; ++i)
-            if (isBAR(addr, i))
-                return i;
-
-        return -1;
-    }
+    std::array<PciBar *, 6> BARs{};

     /**
      * Which base address register (if any) maps the given address?
      * @param addr The address to check.
-     * @retval bar The BAR number (0-5 inclusive),
+     * @retval num The BAR number (0-5 inclusive),
      *             only valid if return value is true.
      * @retval offs The offset from the base address,
      *              only valid if return value is true.
      * @return True iff address maps to a base address register's region.
      */
     bool
-    getBAR(Addr addr, int &bar, Addr &offs)
+    getBAR(Addr addr, int &num, Addr &offs)
     {
-        int b = getBAR(addr);
-        if (b < 0)
-            return false;
-
-        offs = addr - BARAddrs[b];
-        bar = b;
-        return true;
+        for (int i = 0; i < BARs.size(); i++) {
+            auto *bar = BARs[i];
+            if (!bar || !bar->range().contains(addr))
+                continue;
+            num = i;
+            offs = addr - bar->addr();
+            return true;
+        }
+        return false;
     }

   public: // Host configuration interface
@@ -191,7 +334,9 @@
     Tick configDelay;

   public:
-    Addr pciToDma(Addr pci_addr) const {
+    Addr
+    pciToDma(Addr pci_addr) const
+    {
         return hostInterface.dmaAddr(pci_addr);
     }

diff --git a/src/dev/pci/pcireg.h b/src/dev/pci/pcireg.h
index 5f001c7..dc1807b 100644
--- a/src/dev/pci/pcireg.h
+++ b/src/dev/pci/pcireg.h
@@ -50,6 +50,20 @@
 #include "base/bitfield.hh"
 #include "base/bitunion.hh"

+BitUnion16(PciCommandRegister)
+    Bitfield<15, 10> reserved;
+    Bitfield<9> fastBackToBackEn;
+    Bitfield<8> serrEn;
+    Bitfield<7> steppingControl;
+    Bitfield<6> parityErrResp;
+    Bitfield<5> vgaPaletteSnoopEn;
+    Bitfield<4> memWriteInvEn;
+    Bitfield<3> specialCycles;
+    Bitfield<2> busMaster;
+    Bitfield<1> memorySpace;
+    Bitfield<0> ioSpace;
+EndBitUnion(PciCommandRegister)
+
 union PCIConfig {
     uint8_t data[64];

diff --git a/src/dev/storage/Ide.py b/src/dev/storage/Ide.py
index 5edea49..b493f81 100644
--- a/src/dev/storage/Ide.py
+++ b/src/dev/storage/Ide.py
@@ -26,7 +26,7 @@

 from m5.SimObject import SimObject
 from m5.params import *
-from m5.objects.PciDevice import PciDevice
+from m5.objects.PciDevice import PciDevice, PciIoBar

 class IdeID(Enum): vals = ['device0', 'device1']

@@ -50,19 +50,11 @@
     ClassCode = 0x01
     SubClassCode = 0x01
     ProgIF = 0x85
-    BAR0 = 0x00000001
-    BAR1 = 0x00000001
-    BAR2 = 0x00000001
-    BAR3 = 0x00000001
-    BAR4 = 0x00000001
-    BAR5 = 0x00000001
+    BARs = (PciIoBar(size='8B'), PciIoBar(size='4B'), # Primary
+            PciIoBar(size='8B'), PciIoBar(size='4B'), # Secondary
+            PciIoBar(size='16B')) # DMA
     InterruptLine = 0x1f
     InterruptPin = 0x01
-    BAR0Size = '8B'
-    BAR1Size = '4B'
-    BAR2Size = '8B'
-    BAR3Size = '4B'
-    BAR4Size = '16B'

     io_shift = Param.UInt32(0x0, "IO port shift");
     ctrl_offset = Param.UInt32(0x0, "IDE disk control offset")
diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 5efa42b..c2b2847 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -73,11 +73,8 @@

 static const uint16_t timeRegWithDecodeEn = 0x8000;

-IdeController::Channel::Channel(
-        string newName, Addr _cmdSize, Addr _ctrlSize) :
-    _name(newName),
-    cmdAddr(0), cmdSize(_cmdSize), ctrlAddr(0), ctrlSize(_ctrlSize),
-    device0(NULL), device1(NULL), selected(NULL)
+IdeController::Channel::Channel(string newName) :
+    _name(newName), device0(NULL), device1(NULL), selected(NULL)
 {
     bmiRegs.reset();
     bmiRegs.status.dmaCap0 = 1;
@@ -89,13 +86,11 @@
 }

 IdeController::IdeController(Params *p)
-    : PciDevice(p), primary(name() + ".primary", BARSize[0], BARSize[1]),
-    secondary(name() + ".secondary", BARSize[2], BARSize[3]),
-    bmiAddr(0), bmiSize(BARSize[4]),
+    : PciDevice(p), primary(name() + ".primary"),
+    secondary(name() + ".secondary"),
     primaryTiming(htole(timeRegWithDecodeEn)),
     secondaryTiming(htole(timeRegWithDecodeEn)),
     deviceTiming(0), udmaControl(0), udmaTiming(0), ideConfig(0),
-    ioEnabled(false), bmEnabled(false),
     ioShift(p->io_shift), ctrlOffset(p->ctrl_offset)
 {

@@ -126,18 +121,6 @@

     primary.select(false);
     secondary.select(false);
-
-    if ((BARAddrs[0] & ~BAR_IO_MASK) && (!legacyIO[0] || ioShift)) {
-        primary.cmdAddr = BARAddrs[0];  primary.cmdSize = BARSize[0];
-        primary.ctrlAddr = BARAddrs[1]; primary.ctrlSize = BARSize[1];
-    }
-    if ((BARAddrs[2] & ~BAR_IO_MASK) && (!legacyIO[2] || ioShift)) {
-        secondary.cmdAddr = BARAddrs[2];  secondary.cmdSize = BARSize[2];
-        secondary.ctrlAddr = BARAddrs[3]; secondary.ctrlSize = BARSize[3];
-    }
-
-    ioEnabled = (config.command & htole(PCI_CMD_IOSE));
-    bmEnabled = (config.command & htole(PCI_CMD_BME));
 }

 bool
@@ -323,41 +306,6 @@
         }
         pkt->makeAtomicResponse();
     }
-
- /* Trap command register writes and enable IO/BM as appropriate as well as
-     * BARs. */
-    switch(offset) {
-      case PCI0_BASE_ADDR0:
-        if (BARAddrs[0] != 0)
-            primary.cmdAddr = BARAddrs[0];
-        break;
-
-      case PCI0_BASE_ADDR1:
-        if (BARAddrs[1] != 0)
-            primary.ctrlAddr = BARAddrs[1];
-        break;
-
-      case PCI0_BASE_ADDR2:
-        if (BARAddrs[2] != 0)
-            secondary.cmdAddr = BARAddrs[2];
-        break;
-
-      case PCI0_BASE_ADDR3:
-        if (BARAddrs[3] != 0)
-            secondary.ctrlAddr = BARAddrs[3];
-        break;
-
-      case PCI0_BASE_ADDR4:
-        if (BARAddrs[4] != 0)
-            bmiAddr = BARAddrs[4];
-        break;
-
-      case PCI_COMMAND:
- DPRINTF(IdeCtrl, "Writing to PCI Command val: %#x\n", config.command);
-        ioEnabled = (config.command & htole(PCI_CMD_IOSE));
-        bmEnabled = (config.command & htole(PCI_CMD_BME));
-        break;
-    }
     return configDelay;
 }

@@ -497,38 +445,41 @@
     int size = pkt->getSize();
     uint8_t *dataPtr = pkt->getPtr<uint8_t>();

-    if (addr >= primary.cmdAddr &&
-            addr < (primary.cmdAddr + primary.cmdSize)) {
-        addr -= primary.cmdAddr;
+    int bar_num;
+    Addr offset;
+    panic_if(!getBAR(addr, bar_num, offset),
+        "IDE controller access to invalid address: %#x.", addr);
+
+    switch (bar_num) {
+      case 0:
         // linux may have shifted the address by ioShift,
         // here we shift it back, similarly for ctrlOffset.
-        addr >>= ioShift;
-        primary.accessCommand(addr, size, dataPtr, read);
-    } else if (addr >= primary.ctrlAddr &&
-               addr < (primary.ctrlAddr + primary.ctrlSize)) {
-        addr -= primary.ctrlAddr;
-        addr += ctrlOffset;
-        primary.accessControl(addr, size, dataPtr, read);
-    } else if (addr >= secondary.cmdAddr &&
-               addr < (secondary.cmdAddr + secondary.cmdSize)) {
-        addr -= secondary.cmdAddr;
-        secondary.accessCommand(addr, size, dataPtr, read);
-    } else if (addr >= secondary.ctrlAddr &&
-               addr < (secondary.ctrlAddr + secondary.ctrlSize)) {
-        addr -= secondary.ctrlAddr;
-        secondary.accessControl(addr, size, dataPtr, read);
-    } else if (addr >= bmiAddr && addr < (bmiAddr + bmiSize)) {
-        if (!read && !bmEnabled)
-            return;
-        addr -= bmiAddr;
-        if (addr < sizeof(Channel::BMIRegs)) {
-            primary.accessBMI(addr, size, dataPtr, read);
-        } else {
-            addr -= sizeof(Channel::BMIRegs);
-            secondary.accessBMI(addr, size, dataPtr, read);
+        offset >>= ioShift;
+        primary.accessCommand(offset, size, dataPtr, read);
+        break;
+      case 1:
+        offset += ctrlOffset;
+        primary.accessControl(offset, size, dataPtr, read);
+        break;
+      case 2:
+        secondary.accessCommand(offset, size, dataPtr, read);
+        break;
+      case 3:
+        secondary.accessControl(offset, size, dataPtr, read);
+        break;
+      case 4:
+        {
+            PciCommandRegister command = letoh(config.command);
+            if (!read && !command.busMaster)
+                return;
+
+            if (offset < sizeof(Channel::BMIRegs)) {
+                primary.accessBMI(offset, size, dataPtr, read);
+            } else {
+                offset -= sizeof(Channel::BMIRegs);
+                secondary.accessBMI(offset, size, dataPtr, read);
+            }
         }
-    } else {
-        panic("IDE controller access to invalid address: %#x\n", addr);
     }

 #ifndef NDEBUG
@@ -577,22 +528,12 @@
     SERIALIZE_SCALAR(udmaControl);
     SERIALIZE_SCALAR(udmaTiming);
     SERIALIZE_SCALAR(ideConfig);
-
-    // Serialize internal state
-    SERIALIZE_SCALAR(ioEnabled);
-    SERIALIZE_SCALAR(bmEnabled);
-    SERIALIZE_SCALAR(bmiAddr);
-    SERIALIZE_SCALAR(bmiSize);
 }

 void
 IdeController::Channel::serialize(const std::string &base,
                                   CheckpointOut &cp) const
 {
-    paramOut(cp, base + ".cmdAddr", cmdAddr);
-    paramOut(cp, base + ".cmdSize", cmdSize);
-    paramOut(cp, base + ".ctrlAddr", ctrlAddr);
-    paramOut(cp, base + ".ctrlSize", ctrlSize);
     uint8_t command = bmiRegs.command;
     paramOut(cp, base + ".bmiRegs.command", command);
     paramOut(cp, base + ".bmiRegs.reserved0", bmiRegs.reserved0);
@@ -620,21 +561,11 @@
     UNSERIALIZE_SCALAR(udmaControl);
     UNSERIALIZE_SCALAR(udmaTiming);
     UNSERIALIZE_SCALAR(ideConfig);
-
-    // Unserialize internal state
-    UNSERIALIZE_SCALAR(ioEnabled);
-    UNSERIALIZE_SCALAR(bmEnabled);
-    UNSERIALIZE_SCALAR(bmiAddr);
-    UNSERIALIZE_SCALAR(bmiSize);
 }

 void
IdeController::Channel::unserialize(const std::string &base, CheckpointIn &cp)
 {
-    paramIn(cp, base + ".cmdAddr", cmdAddr);
-    paramIn(cp, base + ".cmdSize", cmdSize);
-    paramIn(cp, base + ".ctrlAddr", ctrlAddr);
-    paramIn(cp, base + ".ctrlSize", ctrlSize);
     uint8_t command;
     paramIn(cp, base +".bmiRegs.command", command);
     bmiRegs.command = command;
diff --git a/src/dev/storage/ide_ctrl.hh b/src/dev/storage/ide_ctrl.hh
index 51e1603..b6b8368 100644
--- a/src/dev/storage/ide_ctrl.hh
+++ b/src/dev/storage/ide_ctrl.hh
@@ -72,13 +72,12 @@
             return _name;
         }

-        /** Command and control block registers */
-        Addr cmdAddr, cmdSize, ctrlAddr, ctrlSize;
-
         /** Registers used for bus master interface */
         struct BMIRegs
         {
-            void reset() {
+            void
+            reset()
+            {
                 memset(static_cast<void *>(this), 0, sizeof(*this));
             }

@@ -113,7 +112,7 @@
void accessControl(Addr offset, int size, uint8_t *data, bool read);
         void accessBMI(Addr offset, int size, uint8_t *data, bool read);

-        Channel(std::string newName, Addr _cmdSize, Addr _ctrlSize);
+        Channel(std::string newName);
         ~Channel();

         void serialize(const std::string &base, std::ostream &os) const;
@@ -123,9 +122,6 @@
     Channel primary;
     Channel secondary;

-    /** Bus master interface (BMI) registers */
-    Addr bmiAddr, bmiSize;
-
     /** Registers used in device specific PCI configuration */
     uint16_t primaryTiming, secondaryTiming;
     uint8_t deviceTiming;
diff --git a/src/dev/virtio/VirtIO.py b/src/dev/virtio/VirtIO.py
index ed8cffa..e9456a6 100644
--- a/src/dev/virtio/VirtIO.py
+++ b/src/dev/virtio/VirtIO.py
@@ -39,7 +39,7 @@
 from m5.params import *
 from m5.proxy import *
 from m5.objects.Device import PioDevice
-from m5.objects.PciDevice import PciDevice
+from m5.objects.PciDevice import PciDevice, PciIoBar


 class VirtIODeviceBase(SimObject):
@@ -68,7 +68,7 @@

     ClassCode = 0xff # Misc device

-    BAR0 = 0x00000001 # Anywhere in 32-bit space; IOREG
-    BAR0Size = '0B' # Overridden by the device model
+    # The size is overridden by the device model.
+    BARs = PciIoBar(size='4B')

     InterruptPin = 0x01 # Use #INTA
diff --git a/src/dev/virtio/pci.cc b/src/dev/virtio/pci.cc
index fdded20..78ef4e1 100644
--- a/src/dev/virtio/pci.cc
+++ b/src/dev/virtio/pci.cc
@@ -53,7 +53,7 @@
     // two. Nothing else is supported. Therefore, we need to force
     // that alignment here. We do not touch vio.configSize as this is
     // used to check accesses later on.
-    BARSize[0] = alignToPowerOfTwo(BAR0_SIZE_BASE + vio.configSize);
+    BARs[0]->size(alignToPowerOfTwo(BAR0_SIZE_BASE + vio.configSize));

     vio.registerKickCallback([this]() { kick(); });
 }
diff --git a/src/dev/x86/SouthBridge.py b/src/dev/x86/SouthBridge.py
index 095f88b..61cf95c 100644
--- a/src/dev/x86/SouthBridge.py
+++ b/src/dev/x86/SouthBridge.py
@@ -33,6 +33,7 @@
 from m5.objects.I8254 import I8254
 from m5.objects.I8259 import I8259
 from m5.objects.Ide import IdeController
+from m5.objects.PciDevice import PciLegacyIoBar, PciIoBar
 from m5.objects.PcSpeaker import PcSpeaker
 from m5.SimObject import SimObject

@@ -66,22 +67,15 @@

     # IDE controller
     ide = IdeController(disks=[], pci_func=0, pci_dev=4, pci_bus=0)
-    ide.BAR0 = 0x1f0
-    ide.BAR0LegacyIO = True
-    ide.BAR1 = 0x3f4
-    ide.BAR1Size = '3B'
-    ide.BAR1LegacyIO = True
-    ide.BAR2 = 0x170
-    ide.BAR2LegacyIO = True
-    ide.BAR3 = 0x374
-    ide.BAR3Size = '3B'
-    ide.BAR3LegacyIO = True
-    ide.BAR4 = 1
+    ide.BARs = (PciLegacyIoBar(addr=0x1f0, size=ide.BARs[0].size),
+                PciLegacyIoBar(addr=0x3f4, size='3B'),
+                PciLegacyIoBar(addr=0x170, size=ide.BARs[2].size),
+                PciLegacyIoBar(addr=0x374, size='3B'),
+                PciIoBar(size='16B'))
     ide.Command = 0
     ide.ProgIF = 0x80
     ide.InterruptLine = 14
     ide.InterruptPin = 1
-    ide.LegacyIOBase = x86IOAddress(0)

     def attachIO(self, bus, dma_ports):
         # Route interrupt signals

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35516
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I16d5f8cdf86d7a2d02a6b04d1f9e1b3eb1dd189d
Gerrit-Change-Number: 35516
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabebl...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to