[v3,31/70] i386/tdx: Allows mrconfigid/mrowner/mrownerconfig for TDX_INIT_VM

Commit Message

Xiaoyao Li Nov. 15, 2023, 7:14 a.m. UTC

From: Isaku Yamahata <[e-mail]>

Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
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. They are all in
base64 format.

example
-object tdx-guest, \
  mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
  mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
  mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v

Signed-off-by: Isaku Yamahata <[e-mail]>
Co-developed-by: Xiaoyao Li <[e-mail]>
Signed-off-by: Xiaoyao Li <[e-mail]>
---
Changes in v3:
 - use base64 encoding instread of hex-string;
---
 qapi/qom.json         | 11 +++++-
 target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
 target/i386/kvm/tdx.h |  3 ++
 3 files changed, 98 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé Nov. 15, 2023, 5:32 p.m. UTC | #1

On Wed, Nov 15, 2023 at 02:14:40AM -0500, Xiaoyao Li wrote:
> From: Isaku Yamahata <[e-mail]>
> 
> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
> 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. They are all in
> base64 format.
> 
> example
> -object tdx-guest, \
>   mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>   mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>   mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
> 
> Signed-off-by: Isaku Yamahata <[e-mail]>
> Co-developed-by: Xiaoyao Li <[e-mail]>
> Signed-off-by: Xiaoyao Li <[e-mail]>
> ---
> Changes in v3:
>  - use base64 encoding instread of hex-string;
> ---
>  qapi/qom.json         | 11 +++++-
>  target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
>  target/i386/kvm/tdx.h |  3 ++
>  3 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 3a29659e0155..fd99aa1ff8cc 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -888,10 +888,19 @@
>  #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
>  #     be set, otherwise they refuse to boot.
>  #
> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
> +#
> +# @mrowner: base64 encoded MROWNER SHA384 digest
> +#
> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
> +#
>  # Since: 8.2
>  ##
>  { 'struct': 'TdxGuestProperties',
> -  'data': { '*sept-ve-disable': 'bool' } }
> +  'data': { '*sept-ve-disable': 'bool',
> +            '*mrconfigid': 'str',
> +            '*mrowner': 'str',
> +            '*mrownerconfig': 'str' } }
>  
>  ##
>  # @ThreadContextProperties:
> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
> index 28b3c2765c86..b70efbcab738 100644
> --- a/target/i386/kvm/tdx.c
> +++ b/target/i386/kvm/tdx.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
> +#include "qemu/base64.h"
>  #include "qapi/error.h"
>  #include "qom/object_interfaces.h"
>  #include "standard-headers/asm-x86/kvm_para.h"
> @@ -508,6 +509,8 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>      X86CPU *x86cpu = X86_CPU(cpu);
>      CPUX86State *env = &x86cpu->env;
>      struct kvm_tdx_init_vm *init_vm;
> +    uint8_t *data;
> +    size_t data_len;

Don't declare these here.

>      int r = 0;
>  
>      qemu_mutex_lock(&tdx_guest->lock);
> @@ -518,6 +521,38 @@ int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
>      init_vm = g_malloc0(sizeof(struct kvm_tdx_init_vm) +
>                          sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES);
>  
> +#define SHA384_DIGEST_SIZE  48
> +
> +    if (tdx_guest->mrconfigid) {

> +        data = qbase64_decode(tdx_guest->mrconfigid,
> +                              strlen(tdx_guest->mrconfigid), &data_len, errp);

Declare it here:

    g_autofree uint8_t *data = qbase64_decode(...)


so we aviod the memory leak of 'data' in each if()... block


> +        if (!data || data_len != SHA384_DIGEST_SIZE) {
> +            error_setg(errp, "TDX: failed to decode mrconfigid");
> +            return -1;
> +        }
> +        memcpy(init_vm->mrconfigid, data, data_len);
> +    }
> +
> +    if (tdx_guest->mrowner) {
> +        data = qbase64_decode(tdx_guest->mrowner,
> +                              strlen(tdx_guest->mrowner), &data_len, errp);
> +        if (!data || data_len != SHA384_DIGEST_SIZE) {
> +            error_setg(errp, "TDX: failed to decode mrowner");
> +            return -1;
> +        }
> +        memcpy(init_vm->mrowner, data, data_len);
> +    }
> +
> +    if (tdx_guest->mrownerconfig) {
> +        data = qbase64_decode(tdx_guest->mrownerconfig,
> +                              strlen(tdx_guest->mrownerconfig), &data_len, errp);
> +        if (!data || data_len != SHA384_DIGEST_SIZE) {
> +            error_setg(errp, "TDX: failed to decode mrownerconfig");
> +            return -1;
> +        }
> +        memcpy(init_vm->mrownerconfig, data, data_len);
> +    }
> +
>      r = kvm_vm_enable_cap(kvm_state, KVM_CAP_MAX_VCPUS, 0, ms->smp.cpus);
>      if (r < 0) {
>          error_setg(errp, "Unable to set MAX VCPUS to %d", ms->smp.cpus);
> @@ -567,6 +602,48 @@ static void tdx_guest_set_sept_ve_disable(Object *obj, bool value, Error **errp)
>      }
>  }
> +static void tdx_guest_set_mrconfigid(Object *obj, const char *value, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    tdx->mrconfigid = g_strdup(value);
> +}

g_free(tdx->mrconfigid) first to be sure we don't leak if
the value is set twice.

> +
> +static char * tdx_guest_get_mrowner(Object *obj, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    return g_strdup(tdx->mrowner);
> +}
> +
> +static void tdx_guest_set_mrowner(Object *obj, const char *value, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    tdx->mrconfigid = g_strdup(value);
> +}
> +
> +static char * tdx_guest_get_mrownerconfig(Object *obj, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    return g_strdup(tdx->mrownerconfig);
> +}
> +
> +static void tdx_guest_set_mrownerconfig(Object *obj, const char *value, Error **errp)
> +{
> +    TdxGuest *tdx = TDX_GUEST(obj);
> +
> +    tdx->mrconfigid = g_strdup(value);
> +}
> +
>  /* tdx guest */
>  OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
>                                     tdx_guest,
> @@ -586,6 +663,14 @@ static void tdx_guest_init(Object *obj)
>      object_property_add_bool(obj, "sept-ve-disable",
>                               tdx_guest_get_sept_ve_disable,
>                               tdx_guest_set_sept_ve_disable);
> +    object_property_add_str(obj, "mrconfigid",
> +                            tdx_guest_get_mrconfigid,
> +                            tdx_guest_set_mrconfigid);
> +    object_property_add_str(obj, "mrowner",
> +                            tdx_guest_get_mrowner, tdx_guest_set_mrowner);
> +    object_property_add_str(obj, "mrownerconfig",
> +                            tdx_guest_get_mrownerconfig,
> +                            tdx_guest_set_mrownerconfig);
>  }
>  
>  static void tdx_guest_finalize(Object *obj)
> diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
> index 432077723ac5..6e39ef3bac13 100644
> --- a/target/i386/kvm/tdx.h
> +++ b/target/i386/kvm/tdx.h
> @@ -21,6 +21,9 @@ typedef struct TdxGuest {
>  
>      bool initialized;
>      uint64_t attributes;    /* TD attributes */
> +    char *mrconfigid;       /* base64 encoded sha348 digest */
> +    char *mrowner;          /* base64 encoded sha348 digest */
> +    char *mrownerconfig;    /* base64 encoded sha348 digest */
>  } TdxGuest;
>  
>  #ifdef CONFIG_TDX
> -- 
> 2.34.1
> 

With regards,
Daniel

Markus Armbruster Dec. 1, 2023, 11 a.m. UTC | #2

Xiaoyao Li <[e-mail]> writes:

> From: Isaku Yamahata <[e-mail]>
>
> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
> 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. They are all in
> base64 format.
>
> example
> -object tdx-guest, \
>   mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>   mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>   mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
>
> Signed-off-by: Isaku Yamahata <[e-mail]>
> Co-developed-by: Xiaoyao Li <[e-mail]>
> Signed-off-by: Xiaoyao Li <[e-mail]>
> ---
> Changes in v3:
>  - use base64 encoding instread of hex-string;
> ---
>  qapi/qom.json         | 11 +++++-
>  target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
>  target/i386/kvm/tdx.h |  3 ++
>  3 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 3a29659e0155..fd99aa1ff8cc 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -888,10 +888,19 @@
>  #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
>  #     be set, otherwise they refuse to boot.
>  #
> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
> +#
> +# @mrowner: base64 encoded MROWNER SHA384 digest
> +#
> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest

Can we come up with a description that tells the user a bit more clearly
what we're talking about?  Perhaps starting with this question could
lead us there: what's an MRCONFIGID, and why should I care?

> +#
>  # Since: 8.2
>  ##
>  { 'struct': 'TdxGuestProperties',
> -  'data': { '*sept-ve-disable': 'bool' } }
> +  'data': { '*sept-ve-disable': 'bool',
> +            '*mrconfigid': 'str',
> +            '*mrowner': 'str',
> +            '*mrownerconfig': 'str' } }
>  
>  ##
>  # @ThreadContextProperties:

[...]

Xiaoyao Li Dec. 14, 2023, 3:07 a.m. UTC | #3

On 12/1/2023 7:00 PM, Markus Armbruster wrote:
> Xiaoyao Li <[e-mail]> writes:
> 
>> From: Isaku Yamahata <[e-mail]>
>>
>> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
>> 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. They are all in
>> base64 format.
>>
>> example
>> -object tdx-guest, \
>>    mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>    mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>    mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
>>
>> Signed-off-by: Isaku Yamahata <[e-mail]>
>> Co-developed-by: Xiaoyao Li <[e-mail]>
>> Signed-off-by: Xiaoyao Li <[e-mail]>
>> ---
>> Changes in v3:
>>   - use base64 encoding instread of hex-string;
>> ---
>>   qapi/qom.json         | 11 +++++-
>>   target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
>>   target/i386/kvm/tdx.h |  3 ++
>>   3 files changed, 98 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 3a29659e0155..fd99aa1ff8cc 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -888,10 +888,19 @@
>>   #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
>>   #     be set, otherwise they refuse to boot.
>>   #
>> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
>> +#
>> +# @mrowner: base64 encoded MROWNER SHA384 digest
>> +#
>> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
> 
> Can we come up with a description that tells the user a bit more clearly
> what we're talking about?  Perhaps starting with this question could
> lead us there: what's an MRCONFIGID, and why should I care?

Below are the definition from TDX spec:

MRCONFIGID: Software-defined ID for non-owner-defined configuration of 
the guest TD – e.g., run-time or OS configuration.

MROWNER: Software-defined ID for the guest TD’s owner

MROWNERCONFIG: Software-defined ID for owner-defined configuration of 
the guest TD – e.g., specific to the workload rather than the run-time or OS


They are all attestation related, and input by users who launches the TD 
. Software inside TD can retrieve them with TDREPORT and verify if it is 
the expected value.

MROWNER is to identify the owner of the TD, MROWNERCONFIG is to pass 
OWNER's configuration. And MRCONFIGID contains configuration specific to 
OS level instead of OWNER.

Below is the explanation from Intel inside, hope it can get you more clear:

"These are primarily intended for general purpose, configurable software 
in a minimal TD. So, not a legacy VM image cloud customer wanting to 
move their VM out into the cloud. Also it’s not necessarily the case 
that any workload will use them all.

MROWNER is for declaring the owner of the TD. An example use case would 
be an vHSM TD. HSMs need to know who their administrative contact is. 
You could customize the HSM image and measurements, but then people 
can’t recognize that this is the vHSM product from XYZ. So you put the 
unmodified vHSM stack in the TD, which will include MRTD/RTMRs that 
reflect the vHSM, and the owner’s public key in MROWNER. Now, when the 
vHSM starts up, to determine who is authorized to send commands, it does 
a TDREPORT, and looks at MROWNER.

