On 8/8/19 5:48 AM, Dr. David Alan Gilbert wrote:
> * 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 : "");
> }
thanks, i will address it in next rev.
>> 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'
Hmm, I have been using scripts to automate all these certificating
passing. let me closely look into it and see if its script bug.
>>>> 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
