Hi Daniel,
On Fri, Aug 7, 2026 at 8:38 AM Daniel Henrique Barboza
<[email protected]> wrote:
>
> Hello,
>
> On 7/29/2026 12:21 PM, Anirudh Srinivasan wrote:
> > This device present in Tenstorrent Atlantis has registers that control
> > clocks, reset and other misc functionality in the SoC. This commit adds
> > models for the RCPU, HSIO, PCIE, MM and 2 DDR PRCMs.
> >
> > Co-developed-by: Portia Stephens <[email protected]>
> > Signed-off-by: Portia Stephens <[email protected]>
> > Signed-off-by: Anirudh Srinivasan <[email protected]>
> > ---
> > MAINTAINERS | 2 +
> > hw/misc/meson.build | 1 +
> > hw/misc/trace-events | 4 +
> > hw/misc/tt_atlantis_prcm.c | 436
> > +++++++++++++++++++++++++++++++++++++
> > include/hw/misc/tt_atlantis_prcm.h | 51 +++++
> > 5 files changed, 494 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e25df9493c..49c1a5ad76 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1802,7 +1802,9 @@ R: Portia Stephens <[email protected]>
> > L: [email protected]
> > S: Supported
> > F: docs/system/riscv/tt_*.rst
> > +F: hw/misc/tt_*.c
> > F: hw/riscv/tt_*.c
> > +F: include/hw/misc/tt_*.h
> > F: include/hw/riscv/tt_*.h
> > F: tests/functional/riscv64/test_tt_*.py
> >
> > +}
> > +
> > +static void tt_atlantis_prcm_write(void *opaque, hwaddr offset,
> > + uint64_t data, unsigned size) {
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(opaque);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_GET_CLASS(s);
> > +
> > + trace_tt_atlantis_prcm_write(c->domain, offset, data);
> > +
> > + if (offset >= c->regs_size) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx
> > "\n",
> > + __func__, offset);
> > + return;
> > + }
>
> I'm a bit confused here: in tt_atlantis_prcm_read() we're doing a
> qemu_log_mask() and
> return, and then do a "trace_tt_atlantis...". So in an error case you'll
> have just
> the qemu_log_mask(). Here we're doing a trace at the start of prcm_write()
> and then
> a qemu_log_mask() if something wrong happens.
>
> There's no rule on how you use trace so there's nothing wrong with both
> approaches but
> would be nice if we could stick with one single approach: either do
> trace_tt_... during
> function start or in function success. From what I can tell the most common
> pattern is
> the one used in prcm_read(), i.e. qemu_log_mask on errors and trace_ on
> success.
I think I tried placing the trace for the write at the top of the
function and missed this bit. Will do it this way instead.
>
> > +
> > + switch (TO_REG(offset)) {
> > + default:
> > + s->regs[TO_REG(offset)] = data;
> > + break;
> > + }
>
> This switch is doing nothing. Just do s->regs[TO_REG(offset)] = data
> directly.
Yes, I think I left this in case I was expecting to handle writes to
specific registers here, but that doesn't seem to be needed. WIll
remove this.
>
>
> > +}
> > +
> > +static void tt_atlantis_prcm_rcpu_write(void *opaque, hwaddr offset,
> > + uint64_t data, unsigned size) {
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(opaque);
> > +
> > + tt_atlantis_prcm_write(opaque, offset, data, size);
> > +
> > + bool pll_en_written = false;
> > + uint32_t pll_reg_offset;
> > + switch TO_REG(offset)
> > + {
> > + case RCPU_PLL_RCPU_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_RCPU_CFG;
> > + break;
> > + case RCPU_PLL_NOCC_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_NOCC_CFG;
> > + break;
> > + case RCPU_PLL_HSIO_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_HSIO_CFG;
> > + break;
> > + case RCPU_PLL_PCIE_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_PCIE_CFG;
> > + break;
> > + case RCPU_PLL_MM_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_MM_CFG;
> > + break;
> > + case RCPU_PLL1_MM_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL1_MM_CFG;
> > + break;
> > + case RCPU_PLL_I2S_EN:
> > + pll_en_written = true;
> > + pll_reg_offset = RCPU_PLL_I2S_CFG;
> > + break;
> > + }
> > + if (pll_en_written) {
> > + if (data & PLL_EN) {
> > + s->regs[pll_reg_offset] |= BIT(PLL_LOCK_BIT);
> > + } else {
> > + s->regs[pll_reg_offset] &= ~BIT(PLL_LOCK_BIT);
> > + }
> > + }
>
> Seems like you're using pll_en_written as a flag to detect if we have a match
> in the 'switch' and nothing else, since pll_en_written is always being set to
> 'true'. And if there's no match in the switch() the function just returns.
>
> I suggest removing pll_en_written and doing a return in a default label.
> Something like this:
>
> > + uint32_t pll_reg_offset;
> > + switch TO_REG(offset) {
> > + case RCPU_PLL_RCPU_EN:
> > + pll_reg_offset = RCPU_PLL_RCPU_CFG;
> > + break;
> > + case RCPU_PLL_NOCC_EN:
> > + pll_reg_offset = RCPU_PLL_NOCC_CFG;
> > + break;
> > + case RCPU_PLL_HSIO_EN:
> > + pll_reg_offset = RCPU_PLL_HSIO_CFG;
> > + break;
> > + case RCPU_PLL_PCIE_EN:
> > + pll_reg_offset = RCPU_PLL_PCIE_CFG;
> > + break;
> > + case RCPU_PLL_MM_EN:
> > + pll_reg_offset = RCPU_PLL_MM_CFG;
> > + break;
> > + case RCPU_PLL1_MM_EN:
> > + pll_reg_offset = RCPU_PLL1_MM_CFG;
> > + break;
> > + case RCPU_PLL_I2S_EN:
> > + pll_reg_offset = RCPU_PLL_I2S_CFG;
> > + break;
> default:
> return;
> > + }
>
> > + if (data & PLL_EN) {
> > + s->regs[pll_reg_offset] |= BIT(PLL_LOCK_BIT);
> > + } else {
> > + s->regs[pll_reg_offset] &= ~BIT(PLL_LOCK_BIT);
> > + }
Yes, this looks simpler. Will change it to this.
>
> > +}
> > +
> > +static const MemoryRegionOps tt_atlantis_prcm_ops = {
> > + .read = tt_atlantis_prcm_read,
> > + .write = tt_atlantis_prcm_write,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid.min_access_size = 4,
> > + .valid.max_access_size = 4,
> > +};
> > +
> > +static const MemoryRegionOps tt_atlantis_prcm_rcpu_ops = {
> > + .read = tt_atlantis_prcm_read,
> > + .write = tt_atlantis_prcm_rcpu_write,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid.min_access_size = 4,
> > + .valid.max_access_size = 4,
> > +};
> > +
> > +static void tt_atlantis_prcm_realize(DeviceState *dev, Error **errp)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_GET_CLASS(s);
> > +
> > + if (c->domain >= PRCM_DOMAIN_COUNT) {
> > + error_setg(errp,
> > + "domain %d is larger than max number of possible
> > domains %d",
> > + c->domain, PRCM_DOMAIN_COUNT);
> > + return;
> > + }
> > + s->regs = g_new0(uint32_t, TO_REG(c->regs_size));
> > +
> > + memory_region_init_io(&s->mmio, OBJECT(s), c->ops, s,
> > + TYPE_TT_ATLANTIS_PRCM, c->regs_size);
> > + sysbus_init_mmio(sbd, &s->mmio);
> > +}
> > +
> > +static void tt_atlantis_prcm_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_GET_CLASS(s);
> > +
> > + memset(s->regs, 0, c->regs_size);
> > +}
> > +
> > +static void tt_atlantis_prcm_rcpu_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + tt_atlantis_prcm_reset(dev);
> > +
> > + s->regs[RCPU_PLL_RCPU_CFG] = PLL_RESET_VAL(1, 0, 4, 0, 100, 1);
> > + s->regs[RCPU_PLL_NOCC_CFG] = PLL_RESET_VAL(0, 2, 2, 2, 267, 1);
> > + s->regs[RCPU_NOCC_CLK_CFG] = (0x25f31 << 1) | BIT(0);
> > + s->regs[RCPU_RCPU_DIV_CFG] = (0xb8c2 << 1);
>
> There's a lot of the so called 'magic numbers' around the code. Please put
> them in macros and, preferrably, explain where they're coming from (a
> combination
> of reg values, a hardcoded value that the manual/spec dictates, etc).
Most of the reset values are already covered by the PLL_RESET_VAL
macro, which is defined at the top of this file. For the remaining
few, some are all 0s or all Fs, and the remaining "magic" values come
from the datasheet, so I will add a comment to reflect that.
>
> One more thing:
>
> > + s->regs[RCPU_RCPU_BLK_CG] = 0;
> > + s->regs[RCPU_LSIO_BLK_CG] = 0;
> > + s->regs[RCPU_RCPU_BLK_RST] = 0xffffffff;
> > + s->regs[RCPU_LSIO_BLK_RST] = 0xffffffff;
> > + s->regs[RCPU_PLL_RCPU_EN] = PLL_EN;
> > + s->regs[RCPU_PLL_NOCC_EN] = PLL_EN;
> > + s->regs[RCPU_PLL_HSIO_EN] = 0;
> > + s->regs[RCPU_PLL_MM_EN] = 0;
> > + s->regs[RCPU_PLL1_MM_EN] = 0;
> > + s->regs[RCPU_PLL_I2S_EN] = 0;
> > + s->regs[RCPU_PLL_PCIE_EN] = 0;
> > + s->regs[RCPU_PLL_HSIO_CFG] = PLL_RESET_VAL(0, 1, 2, 1, 50, 0);
> > + s->regs[RCPU_PLL_PCIE_CFG] = PLL_RESET_VAL(0, 2, 3, 1, 250, 0);
> > + s->regs[RCPU_BOOT_MODE_CFG] = RCPU_BOOT_MODE_SD;
> > + s->regs[RCPU_PLL_MM_CFG] = PLL_RESET_VAL(0, 1, 2, 1, 50, 1);
> > + s->regs[RCPU_PLL1_MM_CFG] = PLL_RESET_VAL(0, 1, 2, 1, 50, 1);
> > + s->regs[RCPU_PLL_I2S_CFG] = PLL_RESET_VAL(0, 1, 6, 6, 33, 0);
> > + s->regs[RCPU_I2S_DIV_CFG] = 0xde69ad31;
> > + s->regs[RCPU_BUS_CFG] = NOCC_PLL_BUS_CG_EN | HSIO_PLL_BUS_CG_EN;
> > +}
> > +
> > +static void tt_atlantis_prcm_ddrc_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + tt_atlantis_prcm_reset(dev);
> > +
> > + s->regs[DDRC_STAT] = 0x1;
> > + s->regs[DDRC_DFISTAT] = 0x1;
> > + s->regs[DDRC_SWSTAT] = 0x1;
> > +}
> > +
> > +static void tt_atlantis_prcm_hsio_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + tt_atlantis_prcm_reset(dev);
> > +
> > + s->regs[HSIO_HSIO_DIV_CFG] = 0x20821731;
> > + s->regs[HSIO_HSIO_DIV_CFG1] = 0x8;
> > + s->regs[HSIO_HSIO_BLK_CG] = 0;
> > + s->regs[HSIO_HSIO_BLK_RST] = 0;
> > + s->regs[HSIO_HSIO_GMAC_DIV_CFG] = 0;
> > +}
> > +
> > +static void tt_atlantis_prcm_pcie_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + tt_atlantis_prcm_reset(dev);
> > +
> > + s->regs[PCIE_SUBS_RST_CTL] = 0;
> > + s->regs[PCIE_SUBS_CG_CTL] = 0;
> > + s->regs[PCIE_SUBS_CK_CTL] = 0x44908;
> > +}
> > +
> > +static void tt_atlantis_prcm_mm_reset(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > + tt_atlantis_prcm_reset(dev);
> > +
> > + s->regs[MM_MM_CLK_GATE_CFG] = 0;
> > + s->regs[MM_MM_CLK_DIV_CFG] = 0xddc8312;
> > + s->regs[MM_MM_CLK_SEL_CFG] = 0;
> > + s->regs[MM_MM_RSTN] = 0;
> > + s->regs[MM_MM_CLK_DIV_CFG1] = 0x333;
> > +}
> > +
> > +static void tt_atlantis_prcm_unrealize(DeviceState *dev)
> > +{
> > + TTAtlantisPRCMState *s = TT_ATLANTIS_PRCM(dev);
> > +
> > + g_free(s->regs);
> > +}
> > +
> > +static void tt_atlantis_prcm_class_init(ObjectClass *klass,
> > + const void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_CLASS(klass);
> > +
> > + dc->realize = tt_atlantis_prcm_realize;
> > + dc->unrealize = tt_atlantis_prcm_unrealize;
> > + device_class_set_legacy_reset(dc, tt_atlantis_prcm_reset);
> > + dc->desc = "Tenstorrent Atlantis PRCM Controller";
> > + c->ops = &tt_atlantis_prcm_ops;
> > +}
> > +
> > +static void tt_atlantis_prcm_rcpu_class_init(ObjectClass *klass,
> > + const void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_CLASS(klass);
> > +
> > + dc->realize = tt_atlantis_prcm_realize;
> > + device_class_set_legacy_reset(dc, tt_atlantis_prcm_rcpu_reset);
> > + dc->desc = "Tenstorrent Atlantis RCPU PRCM Controller";
> > + c->domain = PRCM_DOMAIN_RCPU;
> > + c->regs_size = RCPU_DOMAIN_SIZE;
> > + c->ops = &tt_atlantis_prcm_rcpu_ops;
> > +}
> > +
> > +static void tt_atlantis_prcm_hsio_class_init(ObjectClass *klass,
> > + const void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_CLASS(klass);
> > +
> > + dc->realize = tt_atlantis_prcm_realize;
> > + device_class_set_legacy_reset(dc, tt_atlantis_prcm_hsio_reset);
> > + dc->desc = "Tenstorrent Atlantis HSIO PRCM Controller";
> > + c->domain = PRCM_DOMAIN_HSIO;
> > + c->regs_size = HSIO_DOMAIN_SIZE;
> > + c->ops = &tt_atlantis_prcm_ops;
> > +}
> > +
> > +static void tt_atlantis_prcm_pcie_class_init(ObjectClass *klass,
> > + const void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_CLASS(klass);
> > +
> > + dc->realize = tt_atlantis_prcm_realize;
> > + device_class_set_legacy_reset(dc, tt_atlantis_prcm_pcie_reset);
> > + dc->desc = "Tenstorrent Atlantis PCIE PRCM Controller";
> > + c->domain = PRCM_DOMAIN_PCIE;
> > + c->regs_size = PCIE_DOMAIN_SIZE;
> > + c->ops = &tt_atlantis_prcm_ops;
> > +}
> > +
> > +static void tt_atlantis_prcm_mm_class_init(ObjectClass *klass,
> > + const void *data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + TTAtlantisPRCMClass *c = TT_ATLANTIS_PRCM_CLASS(klass);
> > +
> > + dc->realize = tt_atlantis_prcm_realize;
> > + device_class_set_legacy_reset(dc, tt_atlantis_prcm_mm_reset);
>
> Let's not use the legacy reset interface for new devices - we should use the
> Resetabble interface instead. Here's a straightforward example from
> riscv-iommu-sys.c:
Okay, will change over to this.
>
>
> static void riscv_iommu_sys_reset_hold(Object *obj, ResetType type)
> {
> // reset procedure
> }
>
> static void riscv_iommu_sys_class_init(ObjectClass *klass, const void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> rc->phases.hold = riscv_iommu_sys_reset_hold;
> (...)
>
>
> In this case you would do "rc->phases.hold = tt_atlantis_prcm_mm_reset". Same
>
> Note that if the device requires a more elaborated reset procedure you would
> need
> to do more stuff, but from what I can tell this would be enough for this PRCM
> model.
> For more information on reset phases feel free to take a look here:
>
> https://www.qemu.org/docs/master/devel/reset.html
>
>
> Thanks,
> Daniel
I ended up getting 2 replies from you to this mail. Not sure if this
was a bug or not. I have replied to the 2nd one.
