[gem5-dev] Re: Trouble building 20.1.0.0

[Poremba, Matthew via gem5-dev]

[AMD Public Use]

Hi Evey,


I've seen similar issues in the past caused by protobuf. I was able to 
workaround this by passing a non-existent command to the PROTOC variable to 
scons, e.g.:

scons build/X86/gem5.opt -j 9 PROTOC=asdf

Due to the way gem5 stores the build variables, I never had to add that again 
and completely forgot about reporting it. This workaround doesn't work if you 
need protobuf (e.g., for TraceCPU) otherwise I think you have to update 
protobuf.



Reference for other devs: Happens on one of my machines where protoc --version 
returns libprotobuf 3.7.1.


-Matt

From: Yijia Liu via gem5-dev 
Sent: Friday, October 2, 2020 3:30 PM
To: gem5-dev@gem5.org
Cc: Yijia Liu 
Subject: [gem5-dev] Fw: Trouble building 20.1.0.0

[CAUTION: External Email]
Hello gem5,

This should be an easy fix, but I've been looking into it for a couple of hours 
and couldn't figure out what's off.

I downloaded v20.1.0.0 and installed all the dependencies. I tried to build the 
opt binary with
scons build/X86/gem5.opt -j 9

The build failed because of #NDEBUG flag dependent variables weren't in scope. 
E.g.

DPRINTF(EthernetDesc, "Writeback complete curHead %d -> %d\n",

oldHead, curHead);
The error was oldHead wasn't in scope, which was defined as:

#ifndef NDEBUG

long oldHead = curHead;

#endif

In Sconscript,  NDEBUG is clearly not defined for opt. I'm not sure where the 
build picks up the flag from.

 CPPDEFINES = ['TRACING_ON=1'],

Can someone please share insight on what might have set the NDEBUG flag, and 
how to make sure it's undefined throughout the build? There seem to be a lot of 
these conditional definitions so I can't go in and manually change all of them.d

Thanks for the help.

Evey
___
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

[gem5-dev] Fw: Trouble building 20.1.0.0

[Yijia Liu via gem5-dev]

Hello gem5,

This should be an easy fix, but I've been looking into it for a couple of hours 
and couldn't figure out what's off.

I downloaded v20.1.0.0 and installed all the dependencies. I tried to build the 
opt binary with
scons build/X86/gem5.opt -j 9

The build failed because of #NDEBUG flag dependent variables weren't in scope. 
E.g.

DPRINTF(EthernetDesc, "Writeback complete curHead %d -> %d\n",

oldHead, curHead);

The error was oldHead wasn't in scope, which was defined as:

#ifndef NDEBUG

long oldHead = curHead;

#endif

In Sconscript,  NDEBUG is clearly not defined for opt. I'm not sure where the 
build picks up the flag from.

 CPPDEFINES = ['TRACING_ON=1'],

Can someone please share insight on what might have set the NDEBUG flag, and 
how to make sure it's undefined throughout the build? There seem to be a lot of 
these conditional definitions so I can't go in and manually change all of them.d

Thanks for the help.

Evey
___
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

[gem5-dev] Re: PCI memory BARs broken on ARM

[Jason Lowe-Power via gem5-dev]

Hey Gabe,

I believe the developers at AMD have already implemented a lot of similar
things to get their GPU model working. The relevant Jira issue is
https://gem5.atlassian.net/browse/GEM5-470 (I think). There may be other
relevant issues on this Epic: https://gem5.atlassian.net/browse/GEM5-195.

I just want to make sure we rope in everyone who has been doing development
in this space!

Cheers,
Jason

On Thu, Oct 1, 2020 at 9:33 PM Gabe Black via gem5-dev 
wrote:

