* Singh, Brijesh (brijesh.si...@amd.com) wrote: > > On 8/7/19 6:06 AM, Dr. David Alan Gilbert wrote: > > * Singh, Brijesh (brijesh.si...@amd.com) 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 <brijesh.si...@amd.com> > >> --- > >> > >> 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 / dgilb...@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK