From 27316882d75ff09fb08edd48123fe06877d97075 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Thu, 9 Oct 2025 10:27:22 +0200 Subject: [PATCH 1/6] [ot] python/qemu: ot.misc.util: fix retrieve_git_version when called on non git dir hierarchy Signed-off-by: Emmanuel Blot --- python/qemu/ot/util/misc.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python/qemu/ot/util/misc.py b/python/qemu/ot/util/misc.py index eccb3c49abc48..f8715ef7ffdec 100644 --- a/python/qemu/ot/util/misc.py +++ b/python/qemu/ot/util/misc.py @@ -241,7 +241,11 @@ def retrieve_git_version(path: str, max_tag_dist: int = 100) \ gitdir = joinpath(cfgdir, '.git') if exists(gitdir): # either a dir or a file for worktree break - cfgdir = dirname(cfgdir) + pardir = dirname(cfgdir) + if pardir == cfgdir: + # reach top of tree hierarchy without any detected .git directory + return None + cfgdir = pardir else: return None assert cfgdir is not None @@ -258,7 +262,7 @@ def retrieve_git_version(path: str, max_tag_dist: int = 100) \ return '-'.join(filter(None, (gmo.group('tag'), dirty))) if distance <= max_tag_dist: return descstr - return '-'.join(filter(None, (gmo.group('commit'), dirty))) + return '-'.join(filter(None, (gmo.group('commit'), dirty))) except (OSError, ValueError): pass try: From 639a2bc4ee62608bb9ced0c6347116e0f2b5f137 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Tue, 7 Oct 2025 11:58:14 +0200 Subject: [PATCH 2/6] [ot] scripts/opentitan: cfggen.py: add Git reference to generated config file. Signed-off-by: Emmanuel Blot --- scripts/opentitan/cfggen.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/opentitan/cfggen.py b/scripts/opentitan/cfggen.py index 44486d8850395..c78f3e5b617a5 100755 --- a/scripts/opentitan/cfggen.py +++ b/scripts/opentitan/cfggen.py @@ -37,7 +37,7 @@ def hjload(*_, **__): # noqa: E301 from ot.top import OpenTitanTop from ot.util.arg import ArgError from ot.util.log import configure_loggers -from ot.util.misc import alphanum_key, to_bool +from ot.util.misc import alphanum_key, retrieve_git_version, to_bool OtParamRegex = str @@ -126,6 +126,7 @@ def __init__(self): self._clock_groups: dict[str, OtClockGroup] = {} self._mod_clocks: dict[str, list[str]] = {} self._top_name: Optional[str] = None + self._git_version: Optional[str] = None self._exclusions: dict[str, set[str]] = {} @property @@ -141,6 +142,8 @@ def load_config(self, toppath: str) -> None: self._top_name = cfg.get('name') topbase = basename(toppath) + self._git_version = retrieve_git_version(toppath) + for module in cfg.get('module') or []: modtype = module.get('type') moddefs = self.MODULES.get(modtype) @@ -350,6 +353,9 @@ def save(self, variant: str, socid: Optional[str], count: Optional[int], self._generate_ast(cfg, variant, socid) self._generate_clkmgr(cfg, socid) self._generate_pwrmgr(cfg, socid) + if self._git_version: + print(f'# Generated from OpenTitan commit: {self._git_version}\n', + file=ofp) cfg.write(ofp) def show_clocks(self, ofp: Optional[TextIO]) -> None: From 3a0643493dbe7b8bb5f0900858ecbafba6974c42 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Tue, 7 Oct 2025 13:18:01 +0200 Subject: [PATCH 3/6] [ot] python/qemu: ot.otp: add Git version when know to generated files Signed-off-by: Emmanuel Blot --- python/qemu/ot/otp/descriptor.py | 16 ++++++++++++++-- python/qemu/ot/otp/lifecycle.py | 14 +++++++++++--- python/qemu/ot/otp/map.py | 10 +++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/python/qemu/ot/otp/descriptor.py b/python/qemu/ot/otp/descriptor.py index 391e6af67b263..e819758fb0e9f 100644 --- a/python/qemu/ot/otp/descriptor.py +++ b/python/qemu/ot/otp/descriptor.py @@ -79,7 +79,12 @@ def save(self, kind: str, hjname: str, scriptname: str, cfp: TextIO) \ raise NotImplementedError(f'No support for {kind}') attrs = {n: getattr(self, f'_convert_to_{k}') if k else lambda x: x for n, k in self.ATTRS.items() if k is not None} - print(f'/* Generated from {hjname} with {scriptname} */', file=cfp) + msg = f'Generated from {hjname} with {scriptname}' + git_version = self._otpmap.git_version + if git_version: + print(f'/*\n * {msg}\n * Top version: {git_version}\n */', file=cfp) + else: + print(f'/* ${msg} */', file=cfp) print(file=cfp) print('/* clang-format off */', file=cfp) print('/* NOLINTBEGIN */', file=cfp) @@ -205,7 +210,12 @@ def save(self, kind: str, topname: Optional[str], hjname: str, def _save_qemu(self, topname: Optional[str], hjname: str, scriptname: str, cfp: TextIO, slots: list[OtpSlotDescriptor]) -> None: - print(f'/* Generated from {hjname} with {scriptname} */', file=cfp) + msg = f'Generated from {hjname} with {scriptname}' + git_version = self._otpmap.git_version + if git_version: + print(f'/*\n * {msg}\n * Top version: {git_version}\n */', file=cfp) + else: + print(f'/* ${msg} */', file=cfp) print(file=cfp) for slot in slots: if slot.part: @@ -308,6 +318,8 @@ def _save_bmtest(self, topname: Optional[str], hjname: str, scriptname: str, cfp: TextIO, slots: list[OtpSlotDescriptor]) -> None: # pylint: disable=unused-argument print(f'// Generated from {hjname} with {scriptname}', file=cfp) + if self._otpmap.git_version: + print(f'// Top version: {self._otpmap.git_version}', file=cfp) print(file=cfp) rec_p2 = 1 << (len(slots) - 1).bit_length() print(f'#![recursion_limit = "{rec_p2}"]', file=cfp) diff --git a/python/qemu/ot/otp/lifecycle.py b/python/qemu/ot/otp/lifecycle.py index 9e32d01fc6dba..dff2a14a7e83a 100644 --- a/python/qemu/ot/otp/lifecycle.py +++ b/python/qemu/ot/otp/lifecycle.py @@ -10,10 +10,10 @@ from io import StringIO from logging import getLogger from textwrap import fill -from typing import TextIO +from typing import Optional, TextIO import re -from ot.util.misc import camel_to_snake_case, group +from ot.util.misc import camel_to_snake_case, group, retrieve_git_version class OtpLifecycle: @@ -33,6 +33,7 @@ def __init__(self): self._sequences: dict[str, dict[str, list[str]]] = {} self._tables: dict[str, dict[str, str]] = {} self._tokens: dict[str, str] = {} + self._git_version: Optional[str] = None def load(self, svp: TextIO): """Decode LifeCycle information. @@ -46,6 +47,8 @@ def load(self, svp: TextIO): r"\s+\{([^\}]+)\}\s*,?") codes: dict[str, int] = {} sequences: dict[str, dict[str, list[str]]] = {} + if svp.name and isinstance(svp.name, str): + self._git_version = retrieve_git_version(svp.name) svp = StringIO(svp.read()) for line in svp: cmt = line.find('//') @@ -104,7 +107,12 @@ def save(self, kind: str, cfp: TextIO, data_mode: bool) -> None: """ if kind.lower() != 'qemu': raise NotImplementedError(f'No support for {kind}') - print(f'/* Section auto-generated with {__name__} module */', file=cfp) + msg = f'Section auto-generated with {__name__} module' + if self._git_version: + print(f'/*\n * {msg}\n * Top version: {self._git_version}\n */', + file=cfp) + else: + print(f'/* ${msg} */', file=cfp) if data_mode: self._save_data(cfp) else: diff --git a/python/qemu/ot/otp/map.py b/python/qemu/ot/otp/map.py index 6831983707fab..3b4fb29925132 100644 --- a/python/qemu/ot/otp/map.py +++ b/python/qemu/ot/otp/map.py @@ -15,7 +15,7 @@ except ImportError: hjload = None -from ot.util.misc import round_up +from ot.util.misc import retrieve_git_version, round_up class OtpMap: @@ -42,6 +42,7 @@ def __init__(self): self._map: dict = {} self._otp_size = 0 self._partitions: list[OtpPartition] = [] + self._git_version: Optional[str] = None def load(self, hfp: TextIO) -> None: """Parse a HJSON configuration file, typically otp_ctrl_mmap.hjson @@ -49,6 +50,8 @@ def load(self, hfp: TextIO) -> None: if hjload is None: raise ImportError('HJSON module is required') self._map = hjload(hfp, object_pairs_hook=dict) + if hfp.name and isinstance(hfp.name, str): + self._git_version = retrieve_git_version(hfp.name) otp = self._map['otp'] self._otp_size = int(otp['width']) * int(otp['depth']) self._generate_partitions() @@ -59,6 +62,11 @@ def partitions(self) -> dict[str, Any]: """Return the partitions (in any)""" return {p['name']: p for p in self._map.get('partitions', [])} + @property + def git_version(self) -> Optional[str]: + """Return Git version if known.""" + return self._git_version + @classmethod def part_offset(cls, part: dict[str, Any]) -> int: """Get the offset of a partition.""" From 92425eb6473b2256c1e48f9bc624607174332b6e Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Tue, 7 Oct 2025 16:52:18 +0200 Subject: [PATCH 4/6] [ot] scripts/opentitan: autoreg.py: add a configurable register window size limit Signed-off-by: Emmanuel Blot --- scripts/opentitan/autoreg.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/scripts/opentitan/autoreg.py b/scripts/opentitan/autoreg.py index 215bebd3b26fa..a3c559095005d 100755 --- a/scripts/opentitan/autoreg.py +++ b/scripts/opentitan/autoreg.py @@ -86,6 +86,9 @@ class AutoReg: RESETS = ['enter', 'hold', 'exit'] + WINDOW_LIMIT = 64 + """Default window item count limit.""" + def __init__(self, devname: str, ignore_prefix: Optional[str] = None, resets: Optional[list[str]] = None): self._log = getLogger('autoreg') @@ -111,7 +114,7 @@ def generators(cls): actions.sort() return actions - def load(self, hfp: TextIO) -> None: + def load(self, hfp: TextIO, win_lim: Optional[int] = None) -> None: """Load a register map definition from an OT HJSON file.""" if hfp.name: git_version = retrieve_git_version(hfp.name) @@ -164,6 +167,11 @@ def load(self, hfp: TextIO) -> None: item = item['window'] reg = self._parse_register(address, item) count = safe_eval(item['items'], self._parameters) + if count > (win_lim or self.WINDOW_LIMIT): + self._log.warning( + 'Ignoring oversized window %s with %u items', + reg.name, count) + continue fields = reg.fields if not reg.fields: validbits = item.get('validbits') @@ -1117,6 +1125,10 @@ def main(): params.add_argument('-g', '--generate', action='append', choices=generators, required=True, help='what to generate') + params.add_argument('-k', '--out-kind', choices=outkinds, + default=outkinds[0], + help=f'output file format ' + f'(default: {outkinds[0]})') params.add_argument('-n', '--name', default='foo', help='device name') params.add_argument('-p', '--ignore', metavar='PREFIX', @@ -1124,10 +1136,10 @@ def main(): params.add_argument('-r', '--reset', action='append', choices=AutoReg.RESETS, help='generate reset code') - params.add_argument('-k', '--out-kind', choices=outkinds, - default=outkinds[0], - help=f'output file format ' - f'(default: {outkinds[0]})') + params.add_argument('-w', '--win-limit', type=HexInt.parse, + metavar='ITEMS', default=AutoReg.WINDOW_LIMIT, + help=f'Max window size, discard larger ' + f'(default: {AutoReg.WINDOW_LIMIT} items)') extra = argparser.add_argument_group(title='Extras') extra.add_argument('-v', '--verbose', action='count', help='increase verbosity') @@ -1146,7 +1158,7 @@ def main(): areg = reggen(args.name, args.ignore, args.reset) if args.copyright: areg.copyright = args.copyright - areg.load(args.config) + areg.load(args.config, args.win_limit) for gen in args.generate or []: generate = getattr(areg, f'generate_{gen}', None) if not generate: From 0a6afb4699a1957f155d4f715dc021c1bb54e167 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Mon, 6 Oct 2025 17:19:13 +0200 Subject: [PATCH 5/6] [ot] python/qemu: ot: use removeprefix instead of len-based substring. Signed-off-by: Emmanuel Blot --- python/qemu/ot/km/dpe.py | 5 +++-- python/qemu/ot/km/engine.py | 3 ++- python/qemu/ot/otp/partition.py | 6 ++---- scripts/opentitan/autotop.py | 11 +++-------- scripts/opentitan/cfggen.py | 2 +- scripts/opentitan/loghelp.py | 2 +- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/python/qemu/ot/km/dpe.py b/python/qemu/ot/km/dpe.py index 5ae0d627453b4..195250d2f9642 100644 --- a/python/qemu/ot/km/dpe.py +++ b/python/qemu/ot/km/dpe.py @@ -210,9 +210,10 @@ def load_config(self, config_file: TextIO) -> None: if devload in loaded: continue loaded.add(devload) - if not devinst.startswith('rom'): + prefix = 'rom' + if not devinst.startswith(prefix): raise ValueError(f'Invalid ROM instance name: {devinst}') - devidx = int(devinst[len('rom'):]) + devidx = int(devinst.removeprefix(prefix)) try: rom_img = self._roms[devidx] except IndexError: diff --git a/python/qemu/ot/km/engine.py b/python/qemu/ot/km/engine.py index d60c41eac894e..d83c9f3fe0ea3 100644 --- a/python/qemu/ot/km/engine.py +++ b/python/qemu/ot/km/engine.py @@ -305,9 +305,10 @@ def _enumerate_steps_from_log(self, itlog: Iterator[str]) -> \ def _execute_steps(self) -> dict[str, bytes]: results: dict[str, bytes] = {} + prefix = 'KeyManagerDpeStep' for name, step in self._steps.items(): self._log.info('Executing %s (%s: %s)', name, - step.__class__.__name__[len('KeyManagerDpeStep'):], + step.__class__.__name__.removeprefix(prefix), step.dst) if isinstance(step, KeyManagerDpeStepInitialize): res = self._kmd.initialize(step.dst) diff --git a/python/qemu/ot/otp/partition.py b/python/qemu/ot/otp/partition.py index 9f382dda9fc5d..59c42669c6868 100644 --- a/python/qemu/ot/otp/partition.py +++ b/python/qemu/ot/otp/partition.py @@ -217,10 +217,8 @@ def emit(fmt, *args): itvalue = buf.read(itsize) soff = f'[{f"{base+offset:d}":>5s}]' if base is not None else '' offset += itsize - if itname.startswith(f'{pname}_'): - name = f'{pname}:{itname[len(pname)+1:]}' - else: - name = f'{pname}:{itname}' + prefix = f'{pname}_' + name = f'{pname}:{itname.removeprefix(prefix)}' if not match(filter_re, itname, IGNORECASE): continue if itsize > 8: diff --git a/scripts/opentitan/autotop.py b/scripts/opentitan/autotop.py index 5ed34209d58a6..708897cc4de28 100755 --- a/scripts/opentitan/autotop.py +++ b/scripts/opentitan/autotop.py @@ -216,12 +216,10 @@ def _load_interrupts(self, interrupts: dict[str, Any]) \ -> list[tuple[str, str]]: int_list: list[tuple[str, str]] = [] for interrupt in interrupts: - name = interrupt['name'].lower() dev = self.device_name(interrupt['module_name'], True) width = interrupt.get('width', 1) prefix = f'{dev.lower()}_' - if name.startswith(prefix): - name = name[len(prefix):] + name = interrupt['name'].lower().removeprefix(prefix) if width == 1: int_list.append((dev, name)) else: @@ -247,12 +245,9 @@ def _load_alerts(self, cat: str, alerts: dict[str, Any]) \ if cat: cat = f'{cat}_'.upper() for alert in alerts: - name = alert['name'] dev = self.device_name(alert['module_name'], True) - lname = name.lower() prefix = f'{dev.lower()}_' - if lname.startswith(prefix): - lname = lname[len(prefix):] + lname = alert['name'].lower().removeprefix(prefix) alert_list.append((f'{cat}{dev}', lname)) return alert_list @@ -451,7 +446,7 @@ def _generate_bmtest_interrupts(self, tfp): irqname = irq.name.upper() prefix = commonprefix((irqname, dev)) if prefix: - irqname = irqname[len(prefix):].lstrip('_') + irqname = irqname.removeprefix(prefix).lstrip('_') irqs[f'{dev}_{irqname}'] = irq.index print('pub mod irq_num {', file=tfp) max_val = 0 diff --git a/scripts/opentitan/cfggen.py b/scripts/opentitan/cfggen.py index c78f3e5b617a5..c5a3f6350e0aa 100755 --- a/scripts/opentitan/cfggen.py +++ b/scripts/opentitan/cfggen.py @@ -214,7 +214,7 @@ def load_config(self, toppath: str) -> None: if not clk_name.startswith(exp_prefix): raise ValueError(f'Unexpected clock {clk_name} in group' f' {name}') - src_name = clk_name[len(exp_prefix):] + src_name = clk_name.removeprefix(exp_prefix) clk_srcs.append(src_name) if src_name in self._sub_clocks: raise ValueError(f'Refinition of clock {src_name}') diff --git a/scripts/opentitan/loghelp.py b/scripts/opentitan/loghelp.py index 404a6ce8778f1..171f928070329 100755 --- a/scripts/opentitan/loghelp.py +++ b/scripts/opentitan/loghelp.py @@ -64,7 +64,7 @@ def check(lfp: TextIO, comp: str, defs: RegisterDefs): line = line.strip() if not line.startswith(prefix): continue - parts = line[len(prefix):].split(' ', 1) + parts = line.removeprefix(prefix).split(' ', 1) items = dict(tuple(x.strip().split('=', 1)) for x in parts[1].split(',')) vals = {k: int(v, 16) for k, v in items.items()} From a4166a6478a34bd29e05a2e832cc35502db26091 Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 3 Oct 2025 18:22:59 +0200 Subject: [PATCH 6/6] [ot] python/qemu: ot.otp.partition: add default allocation for absorb-able space. Signed-off-by: Emmanuel Blot --- python/qemu/ot/otp/partition.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/python/qemu/ot/otp/partition.py b/python/qemu/ot/otp/partition.py index 59c42669c6868..852f139380f62 100644 --- a/python/qemu/ot/otp/partition.py +++ b/python/qemu/ot/otp/partition.py @@ -286,6 +286,9 @@ def dispatch_absorb(self) -> None: """Request the partition to resize its fields.""" if not self.is_absorb: raise RuntimeError('Partition cannot absorb free space') + if not self.items: + # no field to dispatch spare space into + return absorb_fields = {n: v for n, v in self.items.items() if v.get('absorb', False)} avail_size = self.size @@ -295,15 +298,13 @@ def dispatch_absorb(self) -> None: avail_size -= self.ZER_SIZE avail_blocks = avail_size // OtpMap.BLOCK_SIZE if not absorb_fields: - # a version of OTP where absorb is defined as a partition property - # but not defined for fields. In such a case, it is expected that - # the partition contains a single field. - itemcnt = len(self.items) - if itemcnt != 1: - raise RuntimeError(f'No known absorption method with {itemcnt} ' - f'items in partition {self.name}') - # nothing to do here - return + # This version of OTP defines 'absorb' as a partition property but + # does not document which fields can absorb the extra space. + # Allocate it to the last field of the partition. + last = list(self.items)[-1] + absorb_fields = {last: self.items[last]} + self._log.warning("No absorption defined for partition %s, " + "assigning to last field '%s'", self.name, last) absorb_count = len(absorb_fields) blk_per_field = avail_blocks // absorb_count extra_blocks = avail_blocks % absorb_count @@ -317,7 +318,7 @@ def dispatch_absorb(self) -> None: itfield['size'] += OtpMap.BLOCK_SIZE extra_blocks -= 1 self._log.info('%s.%s size augmented from %u to %u bytes', - self.name, itname, fsize, itfield['size']) + self.name, itname, fsize, itfield['size']) def empty(self) -> None: """Empty the partition, including its digest if any."""