Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters

[Dr. David Alan Gilbert]

* Singh, Brijesh (address@hidden) wrote:
> 
> On 8/7/19 6:06 AM, Dr. David Alan Gilbert wrote:
> > * Singh, Brijesh (address@hidden) wrote:
> >> AMD SEV migration flow requires that target machine's public Diffie-Hellman
> >> key (PDH) and certificate chain must be passed before initiating the guest
> >> migration. User can use QMP 'migrate-set-parameters' to pass the 
> >> certificate
> >> chain. The certificate chain will be used while creating the outgoing
> >> encryption context.
> >>
> >> Signed-off-by: Brijesh Singh <address@hidden>
> >> ---
> >>
> >> I was able to pass the certificate chain through the HMP but somehow
> >> QMP socket interface is not working for me. If anyone has any tips on
> >> what I am missing in the patch then please let me know. In meantime,
> >> I will also continue my investigation on why its not working for me.
> > It looks OK to me; what's the qmp you're trying and what's the failure
> > error?
> 

Before I forget, you've not updated hmp_info_migrate_parameters in
hmp-cmds.c, e.g.:

             MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
             params->has_tls_authz ? params->tls_authz : "");
+        monitor_printf(mon, "%s:'%s'\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_SEV_PDH),
+            params->has_sev_pdh ? params->sev_pdh : "");
     }

> I am not seeing any error. I am using the below command through qmp-shell.
> 
> (qmp) migrate-set-paramaters sev-pdh="...." sev-plat-cert="...."
> sev-amd-cert="..."
> 
> 
> The command does not return any error. I added some debugs in
> migrate_params_test_apply() and qmp_migrate_set_parameters() to see the
> valye of params->has_sev_pdh and its always zero. The functions are
> getting called when I issue the migrate-set-parameters qmp but the
> values are not passed hence the memory_encryption->setup() never gets
> called.

Driving QMP by hand I'm seeing it apparently be stored in the
parameters:

Escape character is '^]'.
{"QMP": {"version": {"qemu": {"micro": 94, "minor": 0, "major": 4}, "package": 
"v4.1.0-rc4-16-ge6e1f28afd-dirty"}, "capabilities": ["oob"]}}
{ "execute": "qmp_capabilities" }
{"return": {}}
{ "execute": "migrate-set-parameters" , "arguments": { "sev-pdh": "foo" } }
{"return": {}}

then from HMP with the patch above:
(qemu) info migrate_parameters 
....
sev-pdh:'foo'

Dave