> Hi folks. We locally just rebased and picked up a change where the ARM PCI
> controller's configuration was fixed so that it had the appropriate
> starting address for memory mappings. Now that that's correct, it means
> that instead of setting memory BARs to 0x4000 (for example) to get them
> in the correct place, they need to be set to 0, and the "hardware" will
> take care of adding in a 0x4000 offset.
>
> That's more correct, but it means that now the BAR needs to be set to 0,
> and the gem5 PCI BAR handling code is a little bit simplistic and wrong in
> that it treats 0 and 0x as magical values and has special behavior
> when the BARs are set to them. Specifically, it treats a 0 as off, and
> doesn't update the ranges the device responds to on its parent bus. This
> isn't a problem on x86 where the memory addresses are usually not 0 since
> there's actual RAM there, and there's generally no offset applied either.
>
> To fix this, I have a big rework in progress which will change how BARs
> are set up for PCI devices across the board. Note that this will not affect
> any of the work Andreas did a while ago setting up a host device, the
> DeviceInterface type, etc., which is all fine. There are two major parts to
> the change I'm making, in python and C++.
>
> First, rather than having several arrays of scalar parameters which
> together control the BARs, a raw BAR value as it would be in the config, a
> flag setting it s a "legacyIO", and a size, (and a legacy IO offset!) each
> BAR is represented by a python object whose type reflects it's job. There
> is one for IO, one for memory, one for the upper 32 bits of a 64 bit BAR,
> and one for legacy IO. They each have appropriate properties like a fixed
> address or a size as appropriate, and are assigned to a device as a
> VectorParam.
>
> Then on the C++ side, rather than try to track things from the raw BAR
> values, config writes are filtered through the BAR objects which know what
> bits to mask, what the corresponding address range should be based on their
> type, etc. I also took this opportunity to clear away a number of clumsy
> bookkeeping mechanisms and bugs/misunderstandings of how BARs work, many of
> them my own from a long time ago. Also, the handling of disabling memory or
> IO BARs through the config "command" byte is now handled centrally, rather
> than being implemented one off in the IDE controller.
>
> Both of these things seem to now be working, so that's great. The reason
> I'm writing an email rather than sending a code review (yet) is that I'm
> also running into a problem with the python config.
>
> The gist of it is that the PCI device creates the VectorParam of BARs. The
> IDE controller then sets the BARs using a python list of BAR objects as a
> default, and that works. Then in the X86 SouthBridge object (x86 is a
> little easier for me to test atm), I've tried to overwrite some of those
> BARs individually to give them new defaults which make sense on x86. That
> fails because the new values are not parented correctly. If I try setting
> the whole VectorParam with a new list, then it works fine. I think the
> problem is that foo.bar[0] = doesn't actually set foo.bar, it extracts bar
> from foo and then sets element 0 in the bar it extracted. The bookkeeping
> for this is wrong somehow.
>
> Anyway, I wanted to send out an email to let people know there's a bug
> here on both counts, in case they ran into any problems. The default ARM
> setup should be mostly ok since the only PCI device seems to be the IDE
> controller, and that uses legacy style fixed IO BARs which seem to work
> fine. I also wanted to let people know I'm fighting with the bug in the
> config system. I'm going to try to figure that one out, but it's pretty
> twisty in there and I'm struggling to understand how all that plumbing
> works. If anybody has any great insights, I'd be happy to hear them!
>
> Gabe
> ___
> 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
___
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

[gem5-dev] Change in gem5/gem5[develop]: util: add update-copyright utility

[Ciro Santilli (Gerrit) via gem5-dev]

Ciro Santilli has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/35535 )



Change subject: util: add update-copyright utility
..

util: add update-copyright utility

Only ARM copyright headers are implemented now, but the script is designed
to be easily generalizable by other organizations.

Change-Id: I4e1803e53f4530f88fb344f56e08ea29fbfcd41d
---
A util/update-copyright
1 file changed, 177 insertions(+), 0 deletions(-)



diff --git a/util/update-copyright b/util/update-copyright
new file mode 100755
index 000..7851646
--- /dev/null
+++ b/util/update-copyright
@@ -0,0 +1,177 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2020 ARM Limited
+# All rights reserved
+#
+# 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.
+
+import argparse
+import datetime
+import re
+
+import git_filter_repo
+
+parser = argparse.ArgumentParser(description=
+"""Update copyright headers on files of a range of commits.
+
+This can be used to easily update copyright headers at once on an entire
+patchset before submitting.
+
+Only files touched by the selected commits are updated.
+
+Example usage:
+
+```
+python3 -m pip install --user git-filter-repo
+./update-copyright HEAD~3 arm
+```
+
+The above would act on the 3 last commits (HEAD~2, HEAD~ and HEAD),
+leaving HEAD~3 unchanged.
+""",
+formatter_class=argparse.RawTextHelpFormatter,
+)
+parser.add_argument('--test',
+action='store_true',
+default=False,
+help="Run unit tests instead of running.")
+parser.add_argument('start',
+default=None,
+nargs='?',
+help="The commit before the last commit to be modified")
+parser.add_argument('org', default='arm', nargs='?', choices=('arm',),
+help="Organization to update the copyright for")
+args = parser.parse_args()
+
+update_arm_copyright_regexp = re.compile(
+b' Copyright \\(c\\) ([0-9,\- ]+) ARM Limited',
+re.IGNORECASE
+)
+
+update_arm_copyright_year_regexp = re.compile(b'(.*?)([0-9]+)$')
+
+def update_copyright_years(m, cur_year):
+'''
+Does e.g.: b'2016, 2018-2019' -> b'2016, 2018-2020'.
+
+:param m: match containing only the years part of the string
+:type m: re.Match
+:return: the new years part of the string
+:rtype: bytes
+'''
+cur_year_bytes = str(cur_year).encode()
+m = update_arm_copyright_year_regexp.match(m.group(1))
+years_prefix = m.group(1)
+old_year_bytes = m.group(2)
+old_year = int(old_year_bytes.decode())
+if old_year == cur_year:
+new_years_string = old_year_bytes
+elif old_year == cur_year - 1:
+if len(years_prefix) > 0 and years_prefix[-1:] == b'-':
+new_years_string = cur_year_bytes
+else:
+new_years_string = old_year_bytes + b'-' + cur_year_bytes
+else:
+new_years_string = old_year_bytes + b', ' + cur_year_bytes
+new_years_string = years_prefix + new_years_string

[gem5-dev] Change in gem5/gem5[develop]: dev: Rework how PCI BARs are set up in python and C++.

