qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Singh, Brijesh" <brijesh.singh@amd.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
	"Singh, Brijesh" <brijesh.singh@amd.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"ehabkost@redhat.com" <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
Date: Fri, 9 Aug 2019 20:00:17 +0000	[thread overview]
Message-ID: <8f3e8479-11e5-d0eb-a1e1-cd166d84a85c@amd.com> (raw)
In-Reply-To: <20190808104834.GE2852@work-vm>


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

  reply	other threads:[~2019-08-09 20:01 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
2019-08-06 19:00   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow Singh, Brijesh
2019-08-07 11:01   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters Singh, Brijesh
2019-08-07 11:06   ` Dr. David Alan Gilbert
2019-08-08  2:25     ` Singh, Brijesh
2019-08-08 10:48       ` Dr. David Alan Gilbert
2019-08-09 20:00         ` Singh, Brijesh [this message]
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 04/14] linux-headers: update kernel header to include SEV migration commands Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state Singh, Brijesh
2019-08-07 16:14   ` Dr. David Alan Gilbert
2019-08-08  2:25     ` Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs Singh, Brijesh
2019-08-07 16:36   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 08/14] target/i386: sev: do not create launch context for an incoming guest Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context Singh, Brijesh
2019-08-08 11:19   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page Singh, Brijesh
2019-08-09 18:54   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page Singh, Brijesh
2019-08-13 17:38   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap Singh, Brijesh
2019-08-13 18:57   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages Singh, Brijesh
2019-08-14 16:37   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker Singh, Brijesh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8f3e8479-11e5-d0eb-a1e1-cd166d84a85c@amd.com \
    --to=brijesh.singh@amd.com \
    --cc=Thomas.Lendacky@amd.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).