Hello Kane,
+ Markus (for ebc29e1beab0 implementation)
On 4/7/25 09:33, Kane Chen wrote:
Hi Cédric/Philippe,
OTP (One-Time Programmable) memory is a type of non-volatile memory
in which each bit can be programmed only once. It is typically used
to store critical and permanent information, such as the chip ID and
secure boot keys. The structure and behavior of OTP memory are
consistent across both the AST1030 and AST2600 platforms.
As Philippe pointed out, this proposal models the OTP memory as a
flash device and utilizes a block backend for persistent storage. In
contrast, existing implementations such as NPCM7xxOTPState,
BCM2835OTPState, and SiFiveUOTPState expose OTP memory via MMIO and
always initialize it in a blank state.
AFAIU, Aspeed SBC is also MMIO based or is there another device,
an eeprom, accessible through an external bus ? How is it
implemented in HW ?
The goal of this design is to
allow the guest system to boot with a pre-configured OTP memory
state.
Yes. This is a valid request. It's not the first time we've had
this kind of requests. The initial content of EEPROM devices are
an example and some machines, like the rainier, have a lot.
If the device can be defined on the command line, like would be
an EEPROM device attached to an I2C bus or a flash device attached
to a SPI bus, we can use a 'drive' property. Something like :
qemu-system-arm -M ast2600-evb \
-blockdev node-name=fmc0,driver=file,filename=/path/to/fmc0.img \
-device mx66u51235f,bus=ssi.0,cs=0x0,drive=fmc0 \
-blockdev node-name=fmc1,driver=file,filename=/path/to/fmc1.img \
-device mx66u51235f,bus=ssi.0,cs=0x1,drive=fmc1 \
-blockdev node-name=spi1,driver=file,filename=/path/to/spi1.img \
-device mx66u51235f,cs=0x0,bus=ssi.1,drive=spi1 \
...
However, the Aspeed SBC device is a platform device and it makes
things more complex : it can not be created on the command line,
it is directly created by the machine and the soc and passing
device properties to specify a blockdev it is not possible :
qemu-system-arm -M ast2600-evb \
-blockdev node-name=otpmem,driver=file,filename=/path/to/otpmem.img \
-device aspeed-sbc,drive=otpmem \
...
To support this, the OTP memory is backed by a file,
simulating persistent flash behavior.
The idea is good but the implementation is problematic.
+static BlockBackend *init_otpmem(int64_t size_bytes)
+{
+ Error *local_err = NULL;
+ BlockDriverState *bs = NULL;
+ BlockBackend *blk = NULL;
+ bool image_created = false;
+ QDict *options;
+ uint32_t i, odd_def = 0xffffffff, even_def = 0, *def;
+
+ if (!g_file_test(OTP_FILE_PATH, G_FILE_TEST_EXISTS)) {
+ bdrv_img_create(OTP_FILE_PATH, "raw", NULL, NULL,
+ NULL, size_bytes, 0, true, &local_err);
+ if (local_err) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to create image %s: %s\n",
+ __func__, OTP_FILE_PATH,
+ error_get_pretty(local_err));
+ error_free(local_err);
+ return NULL;
+ }
+ image_created = true;
+ }
+
+ blk = blk_new(qemu_get_aio_context(),
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
+ 0);
+ if (!blk) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to create BlockBackend\n",
+ __func__);
+ return NULL;
+ }
+
+ options = qdict_new();
+ qdict_put_str(options, "driver", "raw");
+ bs = bdrv_open(OTP_FILE_PATH, NULL, options, BDRV_O_RDWR, &local_err);
+ if (local_err) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to create OTP memory, err = %s\n",
+ __func__, error_get_pretty(local_err));
+ blk_unref(blk);
+ error_free(local_err);
+ return NULL;
+ }
+
+ blk_insert_bs(blk, bs, &local_err);
+ if (local_err) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Failed to insert OTP memory to SBC, err = %s\n",
+ __func__, error_get_pretty(local_err));
+ bdrv_unref(bs);
+ blk_unref(blk);
+ error_free(local_err);
+ return NULL;
+ }
+ bdrv_unref(bs);
...
IMO, this is low level block code that a device model shouldn't have
to deal with. A 'drive' should be used instead. Now, if the qemu-block
maintainers are OK with it, we need their approval.
The OTP memory access flow is as follows:
1. The guest issues a read or write OTP command to the Secure Boot
Controller (SBC)
2. The SBC triggers the corresponding operation in the OTP controller
3. The SBC returns the result to the guest
Since the guest interacts with OTP memory exclusively through the
SBC, the OTP logic is implemented within aspeed_sbc.c.
If there are existing architectural guidelines or design patterns
that should be followed for modeling OTP devices, I would greatly
appreciate your feedback. I am happy to revise the implementation
accordingly and submit updated patches for further review.
Adding a 'drive' property to the aspeed-sbc model shouldn't change
too much the proposal and it will remove the init_otpmem() routine,
which is problematic.
Then, we need to find a way to set the 'drive' property of the
aspeed-sbc model. I suggest using the same method of the edk2 flash
devices of the q35 machine. See ebc29e1beab0. Setting a machine
option would set the drive. Something like :
qemu-system-arm -M ast2600-evb,otpmem=otpmem-drive \
-blockdev node-name=otpmem,driver=file,filename=/path/to/otpmem.img \
...
This machine option would only be defined for machine types needing
it.
Thanks,
C.
Thanks for your comments and review.
Best Regards,
Kane
-----Original Message-----
From: Cédric Le Goater <c...@kaod.org>
Sent: Friday, April 4, 2025 9:54 PM
To: Philippe Mathieu-Daudé <phi...@linaro.org>; Kane Chen
<kane_c...@aspeedtech.com>; Peter Maydell <peter.mayd...@linaro.org>;
Steven Lee <steven_...@aspeedtech.com>; Troy Lee <leet...@gmail.com>;
Jamin Lin <jamin_...@aspeedtech.com>; Andrew Jeffery
<and...@codeconstruct.com.au>; Joel Stanley <j...@jms.id.au>; open
list:ASPEED BMCs <qemu-...@nongnu.org>; open list:All patches CC here
<qemu-devel@nongnu.org>; qemu-block <qemu-bl...@nongnu.org>
Cc: Troy Lee <troy_...@aspeedtech.com>
Subject: Re: [PATCH v1 0/1] hw/misc/aspeed_sbc: Implement OTP memory and
controller
On 4/4/25 15:00, Philippe Mathieu-Daudé wrote:
+qemu-block@
On 4/4/25 14:06, Cédric Le Goater wrote:
Hello,
On 4/2/25 11:14, Kane-Chen-AS wrote:
This patch introduces part of the Secure Boot Controller device,
which consists of several sub-components, including an OTP memory,
OTP controller, cryptographic engine, and boot controller.
In this version, the implementation includes the OTP memory and its
controller. The OTP memory can be programmed from within the guest
OS via a software utility.
What is the OTP memory ? An external flash device or built-in SRAM ?
If the latter, I suggest using an allocated buffer under the SBC
model and avoid the complexity of the BlockBackend implementation and
the definition of a drive on the command line for it. The proposal is
bypassing a lot of QEMU layers for this purpose.
More of the former, a built-in eFuse behaving more like flash. So
using block backend for the storage seems correct to me.
How would you define the drive backend on the command line ?
However I don't think
the implementation belongs to hw/misc/aspeed_sbc; ideally we'd have
some abstract (or interface) implementation in hw/block/otp.c -- with
methods such program_otp_data() --, completed by hw/block/aspeed_otc.c.
I was imagining more something like NPCM7xxOTPState or BCM2835OTPState
and not SiFiveUOTPState.
Current patch might be good enough to start with IMHO.
Have you looked at the next patch and how the backend is handled ?
I will let the block people Ack this patch in that case. It's beyond my skills.
Thanks,
C.
