| Message ID | 20241125195626.856992-12-jean-philippe@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm: Run Arm CCA VMs with KVM | expand |
Jean-Philippe Brucker <jean-philippe@linaro.org> writes: > The Realm Personalization Value (RPV) is provided by the user to > distinguish Realms that have the same initial measurement. > > The user provides up to 64 hexadecimal bytes. They are stored into the > RPV in the same order, zero-padded on the right. > > Cc: Eric Blake <eblake@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Daniel P. Berrangé <berrange@redhat.com> > Cc: Eduardo Habkost <eduardo@habkost.net> > Acked-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > v2->v3: Fix documentation > --- > qapi/qom.json | 15 ++++++ > target/arm/kvm-rme.c | 111 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 126 insertions(+) > > diff --git a/qapi/qom.json b/qapi/qom.json > index a8beeabf1f..f982850bca 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -1068,6 +1068,19 @@ > 'data': { '*cpu-affinity': ['uint16'], > '*node-affinity': ['uint16'] } } > > +## > +# @RmeGuestProperties: > +# > +# Properties for rme-guest objects. > +# > +# @personalization-value: Realm personalization value, as a 64-byte > +# hex string. This optional parameter allows to uniquely identify > +# the VM instance during attestation. (default: 0) QMP commonly uses base64 for encoding binary data. Any particular reason to deviate? Is the "hex string" to be mapped to binary in little or big endian? Not an issue with base64. Nitpick: (default: all-zero), please, for consistency with similar documentation in SevSnpGuestProperties. > +# > +# Since: 9.3 > +## > +{ 'struct': 'RmeGuestProperties', > + 'data': { '*personalization-value': 'str' } } > > ## > # @ObjectType: > @@ -1121,6 +1134,7 @@ > { 'name': 'pr-manager-helper', > 'if': 'CONFIG_LINUX' }, > 'qtest', > + 'rme-guest', > 'rng-builtin', > 'rng-egd', > { 'name': 'rng-random', The commit message claims the patch adds a parameter. It actually adds an entire object type. > @@ -1195,6 +1209,7 @@ > 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', > 'if': 'CONFIG_LINUX' }, > 'qtest': 'QtestProperties', > + 'rme-guest': 'RmeGuestProperties', > 'rng-builtin': 'RngProperties', > 'rng-egd': 'RngEgdProperties', > 'rng-random': { 'type': 'RngRandomProperties', > diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c > index 83a29421df..0be55867ee 100644 > --- a/target/arm/kvm-rme.c > +++ b/target/arm/kvm-rme.c > @@ -23,11 +23,15 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST) > > #define RME_PAGE_SIZE qemu_real_host_page_size() > > +#define RME_MAX_CFG 1 > + > struct RmeGuest { > ConfidentialGuestSupport parent_obj; > Notifier rom_load_notifier; > GSList *ram_regions; > > + uint8_t *personalization_value; > + > hwaddr ram_base; > size_t ram_size; > }; > @@ -43,6 +47,48 @@ typedef struct { > > static RmeGuest *rme_guest; > > +static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp) > +{ > + int ret; > + const char *cfg_str; > + struct kvm_cap_arm_rme_config_item args = { > + .cfg = cfg, > + }; > + > + switch (cfg) { > + case KVM_CAP_ARM_RME_CFG_RPV: > + if (!guest->personalization_value) { > + return 0; > + } > + memcpy(args.rpv, guest->personalization_value, KVM_CAP_ARM_RME_RPV_SIZE); > + cfg_str = "personalization value"; > + break; > + default: > + g_assert_not_reached(); > + } This function gets called with @cfg arguments 0 .. RME_MAX_CFG-1, from rme_configure() right below. RME_MAX_CFG is defined to 1 above. The switch assumes KVM_CAP_ARM_RME_CFG_RPV is zero. Such assumptions are ideally avoided, and alternatively checked at build time. > + > + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0, > + KVM_CAP_ARM_RME_CONFIG_REALM, (intptr_t)&args); > + if (ret) { > + error_setg_errno(errp, -ret, "failed to configure %s", cfg_str); > + } > + return ret; > +} > + > +static int rme_configure(Error **errp) > +{ > + int ret; > + int cfg; > + > + for (cfg = 0; cfg < RME_MAX_CFG; cfg++) { > + ret = rme_configure_one(rme_guest, cfg, errp); > + if (ret) { > + return ret; > + } > + } > + return 0; > +} > + > static int rme_init_ram(hwaddr base, size_t size, Error **errp) > { > int ret; > @@ -123,6 +169,10 @@ static int rme_create_realm(Error **errp) > { > int ret; > > + if (rme_configure(errp)) { > + return -1; > + } > + > ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0, > KVM_CAP_ARM_RME_CREATE_RD); > if (ret) { > @@ -168,8 +218,69 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state) > } > } > > +static char *rme_get_rpv(Object *obj, Error **errp) > +{ > + RmeGuest *guest = RME_GUEST(obj); > + GString *s; > + int i; > + > + if (!guest->personalization_value) { > + return NULL; > + } > + > + s = g_string_sized_new(KVM_CAP_ARM_RME_RPV_SIZE * 2 + 1); > + > + for (i = 0; i < KVM_CAP_ARM_RME_RPV_SIZE; i++) { > + g_string_append_printf(s, "%02x", guest->personalization_value[i]); > + } The size of the destrination string is known at compile time. Why grow it dynamically? Base64 would take less code. > + > + return g_string_free(s, /* free_segment */ false); > +} > + > +static void rme_set_rpv(Object *obj, const char *value, Error **errp) > +{ > + RmeGuest *guest = RME_GUEST(obj); > + size_t len = strlen(value); > + uint8_t *out; > + int i = 1; > + int ret; > + > + g_free(guest->personalization_value); > + guest->personalization_value = out = g_malloc0(KVM_CAP_ARM_RME_RPV_SIZE); > + > + /* Two chars per byte */ > + if (len > KVM_CAP_ARM_RME_RPV_SIZE * 2) { > + error_setg(errp, "Realm Personalization Value is too large"); > + return; > + } > + > + /* First byte may have a single char */ > + if (len % 2) { > + ret = sscanf(value, "%1hhx", out++); > + } else { > + ret = sscanf(value, "%2hhx", out++); > + i++; > + } > + if (ret != 1) { > + error_setg(errp, "Invalid Realm Personalization Value"); > + return; > + } > + > + for (; i < len; i += 2) { > + ret = sscanf(value + i, "%2hhx", out++); > + if (ret != 1) { > + error_setg(errp, "Invalid Realm Personalization Value"); > + return; > + } > + } Looks like this supports hex strings shorter than KVM_CAP_ARM_RME_RPV_SIZE * 2. How these get padded is not documented. Fixable, but are you sure the convenience is worth the complexity? Again, base64 would take less code. > +} > + > static void rme_guest_class_init(ObjectClass *oc, void *data) > { > + object_class_property_add_str(oc, "personalization-value", rme_get_rpv, > + rme_set_rpv); > + object_class_property_set_description(oc, "personalization-value", > + "Realm personalization value (512-bit hexadecimal number)"); > } > > static void rme_guest_init(Object *obj)
On Tue, Nov 26, 2024 at 08:20:42AM +0100, Markus Armbruster wrote: > Jean-Philippe Brucker <jean-philippe@linaro.org> writes: > > > The Realm Personalization Value (RPV) is provided by the user to > > distinguish Realms that have the same initial measurement. > > > > The user provides up to 64 hexadecimal bytes. They are stored into the > > RPV in the same order, zero-padded on the right. > > > > Cc: Eric Blake <eblake@redhat.com> > > Cc: Markus Armbruster <armbru@redhat.com> > > Cc: Daniel P. Berrangé <berrange@redhat.com> > > Cc: Eduardo Habkost <eduardo@habkost.net> > > Acked-by: Markus Armbruster <armbru@redhat.com> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > --- > > v2->v3: Fix documentation > > --- > > qapi/qom.json | 15 ++++++ > > target/arm/kvm-rme.c | 111 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 126 insertions(+) > > > > diff --git a/qapi/qom.json b/qapi/qom.json > > index a8beeabf1f..f982850bca 100644 > > --- a/qapi/qom.json > > +++ b/qapi/qom.json > > @@ -1068,6 +1068,19 @@ > > 'data': { '*cpu-affinity': ['uint16'], > > '*node-affinity': ['uint16'] } } > > > > +## > > +# @RmeGuestProperties: > > +# > > +# Properties for rme-guest objects. > > +# > > +# @personalization-value: Realm personalization value, as a 64-byte > > +# hex string. This optional parameter allows to uniquely identify > > +# the VM instance during attestation. (default: 0) > > QMP commonly uses base64 for encoding binary data. Any particular > reason to deviate? > > Is the "hex string" to be mapped to binary in little or big endian? Not > an issue with base64. Agreed, using base64 is preferrable for consistency. > > Nitpick: (default: all-zero), please, for consistency with similar > documentation in SevSnpGuestProperties. > > > +# > > +# Since: 9.3 There is never any x.3 version of QEMU, as we bump the major version once a year. IOW, next release you could target this for will be 10.0 > > +## > > +{ 'struct': 'RmeGuestProperties', > > + 'data': { '*personalization-value': 'str' } } > > > > ## > > # @ObjectType: > > @@ -1121,6 +1134,7 @@ > > { 'name': 'pr-manager-helper', > > 'if': 'CONFIG_LINUX' }, > > 'qtest', > > + 'rme-guest', > > 'rng-builtin', > > 'rng-egd', > > { 'name': 'rng-random', > > The commit message claims the patch adds a parameter. It actually adds > an entire object type. The object should be added to qom.json earlier in this series when the RmeGuest class is defined, then this patch would merely be adding the parameter. > > > @@ -1195,6 +1209,7 @@ > > 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', > > 'if': 'CONFIG_LINUX' }, > > 'qtest': 'QtestProperties', > > + 'rme-guest': 'RmeGuestProperties', > > 'rng-builtin': 'RngProperties', > > 'rng-egd': 'RngEgdProperties', > > 'rng-random': { 'type': 'RngRandomProperties', With regards, Daniel
On Tue, Nov 26, 2024 at 08:20:42AM +0100, Markus Armbruster wrote: > > +# @personalization-value: Realm personalization value, as a 64-byte > > +# hex string. This optional parameter allows to uniquely identify > > +# the VM instance during attestation. (default: 0) > > QMP commonly uses base64 for encoding binary data. Any particular > reason to deviate? No, I think base64 is fine for this > > Is the "hex string" to be mapped to binary in little or big endian? Not > an issue with base64. (It was big endian with padding on the right) > > Nitpick: (default: all-zero), please, for consistency with similar > documentation in SevSnpGuestProperties. > > > +# > > +# Since: 9.3 > > +## > > +{ 'struct': 'RmeGuestProperties', > > + 'data': { '*personalization-value': 'str' } } > > > > ## > > # @ObjectType: > > @@ -1121,6 +1134,7 @@ > > { 'name': 'pr-manager-helper', > > 'if': 'CONFIG_LINUX' }, > > 'qtest', > > + 'rme-guest', > > 'rng-builtin', > > 'rng-egd', > > { 'name': 'rng-random', > > The commit message claims the patch adds a parameter. It actually adds > an entire object type. Yes as Daniel noted this belongs in an earlier patch > > > @@ -1195,6 +1209,7 @@ > > 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', > > 'if': 'CONFIG_LINUX' }, > > 'qtest': 'QtestProperties', > > + 'rme-guest': 'RmeGuestProperties', > > 'rng-builtin': 'RngProperties', > > 'rng-egd': 'RngEgdProperties', > > 'rng-random': { 'type': 'RngRandomProperties', > > diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c > > index 83a29421df..0be55867ee 100644 > > --- a/target/arm/kvm-rme.c > > +++ b/target/arm/kvm-rme.c > > @@ -23,11 +23,15 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST) > > > > #define RME_PAGE_SIZE qemu_real_host_page_size() > > > > +#define RME_MAX_CFG 1 > > + > > struct RmeGuest { > > ConfidentialGuestSupport parent_obj; > > Notifier rom_load_notifier; > > GSList *ram_regions; > > > > + uint8_t *personalization_value; > > + > > hwaddr ram_base; > > size_t ram_size; > > }; > > @@ -43,6 +47,48 @@ typedef struct { > > > > static RmeGuest *rme_guest; > > > > +static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp) > > +{ > > + int ret; > > + const char *cfg_str; > > + struct kvm_cap_arm_rme_config_item args = { > > + .cfg = cfg, > > + }; > > + > > + switch (cfg) { > > + case KVM_CAP_ARM_RME_CFG_RPV: > > + if (!guest->personalization_value) { > > + return 0; > > + } > > + memcpy(args.rpv, guest->personalization_value, KVM_CAP_ARM_RME_RPV_SIZE); > > + cfg_str = "personalization value"; > > + break; > > + default: > > + g_assert_not_reached(); > > + } > > This function gets called with @cfg arguments 0 .. RME_MAX_CFG-1, from > rme_configure() right below. RME_MAX_CFG is defined to 1 above. > > The switch assumes KVM_CAP_ARM_RME_CFG_RPV is zero. Such assumptions > are ideally avoided, and alternatively checked at build time. Ok, I'll change this Thanks, Jean
On Tue, Nov 26, 2024 at 12:47:59PM +0000, Daniel P. Berrangé wrote: > On Tue, Nov 26, 2024 at 08:20:42AM +0100, Markus Armbruster wrote: > > Jean-Philippe Brucker <jean-philippe@linaro.org> writes: > > > > > The Realm Personalization Value (RPV) is provided by the user to > > > distinguish Realms that have the same initial measurement. > > > > > > The user provides up to 64 hexadecimal bytes. They are stored into the > > > RPV in the same order, zero-padded on the right. > > > > > > Cc: Eric Blake <eblake@redhat.com> > > > Cc: Markus Armbruster <armbru@redhat.com> > > > Cc: Daniel P. Berrangé <berrange@redhat.com> > > > Cc: Eduardo Habkost <eduardo@habkost.net> > > > Acked-by: Markus Armbruster <armbru@redhat.com> > > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > > --- > > > v2->v3: Fix documentation > > > --- > > > qapi/qom.json | 15 ++++++ > > > target/arm/kvm-rme.c | 111 +++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 126 insertions(+) > > > > > > diff --git a/qapi/qom.json b/qapi/qom.json > > > index a8beeabf1f..f982850bca 100644 > > > --- a/qapi/qom.json > > > +++ b/qapi/qom.json > > > @@ -1068,6 +1068,19 @@ > > > 'data': { '*cpu-affinity': ['uint16'], > > > '*node-affinity': ['uint16'] } } > > > > > > +## > > > +# @RmeGuestProperties: > > > +# > > > +# Properties for rme-guest objects. > > > +# > > > +# @personalization-value: Realm personalization value, as a 64-byte > > > +# hex string. This optional parameter allows to uniquely identify > > > +# the VM instance during attestation. (default: 0) > > > > QMP commonly uses base64 for encoding binary data. Any particular > > reason to deviate? > > > > Is the "hex string" to be mapped to binary in little or big endian? Not > > an issue with base64. > > Agreed, using base64 is preferrable for consistency. Ack > > > > > Nitpick: (default: all-zero), please, for consistency with similar > > documentation in SevSnpGuestProperties. > > > > > +# > > > +# Since: 9.3 > > There is never any x.3 version of QEMU, as we bump the major > version once a year. IOW, next release you could target this > for will be 10.0 Ok good to know Thanks, Jean > > > > +## > > > +{ 'struct': 'RmeGuestProperties', > > > + 'data': { '*personalization-value': 'str' } } > > > > > > ## > > > # @ObjectType: > > > @@ -1121,6 +1134,7 @@ > > > { 'name': 'pr-manager-helper', > > > 'if': 'CONFIG_LINUX' }, > > > 'qtest', > > > + 'rme-guest', > > > 'rng-builtin', > > > 'rng-egd', > > > { 'name': 'rng-random', > > > > The commit message claims the patch adds a parameter. It actually adds > > an entire object type. > > The object should be added to qom.json earlier in this series when > the RmeGuest class is defined, then this patch would merely be adding > the parameter. > > > > > > @@ -1195,6 +1209,7 @@ > > > 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', > > > 'if': 'CONFIG_LINUX' }, > > > 'qtest': 'QtestProperties', > > > + 'rme-guest': 'RmeGuestProperties', > > > 'rng-builtin': 'RngProperties', > > > 'rng-egd': 'RngEgdProperties', > > > 'rng-random': { 'type': 'RngRandomProperties', > > 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 :| >
diff --git a/qapi/qom.json b/qapi/qom.json index a8beeabf1f..f982850bca 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -1068,6 +1068,19 @@ 'data': { '*cpu-affinity': ['uint16'], '*node-affinity': ['uint16'] } } +## +# @RmeGuestProperties: +# +# Properties for rme-guest objects. +# +# @personalization-value: Realm personalization value, as a 64-byte +# hex string. This optional parameter allows to uniquely identify +# the VM instance during attestation. (default: 0) +# +# Since: 9.3 +## +{ 'struct': 'RmeGuestProperties', + 'data': { '*personalization-value': 'str' } } ## # @ObjectType: @@ -1121,6 +1134,7 @@ { 'name': 'pr-manager-helper', 'if': 'CONFIG_LINUX' }, 'qtest', + 'rme-guest', 'rng-builtin', 'rng-egd', { 'name': 'rng-random', @@ -1195,6 +1209,7 @@ 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', 'if': 'CONFIG_LINUX' }, 'qtest': 'QtestProperties', + 'rme-guest': 'RmeGuestProperties', 'rng-builtin': 'RngProperties', 'rng-egd': 'RngEgdProperties', 'rng-random': { 'type': 'RngRandomProperties', diff --git a/target/arm/kvm-rme.c b/target/arm/kvm-rme.c index 83a29421df..0be55867ee 100644 --- a/target/arm/kvm-rme.c +++ b/target/arm/kvm-rme.c @@ -23,11 +23,15 @@ OBJECT_DECLARE_SIMPLE_TYPE(RmeGuest, RME_GUEST) #define RME_PAGE_SIZE qemu_real_host_page_size() +#define RME_MAX_CFG 1 + struct RmeGuest { ConfidentialGuestSupport parent_obj; Notifier rom_load_notifier; GSList *ram_regions; + uint8_t *personalization_value; + hwaddr ram_base; size_t ram_size; }; @@ -43,6 +47,48 @@ typedef struct { static RmeGuest *rme_guest; +static int rme_configure_one(RmeGuest *guest, uint32_t cfg, Error **errp) +{ + int ret; + const char *cfg_str; + struct kvm_cap_arm_rme_config_item args = { + .cfg = cfg, + }; + + switch (cfg) { + case KVM_CAP_ARM_RME_CFG_RPV: + if (!guest->personalization_value) { + return 0; + } + memcpy(args.rpv, guest->personalization_value, KVM_CAP_ARM_RME_RPV_SIZE); + cfg_str = "personalization value"; + break; + default: + g_assert_not_reached(); + } + + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0, + KVM_CAP_ARM_RME_CONFIG_REALM, (intptr_t)&args); + if (ret) { + error_setg_errno(errp, -ret, "failed to configure %s", cfg_str); + } + return ret; +} + +static int rme_configure(Error **errp) +{ + int ret; + int cfg; + + for (cfg = 0; cfg < RME_MAX_CFG; cfg++) { + ret = rme_configure_one(rme_guest, cfg, errp); + if (ret) { + return ret; + } + } + return 0; +} + static int rme_init_ram(hwaddr base, size_t size, Error **errp) { int ret; @@ -123,6 +169,10 @@ static int rme_create_realm(Error **errp) { int ret; + if (rme_configure(errp)) { + return -1; + } + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_RME, 0, KVM_CAP_ARM_RME_CREATE_RD); if (ret) { @@ -168,8 +218,69 @@ static void rme_vm_state_change(void *opaque, bool running, RunState state) } } +static char *rme_get_rpv(Object *obj, Error **errp) +{ + RmeGuest *guest = RME_GUEST(obj); + GString *s; + int i; + + if (!guest->personalization_value) { + return NULL; + } + + s = g_string_sized_new(KVM_CAP_ARM_RME_RPV_SIZE * 2 + 1); + + for (i = 0; i < KVM_CAP_ARM_RME_RPV_SIZE; i++) { + g_string_append_printf(s, "%02x", guest->personalization_value[i]); + } + + return g_string_free(s, /* free_segment */ false); +} + +static void rme_set_rpv(Object *obj, const char *value, Error **errp) +{ + RmeGuest *guest = RME_GUEST(obj); + size_t len = strlen(value); + uint8_t *out; + int i = 1; + int ret; + + g_free(guest->personalization_value); + guest->personalization_value = out = g_malloc0(KVM_CAP_ARM_RME_RPV_SIZE); + + /* Two chars per byte */ + if (len > KVM_CAP_ARM_RME_RPV_SIZE * 2) { + error_setg(errp, "Realm Personalization Value is too large"); + return; + } + + /* First byte may have a single char */ + if (len % 2) { + ret = sscanf(value, "%1hhx", out++); + } else { + ret = sscanf(value, "%2hhx", out++); + i++; + } + if (ret != 1) { + error_setg(errp, "Invalid Realm Personalization Value"); + return; + } + + for (; i < len; i += 2) { + ret = sscanf(value + i, "%2hhx", out++); + if (ret != 1) { + error_setg(errp, "Invalid Realm Personalization Value"); + return; + } + } +} + static void rme_guest_class_init(ObjectClass *oc, void *data) { + object_class_property_add_str(oc, "personalization-value", rme_get_rpv, + rme_set_rpv); + object_class_property_set_description(oc, "personalization-value", + "Realm personalization value (512-bit hexadecimal number)"); } static void rme_guest_init(Object *obj)