> 
> > Dave
> >
> >>  migration/migration.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >>  monitor/hmp-cmds.c    | 18 +++++++++++++
> >>  qapi/migration.json   | 41 ++++++++++++++++++++++++++---
> >>  3 files changed, 116 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/migration/migration.c b/migration/migration.c
> >> index 8a607fe1e2..de66a0eb7e 100644
> >> --- a/migration/migration.c
> >> +++ b/migration/migration.c
> >> @@ -783,6 +783,12 @@ MigrationParameters 
> >> *qmp_query_migrate_parameters(Error **errp)
> >>      params->announce_rounds = s->parameters.announce_rounds;
> >>      params->has_announce_step = true;
> >>      params->announce_step = s->parameters.announce_step;
> >> +    params->has_sev_pdh = true;
> >> +    params->sev_pdh = g_strdup(s->parameters.sev_pdh);
> >> +    params->has_sev_plat_cert = true;
> >> +    params->sev_plat_cert = g_strdup(s->parameters.sev_plat_cert);
> >> +    params->has_sev_amd_cert = true;
> >> +    params->sev_amd_cert = g_strdup(s->parameters.sev_amd_cert);
> >>  
> >>      return params;
> >>  }
> >> @@ -1289,6 +1295,18 @@ static void 
> >> migrate_params_test_apply(MigrateSetParameters *params,
> >>      if (params->has_announce_step) {
> >>          dest->announce_step = params->announce_step;
> >>      }
> >> +    if (params->has_sev_pdh) {
> >> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> >> +        dest->sev_pdh = g_strdup(params->sev_pdh->u.s);
> >> +    }
> >> +    if (params->has_sev_plat_cert) {
> >> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> >> +        dest->sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
> >> +    }
> >> +    if (params->has_sev_amd_cert) {
> >> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> >> +        dest->sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> >> +    }
> >>  }
> >>  
> >>  static void migrate_params_apply(MigrateSetParameters *params, Error 
> >> **errp)
> >> @@ -1390,6 +1408,21 @@ static void 
> >> migrate_params_apply(MigrateSetParameters *params, Error **errp)
> >>      if (params->has_announce_step) {
> >>          s->parameters.announce_step = params->announce_step;
> >>      }
> >> +    if (params->has_sev_pdh) {
> >> +        g_free(s->parameters.sev_pdh);
> >> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_pdh = g_strdup(params->sev_pdh->u.s);
> >> +    }
> >> +    if (params->has_sev_plat_cert) {
> >> +        g_free(s->parameters.sev_plat_cert);
> >> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_plat_cert = 
> >> g_strdup(params->sev_plat_cert->u.s);
> >> +    }
> >> +    if (params->has_sev_amd_cert) {
> >> +        g_free(s->parameters.sev_amd_cert);
> >> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> >> +    }
> >>  }
> >>  
> >>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error 
> >> **errp)
> >> @@ -1410,6 +1443,27 @@ void 
> >> qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> >>          params->tls_hostname->type = QTYPE_QSTRING;
> >>          params->tls_hostname->u.s = strdup("");
> >>      }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_pdh
> >> +        && params->sev_pdh->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_pdh->u.n);
> >> +        params->sev_pdh->type = QTYPE_QSTRING;
> >> +        params->sev_pdh->u.s = strdup("");
> >> +    }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_plat_cert
> >> +        && params->sev_plat_cert->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_plat_cert->u.n);
> >> +        params->sev_plat_cert->type = QTYPE_QSTRING;
> >> +        params->sev_plat_cert->u.s = strdup("");
> >> +    }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_amd_cert
> >> +        && params->sev_amd_cert->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_amd_cert->u.n);
> >> +        params->sev_amd_cert->type = QTYPE_QSTRING;
> >> +        params->sev_amd_cert->u.s = strdup("");
> >> +    }
> >>  
> >>      migrate_params_test_apply(params, &tmp);
> >>  
> >> @@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
> >>      qemu_mutex_destroy(&ms->qemu_file_lock);
> >>      g_free(params->tls_hostname);
> >>      g_free(params->tls_creds);
> >> +    g_free(params->sev_pdh);
> >> +    g_free(params->sev_plat_cert);
> >> +    g_free(params->sev_amd_cert);
> >>      qemu_sem_destroy(&ms->rate_limit_sem);
> >>      qemu_sem_destroy(&ms->pause_sem);
> >>      qemu_sem_destroy(&ms->postcopy_pause_sem);
> >> @@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
> >>      params->has_announce_rounds = true;
> >>      params->has_announce_step = true;
> >>  
> >> +    params->sev_pdh = g_strdup("");
> >> +    params->sev_plat_cert = g_strdup("");
> >> +    params->sev_amd_cert = g_strdup("");
> >> +
> >>      qemu_sem_init(&ms->postcopy_pause_sem, 0);
> >>      qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
> >>      qemu_sem_init(&ms->rp_state.rp_sem, 0);
> >> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> >> index 5ca3ebe942..354219f27a 100644
> >> --- a/monitor/hmp-cmds.c
> >> +++ b/monitor/hmp-cmds.c
> >> @@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> >> QDict *qdict)
> >>          p->has_announce_step = true;
> >>          visit_type_size(v, param, &p->announce_step, &err);
> >>          break;
> >> +    case MIGRATION_PARAMETER_SEV_PDH:
> >> +        p->has_sev_pdh = true;
> >> +        p->sev_pdh = g_new0(StrOrNull, 1);
> >> +        p->sev_pdh->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
> >> +        break;
> >> +    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
> >> +        p->has_sev_plat_cert = true;
> >> +        p->sev_plat_cert = g_new0(StrOrNull, 1);
> >> +        p->sev_plat_cert->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
> >> +        break;
> >> +    case MIGRATION_PARAMETER_SEV_AMD_CERT:
> >> +        p->has_sev_amd_cert = true;
> >> +        p->sev_amd_cert = g_new0(StrOrNull, 1);
> >> +        p->sev_amd_cert->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
> >> +        break;
> >>      default:
> >>          assert(0);
> >>      }
> >> diff --git a/qapi/migration.json b/qapi/migration.json
> >> index 9cfbaf8c6c..bb07995d2c 100644
> >> --- a/qapi/migration.json
> >> +++ b/qapi/migration.json
> >> @@ -580,6 +580,15 @@
> >>  # @max-cpu-throttle: maximum cpu throttle percentage.
> >>  #                    Defaults to 99. (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in 
> >> base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded 
> >> in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  { 'enum': 'MigrationParameter',
> >> @@ -592,7 +601,7 @@
> >>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
> >>             'multifd-channels',
> >>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> >> -           'max-cpu-throttle' ] }
> >> +           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' 
> >> ] }
> >>  
> >>  ##
> >>  # @MigrateSetParameters:
> >> @@ -682,6 +691,15 @@
> >>  # @max-cpu-throttle: maximum cpu throttle percentage.
> >>  #                    The default value is 99. (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in 
> >> base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded 
> >> in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  # TODO either fuse back into MigrationParameters, or make
> >> @@ -707,7 +725,10 @@
> >>              '*multifd-channels': 'int',
> >>              '*xbzrle-cache-size': 'size',
> >>              '*max-postcopy-bandwidth': 'size',
> >> -      '*max-cpu-throttle': 'int' } }
> >> +            '*max-cpu-throttle': 'int',
> >> +            '*sev-pdh':'StrOrNull',
> >> +            '*sev-plat-cert': 'StrOrNull',
> >> +            '*sev-amd-cert' : 'StrOrNull' } }
> >>  
> >>  ##
> >>  # @migrate-set-parameters:
> >> @@ -817,6 +838,15 @@
> >>  #                    Defaults to 99.
> >>  #                     (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in 
> >> base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded 
> >> in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  { 'struct': 'MigrationParameters',
> >> @@ -839,8 +869,11 @@
> >>              '*block-incremental': 'bool' ,
> >>              '*multifd-channels': 'uint8',
> >>              '*xbzrle-cache-size': 'size',
> >> -      '*max-postcopy-bandwidth': 'size',
> >> -            '*max-cpu-throttle':'uint8'} }
> >> +            '*max-postcopy-bandwidth': 'size',
> >> +            '*max-cpu-throttle':'uint8',
> >> +            '*sev-pdh':'str',
> >> +            '*sev-plat-cert': 'str',
> >> +            '*sev-amd-cert' : 'str'} }
> >>  
> >>  ##
> >>  # @query-migrate-parameters:
> >> -- 
> >> 2.17.1
> >>
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK