dev: RegisterBank addRegistersAt for fragmented reg banks (#902)

One of the limitations of the RegBank class is that it does not allow
you to pass a non-contiguous set of registers. Its simplest form will
just accept an initializer list of registers and it will store them in
sequence.

A more refined version [1] will optionally accept an offset value to be
passed alongside the register reference. This is not meant to be used by
the register bank to store the register at the provided offset.

It is rather used by the bank to sanity check the register sits exactly
at the provided range.

The way to work around this for a fragemented register space is to
explicitly allocate RAZ/RAO blocks as registers and to pass them to
addRegisters together with the others. (See the SysSecCtrl [2] as an
example)

This makes it a bit tedious to model a register bank with gaps between
its registers. First, the exact number and position of the gaps needs to
be extraced from a spec. These sometimes report only implemented
registers and their offset, and omit to document gaps/reserved space. So
a developer needs to manually add register offset and size to check if
all registers are contiguous. Second, these reserved register blocks
need to be instantiated in the bank adding boilerplate code and
affecting readibility.

For these reasons we add a new registration method, called
addRegisters*At*. It reuses the RegisterAdder class but this time the
offset field is really used to instruct the bank where the register
should be mapped. The method is templated and the template parameter
tells the bank which register type should be used to fill the remaining
space. We make the RegBank the owner of this filler space (registers are
generated internally within addRegistersAt).

[1]: https://github.com/gem5/gem5/blob/stable/src/dev/reg_bank.hh#L106
[2]: https://github.com/gem5/gem5/blob/stable/src/dev/arm/ssc.cc#L48

Change-Id: I614ae6e9eeb40b365ac9b6dd8b75abbfdb9cb687

Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
This commit is contained in:
Giacomo Travaglini
2024-03-01 15:32:40 +00:00
committed by GitHub
parent 9bd71bff0c
commit c0e5d58a96
2 changed files with 162 additions and 11 deletions

View File

@@ -1,4 +1,16 @@
/*
* Copyright (c) 2024 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.
*
* Copyright 2020 Google, Inc.
*
* Redistribution and use in source and binary forms, with or without
@@ -41,6 +53,7 @@
#include <sstream>
#include <utility>
#include "base/addr_range.hh"
#include "base/bitfield.hh"
#include "base/debug.hh"
#include "base/logging.hh"
@@ -91,17 +104,26 @@
* read only properties of the RegisterBank instance.
*
* To add actual registers to the RegisterBank (discussed below), you can use
* either the addRegister method which adds a single register, or addRegisters
* which adds an initializer list of them all at once. The register will be
* appended to the end of the bank as they're added, contiguous to the
* existing registers. The size of the bank is automatically accumulated as
* registers are added.
* either the addRegister method which adds a single register, or
* addRegisters/addRegistersAt which add an initializer list of them all at
* once.
*
* When adding a lot of registers, you might accidentally add an extra,
* or accidentally skip one in a long list. Because the offset is handled
* automatically, some of your registers might end up shifted higher or lower
* than you expect. To help mitigate this, you can set what offset you expect
* a register to have by specifying it as an offset, register pair.
* For addRegister and addRegisters, the registers will be appended to
* the end of the bank as they're added, contiguous to the existing registers.
* The size of the bank is automatically accumulated as registers are added.
*
* For addRegistersAt, an offset field is used to instruct the bank where the
* register should be mapped. So the entries of the initializer list will be a
* set of offset-register pair. The method is templated and the template
* parameter tells the bank which register type should be used to fill the
* remaining space. We make the RegBank the owner of this filler space
* (registers are generated internally within addRegistersAt).
*
* When adding a lot of registers with addRegisters, you might accidentally add
* an extra, or accidentally skip one in a long list. Because the offset is
* handled automatically, some of your registers might end up shifted higher or
* lower than you expect. To help mitigate this, you can set what offset you
* expect a register to have by specifying it as an offset, register pair.
*
* addRegisters({{0x1000, reg0}, reg1, reg2});
*
@@ -887,6 +909,7 @@ class RegisterBank : public RegisterBankBase
Addr _base = 0;
Addr _size = 0;
const std::string _name;
std::vector<std::unique_ptr<RegisterBase>> owned;
public:
@@ -955,6 +978,47 @@ class RegisterBank : public RegisterBankBase
}
}
template <class FillerReg>
void
addRegistersAt(std::initializer_list<RegisterAdder> adders)
{
panic_if(std::empty(adders),
"Adding an empty list of registers to %s?", name());
std::vector<RegisterAdder> vec{adders};
std::sort(vec.begin(), vec.end(),
[] (const auto& first, const auto& second) {
return first.offset.value() < second.offset.value();
}
);
for (auto &adder: vec) {
assert(adder.offset && adder.reg);
const Addr offset = _base + _size;
// Here we check if there is a hole (gap) between the start of the
// new register and the end of the current register bank. A positive
// gap means we need to fill the hole with the provided filler.
// If gap is negative, it means previous register is overlapping
// with the start address of the current one, and we should panic
if (int gap = adder.offset.value() - offset; gap != 0) {
panic_if(gap < 0, "Overlapping register added to the bank: %s\n",
adder.reg.value()->name());
// Use the filler register to fill the address range gap
AddrRange hole_range(offset, offset + gap);
owned.push_back(std::make_unique<FillerReg>(hole_range.to_string(), gap));
_offsetMap.emplace(offset, *owned.back().get());
_size += gap;
}
// Now insert the register at the specified offset.
auto *reg = adder.reg.value();
_offsetMap.emplace(adder.offset.value(), *reg);
_size += reg->size();
}
}
void addRegister(RegisterAdder reg) { addRegisters({reg}); }
Addr base() const { return _base; }

View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020 ARM Limited
* Copyright (c) 2020, 2024 Arm Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -1100,6 +1100,93 @@ TEST_F(RegisterBankTest, AddRegistersWithOffsetChecks)
EXPECT_EQ(emptyBank.size(), 12);
}
/**
* This test is using addRegistersAt method to store
* overlapping registers to the empty bank. This should not
* be permitted and the method should panic
*
* [ reg2 ]
* [ reg1 ] |
* [ reg0 ] | |
* | | | |
* 0x0 0x4 0x6 0x8
*/
TEST_F(RegisterBankTest, AddRegistersAtOffsetDeath)
{
gtestLogOutput.str("");
auto base = emptyBank.base();
EXPECT_ANY_THROW(
emptyBank.addRegistersAt<RegisterBankLE::RegisterRao>(
{{base + 0x0, reg0},
{base + 0x2, reg1},
{base + 0x4, reg2}}));
std::string actual = gtestLogOutput.str();
EXPECT_THAT(actual, HasSubstr("Overlapping register"));
EXPECT_THAT(actual, HasSubstr("reg1"));
}
/**
* This test is using addRegistersAt method to store
* contiguous registers to the empty bank, similarly
* to what we would do when relying on addRegisters.
* The test will check size is updated consistently
* with the latter method
*
* [ reg0 ][ reg1 ][ reg2 ]
* | | | |
* 0x0 0x4 0x8 0xc
*/
TEST_F(RegisterBankTest, AddRegistersAtOffsetContiguous)
{
auto base = emptyBank.base();
EXPECT_EQ(emptyBank.size(), 0);
emptyBank.addRegistersAt<RegisterBankLE::RegisterRao>(
{{base + 0x0, reg0},
{base + 0x4, reg1},
{base + 0x8, reg2}});
EXPECT_EQ(emptyBank.size(), 0xc);
}
/**
* This test is using addRegistersAt method to store
* non-contiguous registers to the empty bank.
* As the RegisterRao data type is passed as a template
* argument, the gaps between the registers are filled
* with rao registers.
* We check raos are correctly inserted
*
* [reg0][rao0][reg1][rao1][reg2]
* | | | |
* 0x0 0x8 0x10 0x14
*/
TEST_F(RegisterBankTest, AddRegistersAtOffsetSparse)
{
auto base = emptyBank.base();
EXPECT_EQ(emptyBank.size(), 0);
emptyBank.addRegistersAt<RegisterBankLE::RegisterRao>(
{{base + 0x0, reg0},
{base + 0x8, reg1},
{base + 0x10, reg2}});
EXPECT_EQ(emptyBank.size(), 0x14);
emptyBank.read(base + 0x0, buf.data(), 4);
EXPECT_EQ(reg0.get(), *reinterpret_cast<uint32_t*>(buf.data()));
emptyBank.read(base + 0x4, buf.data(), 4);
EXPECT_EQ(0xffffffff, *reinterpret_cast<uint32_t*>(buf.data()));
emptyBank.read(base + 0x8, buf.data(), 4);
EXPECT_EQ(reg1.get(), *reinterpret_cast<uint32_t*>(buf.data()));
emptyBank.read(base + 0xc, buf.data(), 4);
EXPECT_EQ(0xffffffff, *reinterpret_cast<uint32_t*>(buf.data()));
emptyBank.read(base + 0x10, buf.data(), 4);
EXPECT_EQ(reg2.get(), *reinterpret_cast<uint32_t*>(buf.data()));
}
TEST_F(RegisterBankTest, BadRegisterOffsetDeath)
{
gtestLogOutput.str("");