Extending this model, there could be important configuration information 
from the owner. In that case, MROWNERCONFIG is set to the hash of the 
config file that the vHSM should accept.

This results in an attestable environment that explicitly indicates that 
it’s a well recognized vHSM TD, being administered by MROWNER and 
loading the configuration information that matches MROWNERCONFIG.

Extending this idea of configuration of generally recognized software, 
it could be that there is a shim OS under the vHSM that itself is 
configurable. So MRCONFIGID, which isn’t a great name, can include 
configuration information intended for the OS level. The ID is 
confusing, but MRCONFIGID was the name we used for this register for 
SGX, so we kept the name."

>> +#
>>   # Since: 8.2
>>   ##
>>   { 'struct': 'TdxGuestProperties',
>> -  'data': { '*sept-ve-disable': 'bool' } }
>> +  'data': { '*sept-ve-disable': 'bool',
>> +            '*mrconfigid': 'str',
>> +            '*mrowner': 'str',
>> +            '*mrownerconfig': 'str' } }
>>   
>>   ##
>>   # @ThreadContextProperties:
> 
> [...]
>

Markus Armbruster Dec. 18, 2023, 1:46 p.m. UTC | #4

Xiaoyao Li <[e-mail]> writes:

> On 12/1/2023 7:00 PM, Markus Armbruster wrote:
>> Xiaoyao Li <[e-mail]> writes:
>> 
>>> From: Isaku Yamahata <[e-mail]>
>>>
>>> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
>>> 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. They are all in
>>> base64 format.
>>>
>>> example
>>> -object tdx-guest, \
>>>    mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>>    mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>>    mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
>>>
>>> Signed-off-by: Isaku Yamahata <[e-mail]>
>>> Co-developed-by: Xiaoyao Li <[e-mail]>
>>> Signed-off-by: Xiaoyao Li <[e-mail]>
>>> ---
>>> Changes in v3:
>>>   - use base64 encoding instread of hex-string;
>>> ---
>>>   qapi/qom.json         | 11 +++++-
>>>   target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
>>>   target/i386/kvm/tdx.h |  3 ++
>>>   3 files changed, 98 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 3a29659e0155..fd99aa1ff8cc 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -888,10 +888,19 @@
>>>  #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
>>>  #     be set, otherwise they refuse to boot.
>>>  #
>>> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
>>> +#
>>> +# @mrowner: base64 encoded MROWNER SHA384 digest
>>> +#
>>> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
>>
>> Can we come up with a description that tells the user a bit more clearly
>> what we're talking about?  Perhaps starting with this question could
>> lead us there: what's an MRCONFIGID, and why should I care?
>
> Below are the definition from TDX spec:
>
> MRCONFIGID: Software-defined ID for non-owner-defined configuration of the guest TD – e.g., run-time or OS configuration.
>
> MROWNER: Software-defined ID for the guest TD’s owner
>
> MROWNERCONFIG: Software-defined ID for owner-defined configuration of the guest TD – e.g., specific to the workload rather than the run-time or OS

Have you considered using this for the doc comments?  I'd omit
"software-defined" in this context.

> They are all attestation related, and input by users who launches the TD . Software inside TD can retrieve them with TDREPORT and verify if it is the expected value.
>
> MROWNER is to identify the owner of the TD, MROWNERCONFIG is to pass OWNER's configuration. And MRCONFIGID contains configuration specific to OS level instead of OWNER.
>
> Below is the explanation from Intel inside, hope it can get you more clear:
>
> "These are primarily intended for general purpose, configurable software in a minimal TD. So, not a legacy VM image cloud customer wanting to move their VM out into the cloud. Also it’s not necessarily the case that any workload will use them all.
>
> MROWNER is for declaring the owner of the TD. An example use case would be an vHSM TD. HSMs need to know who their administrative contact is. You could customize the HSM image and measurements, but then people can’t recognize that this is the vHSM product from XYZ. So you put the unmodified vHSM stack in the TD, which will include MRTD/RTMRs that reflect the vHSM, and the owner’s public key in MROWNER. Now, when the vHSM starts up, to determine who is authorized to send commands, it does a TDREPORT, and looks at MROWNER.
>
> Extending this model, there could be important configuration information from the owner. In that case, MROWNERCONFIG is set to the hash of the config file that the vHSM should accept.
>
> This results in an attestable environment that explicitly indicates that it’s a well recognized vHSM TD, being administered by MROWNER and loading the configuration information that matches MROWNERCONFIG.
>
> Extending this idea of configuration of generally recognized software, it could be that there is a shim OS under the vHSM that itself is configurable. So MRCONFIGID, which isn’t a great name, can include configuration information intended for the OS level. The ID is confusing, but MRCONFIGID was the name we used for this register for SGX, so we kept the name."

