On 6/22/21 11:44 AM, Dov Murik wrote:
> On 21/06/2021 23:32, Philippe Mathieu-Daudé wrote:
>> and added qemu_uuid_copy() to complete the API, but that's fine.
>
> I think simple C assignment works for structs (and hence QemuUUID),
> something like:
>
> SevHashTable *ht = ...;
> ht->guid = sev_hash_table_header_guid;
>
> (where both ht->guid and sev_hash_table_header_guid are QemuUUID.)
OK.
>>> + if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) {
>>
>> If we never use the data_len argument, can we simplify the prototype?
>
> The current uses for the OVMF reset vector GUIDed table is for simple
> structs with known length (secret injection page address, SEV-ES reset
> address, SEV table of hashes address). But keeping the length there
> allows adding variable-sized entries such as strings/blobs.
OK. Good opportunity to document the prototype declaration ;)
>
>>
>>> + error_setg(errp, "SEV: kernel specified but OVMF has no hash table
>>> guid");
>>> + return false;
>>> + }
>>> + area = (SevHashTableDescriptor *)data;
>>> +
>>> + ht = qemu_map_ram_ptr(NULL, area->base);
>>> +
>>> + /* Populate the hashes table header */
>>> + memcpy(ht->guid, sev_hash_table_header_guid, sizeof(ht->guid));
>>> + ht->len = sizeof(*ht);
>>> +
>>> + /* Calculate hash of kernel command-line */
>>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->cmdline_data,
>>> + ctx->cmdline_size,
>>> + &hash, &hash_len, errp) < 0) {
>>> + return false;
>>> + }
>>
>> Maybe move the qcrypto_hash_bytes() call before filling ht?
>
> (below)
>
>>
>>> + e = &ht->entries[ht_index++];
>>> + fill_sev_hash_table_entry(e, sev_cmdline_entry_guid, hash, hash_len);
>>> +
>>> + /* Calculate hash of initrd */
>>> + if (ctx->initrd_data) {
>>> + if (qcrypto_hash_bytes(QCRYPTO_HASH_ALG_SHA256, ctx->initrd_data,
>>> + ctx->initrd_size, &hash, &hash_len, errp) <
>>> 0) {
>>> + return false;
>>> + }
>>
>> Ah, now I see the pattern. Hmm OK then.
>>
>
> But this might change if initrd_hash is no longer optional (see separate
> self-reply to this patch). In such a case I'll probably first calculate
> all the three hashes, and then fill in the SevHashTable struct.
Yes, sounds simpler.
Regards,
Phil.
