Re: [PATCH 7/8] hw/sd/sdcard: Handle RPMB MAC field

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

+Dan

On 25/8/25 18:12, Jan Kiszka wrote:
> On 25.08.25 11:47, Philippe Mathieu-Daudé wrote:
>> Hi Jan,
>>
>> On 24/8/25 09:18, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Implement correct setting of the MAC field when passing RPMB frames back
>>> to the guest. Also check the MAC on authenticated write requests.
>>>
>>> As this depends on HMAC support for QCRYPTO_HASH_ALGO_SHA256, only
>>> register the eMMC class if that is available.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>    hw/sd/sd.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 89 insertions(+), 1 deletion(-)
>>
>>
>>> @@ -3122,6 +3201,7 @@ static const TypeInfo sd_types[] = {
>>>            .parent         = TYPE_SD_CARD,
>>>            .class_init     = sd_spi_class_init,
>>>        },
>>> +    /* must be last element */
>>>        {
>>>            .name           = TYPE_EMMC,
>>>            .parent         = TYPE_SDMMC_COMMON,
>>> @@ -3129,4 +3209,12 @@ static const TypeInfo sd_types[] = {
>>>        },
>>>    };
>>>    -DEFINE_TYPES(sd_types)
>>> +static void sd_register_types(void)
>>> +{
>>> +    int num = ARRAY_SIZE(sd_types);
>>> +    if (!qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256)) {
>>> +        num--;
>>
>> Instead, expose RPMB feature in CSD when HMAC supported?
>>
>> Something in emmc_set_ext_csd() in the lines of:
>>
>>    if (qcrypto_hmac_supports(QCRYPTO_HASH_ALGO_SHA256)) {
>>        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;
>>    } else {
>>        sd->ext_csd[EXT_CSD_REV] = 3;
>>    }
> 
> I need to check if revision 5 still had RPMB as optional (current ones
> definitely require it), but I don't think rolling back to revision 3
> would be good idea. If start to add more features from newer revisions,
> that may cause even more weird results from the user perspective. I'm
> not saying we are fully compliant in one or the other version, rather
> that we need to work towards becoming so. Have to support multiple
> versions along that will not make it easier.

Daniel, do you have a rough idea how many of our build config do
not support QCRYPTO_HASH_ALGO_SHA256?
(looking about making the SD device unconditional to it).

>>> +    }
>>> +    type_register_static_array(sd_types, num);
>>> +}
>>> +type_init(sd_register_types);
>>
>