On Wed, Aug 27, 2025 at 07:53:12AM +0200, Jan Kiszka wrote:
> On 26.08.25 12:18, Daniel P. Berrangé wrote:
> > On Mon, Aug 25, 2025 at 06:30:52PM +0200, Philippe Mathieu-Daudé wrote:
> >> +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.kis...@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.kis...@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).
> >
> > That's always available, since we can get it from 'glib' even when no
> > crypto libs are linked.
> >
>
> Perfect, makes things simpler.
>
> So what is best practice, assert() availability or silently assume that
> it is there?
Best practice is always to propagate errors, but if your call chain can't
do that, then in this case you can assert or use &error_abort since this
should never trigger.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
