Skip to content

ot_earlgrey: add JTAG support to LC Ctrl #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: ot-9.2.0
Choose a base branch
from
Draft

ot_earlgrey: add JTAG support to LC Ctrl #274

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 86 additions & 4 deletions hw/riscv/ot_earlgrey.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@
#include "hw/jtag/tap_ctrl.h"
#include "hw/jtag/tap_ctrl_rbb.h"
#include "hw/misc/pulp_rv_dm.h"
#include "hw/opentitan/ot_address_space.h"
#include "hw/opentitan/ot_aes.h"
#include "hw/opentitan/ot_alert.h"
#include "hw/opentitan/ot_aon_timer.h"
#include "hw/opentitan/ot_ast_eg.h"
#include "hw/opentitan/ot_clkmgr.h"
#include "hw/opentitan/ot_common.h"
#include "hw/opentitan/ot_csrng.h"
#include "hw/opentitan/ot_dm_tl.h"
#include "hw/opentitan/ot_edn.h"
#include "hw/opentitan/ot_entropy_src.h"
#include "hw/opentitan/ot_flash.h"
Expand Down Expand Up @@ -99,6 +101,8 @@ static void ot_eg_soc_otp_ctrl_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent);
static void ot_eg_soc_tap_ctrl_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent);
static void ot_eg_soc_lc_ctrl_tap_ctrl_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent);
static void ot_eg_soc_spi_device_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent);
static void ot_eg_soc_uart_configure(DeviceState *dev, const IbexDeviceDef *def,
Expand All @@ -110,6 +114,14 @@ static void ot_eg_soc_usbdev_configure(
/* Constants */
/* ------------------------------------------------------------------------ */

enum OtEgMemoryRegion {
OT_EG_DEFAULT_MEMORY_REGION,
OT_EG_LC_CTRL_MEMORY_REGION,
};

#define LC_CTRL_MEMORY(_addr_) \
IBEX_MEMMAP_MAKE_REG((_addr_), OT_EG_LC_CTRL_MEMORY_REGION)

enum OtEGSocDevice {
OT_EG_SOC_DEV_ADC_CTRL,
OT_EG_SOC_DEV_AES,
Expand All @@ -120,6 +132,7 @@ enum OtEGSocDevice {
OT_EG_SOC_DEV_CSRNG,
OT_EG_SOC_DEV_DM,
OT_EG_SOC_DEV_DTM,
OT_EG_SOC_DEV_LC_CTRL_DTM,
OT_EG_SOC_DEV_EDN0,
OT_EG_SOC_DEV_EDN1,
OT_EG_SOC_DEV_ENTROPY_SRC,
Expand Down Expand Up @@ -147,13 +160,15 @@ enum OtEGSocDevice {
OT_EG_SOC_DEV_ROM_CTRL,
OT_EG_SOC_DEV_RSTMGR,
OT_EG_SOC_DEV_RV_DM,
OT_EG_SOC_DEV_DM_LC_CTRL,
OT_EG_SOC_DEV_SENSOR_CTRL,
OT_EG_SOC_DEV_SPI_DEVICE,
OT_EG_SOC_DEV_SPI_HOST0,
OT_EG_SOC_DEV_SPI_HOST1,
OT_EG_SOC_DEV_SRAM_MAIN_CTRL,
OT_EG_SOC_DEV_SYSRST_CTRL,
OT_EG_SOC_DEV_TAP_CTRL,
OT_EG_SOC_DEV_LC_CTRL_TAP_CTRL,
OT_EG_SOC_DEV_TIMER,
OT_EG_SOC_DEV_UART0,
OT_EG_SOC_DEV_UART1,
Expand Down Expand Up @@ -195,6 +210,11 @@ enum OtEGBoardDevice {
OT_EG_BOARD_DEV_COUNT,
};

#define OT_EG_DEBUG_LC_CTRL_ADDR 0x0u
#define OT_EG_DEBUG_LC_CTRL_SIZE 0x200u
Copy link

@AlexJones0 AlexJones0 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this size defined? I can't find a source on earlgrey_1.0.0 but on master this seems like it should be 0x1000 instead?

Copy link
Author

@luismarques luismarques Nov 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the bus is private, I just used the maximum allowed size, given the abits=7 (2^7 * 4 == 0x200, with the 4 being for the 1:4 mapping adjustment). That way we can keep the actual size needed known only inside the ot_lc_ctrl.

Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth adding a comment to keep track of this, and/or use a defined constant for the bit width (7) and a macro based on it to compute the actual range?

#define OT_EG_DEBUG_LC_CTRL_DMI_ADDR (OT_EG_DEBUG_LC_CTRL_ADDR / 4)
Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit pick: either 4u or sizeof(uint32_t)

#define OT_EG_DEBUG_LC_CTRL_DMI_SIZE (OT_EG_DEBUG_LC_CTRL_SIZE / 4)

#define OT_EG_IBEX_WRAPPER_NUM_REGIONS 2u

static const uint8_t ot_eg_pmp_cfgs[] = {
Expand Down Expand Up @@ -360,6 +380,14 @@ static const IbexDeviceDef ot_eg_soc_devices[] = {
IBEX_DEV_UINT_PROP("idcode", EG_RV_DM_TAP_IDCODE)
),
},
[OT_EG_SOC_DEV_LC_CTRL_TAP_CTRL] = {
.type = TYPE_TAP_CTRL_RBB,
.cfg = &ot_eg_soc_lc_ctrl_tap_ctrl_configure,
.prop = IBEXDEVICEPROPDEFS(
IBEX_DEV_UINT_PROP("ir_length", IBEX_TAP_IR_LENGTH),
IBEX_DEV_UINT_PROP("idcode", EG_LC_CTRL_TAP_IDCODE)
),
},
[OT_EG_SOC_DEV_DTM] = {
.type = TYPE_RISCV_DTM,
.link = IBEXDEVICELINKDEFS(
Expand All @@ -369,6 +397,15 @@ static const IbexDeviceDef ot_eg_soc_devices[] = {
IBEX_DEV_UINT_PROP("abits", 7u)
),
},
[OT_EG_SOC_DEV_LC_CTRL_DTM] = {
.type = TYPE_RISCV_DTM,
.link = IBEXDEVICELINKDEFS(
OT_EG_SOC_DEVLINK("tap-ctrl", LC_CTRL_TAP_CTRL)
),
.prop = IBEXDEVICEPROPDEFS(
IBEX_DEV_UINT_PROP("abits", 7u)
),
},
[OT_EG_SOC_DEV_DM] = {
.type = TYPE_RISCV_DM,
.cfg = &ot_eg_soc_dm_configure,
Expand Down Expand Up @@ -734,7 +771,8 @@ static const IbexDeviceDef ot_eg_soc_devices[] = {
[OT_EG_SOC_DEV_LC_CTRL] = {
.type = TYPE_OT_LC_CTRL,
.memmap = MEMMAPENTRIES(
{ .base = 0x40140000u }
{ .base = 0x40140000u },
{ .base = LC_CTRL_MEMORY(OT_EG_DEBUG_LC_CTRL_ADDR) }
),
.gpio = IBEXGPIOCONNDEFS(
OT_EG_SOC_RSP(OT_PWRMGR_LC, PWRMGR),
Expand Down Expand Up @@ -1364,6 +1402,19 @@ static const IbexDeviceDef ot_eg_soc_devices[] = {
OT_EG_SOC_GPIO_ALERT(0, 40)
),
},
[OT_EG_SOC_DEV_DM_LC_CTRL] = {
.type = TYPE_OT_DM_TL,
.link = IBEXDEVICELINKDEFS(
OT_EG_SOC_DEVLINK("dtm", LC_CTRL_DTM),
OT_EG_SOC_DEVLINK("tl_dev", LC_CTRL)
),
.prop = IBEXDEVICEPROPDEFS(
IBEX_DEV_UINT_PROP("dmi_addr", OT_EG_DEBUG_LC_CTRL_DMI_ADDR),
Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit pick: maybe OT_EG_LC_CTRL_DMI_ADDR to match OT_EG_LC_CTRL_MEMORY_REGION? Dmi already stands for DEBUG.

IBEX_DEV_UINT_PROP("dmi_size", OT_EG_DEBUG_LC_CTRL_DMI_SIZE),
IBEX_DEV_UINT_PROP("tl_addr", OT_EG_DEBUG_LC_CTRL_ADDR),
IBEX_DEV_STRING_PROP("tl_as_name", "ot-lc-ctrl-dmi-as")
Copy link

@AlexJones0 AlexJones0 Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this name "ot-lc-ctrl-dmi-as" is repeated as a hardcoded string, maybe extract it out to a:

#define LC_CTRL_ADDR_SPACE "ot-lc-ctrl-dmi-as"

or equivalent?

)
},
[OT_EG_SOC_DEV_PLIC] = {
.type = TYPE_SIFIVE_PLIC,
.memmap = MEMMAPENTRIES(
Expand Down Expand Up @@ -1609,6 +1660,20 @@ static void ot_eg_soc_tap_ctrl_configure(
}
}

static void ot_eg_soc_lc_ctrl_tap_ctrl_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent)
{
(void)parent;
(void)def;

Chardev *chr;

chr = ibex_get_chardev_by_id("taprbb-lc-ctrl");
if (chr) {
qdev_prop_set_chr(dev, "chardev", chr);
}
}

static void ot_eg_soc_spi_device_configure(
DeviceState *dev, const IbexDeviceDef *def, DeviceState *parent)
{
Expand Down Expand Up @@ -1754,9 +1819,26 @@ static void ot_eg_soc_realize(DeviceState *dev, Error **errp)
ot_eg_soc_devices,
ARRAY_SIZE(ot_eg_soc_devices));

MemoryRegion *mrs[] = { get_system_memory(), NULL, NULL, NULL };
ibex_map_devices(s->devices, mrs, ot_eg_soc_devices,
ARRAY_SIZE(ot_eg_soc_devices));
MemoryRegion *lc_ctrl_mr = g_new0(MemoryRegion, 1u);
memory_region_init(lc_ctrl_mr, OBJECT(dev), "lc-ctrl-dmi.xbar",
Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe try to use the same radix for all related naming, i.e. "lc-ctrl.xbar", or better:

TYPE_OT_LC_CTRL ".xbar"
TYPE_OT_LC_CTRL ".as"
// see note in another comment about as->name
IBEX_DEV_STRING_PROP("tl_as_name", TYPE_OT_LC_CTRL ".as")

OT_EG_DEBUG_LC_CTRL_SIZE);

MemoryRegion *mrs[IBEX_MEMMAP_REGIDX_COUNT] = {
[OT_EG_DEFAULT_MEMORY_REGION] = get_system_memory(),
[OT_EG_LC_CTRL_MEMORY_REGION] = lc_ctrl_mr,
};
ibex_map_devices_mask(s->devices, mrs, ot_eg_soc_devices,
ARRAY_SIZE(ot_eg_soc_devices),
IBEX_MEMMAP_MAKE_REG_MASK(
OT_EG_DEFAULT_MEMORY_REGION) |
IBEX_MEMMAP_MAKE_REG_MASK(
OT_EG_LC_CTRL_MEMORY_REGION));
Object *oas;
AddressSpace *as = g_new0(AddressSpace, 1u);
address_space_init(as, lc_ctrl_mr, "lc-ctrl.as");
Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment about the role of this new AS?

oas = object_new(TYPE_OT_ADDRESS_SPACE);
object_property_add_child(OBJECT(dev), "ot-lc-ctrl-dmi-as", oas);
Copy link

@rivos-eblot rivos-eblot Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use object_property_add_child(OBJECT(dev), as->name, oas);
for the property name. It is probably easier when debugging to have the AS refered by its name.

ot_address_space_set(OT_ADDRESS_SPACE(oas), as);

qdev_connect_gpio_out_named(DEVICE(s->devices[OT_EG_SOC_DEV_RSTMGR]),
OT_RSTMGR_SOC_RST, 0,
Expand Down
Loading