Include a reference to this document?

>>> +#
>>>  # Since: 8.2
>>>  ##
>>>  { 'struct': 'TdxGuestProperties',
>>> -  'data': { '*sept-ve-disable': 'bool' } }
>>> +  'data': { '*sept-ve-disable': 'bool',
>>> +            '*mrconfigid': 'str',
>>> +            '*mrowner': 'str',
>>> +            '*mrownerconfig': 'str' } }
>>>   ##
>>>   # @ThreadContextProperties:
>> [...]
>>

Xiaoyao Li Dec. 19, 2023, 8:27 a.m. UTC | #5

On 12/18/2023 9:46 PM, Markus Armbruster wrote:
> Xiaoyao Li <[e-mail]> writes:
> 
>> On 12/1/2023 7:00 PM, Markus Armbruster wrote:
>>> Xiaoyao Li <[e-mail]> writes:
>>>
>>>> From: Isaku Yamahata <[e-mail]>
>>>>
>>>> Three sha384 hash values, mrconfigid, mrowner and mrownerconfig, of a TD
>>>> 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. They are all in
>>>> base64 format.
>>>>
>>>> example
>>>> -object tdx-guest, \
>>>>     mrconfigid=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>>>     mrowner=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v,\
>>>>     mrownerconfig=ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8BI0VniavN7wEjRWeJq83v
>>>>
>>>> Signed-off-by: Isaku Yamahata <[e-mail]>
>>>> Co-developed-by: Xiaoyao Li <[e-mail]>
>>>> Signed-off-by: Xiaoyao Li <[e-mail]>
>>>> ---
>>>> Changes in v3:
>>>>    - use base64 encoding instread of hex-string;
>>>> ---
>>>>    qapi/qom.json         | 11 +++++-
>>>>    target/i386/kvm/tdx.c | 85 +++++++++++++++++++++++++++++++++++++++++++
>>>>    target/i386/kvm/tdx.h |  3 ++
>>>>    3 files changed, 98 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>>> index 3a29659e0155..fd99aa1ff8cc 100644
>>>> --- a/qapi/qom.json
>>>> +++ b/qapi/qom.json
>>>> @@ -888,10 +888,19 @@
>>>>   #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
>>>>   #     be set, otherwise they refuse to boot.
>>>>   #
>>>> +# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
>>>> +#
>>>> +# @mrowner: base64 encoded MROWNER SHA384 digest
>>>> +#
>>>> +# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
>>>
>>> Can we come up with a description that tells the user a bit more clearly
>>> what we're talking about?  Perhaps starting with this question could
>>> lead us there: what's an MRCONFIGID, and why should I care?
>>
>> Below are the definition from TDX spec:
>>
>> MRCONFIGID: Software-defined ID for non-owner-defined configuration of the guest TD – e.g., run-time or OS configuration.
>>
>> MROWNER: Software-defined ID for the guest TD’s owner
>>
>> MROWNERCONFIG: Software-defined ID for owner-defined configuration of the guest TD – e.g., specific to the workload rather than the run-time or OS
> 
> Have you considered using this for the doc comments?  I'd omit
> "software-defined" in this context.

sure. I will use them in the next version.

