Improve otptool.py, fix invalid destination address in ibex_wrapper remapping feature

docs/opentitan/otptool.md

@@ -9,10 +9,10 @@ controller virtual device.
 usage: otptool.py [-h] [-j HJSON] [-m VMEM] [-l SV] [-o FILE] [-r RAW]
                   [-k {auto,otp,fuz}] [-e BITS] [-C CONFIG] [-c INT] [-i INT]
                   [-w] [-n] [-f PART:FIELD] [--no-version] [-s] [-E] [-D] [-U]
-                  [--empty PARTITION] [--erase PART:FIELD]
-                  [--clear-bit CLEAR_BIT] [--set-bit SET_BIT]
-                  [--toggle-bit TOGGLE_BIT] [--fix-ecc]
-                  [-G {LCVAL,LCTPL,PARTS,REGS}] [-v] [-d]
+                  [-G {LCVAL,LCTPL,PARTS,REGS}] [-F]
+                  [--change PART:FIELD=VALUE] [--empty PARTITION]
+                  [--erase PART:FIELD] [--clear-bit CLEAR_BIT]
+                  [--set-bit SET_BIT] [--toggle-bit TOGGLE_BIT] [-v] [-d]
 
 QEMU OT tool to manage OTP files.
 
@@ -44,6 +44,11 @@ Commands:
   -E, --ecc-recover     attempt to recover errors with ECC
   -D, --digest          check the OTP HW partition digest
   -U, --update          update RAW file after ECC recovery or bit changes
+  -G, --generate {LCVAL,LCTPL,PARTS,REGS}
+                        generate C code, see doc for options
+  -F, --fix-ecc         rebuild ECC
+  --change PART:FIELD=VALUE
+                        change the content of an OTP field
   --empty PARTITION     reset the content of a whole partition, including its
                         digest if any
   --erase PART:FIELD    clear out an OTP field
@@ -52,9 +57,6 @@ Commands:
   --set-bit SET_BIT     set a bit at specified location
   --toggle-bit TOGGLE_BIT
                         toggle a bit at specified location
-  --fix-ecc             rebuild ECC
-  -G, --generate {LCVAL,LCTPL,PARTS,REGS}
-                        generate C code, see doc for options
 
 Extras:
   -v, --verbose         increase verbosity
@@ -111,6 +113,9 @@ Fuse RAW images only use the v1 type.
 * `-e` specify how many bits are used in the VMEM file to store ECC information. Note that ECC
   information is not stored in the QEMU RAW file for now.
 
+* `-F` may be used to rebuild the ECC values for all slots that have been modified using any
+  modification operation, and any detected error.
+
 * `-f` select which partition(s) and partition field(s) should be shown when option `-s` is used.
   When not specified, all partitions and fields are reported.
 