[Gabe Black (Gerrit) via gem5-dev]

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=0x1c07, 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 = 0x1C1A, BAR0Size = '256B',
-BAR1 = 0x1C1A0100, 

[gem5-dev] Change in gem5/gem5[develop]: x86: Change how IO port devices are structured in the PC platform.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: x86: Change how IO port devices are structured in the PC  
platform.

..

x86: Change how IO port devices are structured in the PC platform.

Before, for historical reasons, the PCI host device was the default
responder on the IO bus, meaning that when there was any type of
transaction which didn't have a device to go to, it would end up
looking like a PCI config transaction. It's very unlikely that this is
what it actually was, and what would happen would be arbitrary and
probably not helpful.

Also, there was no device in place to respond to accesses in x86's IO
port address space. On a real system, these accesses just return junk
and are otherwise legal. On systems where there would be physical bus
wires they would probably return whatever the last data on the bus was.

This would have been helpful when the platform was first being set up
because it would make it obvious when the OS tried to access a device
that wasn't implemented, but there were a few cases where it would
purposefully fiddle with ports with nothing on them. These had one off
backing devices in the config which would handle the accesses
harmlessly, but if the OS changed and tried to access other ports, the
configs would need to be updated.

Now, the PCI host is just another device on the bus. It claims all of
the PCI config space addresses, so any config access, even ones which
don't go with a device, will go to it, and it can respond with all 1s
like it's supposed to.

In it's place, the default responder is now a bus. On that bus is
a device which responds to the entire IO port address range with 0s.
The default on *that* bus is a device which will mark any accesses
as bad.

With this setup, accesses which don't go to a device, including a
device on the IO port address space, will go to the IO bus's default
port. There, if the access was an IO access, it will go to the device
which replies successfully with all 0s. If not, it's marked as an
error.

The device which backs the entire IO address space doesn't conflict
with the actual IO devices, since the access will only go towards it
if it's otherwise unclaimed, and the devices on the default bus don't
participate in routing on the higher level IO bus.

Change-Id: Ie02ad7165dfad3ee6f4a762e2f01f7f1b8225168
---
M src/dev/x86/Pc.py
1 file changed, 16 insertions(+), 13 deletions(-)



diff --git a/src/dev/x86/Pc.py b/src/dev/x86/Pc.py
index a0a9825..0ed2648 100644
--- a/src/dev/x86/Pc.py
+++ b/src/dev/x86/Pc.py
@@ -27,12 +27,13 @@
 from m5.params import *
 from m5.proxy import *

-from m5.objects.Device import IsaFake
+from m5.objects.Device import IsaFake, BadAddr
 from m5.objects.Platform import Platform
 from m5.objects.SouthBridge import SouthBridge
 from m5.objects.Terminal import Terminal
 from m5.objects.Uart import Uart8250
 from m5.objects.PciHost import GenericPciHost
+from m5.objects.XBar import IOXBar

 def x86IOAddress(port):
 IO_address_space_base = 0x8000
@@ -52,14 +53,6 @@
 south_bridge = SouthBridge()
 pci_host = PcPciHost()

-# "Non-existant" ports used for timing purposes by the linux kernel
-i_dont_exist1 = IsaFake(pio_addr=x86IOAddress(0x80), pio_size=1)
-i_dont_exist2 = IsaFake(pio_addr=x86IOAddress(0xed), pio_size=1)
-
-# Ports behind the pci config and data regsiters. These don't do  
anything,

-# but the linux kernel fiddles with them anway.
-behind_pci = IsaFake(pio_addr=x86IOAddress(0xcf8), pio_size=8)
-
 # Serial port and terminal
 com_1 = Uart8250()
 com_1.pio_addr = x86IOAddress(0x3f8)
@@ -73,14 +66,24 @@
 # A device to catch accesses to the non-existant floppy controller.
 fake_floppy = IsaFake(pio_addr=x86IOAddress(0x3f2), pio_size=2)

+# A bus for accesses not claimed by a specific device.
+default_bus = IOXBar()
+
+# A device to handle accesses to unclaimed IO ports.
+empty_isa = IsaFake(pio_addr=x86IOAddress(0), pio_size='64kB',
+ret_data8=0, ret_data16=0, ret_data32=0,  
ret_data64=0,

+pio=default_bus.mem_side_ports)
+
+# A device to handle any other type of unclaimed access.
+bad_addr = BadAddr(pio=default_bus.default)
+
 def attachIO(self, bus, dma_ports = []):
 self.south_bridge.attachIO(bus, dma_ports)
-self.i_dont_exist1.pio = bus.mem_side_ports
-self.i_dont_exist2.pio = bus.mem_side_ports
-self.behind_pci.pio = bus.mem_side_ports
 self.com_1.pio = bus.mem_side_ports
 self.fake_com_2.pio = bus.mem_side_ports
 self.fake_com_3.pio = bus.mem_side_ports
 self.fake_com_4.pio = bus.mem_side_ports
 self.fake_floppy.pio = bus.mem_side_ports
-self.pci_host.pio = bus.default
+self.pci_host.pio = bus.mem_side_ports
+
+