>> They are all attestation related, and input by users who launches the TD . Software inside TD can retrieve them with TDREPORT and verify if it is the expected value.
>>
>> MROWNER is to identify the owner of the TD, MROWNERCONFIG is to pass OWNER's configuration. And MRCONFIGID contains configuration specific to OS level instead of OWNER.
>>
>> Below is the explanation from Intel inside, hope it can get you more clear:
>>
>> "These are primarily intended for general purpose, configurable software in a minimal TD. So, not a legacy VM image cloud customer wanting to move their VM out into the cloud. Also it’s not necessarily the case that any workload will use them all.
>>
>> MROWNER is for declaring the owner of the TD. An example use case would be an vHSM TD. HSMs need to know who their administrative contact is. You could customize the HSM image and measurements, but then people can’t recognize that this is the vHSM product from XYZ. So you put the unmodified vHSM stack in the TD, which will include MRTD/RTMRs that reflect the vHSM, and the owner’s public key in MROWNER. Now, when the vHSM starts up, to determine who is authorized to send commands, it does a TDREPORT, and looks at MROWNER.
>>
>> Extending this model, there could be important configuration information from the owner. In that case, MROWNERCONFIG is set to the hash of the config file that the vHSM should accept.
>>
>> This results in an attestable environment that explicitly indicates that it’s a well recognized vHSM TD, being administered by MROWNER and loading the configuration information that matches MROWNERCONFIG.
>>
>> Extending this idea of configuration of generally recognized software, it could be that there is a shim OS under the vHSM that itself is configurable. So MRCONFIGID, which isn’t a great name, can include configuration information intended for the OS level. The ID is confusing, but MRCONFIGID was the name we used for this register for SGX, so we kept the name."
> 
> Include a reference to this document?

That was the email reply from internal attestation folks.

but I can add the link to this mail in the version.

>>>> +#
>>>>   # Since: 8.2
>>>>   ##
>>>>   { 'struct': 'TdxGuestProperties',
>>>> -  'data': { '*sept-ve-disable': 'bool' } }
>>>> +  'data': { '*sept-ve-disable': 'bool',
>>>> +            '*mrconfigid': 'str',
>>>> +            '*mrowner': 'str',
>>>> +            '*mrownerconfig': 'str' } }
>>>>    ##
>>>>    # @ThreadContextProperties:
>>> [...]
>>>
>

Patch

diff --git a/qapi/qom.json b/qapi/qom.json
index 3a29659e0155..fd99aa1ff8cc 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -888,10 +888,19 @@ 
 #     pages.  Some guest OS (e.g., Linux TD guest) may require this to
 #     be set, otherwise they refuse to boot.
 #
+# @mrconfigid: base64 encoded MRCONFIGID SHA384 digest
+#
+# @mrowner: base64 encoded MROWNER SHA384 digest
+#
+# @mrownerconfig: base64 MROWNERCONFIG SHA384 digest
+#
 # Since: 8.2
 ##
 { 'struct': 'TdxGuestProperties',
-  'data': { '*sept-ve-disable': 'bool' } }
+  'data': { '*sept-ve-disable': 'bool',
+            '*mrconfigid': 'str',
+            '*mrowner': 'str',
+            '*mrownerconfig': 'str' } }
 
 ##
 # @ThreadContextProperties:
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index 28b3c2765c86..b70efbcab738 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -13,6 +13,7 @@ 
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/base64.h"
 #include "qapi/error.h"
 #include "qom/object_interfaces.h"
 #include "standard-headers/asm-x86/kvm_para.h"
@@ -508,6 +509,8 @@  int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
     X86CPU *x86cpu = X86_CPU(cpu);
     CPUX86State *env = &x86cpu->env;
     struct kvm_tdx_init_vm *init_vm;
+    uint8_t *data;
+    size_t data_len;
     int r = 0;
 
     qemu_mutex_lock(&tdx_guest->lock);
@@ -518,6 +521,38 @@  int tdx_pre_create_vcpu(CPUState *cpu, Error **errp)
     init_vm = g_malloc0(sizeof(struct kvm_tdx_init_vm) +
                         sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES);
 
