Re: [PATCH v3 10/26] target/arm/kvm-rme: Add Realm Personalization Value parameter

[Jean-Philippe Brucker]

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 :|
>