@@ -164,17 +169,29 @@ Fuse RAW images only use the v1 type.
   only intended to corrupt the OTP content so that HW & SW behavior may be exercised should such
   a condition exists. See [Bit position syntax](#bit-syntax) for how to specify a bit.
 
-* `--empty` reset a whole parition, including its digest if any and ECC bits. This option is only
+* `--change` updates the value of a field in the specified partition. The flag may be repeated.
+  Note that changing the partition content without specifying the `-F` flag may result into a
+  corrupted partition. `otptool.py` accepts the following value syntax:
+
+  - `"string"` or `'string'`: an arbitrary string, which is automatically encoded into UTF8 raw
+    bytes. Note that the shell may interpret the single or double quote characters, so it may be
+    required to use `'part:field="string"'` as the argument to bypass any shell replacement,
+  - `true`, `false`, `on`, `off`, `enabled`, `disabled` as a boolean specifier. The boolean value is
+    encoded according to the OTP map into an hardened boolean or a MUBI value,
+  - an hexadecimal value starting with with `0x`
+  - a string of hexadecimal nibbles, _e.g._ `01020304abcdef`, which is converted into raw bytes
+
+  If the partition field is larger than the specified value, the value is left-extended with NUL
+  bytes. Values larger than the partition field length are rejected.
+
+* `--empty` reset a whole partition, including its digest if any and ECC bits. This option is only
   intended for test purposes. This flag may be repeated. Partition(s) can be specified either by
   their index or their name.
 
 * `--erase` reset a specific field within a partition. The flag may be repeated.
 
 * `--no-version` disable OTP image version reporting when `-s` is used.
 
-* `--fix-ecc` may be used to rebuild the ECC values for all slots that have been modified using the
-  ECC modification operations, and any detected error.
-
 * `--set-bit` sets the specified bit in the OTP data. This flag may be repeated. This option is
    only intended to corrupt the OTP content so that HW & SW behavior may be exercised should such
    a condition exists. See [Bit position syntax](#bit-syntax) for how to specify a bit.
@@ -205,7 +222,7 @@ If the bit is larger than the data slot, it indicates the location with the ECC
 fuses are organized as 16-bit slots wtih 6-bit ECC, bit 0 to 15 indicates a bit into the data slot,
 while bit 16 to 21 indicates an ECC bit.
 
-It is possible to tell the script to rebuild the ECC value for the modified bits, using `--fix-ecc`.
+It is possible to tell the script to rebuild the ECC value for the modified bits, using `-F`.
 The default behavior is to not automatically update the ECC value, as the primary usage for these
 bit modification operations is to test the error detection and correction feature.
 

hw/opentitan/ot_ibex_wrapper_dj.c

@@ -921,8 +921,9 @@ static void ot_ibex_wrapper_dj_update_remap(OtIbexWrapperDjState *s, bool doi,
         }
         /* enable */
         uint32_t map_size = (-src_match_i & (src_match_i + 1u)) << 1u;
-        uint32_t src_base = src_match_i & ~(map_size - 1u);
-        uint32_t dst_base = remap_addr_i;
+        uint32_t map_mask = ~(map_size - 1u);
+        uint32_t src_base = src_match_i & map_mask;
+        uint32_t dst_base = remap_addr_i & map_mask;
 
         ot_ibex_wrapper_dj_remapper_destroy(s, slot);
         ot_ibex_wrapper_dj_remapper_create(s, slot, (hwaddr)dst_base,

hw/opentitan/ot_ibex_wrapper_eg.c

@@ -380,8 +380,9 @@ static void ot_ibex_wrapper_eg_update_remap(OtIbexWrapperEgState *s, bool doi,
         }
         /* enable */
         uint32_t map_size = (-src_match_i & (src_match_i + 1u)) << 1u;
-        uint32_t src_base = src_match_i & ~(map_size - 1u);
-        uint32_t dst_base = remap_addr_i;
+        uint32_t map_mask = ~(map_size - 1u);
+        uint32_t src_base = src_match_i & map_mask;
+        uint32_t dst_base = remap_addr_i & map_mask;
 
         ot_ibex_wrapper_eg_remapper_destroy(s, slot);
         ot_ibex_wrapper_eg_remapper_create(s, slot, (hwaddr)dst_base,

python/qemu/ot/.pylintrc

@@ -10,6 +10,7 @@ disable=
     too-many-lines,
     too-many-locals,
     too-many-nested-blocks,
+    too-many-positional-arguments,
     too-many-public-methods,
     too-many-statements,
     unspecified-encoding

python/qemu/ot/otp/image.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2023-2024 Rivos, Inc.
+# Copyright (c) 2023-2025 Rivos, Inc.
 # SPDX-License-Identifier: Apache2
 
 """OTP QEMU image.
@@ -60,6 +60,7 @@ def __init__(self, ecc_bits: Optional[int] = None):
         self._log = getLogger('otp.img')
         self._header: dict[str, Any] = {}
         self._magic = b''
+        self._changed = False
         self._data = bytearray()
         self._ecc = bytearray()
         if ecc_bits is None:
@@ -88,6 +89,14 @@ def is_opentitan(self) -> bool:
         """Report whether the current image contains OpenTitan OTP data."""
         return self._magic == b'vOTP'
 
+    @property
+    def changed(self) -> bool:
+        """Report whether the content of the partition (data or ecc) has been
+           modified since it has been loaded or since the last time it has been
+           saved.
+        """
+        return self._changed
+
     @classproperty
     def vmem_kinds(cls) -> list[str]:
         """Reports the supported content kinds of VMEM files."""
@@ -114,6 +123,7 @@ def load_raw(self, rfp: BinaryIO) -> None:
         if header['version'] > 1:
             self._digest_iv = header['digiv']
             self._digest_constant = header['digfc']
+        self._changed = False
 
     def save_raw(self, rfp: BinaryIO) -> None:
         """Save OTP image as a QEMU 'RAW' image stream."""
@@ -124,6 +134,7 @@ def save_raw(self, rfp: BinaryIO) -> None:
         self._pad(rfp)
         rfp.write(self._ecc)
         self._pad(rfp, 4096)
+        self._changed = False
 
     def load_vmem(self, vfp: TextIO, vmem_kind: Optional[str] = None,
                   swap: bool = True):
@@ -201,6 +212,7 @@ def load_vmem(self, vfp: TextIO, vmem_kind: Optional[str] = None,
         if not vkind:
             raise ValueError('Unable to detect VMEM find, please specify')
         self._magic = f'v{vkind[:3].upper()}'.encode()
+        self._changed = False
 
     def load_lifecycle(self, lcext: OtpLifecycleExtension) -> None:
         """Load lifecyle values."""
@@ -394,21 +406,36 @@ def empty_partition(self, partition: Union[int, str]) -> None:
         part.empty()
         if not part.is_empty:
             raise RuntimeError(f"Unable to empty partition '{partition}'")
-        content = BytesIO()
-        part.save(content)
-        data = content.getvalue()
-        length = len(data)
-        offset = self._part_offsets[partix]
-        self._data[offset:offset+length] = data
-        self._ecc[offset // 2:(offset+length)//2] = bytes(length//2)
+        start, end = self._reload(partix, False)
+        self._ecc[start // 2:end // 2] = bytes((end-start) // 2)
+
+    def change_field(self, partition: Union[int, str], field: str,
+                     value: Union[bytes, bytearray, bool, int, str]) -> None:
+        """Change the content of a field within a partition.
+
+           :param partition: the name or the index of the partition to erase
+           :param field: the name of the field to erase
+           :param value: the new value of the field
+
+           Valid value types depend on the actual partition's field.
+
+           ECC is not automatically updated
+        """
+        partix, part = self._retrieve_partition(partition)
+        part.change_field(field, value)
+        self._reload(partix, True)
 
     def erase_field(self, partition: Union[int, str], field: str) -> None:
         """Erase (reset) the content of a field within a partition.
 
+           :param partition: the name or the index of the partition to erase
            :param field: the name of the field to erase
+
+           ECC is not automatically updated
         """
-        part = self._retrieve_partition(partition)[1]
+        partix, part = self._retrieve_partition(partition)
         part.erase_field(field)
+        self._reload(partix, True)
 
     @staticmethod
     def bit_parity(data: int) -> int:
@@ -475,6 +502,7 @@ def verify_ecc(self, recover: bool) -> tuple[int, int]:
                 if recover:
                     self._data[off:off+granule] = fchunk.to_bytes(granule,
                                                                   'little')
+                    self._changed = True
                     updated_parts.add(partition)
                 err_cnt += 1
             elif err < 0:
@@ -524,6 +552,7 @@ def fix_ecc(self) -> int:
             self._log.info('New ECC @ 0x%04x: 0x%x -> 0x%x', off, old_ecc,
                            new_ecc)
             self._ecc[eccoff] = new_ecc
+            self._changed = True
         self._dirty_offsets.clear()
         return dirty_len
 
@@ -663,6 +692,7 @@ def _change_bits(self, bits: Sequence[tuple[int, int]],
                     self._ecc[eccoff] = ecc.to_bytes(ecclen, 'little')
                     ecc = int.from_bytes(self._ecc[eccoff:eccoff+ecclen],
                                          'little')
+                self._changed = True
             else:  # Data bit
                 chunk = int.from_bytes(self._data[off:off+granule], 'little')
                 bitval = 1 << bit
@@ -677,6 +707,7 @@ def _change_bits(self, bits: Sequence[tuple[int, int]],
                                bit, off, old, chunk)
                 self._data[off:off+granule] = chunk.to_bytes(granule, 'little')
                 self._dirty_offsets.append(off)
+                self._changed = True
 
     def _get_partition_bounds(self, partref: Union[str, OtpPartition]) \
             -> Optional[tuple[int, int]]:
@@ -712,7 +743,7 @@ def _retrieve_partition(self, partition: Union[int, str]) \
             except IndexError:
                 pass
         elif isinstance(partition, str):
-            partname = partition.lower()
+            partname = partition.lower().replace('_', '')
             try:
                 partix, part = {(i, p) for i, p in enumerate(self._partitions)
                                 if p.__class__.__name__[:-4].lower() ==
@@ -722,3 +753,20 @@ def _retrieve_partition(self, partition: Union[int, str]) \
         if not part or partix is None:
             raise ValueError(f"Unknown partition '{partition}'")
         return partix, part
+
+    def _reload(self, partix: int, flag_dirty: bool = True) -> tuple[int, int]:
+        partition = self._partitions[partix]
+        content = BytesIO()
+        partition.save(content)
+        data = content.getvalue()
+        length = len(data)
+        start = self._part_offsets[partix]
+        granule = self._ecc_granule
+        if flag_dirty:
+            for offset in range(0, length, granule):
+                if self._data[start+offset:start+offset+granule] != \
+                        data[offset:offset+2]:
+                    self._dirty_offsets.append(start+offset)
+        self._data[start:start+length] = data
+        self._changed = True
+        return (start, start+length)