+#define SHA384_DIGEST_SIZE  48
+
+    if (tdx_guest->mrconfigid) {
+        data = qbase64_decode(tdx_guest->mrconfigid,
+                              strlen(tdx_guest->mrconfigid), &data_len, errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrconfigid");
+            return -1;
+        }
+        memcpy(init_vm->mrconfigid, data, data_len);
+    }
+
+    if (tdx_guest->mrowner) {
+        data = qbase64_decode(tdx_guest->mrowner,
+                              strlen(tdx_guest->mrowner), &data_len, errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrowner");
+            return -1;
+        }
+        memcpy(init_vm->mrowner, data, data_len);
+    }
+
+    if (tdx_guest->mrownerconfig) {
+        data = qbase64_decode(tdx_guest->mrownerconfig,
+                              strlen(tdx_guest->mrownerconfig), &data_len, errp);
+        if (!data || data_len != SHA384_DIGEST_SIZE) {
+            error_setg(errp, "TDX: failed to decode mrownerconfig");
+            return -1;
+        }
+        memcpy(init_vm->mrownerconfig, data, data_len);
+    }
+
     r = kvm_vm_enable_cap(kvm_state, KVM_CAP_MAX_VCPUS, 0, ms->smp.cpus);
     if (r < 0) {
         error_setg(errp, "Unable to set MAX VCPUS to %d", ms->smp.cpus);
@@ -567,6 +602,48 @@  static void tdx_guest_set_sept_ve_disable(Object *obj, bool value, Error **errp)
     }
 }
 
+static char * tdx_guest_get_mrconfigid(Object *obj, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    return g_strdup(tdx->mrconfigid);
+}
+
+static void tdx_guest_set_mrconfigid(Object *obj, const char *value, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    tdx->mrconfigid = g_strdup(value);
+}
+
+static char * tdx_guest_get_mrowner(Object *obj, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    return g_strdup(tdx->mrowner);
+}
+
+static void tdx_guest_set_mrowner(Object *obj, const char *value, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    tdx->mrconfigid = g_strdup(value);
+}
+
+static char * tdx_guest_get_mrownerconfig(Object *obj, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    return g_strdup(tdx->mrownerconfig);
+}
+
+static void tdx_guest_set_mrownerconfig(Object *obj, const char *value, Error **errp)
+{
+    TdxGuest *tdx = TDX_GUEST(obj);
+
+    tdx->mrconfigid = g_strdup(value);
+}
+
 /* tdx guest */
 OBJECT_DEFINE_TYPE_WITH_INTERFACES(TdxGuest,
                                    tdx_guest,
@@ -586,6 +663,14 @@  static void tdx_guest_init(Object *obj)
     object_property_add_bool(obj, "sept-ve-disable",
                              tdx_guest_get_sept_ve_disable,
                              tdx_guest_set_sept_ve_disable);
+    object_property_add_str(obj, "mrconfigid",
+                            tdx_guest_get_mrconfigid,
+                            tdx_guest_set_mrconfigid);
+    object_property_add_str(obj, "mrowner",
+                            tdx_guest_get_mrowner, tdx_guest_set_mrowner);
+    object_property_add_str(obj, "mrownerconfig",
+                            tdx_guest_get_mrownerconfig,
+                            tdx_guest_set_mrownerconfig);
 }
 
 static void tdx_guest_finalize(Object *obj)
diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h
index 432077723ac5..6e39ef3bac13 100644
--- a/target/i386/kvm/tdx.h
+++ b/target/i386/kvm/tdx.h
@@ -21,6 +21,9 @@  typedef struct TdxGuest {
 
     bool initialized;
     uint64_t attributes;    /* TD attributes */
+    char *mrconfigid;       /* base64 encoded sha348 digest */
+    char *mrowner;          /* base64 encoded sha348 digest */
+    char *mrownerconfig;    /* base64 encoded sha348 digest */
 } TdxGuest;
 
 #ifdef CONFIG_TDX