On 03.11.25 16:08, Philippe Mathieu-Daudé wrote:
> Hi Jan,
>
> On 17/10/25 14:03, Jan Kiszka wrote:
>> From: Jan Kiszka <[email protected]>
>>
>> The Replay Protected Memory Block (RPMB) is available since eMMC 4.4
>> which has been obsoleted by 4.41. Therefore lift the provided
>> EXT_CSD_REV to 5 (4.41) and provide the basic logic to implement basic
>> support for it. This allows to set the authentication key, read the
>> write counter and authenticated perform data read and write requests.
>> Those aren't actually authenticated yet, support for that will be added
>> later.
>>
>> The RPMB image needs to be added to backing block images after potential
>> boot partitions and before the user data. It's size is controlled by
>> the rpmb-partition-size property.
>>
>> Also missing in this version (and actually not only for RPMB bits) is
>> persistence of registers that are supposed to survive power cycles. Most
>> prominent are the write counters or the authentication key. This feature
>> can be added later, e.g. by append a state structure to the backing
>> block image.
>>
>> Signed-off-by: Jan Kiszka <[email protected]>
>> ---
>> hw/sd/sd.c | 206 +++++++++++++++++++++++++++++++++++++++--
>> hw/sd/sdmmc-internal.h | 21 +++++
>> hw/sd/trace-events | 2 +
>> 3 files changed, 221 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 305ea251cb..918fe9f79f 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -117,6 +117,20 @@ typedef struct SDProto {
>> } cmd[SDMMC_CMD_MAX], acmd[SDMMC_CMD_MAX];
>> } SDProto;
>>
>
> #define RPMB_STUFF_LEN 196
>
>> +#define RPMB_KEY_MAC_LEN 32
>> +
>> +typedef
>
> QEMU_PACKED (better safe than sorry).
>
>> struct {
>> + uint8_t stuff_bytes[196];
>> + uint8_t key_mac[RPMB_KEY_MAC_LEN];
>> + uint8_t data[256];
>> + uint8_t nonce[16];
>> + uint32_t write_counter;
>> + uint16_t address;
>> + uint16_t block_count;
>> + uint16_t result;
>> + uint16_t req_resp;
>> +} RPMBDataFrame;
>> +
>> struct SDState {
>> DeviceState parent_obj;
>> @@ -140,6 +154,7 @@ struct SDState {
>> uint8_t spec_version;
>> uint64_t boot_part_size;
>> + uint64_t rpmb_part_size;
>> BlockBackend *blk;
>> uint8_t boot_config;
>> @@ -172,6 +187,10 @@ struct SDState {
>> uint32_t data_offset;
>> size_t data_size;
>> uint8_t data[512];
>> + RPMBDataFrame rpmb_result;
>> + uint32_t rpmb_write_counter;
>> + uint8_t rpmb_key[32];
>> + uint8_t rpmb_key_set;
>
> Matter of style:
>
> struct {
> uint32_t write_counter;
> uint8_t key[RPMB_KEY_MAC_LEN];
> uint8_t key_set;
> RPMBDataFrame result;
> } rpmb;
>
>> QEMUTimer *ocr_power_timer;
>> uint8_t dat_lines;
>> bool cmd_line;
>> @@ -506,7 +525,9 @@ static void emmc_set_ext_csd(SDState *sd, uint64_t
>> size)
>> sd->ext_csd[205] = 0x46; /* Min read perf for 4bit@26Mhz */
>> sd->ext_csd[EXT_CSD_CARD_TYPE] = 0b11;
>> sd->ext_csd[EXT_CSD_STRUCTURE] = 2;
>> - sd->ext_csd[EXT_CSD_REV] = 3;
>> + sd->ext_csd[EXT_CSD_REV] = 5;
>> + sd->ext_csd[EXT_CSD_RPMB_MULT] = sd->rpmb_part_size / (128 * KiB);
>> + sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0b111;
>> /* Mode segment (RW) */
>> sd->ext_csd[EXT_CSD_PART_CONFIG] = sd->boot_config;
>> @@ -834,7 +855,8 @@ static uint32_t sd_blk_len(SDState *sd)
>> /*
>> * This requires a disk image that has two boot partitions inserted
>> at the
>> * beginning of it, followed by an RPMB partition. The size of the boot
>> - * partitions is the "boot-partition-size" property.
>> + * partitions is the "boot-partition-size" property, the one of the RPMB
>> + * partition is 'rpmb-partition-size'.
>> */
>> static uint32_t sd_part_offset(SDState *sd)
>> {
>> @@ -848,11 +870,13 @@ static uint32_t sd_part_offset(SDState *sd)
>> & EXT_CSD_PART_CONFIG_ACC_MASK;
>> switch (partition_access) {
>> case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
>> - return sd->boot_part_size * 2;
>> + return sd->boot_part_size * 2 + sd->rpmb_part_size;
>> case EXT_CSD_PART_CONFIG_ACC_BOOT1:
>> return 0;
>> case EXT_CSD_PART_CONFIG_ACC_BOOT2:
>> return sd->boot_part_size * 1;
>> + case EXT_CSD_PART_CONFIG_ACC_RPMB:
>> + return sd->boot_part_size * 2;
>> default:
>> g_assert_not_reached();
>> }
>> @@ -891,7 +915,7 @@ static void sd_reset(DeviceState *dev)
>> }
>> size = sect << HWBLOCK_SHIFT;
>> if (sd_is_emmc(sd)) {
>> - size -= sd->boot_part_size * 2;
>> + size -= sd->boot_part_size * 2 + sd->rpmb_part_size;
>> }
>> sect = sd_addr_to_wpnum(size) + 1;
>> @@ -979,6 +1003,34 @@ static const VMStateDescription sd_ocr_vmstate = {
>> },
>> };
>> +static bool vmstate_needed_for_rpmb(void *opaque)
>> +{
>> + SDState *sd = opaque;
>> +
>> + return sd->rpmb_part_size > 0;
>> +}
>> +
>> +static const VMStateDescription emmc_rpmb_vmstate = {
>> + .name = "sd-card/ext_csd_modes-state",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .needed = vmstate_needed_for_rpmb,
>> + .fields = (const VMStateField[]) {
>> + VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState,
>> RPMB_KEY_MAC_LEN),
>> + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256),
>> + VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16),
>> + VMSTATE_UINT32(rpmb_result.write_counter, SDState),
>> + VMSTATE_UINT16(rpmb_result.address, SDState),
>> + VMSTATE_UINT16(rpmb_result.block_count, SDState),
>> + VMSTATE_UINT16(rpmb_result.result, SDState),
>> + VMSTATE_UINT16(rpmb_result.req_resp, SDState),
>> + VMSTATE_UINT32(rpmb_write_counter, SDState),
>> + VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32),
>> + VMSTATE_UINT8(rpmb_key_set, SDState),
>> + VMSTATE_END_OF_LIST()
>> + },
>> +};
>> +
>> static bool vmstate_needed_for_emmc(void *opaque)
>> {
>> SDState *sd = opaque;
>> @@ -1045,6 +1097,7 @@ static const VMStateDescription sd_vmstate = {
>> .subsections = (const VMStateDescription * const []) {
>> &sd_ocr_vmstate,
>> &emmc_extcsd_vmstate,
>> + &emmc_rpmb_vmstate,
>> NULL
>> },
>> };
>> @@ -1067,6 +1120,105 @@ static void sd_blk_write(SDState *sd, uint64_t
>> addr, uint32_t len)
>> }
>> }
>> +static void emmc_rpmb_blk_read(SDState *sd, uint64_t addr, uint32_t
>> len)
>> +{
>> + uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp);
>> + uint16_t result = be16_to_cpu(sd->rpmb_result.result);
>> + unsigned int curr_block = 0;
>> +
>> + if ((result & ~RPMB_RESULT_COUTER_EXPIRED) == RPMB_RESULT_OK &&
>> + resp == RPMB_RESP(RPMB_REQ_AUTH_DATA_READ)) {
>> + curr_block = be16_to_cpu(sd->rpmb_result.address);
>> + if (sd->rpmb_result.block_count == 0) {
>> + sd->rpmb_result.block_count = cpu_to_be16(sd-
>> >multi_blk_cnt);
>> + } else {
>> + curr_block += be16_to_cpu(sd->rpmb_result.block_count) -
>> + sd->multi_blk_cnt;
>> + }
>> + addr = curr_block * 256 + sd_part_offset(sd);
>> + if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) <
>> 0) {
>
> Would be nice to re-use sd_blk_read(), but I notice we want to read the
> frame data to then copy the whole message into sd->data.
>
>> + fprintf(stderr, "sd_blk_read: read error on host side\n");
>
> Although a pre-existing pattern in this file, no new fprintf(stderr)
> please. Better use the Error* API, otherwise error_report().
>
> error_report("sd_blk_read: read error on host side");
>
>> + memset(sd->rpmb_result.data, 0, sizeof(sd-
>> >rpmb_result.data));
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_READ_FAILURE |
>> + (result & RPMB_RESULT_COUTER_EXPIRED));
>> + }
>> + }
>> + memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result));
>> +
>> + trace_sdcard_rpmb_read_block(resp, curr_block,
>> + be16_to_cpu(sd->rpmb_result.result));
>> +}
>> +
>> +static void emmc_rpmb_blk_write(SDState *sd, uint64_t addr, uint32_t
>> len)
>> +{
>> + RPMBDataFrame *frame = (RPMBDataFrame *)sd->data;
>> + uint16_t req = be16_to_cpu(frame->req_resp);
>> +
>> + if (req == RPMB_REQ_READ_RESULT) {
>> + /* just return the current result register */
>> + goto exit;
>> + }
>> + memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result));
>> + memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd-
>> >rpmb_result.nonce));
>> + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK);
>> + sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req));
>> +
>> + if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) {
>> + sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY);
>> + goto exit;
>> + }
>> +
>> + switch (req) {
>> + case RPMB_REQ_PROGRAM_AUTH_KEY:
>> + if (sd->rpmb_key_set) {
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
>> + break;
>> + }
>> + memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key));
>> + sd->rpmb_key_set = 1;
>> + break;
>> + case RPMB_REQ_READ_WRITE_COUNTER:
>> + sd->rpmb_result.write_counter = cpu_to_be32(sd-
>> >rpmb_write_counter);
>> + break;
>> + case RPMB_REQ_AUTH_DATA_WRITE:
>> + /* We only support single-block writes so far */
>> + if (sd->multi_blk_cnt != 1) {
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
>> + break;
>> + }
>> + if (sd->rpmb_write_counter == 0xffffffff) {
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
>> + break;
>> + }
>> + if (be32_to_cpu(frame->write_counter) != sd-
>> >rpmb_write_counter) {
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
>> + break;
>> + }
>> + sd->rpmb_result.address = frame->address;
>> + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
>> + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) {
>> + fprintf(stderr, "sd_blk_write: write error on host side\n");
>
> error_report("sd_blk_write: write error on host side");
>
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
>> + } else {
>> + sd->rpmb_write_counter++;
>> + }
>> + sd->rpmb_result.write_counter = cpu_to_be32(sd-
>> >rpmb_write_counter);
>> + break;
>> + case RPMB_REQ_AUTH_DATA_READ:
>> + sd->rpmb_result.address = frame->address;
>> + break;
>> + default:
>> + qemu_log_mask(LOG_UNIMP, "RPMB request %d not implemented\n",
>> req);
>> + sd->rpmb_result.result =
>> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
>> + break;
>> + }
>> +exit:
>> + if (sd->rpmb_write_counter == 0xffffffff) {
>> + sd->rpmb_result.result |=
>> cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED);
>> + }
>> + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd-
>> >rpmb_result.result));
>> +}
>> +
>> static void sd_erase(SDState *sd)
>> {
>> uint64_t erase_start = sd->erase_start;
>> @@ -1180,6 +1332,19 @@ static void emmc_function_switch(SDState *sd,
>> uint32_t arg)
>> break;
>> }
>> + if (index == EXT_CSD_PART_CONFIG) {
>> + uint8_t part = b & EXT_CSD_PART_CONFIG_ACC_MASK;
>> +
>> + if (((part == EXT_CSD_PART_CONFIG_ACC_BOOT1 ||
>> + part == EXT_CSD_PART_CONFIG_ACC_BOOT2) && !sd-
>> >boot_part_size) ||
>> + (part == EXT_CSD_PART_CONFIG_ACC_RPMB && !sd-
>> >rpmb_part_size)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "MMC switching to illegal partition\n");
>> + sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
>> + return;
>> + }
>> + }
>> +
>> trace_sdcard_ext_csd_update(index, sd->ext_csd[index], b);
>> sd->ext_csd[index] = b;
>> }
>> @@ -2378,6 +2543,7 @@ static bool sd_generic_read_byte(SDState *sd,
>> uint8_t *value)
>> static void sd_write_byte(SDState *sd, uint8_t value)
>> {
>> + unsigned int partition_access;
>> int i;
>> if (!sd->blk || !blk_is_inserted(sd->blk)) {
>> @@ -2427,7 +2593,13 @@ static void sd_write_byte(SDState *sd, uint8_t
>> value)
>> if (sd->data_offset >= sd->blk_len) {
>> /* TODO: Check CRC before committing */
>> sd->state = sd_programming_state;
>> - sd_blk_write(sd, sd->data_start, sd->data_offset);
>> + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG]
>> + & EXT_CSD_PART_CONFIG_ACC_MASK;
>> + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) {
>> + emmc_rpmb_blk_write(sd, sd->data_start, sd-
>> >data_offset);
>> + } else {
>> + sd_blk_write(sd, sd->data_start, sd->data_offset);
>> + }
>> sd->blk_written++;
>> sd->data_start += sd->blk_len;
>> sd->data_offset = 0;
>> @@ -2510,6 +2682,7 @@ static uint8_t sd_read_byte(SDState *sd)
>> {
>> /* TODO: Append CRCs */
>> const uint8_t dummy_byte = 0x00;
>> + unsigned int partition_access;
>> uint8_t ret;
>> uint32_t io_len;
>> @@ -2553,7 +2726,13 @@ static uint8_t sd_read_byte(SDState *sd)
>> sd->data_start, io_len)) {
>> return dummy_byte;
>> }
>> - sd_blk_read(sd, sd->data_start, io_len);
>> + partition_access = sd->ext_csd[EXT_CSD_PART_CONFIG]
>> + & EXT_CSD_PART_CONFIG_ACC_MASK;
>> + if (partition_access == EXT_CSD_PART_CONFIG_ACC_RPMB) {
>> + emmc_rpmb_blk_read(sd, sd->data_start, io_len);
>> + } else {
>> + sd_blk_read(sd, sd->data_start, io_len);
>> + }
>> }
>> ret = sd->data[sd->data_offset ++];
>> @@ -2805,7 +2984,7 @@ static void sd_realize(DeviceState *dev, Error
>> **errp)
>> blk_size = blk_getlength(sd->blk);
>> }
>> if (blk_size >= 0) {
>> - blk_size -= sd->boot_part_size * 2;
>> + blk_size -= sd->boot_part_size * 2 + sd->rpmb_part_size;
>> if (blk_size > SDSC_MAX_CAPACITY) {
>> if (sd_is_emmc(sd) && blk_size % (1 << HWBLOCK_SHIFT) !=
>> 0) {
>> int64_t blk_size_aligned =
>> @@ -2844,13 +3023,23 @@ static void sd_realize(DeviceState *dev, Error
>> **errp)
>> "The boot partition size must be multiples
>> of 128K"
>> "and not larger than 32640K.\n");
>> }
>> + if (sd->rpmb_part_size % (128 * KiB) ||
>> + sd->rpmb_part_size > 128 * 128 * KiB) {
>> + char *size_str = size_to_str(sd->boot_part_size);
>> +
>> + error_setg(errp, "Invalid RPMB partition size: %s", size_str);
>> + g_free(size_str);
>> + error_append_hint(errp,
>> + "The RPMB partition size must be multiples
>> of 128K"
>> + "and not larger than 16384K.\n");
>> + }
>> }
>> static void emmc_realize(DeviceState *dev, Error **errp)
>> {
>> SDState *sd = SDMMC_COMMON(dev);
>> - sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.3 */
>> + sd->spec_version = SD_PHY_SPECv3_01_VERS; /* Actually v4.5 */
>> sd_realize(dev, errp);
>> }
>> @@ -2867,6 +3056,7 @@ static const Property sd_properties[] = {
>> static const Property emmc_properties[] = {
>> DEFINE_PROP_UINT64("boot-partition-size", SDState,
>> boot_part_size, 0),
>> DEFINE_PROP_UINT8("boot-config", SDState, boot_config, 0x0),
>> + DEFINE_PROP_UINT64("rpmb-partition-size", SDState,
>> rpmb_part_size, 0),
>> };
>> static void sdmmc_common_class_init(ObjectClass *klass, const void
>> *data)
>> diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
>> index ce6bc4e6ec..c4a9aa8edf 100644
>> --- a/hw/sd/sdmmc-internal.h
>> +++ b/hw/sd/sdmmc-internal.h
>> @@ -118,9 +118,30 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass,
>> SDMMC_COMMON, TYPE_SDMMC_COMMON)
>> #define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0)
>> #define EXT_CSD_PART_CONFIG_ACC_BOOT1 (0x1)
>> #define EXT_CSD_PART_CONFIG_ACC_BOOT2 (0x2)
>> +#define EXT_CSD_PART_CONFIG_ACC_RPMB (0x3)
>> #define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3)
>> #define EXT_CSD_PART_CONFIG_EN_BOOT0 (0x1 << 3)
>> #define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3)
>> +#define RPMB_REQ_PROGRAM_AUTH_KEY (1)
>> +#define RPMB_REQ_READ_WRITE_COUNTER (2)
>> +#define RPMB_REQ_AUTH_DATA_WRITE (3)
>> +#define RPMB_REQ_AUTH_DATA_READ (4)
>> +#define RPMB_REQ_READ_RESULT (5)
>> +#define RPMB_REQ_AUTH_CONFIG_WRITE (6)
>> +#define RPMB_REQ_AUTH_CONFIG_READ (7)
>> +
>> +#define RPMB_RESP(__req__) ((__req__) << 8)
>> +
>> +#define RPMB_RESULT_OK (0)
>> +#define RPMB_RESULT_GENERAL_FAILURE (1)
>> +#define RPMB_RESULT_AUTH_FAILURE (2)
>> +#define RPMB_RESULT_COUNTER_FAILURE (3)
>> +#define RPMB_RESULT_ADDRESS_FAILURE (4)
>> +#define RPMB_RESULT_WRITE_FAILURE (5)
>> +#define RPMB_RESULT_READ_FAILURE (6)
>> +#define RPMB_RESULT_NO_AUTH_KEY (7)
>
>> +#define RPMB_RESULT_COUTER_EXPIRED (0x80)
>> +
>> #endif
>
> If you are OK, I'd like to squash:
>
> -- >8 --diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
> index c4a9aa8edf6..c115f472efe 100644
> --- a/hw/sd/sdmmc-internal.h
> +++ b/hw/sd/sdmmc-internal.h
> @@ -144,2 +144,3 @@ DECLARE_OBJ_CHECKERS(SDState, SDCardClass,
> SDMMC_COMMON, TYPE_SDMMC_COMMON)
> #define RPMB_RESULT_NO_AUTH_KEY (7)
> +
> #define RPMB_RESULT_COUTER_EXPIRED (0x80)
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f8883860fb1..ac8f6b94746 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -119,8 +119,10 @@ typedef struct SDProto {
>
> +#define RPMB_STUFF_LEN 196
> #define RPMB_KEY_MAC_LEN 32
> +#define RPMB_DATA_LEN 256 /* one RPMB block is half a sector */
>
> -typedef struct {
> - uint8_t stuff_bytes[196];
> +typedef struct QEMU_PACKED {
> + uint8_t stuff_bytes[RPMB_STUFF_LEN];
> uint8_t key_mac[RPMB_KEY_MAC_LEN];
> - uint8_t data[256];
> + uint8_t data[RPMB_DATA_LEN];
> uint8_t nonce[16];
You left the nonce length without a constant now.
> @@ -133,2 +135,5 @@ typedef struct {
>
> +QEMU_BUILD_BUG_MSG(sizeof(RPMBDataFrame) != 512,
> + "invalid RPMBDataFrame size");
> +
> struct SDState {
> @@ -191,3 +196,3 @@ struct SDState {
> uint32_t rpmb_write_counter;
> - uint8_t rpmb_key[32];
> + uint8_t rpmb_key[RPMB_KEY_MAC_LEN];
> uint8_t rpmb_key_set;
> @@ -1019,3 +1024,3 @@ static const VMStateDescription emmc_rpmb_vmstate = {
> VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState,
> RPMB_KEY_MAC_LEN),
> - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, 256),
> + VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN),
> VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16),
> @@ -1137,5 +1142,6 @@ static void emmc_rpmb_blk_read(SDState *sd,
> uint64_t addr, uint32_t len)
> }
> - addr = curr_block * 256 + sd_part_offset(sd);
> - if (blk_pread(sd->blk, addr, 256, sd->rpmb_result.data, 0) < 0) {
> - fprintf(stderr, "sd_blk_read: read error on host side\n");
> + addr = curr_block * RPMB_DATA_LEN + sd_part_offset(sd);
> + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN,
> + sd->rpmb_result.data, 0) < 0) {
> + error_report("sd_blk_read: read error on host side");
> memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
> @@ -1197,5 +1203,5 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> sd->rpmb_result.address = frame->address;
> - addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
> - if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) {
> - fprintf(stderr, "sd_blk_write: write error on host side\n");
> + addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN +
> sd_part_offset(sd);
> + if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) <
> 0) {
> + error_report("sd_blk_write: write error on host side");
> sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> @@ -3027,3 +3033,3 @@ static void sd_realize(DeviceState *dev, Error
> **errp)
> }
> - if (sd->rpmb_part_size % (128 * KiB) ||
> + if (!QEMU_IS_ALIGNED(sd->rpmb_part_size, 128 * KiB) ||
> sd->rpmb_part_size > 128 * 128 * KiB) {
> ---
>
> And on top, if you don't mind (but can do that later):
>
> -- >8 --
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index ac8f6b94746..71f396cb4d6 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -195,4 +195,6 @@ struct SDState {
> - RPMBDataFrame rpmb_result;
> - uint32_t rpmb_write_counter;
> - uint8_t rpmb_key[RPMB_KEY_MAC_LEN];
> - uint8_t rpmb_key_set;
> + struct {
> + uint32_t write_counter;
> + uint8_t key[RPMB_KEY_MAC_LEN];
> + uint8_t key_set;
> + RPMBDataFrame result;
> + } rpmb;
> @@ -1024,11 +1026,11 @@ static const VMStateDescription
> emmc_rpmb_vmstate = {
> - VMSTATE_UINT8_ARRAY(rpmb_result.key_mac, SDState,
> RPMB_KEY_MAC_LEN),
> - VMSTATE_UINT8_ARRAY(rpmb_result.data, SDState, RPMB_DATA_LEN),
> - VMSTATE_UINT8_ARRAY(rpmb_result.nonce, SDState, 16),
> - VMSTATE_UINT32(rpmb_result.write_counter, SDState),
> - VMSTATE_UINT16(rpmb_result.address, SDState),
> - VMSTATE_UINT16(rpmb_result.block_count, SDState),
> - VMSTATE_UINT16(rpmb_result.result, SDState),
> - VMSTATE_UINT16(rpmb_result.req_resp, SDState),
> - VMSTATE_UINT32(rpmb_write_counter, SDState),
> - VMSTATE_UINT8_ARRAY(rpmb_key, SDState, 32),
> - VMSTATE_UINT8(rpmb_key_set, SDState),
> + VMSTATE_UINT8_ARRAY(rpmb.result.key_mac, SDState,
> RPMB_KEY_MAC_LEN),
> + VMSTATE_UINT8_ARRAY(rpmb.result.data, SDState, RPMB_DATA_LEN),
> + VMSTATE_UINT8_ARRAY(rpmb.result.nonce, SDState, 16),
> + VMSTATE_UINT32(rpmb.result.write_counter, SDState),
> + VMSTATE_UINT16(rpmb.result.address, SDState),
> + VMSTATE_UINT16(rpmb.result.block_count, SDState),
> + VMSTATE_UINT16(rpmb.result.result, SDState),
> + VMSTATE_UINT16(rpmb.result.req_resp, SDState),
> + VMSTATE_UINT32(rpmb.write_counter, SDState),
> + VMSTATE_UINT8_ARRAY(rpmb.key, SDState, 32),
> + VMSTATE_UINT8(rpmb.key_set, SDState),
> @@ -1130,2 +1132,2 @@ static void emmc_rpmb_blk_read(SDState *sd,
> uint64_t addr, uint32_t len)
> - uint16_t resp = be16_to_cpu(sd->rpmb_result.req_resp);
> - uint16_t result = be16_to_cpu(sd->rpmb_result.result);
> + uint16_t resp = be16_to_cpu(sd->rpmb.result.req_resp);
> + uint16_t result = be16_to_cpu(sd->rpmb.result.result);
> @@ -1136,3 +1138,3 @@ static void emmc_rpmb_blk_read(SDState *sd,
> uint64_t addr, uint32_t len)
> - curr_block = be16_to_cpu(sd->rpmb_result.address);
> - if (sd->rpmb_result.block_count == 0) {
> - sd->rpmb_result.block_count = cpu_to_be16(sd->multi_blk_cnt);
> + curr_block = be16_to_cpu(sd->rpmb.result.address);
> + if (sd->rpmb.result.block_count == 0) {
> + sd->rpmb.result.block_count = cpu_to_be16(sd->multi_blk_cnt);
> @@ -1140 +1142 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t
> addr, uint32_t len)
> - curr_block += be16_to_cpu(sd->rpmb_result.block_count) -
> + curr_block += be16_to_cpu(sd->rpmb.result.block_count) -
> @@ -1144,2 +1146 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t
> addr, uint32_t len)
> - if (blk_pread(sd->blk, addr, RPMB_DATA_LEN,
> - sd->rpmb_result.data, 0) < 0) {
> + if (blk_pread(sd->blk, addr, RPMB_DATA_LEN, sd-
>>rpmb.result.data, 0) < 0) {
> @@ -1147,2 +1148,2 @@ static void emmc_rpmb_blk_read(SDState *sd,
> uint64_t addr, uint32_t len)
> - memset(sd->rpmb_result.data, 0, sizeof(sd->rpmb_result.data));
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_READ_FAILURE |
> + memset(sd->rpmb.result.data, 0, sizeof(sd->rpmb.result.data));
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_READ_FAILURE |
> @@ -1152 +1153 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t
> addr, uint32_t len)
> - memcpy(sd->data, &sd->rpmb_result, sizeof(sd->rpmb_result));
> + memcpy(sd->data, &sd->rpmb.result, sizeof(sd->rpmb.result));
> @@ -1155 +1156 @@ static void emmc_rpmb_blk_read(SDState *sd, uint64_t
> addr, uint32_t len)
> - be16_to_cpu(sd->rpmb_result.result));
> + be16_to_cpu(sd->rpmb.result.result));
> @@ -1167,4 +1168,4 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - memset(&sd->rpmb_result, 0, sizeof(sd->rpmb_result));
> - memcpy(sd->rpmb_result.nonce, frame->nonce, sizeof(sd-
>>rpmb_result.nonce));
> - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_OK);
> - sd->rpmb_result.req_resp = cpu_to_be16(RPMB_RESP(req));
> + memset(&sd->rpmb.result, 0, sizeof(sd->rpmb.result));
> + memcpy(sd->rpmb.result.nonce, frame->nonce, sizeof(sd-
>>rpmb.result.nonce));
> + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_OK);
> + sd->rpmb.result.req_resp = cpu_to_be16(RPMB_RESP(req));
> @@ -1172,2 +1173,2 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - if (!sd->rpmb_key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) {
> - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY);
> + if (!sd->rpmb.key_set && req != RPMB_REQ_PROGRAM_AUTH_KEY) {
> + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_NO_AUTH_KEY);
> @@ -1179,2 +1180,2 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - if (sd->rpmb_key_set) {
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> + if (sd->rpmb.key_set) {
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> @@ -1183,2 +1184,2 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - memcpy(sd->rpmb_key, frame->key_mac, sizeof(sd->rpmb_key));
> - sd->rpmb_key_set = 1;
> + memcpy(sd->rpmb.key, frame->key_mac, sizeof(sd->rpmb.key));
> + sd->rpmb.key_set = 1;
> @@ -1187 +1188 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.write_counter = cpu_to_be32(sd-
>>rpmb_write_counter);
> + sd->rpmb.result.write_counter = cpu_to_be32(sd-
>>rpmb.write_counter);
> @@ -1192 +1193 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
> @@ -1195,2 +1196,2 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - if (sd->rpmb_write_counter == 0xffffffff) {
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> + if (sd->rpmb.write_counter == 0xffffffff) {
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> @@ -1199,2 +1200,2 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - if (be32_to_cpu(frame->write_counter) != sd->rpmb_write_counter) {
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
> + if (be32_to_cpu(frame->write_counter) != sd->rpmb.write_counter) {
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_COUNTER_FAILURE);
> @@ -1203,3 +1204,3 @@ static void emmc_rpmb_blk_write(SDState *sd,
> uint64_t addr, uint32_t len)
> - sd->rpmb_result.address = frame->address;
> - addr = be16_to_cpu(frame->address) * RPMB_DATA_LEN +
> sd_part_offset(sd);
> - if (blk_pwrite(sd->blk, addr, RPMB_DATA_LEN, frame->data, 0) <
> 0) {
> + sd->rpmb.result.address = frame->address;
> + addr = be16_to_cpu(frame->address) * 256 + sd_part_offset(sd);
> + if (blk_pwrite(sd->blk, addr, 256, frame->data, 0) < 0) {
> @@ -1207 +1208 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> + sd->rpmb.result.result =
> cpu_to_be16(RPMB_RESULT_WRITE_FAILURE);
> @@ -1209 +1210 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_write_counter++;
> + sd->rpmb.write_counter++;
> @@ -1211 +1212 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.write_counter = cpu_to_be32(sd-
>>rpmb_write_counter);
> + sd->rpmb.result.write_counter = cpu_to_be32(sd-
>>rpmb.write_counter);
> @@ -1214 +1215 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.address = frame->address;
> + sd->rpmb.result.address = frame->address;
> @@ -1218 +1219 @@ static void emmc_rpmb_blk_write(SDState *sd, uint64_t
> addr, uint32_t len)
> - sd->rpmb_result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
> + sd->rpmb.result.result = cpu_to_be16(RPMB_RESULT_GENERAL_FAILURE);
> @@ -1222,2 +1223,2 @@ exit:
> - if (sd->rpmb_write_counter == 0xffffffff) {
> - sd->rpmb_result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED);
> + if (sd->rpmb.write_counter == 0xffffffff) {
> + sd->rpmb.result.result |= cpu_to_be16(RPMB_RESULT_COUTER_EXPIRED);
> @@ -1225 +1226 @@ exit:
> - trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd-
>>rpmb_result.result));
> + trace_sdcard_rpmb_write_block(req, be16_to_cpu(sd-
>>rpmb.result.result));
> ---
>
> Regards,
>
> Phil.
Let me pick up and test those changes for v6.
Thanks,
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
