| Message ID | 20230818095041.1973309-21-xiaoyao.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TDX QEMU support | expand |
On Fri, Aug 18, 2023 at 05:50:03AM -0400, Xiaoyao Li wrote: > From: Isaku Yamahata <isaku.yamahata@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.yamahata@intel.com> > Signed-off-by: Xiaoyao Li <xiaoyao.li@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. 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 > +# > # 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 73da15377ec3..33d015a08c34 100644 > --- a/target/i386/kvm/tdx.c > +++ b/target/i386/kvm/tdx.c > @@ -521,6 +521,13 @@ int tdx_pre_create_vcpu(CPUState *cpu) > init_vm->cpuid.nent = kvm_x86_arch_cpuid(env, init_vm->cpuid.entries, 0); > init_vm->attributes = tdx_guest->attributes; > > + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrconfigid) != sizeof(tdx_guest->mrconfigid)); > + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrowner) != sizeof(tdx_guest->mrowner)); > + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrownerconfig) != sizeof(tdx_guest->mrownerconfig)); > + memcpy(init_vm->mrconfigid, tdx_guest->mrconfigid, sizeof(tdx_guest->mrconfigid)); > + memcpy(init_vm->mrowner, tdx_guest->mrowner, sizeof(tdx_guest->mrowner)); > + memcpy(init_vm->mrownerconfig, tdx_guest->mrownerconfig, sizeof(tdx_guest->mrownerconfig)); > + > do { > r = tdx_vm_ioctl(KVM_TDX_INIT_VM, 0, init_vm); > } while (r == -EAGAIN); > @@ -575,6 +582,12 @@ 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_sha384(obj, "mrconfigid", tdx->mrconfigid, > + OBJ_PROP_FLAG_READWRITE); > + object_property_add_sha384(obj, "mrowner", tdx->mrowner, > + OBJ_PROP_FLAG_READWRITE); > + object_property_add_sha384(obj, "mrownerconfig", tdx->mrownerconfig, > + OBJ_PROP_FLAG_READWRITE); > } > > static void tdx_guest_finalize(Object *obj) > diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h > index 46a24ee8c7cc..68f8327f2231 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 */ > + uint8_t mrconfigid[48]; /* sha348 digest */ > + uint8_t mrowner[48]; /* sha348 digest */ > + uint8_t mrownerconfig[48]; /* sha348 digest */ > } TdxGuest; > > #ifdef CONFIG_TDX > -- > 2.34.1 > With regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Fri, Aug 18, 2023 at 05:50:03AM -0400, Xiaoyao Li wrote: >> From: Isaku Yamahata <isaku.yamahata@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.yamahata@intel.com> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@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: [...]
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) +# # 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 73da15377ec3..33d015a08c34 100644 --- a/target/i386/kvm/tdx.c +++ b/target/i386/kvm/tdx.c @@ -521,6 +521,13 @@ int tdx_pre_create_vcpu(CPUState *cpu) init_vm->cpuid.nent = kvm_x86_arch_cpuid(env, init_vm->cpuid.entries, 0); init_vm->attributes = tdx_guest->attributes; + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrconfigid) != sizeof(tdx_guest->mrconfigid)); + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrowner) != sizeof(tdx_guest->mrowner)); + QEMU_BUILD_BUG_ON(sizeof(init_vm->mrownerconfig) != sizeof(tdx_guest->mrownerconfig)); + memcpy(init_vm->mrconfigid, tdx_guest->mrconfigid, sizeof(tdx_guest->mrconfigid)); + memcpy(init_vm->mrowner, tdx_guest->mrowner, sizeof(tdx_guest->mrowner)); + memcpy(init_vm->mrownerconfig, tdx_guest->mrownerconfig, sizeof(tdx_guest->mrownerconfig)); + do { r = tdx_vm_ioctl(KVM_TDX_INIT_VM, 0, init_vm); } while (r == -EAGAIN); @@ -575,6 +582,12 @@ 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_sha384(obj, "mrconfigid", tdx->mrconfigid, + OBJ_PROP_FLAG_READWRITE); + object_property_add_sha384(obj, "mrowner", tdx->mrowner, + OBJ_PROP_FLAG_READWRITE); + object_property_add_sha384(obj, "mrownerconfig", tdx->mrownerconfig, + OBJ_PROP_FLAG_READWRITE); } static void tdx_guest_finalize(Object *obj) diff --git a/target/i386/kvm/tdx.h b/target/i386/kvm/tdx.h index 46a24ee8c7cc..68f8327f2231 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 */ + uint8_t mrconfigid[48]; /* sha348 digest */ + uint8_t mrowner[48]; /* sha348 digest */ + uint8_t mrownerconfig[48]; /* sha348 digest */ } TdxGuest; #ifdef CONFIG_TDX