On Tue, Aug 21, 2018 at 11:20:27AM +0800, Han Han wrote:

Add sub-command nodesevinfo to get node infomation of AMD SEV feature.

Signed-off-by: Han Han <hhan@redhat.com> --- tools/virsh-host.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 5 ++++ 2 files changed, 71 insertions(+)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 16f504bafe..0bcd71a2b8 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -952,6 +952,67 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) return ret; }

+/* + * "nodesevinfo" command + */ +static const vshCmdInfo info_nodesevinfo[] = { + {.name = "help", + .data = N_("AMD SEV feature information.")

s/feature/platform Note that this is part of the "node" subsystem, which means it is always going to be tied to the firmware on a given node, thus "platform data".

+ }, + {.name = "desc", + .data = N_("Returns information of SEV feature about the node.")

Returns SEV platform-specific data.

+ }, + {.name = NULL} +}; + +static bool +cmdNodesevinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)

s/sev/SEV/

+{ + virTypedParameterPtr params = NULL; + int nparams = 0; + unsigned int flags = 0; + bool ret = false; + size_t i; + virshControlPtr priv = ctl->privData; + + if (nparams == 0) { + /* Get the number of SEV info parameters */ + if (virNodeGetSEVInfo(priv->conn, NULL, &nparams, flags) != 0) {

Have you actually tried the patches? Because ^this causes virsh to segfault because this is not a legacy API where you need to query the number of params first and then pre-allocate the pointer to store the data. We don't need that anymore, remote driver is already able to allocate the memory for the caller.

+ vshError(ctl, "%s", + _("Unable to get number of SEV info parameters")); + goto cleanup; + } + } + + if (nparams == 0) { + ret = true; + goto cleanup; + } + + /* Now get all the SEV info parameters */ + params = vshCalloc(ctl, nparams, sizeof(params));

no need for ^this...

+ if (virNodeGetSEVInfo(priv->conn, ¶ms, &nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to get SEV info parameters")); + goto cleanup; + } + + /* XXX: Need to sort the returned params once new parameter + * fields not of shared memory are added.

I'm not sure I see a problem ^here, can you elaborate please?

+ */ + vshPrint(ctl, _("SEV info:\n")); + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "\t%-15s %s\n", params[i].field, str); + VIR_FREE(str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + /* * "nodesuspend" command */ @@ -1900,6 +1961,11 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = info_nodememstats, .flags = 0 }, + {.name = "nodesevinfo", + .handler = cmdNodesevinfo, + .info = info_nodesevinfo, + .flags = 0 + }, {.name = "nodesuspend", .handler = cmdNodeSuspend, .opts = opts_node_suspend, diff --git a/tools/virsh.pod b/tools/virsh.pod index 4e118851f8..ea513c0acc 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -317,6 +317,11 @@ of cpu statistics during 1 second. Returns memory stats of the node. If I<cell> is specified, this will print the specified cell statistics only.

+=item B<nodesevinfo> + +Display AMD's SEV feature of this host, including PDH, cert-chain, cbitpos +and reduced-phys-bits.

Returns AMD's SEV platform-specific data. Availability of the fields depends on the version of the SEV firmware. Explanation of fields: pdh - Platform Diffie-Hellman key cert-chain - certificate chain used to verify authenticity of the platform cbitpos - C-bit position, i.e. which physical address bit marks protection on memory pages reduced-phys-bits - how many physical address bits we lost due to memory encryption Erik