From: Kane-Chen-AS <kane_chen@aspeedtech.com>
Implement correct OTP programming behavior for Aspeed OTP:
- Support read-modify-write flow with one-way bit programming:
* prog_bit uses 0s as the "to-be-programmed" mask.
* Even-indexed words: 0->1, odd-indexed words: 1->0.
* Reject non-programmable requests and log conflicts.
- Enable unaligned accesses in MemoryRegionOps.
Since each OTP address maps to a 1DW (4B) or 2DW (8B) block in the
backing store, upper-layer accesses may be unaligned to block
boundaries.
This matches the irreversible, word-parity-dependent programming rules
of Aspeed SoCs and exposes changes via QEMU trace events.
Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
---
hw/nvram/aspeed_otp.c | 80 ++++++++++++++++++++++++++++++++++++++++++-
hw/nvram/trace-events | 5 +++
2 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/hw/nvram/aspeed_otp.c b/hw/nvram/aspeed_otp.c
index abb3731823..66b2bfea28 100644
--- a/hw/nvram/aspeed_otp.c
+++ b/hw/nvram/aspeed_otp.c
@@ -12,6 +12,7 @@
#include "system/block-backend.h"
#include "hw/qdev-properties.h"
#include "hw/nvram/aspeed_otp.h"
+#include "hw/nvram/trace.h"
static uint64_t aspeed_otp_read(void *opaque, hwaddr offset, unsigned size)
{
@@ -23,12 +24,87 @@ static uint64_t aspeed_otp_read(void *opaque, hwaddr offset, unsigned size)
return val;
}
+static bool valid_program_data(uint32_t otp_addr,
+ uint32_t value, uint32_t prog_bit)
+{
+ uint32_t programmed_bits, has_programmable_bits;
+ bool is_odd = otp_addr & 1;
+
+ /*
+ * prog_bit uses 0s to indicate target bits to program:
+ * - if OTP word is even-indexed, programmed bits flip 0->1
+ * - if odd, bits flip 1->0
+ * Bit programming is one-way only and irreversible.
+ */
+ if (is_odd) {
+ programmed_bits = ~value & prog_bit;
+ } else {
+ programmed_bits = value & (~prog_bit);
+ }
+
+ /* If any bit can be programmed, accept the request */
+ has_programmable_bits = value ^ (~prog_bit);
+
+ if (programmed_bits) {
+ trace_aspeed_otp_prog_conflict(otp_addr, programmed_bits);
+ for (int i = 0; i < 32; ++i) {
+ if (programmed_bits & (1U << i)) {
+ trace_aspeed_otp_prog_bit(i);
+ }
+ }
+ }
+
+ return has_programmable_bits != 0;
+}
+
+static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
+ uint32_t prog_bit, uint32_t *value)
+{
+ AspeedOTPState *s = opaque;
+ bool is_odd = otp_addr & 1;
+ uint32_t otp_offset = otp_addr << 2;
+
+ memcpy(value, s->storage + otp_offset, sizeof(uint32_t));
+
+ if (!valid_program_data(otp_addr, *value, prog_bit)) {
+ return false;
+ }
+
+ if (is_odd) {
+ *value &= ~prog_bit;
+ } else {
+ *value |= ~prog_bit;
+ }
+
+ return true;
+}
+
static void aspeed_otp_write(void *opaque, hwaddr otp_addr,
uint64_t val, unsigned size)
{
AspeedOTPState *s = opaque;
+ uint32_t otp_offset, value;
- memcpy(s->storage + otp_addr, &val, size);
+ if (!program_otpmem_data(s, otp_addr, val, &value)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to program data, value = %x, bit = %lx\n",
+ __func__, value, val);
+ return;
+ }
+
+ otp_offset = otp_addr << 2;
+ memcpy(s->storage + otp_offset, &value, size);
+
+ if (s->blk) {
+ if (blk_pwrite(s->blk, otp_offset, size, &value, 0) < 0) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to write %x to %x\n",
+ __func__, value, otp_offset);
+
+ return;
+ }
+ }
+ trace_aspeed_otp_prog(otp_offset, val, value);
}
static bool aspeed_otp_init_storage(AspeedOTPState *s, Error **errp)
@@ -63,6 +139,8 @@ static const MemoryRegionOps aspeed_otp_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.valid.min_access_size = 1,
.valid.max_access_size = 4,
+ .valid.unaligned = true,
+ .impl.unaligned = true
};
static void aspeed_otp_realize(DeviceState *dev, Error **errp)
diff --git a/hw/nvram/trace-events b/hw/nvram/trace-events
index 5e33b24d47..7084bf70d3 100644
--- a/hw/nvram/trace-events
+++ b/hw/nvram/trace-events
@@ -1,5 +1,10 @@
# See docs/devel/tracing.rst for syntax documentation.
+# aspeed_otp.c
+aspeed_otp_prog(uint32_t addr, uint32_t prog_value, uint32_t value) "OTP Memory program: addr 0x%" PRIx32 " prog_value 0x%" PRIx32 " value 0x%" PRIx32
+aspeed_otp_prog_conflict(uint32_t addr, uint32_t bits) "Conflict at addr=0x%x, bits=0x%08x"
+aspeed_otp_prog_bit(int bit) "Programmed bit %d"
+
# ds1225y.c
nvram_read(uint32_t addr, uint32_t ret) "read addr %d: 0x%02x"
nvram_write(uint32_t addr, uint32_t old, uint32_t val) "write addr %d: 0x%02x -> 0x%02x"
--
2.43.0On 8/12/25 11:40, Kane Chen wrote:
> From: Kane-Chen-AS <kane_chen@aspeedtech.com>
>
> Implement correct OTP programming behavior for Aspeed OTP:
> - Support read-modify-write flow with one-way bit programming:
> * prog_bit uses 0s as the "to-be-programmed" mask.
> * Even-indexed words: 0->1, odd-indexed words: 1->0.
> * Reject non-programmable requests and log conflicts.
> - Enable unaligned accesses in MemoryRegionOps.
> Since each OTP address maps to a 1DW (4B) or 2DW (8B) block in the
> backing store, upper-layer accesses may be unaligned to block
> boundaries.
>
> This matches the irreversible, word-parity-dependent programming rules
> of Aspeed SoCs and exposes changes via QEMU trace events.
>
> Signed-off-by: Kane-Chen-AS <kane_chen@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> hw/nvram/aspeed_otp.c | 80 ++++++++++++++++++++++++++++++++++++++++++-
> hw/nvram/trace-events | 5 +++
> 2 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/hw/nvram/aspeed_otp.c b/hw/nvram/aspeed_otp.c
> index abb3731823..66b2bfea28 100644
> --- a/hw/nvram/aspeed_otp.c
> +++ b/hw/nvram/aspeed_otp.c
> @@ -12,6 +12,7 @@
> #include "system/block-backend.h"
> #include "hw/qdev-properties.h"
> #include "hw/nvram/aspeed_otp.h"
> +#include "hw/nvram/trace.h"
>
> static uint64_t aspeed_otp_read(void *opaque, hwaddr offset, unsigned size)
> {
> @@ -23,12 +24,87 @@ static uint64_t aspeed_otp_read(void *opaque, hwaddr offset, unsigned size)
> return val;
> }
>
> +static bool valid_program_data(uint32_t otp_addr,
> + uint32_t value, uint32_t prog_bit)
> +{
> + uint32_t programmed_bits, has_programmable_bits;
> + bool is_odd = otp_addr & 1;
> +
> + /*
> + * prog_bit uses 0s to indicate target bits to program:
> + * - if OTP word is even-indexed, programmed bits flip 0->1
> + * - if odd, bits flip 1->0
> + * Bit programming is one-way only and irreversible.
> + */
> + if (is_odd) {
> + programmed_bits = ~value & prog_bit;
> + } else {
> + programmed_bits = value & (~prog_bit);
> + }
> +
> + /* If any bit can be programmed, accept the request */
> + has_programmable_bits = value ^ (~prog_bit);
> +
> + if (programmed_bits) {
> + trace_aspeed_otp_prog_conflict(otp_addr, programmed_bits);
> + for (int i = 0; i < 32; ++i) {
> + if (programmed_bits & (1U << i)) {
> + trace_aspeed_otp_prog_bit(i);
> + }
> + }
> + }
> +
> + return has_programmable_bits != 0;
> +}
> +
> +static bool program_otpmem_data(void *opaque, uint32_t otp_addr,
> + uint32_t prog_bit, uint32_t *value)
> +{
> + AspeedOTPState *s = opaque;
> + bool is_odd = otp_addr & 1;
> + uint32_t otp_offset = otp_addr << 2;
> +
> + memcpy(value, s->storage + otp_offset, sizeof(uint32_t));
> +
> + if (!valid_program_data(otp_addr, *value, prog_bit)) {
> + return false;
> + }
> +
> + if (is_odd) {
> + *value &= ~prog_bit;
> + } else {
> + *value |= ~prog_bit;
> + }
> +
> + return true;
> +}
> +
> static void aspeed_otp_write(void *opaque, hwaddr otp_addr,
> uint64_t val, unsigned size)
> {
> AspeedOTPState *s = opaque;
> + uint32_t otp_offset, value;
>
> - memcpy(s->storage + otp_addr, &val, size);
> + if (!program_otpmem_data(s, otp_addr, val, &value)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Failed to program data, value = %x, bit = %lx\n",
> + __func__, value, val);
> + return;
> + }
> +
> + otp_offset = otp_addr << 2;
> + memcpy(s->storage + otp_offset, &value, size);
> +
> + if (s->blk) {
> + if (blk_pwrite(s->blk, otp_offset, size, &value, 0) < 0) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Failed to write %x to %x\n",
> + __func__, value, otp_offset);
> +
> + return;
> + }
> + }
> + trace_aspeed_otp_prog(otp_offset, val, value);
> }
>
> static bool aspeed_otp_init_storage(AspeedOTPState *s, Error **errp)
> @@ -63,6 +139,8 @@ static const MemoryRegionOps aspeed_otp_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> .valid.min_access_size = 1,
> .valid.max_access_size = 4,
> + .valid.unaligned = true,
> + .impl.unaligned = true
> };
>
> static void aspeed_otp_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/nvram/trace-events b/hw/nvram/trace-events
> index 5e33b24d47..7084bf70d3 100644
> --- a/hw/nvram/trace-events
> +++ b/hw/nvram/trace-events
> @@ -1,5 +1,10 @@
> # See docs/devel/tracing.rst for syntax documentation.
>
> +# aspeed_otp.c
> +aspeed_otp_prog(uint32_t addr, uint32_t prog_value, uint32_t value) "OTP Memory program: addr 0x%" PRIx32 " prog_value 0x%" PRIx32 " value 0x%" PRIx32
> +aspeed_otp_prog_conflict(uint32_t addr, uint32_t bits) "Conflict at addr=0x%x, bits=0x%08x"
> +aspeed_otp_prog_bit(int bit) "Programmed bit %d"
> +
> # ds1225y.c
> nvram_read(uint32_t addr, uint32_t ret) "read addr %d: 0x%02x"
> nvram_write(uint32_t addr, uint32_t old, uint32_t val) "write addr %d: 0x%02x -> 0x%02x"
© 2016 - 2025 Red Hat, Inc.