Daniel P. Berrangé <berra...@redhat.com> writes:
> On Fri, Aug 18, 2023 at 05:50:03AM -0400, Xiaoyao Li wrote:
>> From: Isaku Yamahata <isaku.yamah...@intel.com>
>>
>> When creating TDX vm, three sha384 hash values can be provided for
>> TDX attestation.
>>
>> So far they were hard coded as 0. Now allow user to specify those values
>> via property mrconfigid, mrowner and mrownerconfig. Choose hex-encoded
>> string as format since it's friendly for user to input.
>>
>> example
>> -object tdx-guest, \
>>
>> mrconfigid=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef,
>> \
>>
>> mrowner=fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210,
>> \
>>
>> mrownerconfig=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
>>
>> Signed-off-by: Isaku Yamahata <isaku.yamah...@intel.com>
>> Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
>> ---
>> TODO:
>> - community requests to use base64 encoding if no special reason
>> ---
>> qapi/qom.json | 11 ++++++++++-
>> target/i386/kvm/tdx.c | 13 +++++++++++++
>> target/i386/kvm/tdx.h | 3 +++
>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index cc08b9a98df9..87c1d440f331 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -873,10 +873,19 @@
>> #
>> # @sept-ve-disable: bit 28 of TD attributes (default: 0)
>> #
>> +# @mrconfigid: MRCONFIGID SHA384 hex string of 48 * 2 length (default: 0)
>> +#
>> +# @mrowner: MROWNER SHA384 hex string of 48 * 2 length (default: 0)
>> +#
>> +# @mrownerconfig: MROWNERCONFIG SHA384 hex string of 48 * 2 length
>> (default: 0)
>
> Per previous patch, I suggest these should all be passed in base64
> instead of hex.
I'm upgrading this suggestion to a demand: we use base64 for encoding
binary data everywhere in QAPI/QMP. Consistency matters.
> Also 'default: 0' makes no sense for a string,
> which would be 'default: nil', and no need to document that as
> the default is implicit from the fact that its an optional string
> field. So eg
>
> @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
Agree.
The member names are abbreviations all run together, wheras QAPI/QMP
favors words-separated-with-dashes. If you invented them, please change
them to QAPI/QMP style. If they are established TDX terminology, keep
them as they are, but please point to your evidence.
>> +#
>> # Since: 8.2
>> ##
>> { 'struct': 'TdxGuestProperties',
>> - 'data': { '*sept-ve-disable': 'bool' } }
>> + 'data': { '*sept-ve-disable': 'bool',
>> + '*mrconfigid': 'str',
>> + '*mrowner': 'str',
>> + '*mrownerconfig': 'str' } }
>>
>> ##
>> # @ThreadContextProperties:
[...]
