On 5/18/22 09:59, Haibin Huang wrote:
> the QMP capabilities:
> {"return":
> {
> "sgx": true,
> "section-size": 1024,
> "flc": true
> }
> }
>
> the domain capabilities:
> <sgx>
> <flc>yes</flc>
> <epc_size>1</epc_size>
> </sgx>
>
> Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> ---
> src/conf/schemas/domaincaps.rng | 22 +++-
> src/qemu/qemu_capabilities.c | 121 ++++++++++++++++++
> src/qemu/qemu_capabilities.h | 4 +
> src/qemu/qemu_capspriv.h | 4 +
> .../caps_6.2.0.x86_64.replies | 22 +++-
> .../caps_6.2.0.x86_64.xml | 5 +
> .../caps_7.0.0.x86_64.replies | 22 +++-
> .../caps_7.0.0.x86_64.xml | 5 +
> .../caps_7.1.0.x86_64.replies | 21 ++-
> 9 files changed, 213 insertions(+), 13 deletions(-)
>
> diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng
> index 9cbc2467ab..5ace30ae0d 100644
> --- a/src/conf/schemas/domaincaps.rng
> +++ b/src/conf/schemas/domaincaps.rng
> @@ -270,6 +270,9 @@
> <optional>
> <ref name="sev"/>
> </optional>
> + <optional>
> + <ref name='sgx'/>
> + </optional>
> </element>
> </define>
>
> @@ -330,7 +333,24 @@
> </element>
> </define>
>
> - <define name="value">
> + <define name='sgx'>
> + <element name='sgx'>
> + <ref name='supported'/>
> + <optional>
> + <element name='flc'>
> + <data type='string'/>
> + </element>
> + <element name='epc_size'>
> + <attribute name="unit">
> + <value>KiB</value>
> + </attribute>
> + <data type='unsignedInt'/>
> + </element>
> + </optional>
> + </element>
> + </define>
> +
This doesn't belong here. In this commit you are not touching domcaps at
all. Here you are just detecting SGX capabilities. Therefore, the commit
message should reflect that and this change should go into the commit
where domcaps are formatted.
> + <define name='value'>
I'm puzzled by this change. We use double quotes, so please stick with that.
> <zeroOrMore>
> <element name="value">
> <text/>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index a59d839d85..0d16762a0b 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -675,6 +675,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>
> /* 430 */
> "chardev.qemu-vdagent", /* QEMU_CAPS_CHARDEV_QEMU_VDAGENT */
> + "sgx-epc", /* QEMU_CAPS_SGX_EPC */
> );
>
>
> @@ -756,6 +757,8 @@ struct _virQEMUCaps {
>
> virSEVCapability *sevCapabilities;
>
> + virSGXCapability *sgxCapabilities;
> +
> /* Capabilities which may differ depending on the accelerator. */
> virQEMUCapsAccel kvm;
> virQEMUCapsAccel hvf;
> @@ -1398,6 +1401,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
> { "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
> { "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI },
> { "virtio-iommu-pci", QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI },
> + { "sgx-epc", QEMU_CAPS_SGX_EPC },
> };
>
>
> @@ -1974,6 +1978,22 @@ virQEMUCapsSEVInfoCopy(virSEVCapability **dst,
> }
>
>
> +static int
> +virQEMUCapsSGXInfoCopy(virSGXCapabilityPtr *dst,
> + virSGXCapabilityPtr src)
> +{
> + g_autoptr(virSGXCapability) tmp = NULL;
> +
> + tmp = g_new0(virSGXCapability, 1);
> +
> + tmp->flc = src->flc;
> + tmp->epc_size = src->epc_size;
> +
> + *dst = g_steal_pointer(&tmp);
> + return 0;
> +}
> +
> +
> static void
> virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccel *dst,
> virQEMUCapsAccel *src)
> @@ -2055,6 +2075,12 @@ virQEMUCaps *virQEMUCapsNewCopy(virQEMUCaps *qemuCaps)
> qemuCaps->sevCapabilities) < 0)
> return NULL;
>
> +
> + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC) &&
> + virQEMUCapsSGXInfoCopy(&ret->sgxCapabilities,
> + qemuCaps->sgxCapabilities) < 0)
> + return NULL;
> +
> return g_steal_pointer(&ret);
> }
>
> @@ -2618,6 +2644,13 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps)
> }
>
>
> +virSGXCapabilityPtr
> +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps)
> +{
> + return qemuCaps->sgxCapabilities;
> +}
> +
> +
> static int
> virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps,
> qemuMonitor *mon)
> @@ -3444,6 +3477,31 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps,
> }
>
>
> +static int
> +virQEMUCapsProbeQMPSGXCapabilities(virQEMUCaps *qemuCaps,
> + qemuMonitor *mon)
> +{
> + int rc = -1;
> + virSGXCapability *caps = NULL;
> +
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC))
> + return 0;
> +
> + if ((rc = qemuMonitorGetSGXCapabilities(mon, &caps)) < 0)
> + return -1;
> +
> + /* SGX isn't actually supported */
> + if (rc == 0) {
> + virQEMUCapsClear(qemuCaps, QEMU_CAPS_SGX_EPC);
> + return 0;
> + }
> +
> + virSGXCapabilitiesFree(qemuCaps->sgxCapabilities);
> + qemuCaps->sgxCapabilities = caps;
> + return 0;
> +}
> +
> +
> /*
> * Filter for features which should never be passed to QEMU. Either because
> * QEMU never supported them or they were dropped as they never did anything
> @@ -4222,6 +4280,42 @@ virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)
> }
>
>
> +static int
> +virQEMUCapsParseSGXInfo(virQEMUCaps *qemuCaps,
> + xmlXPathContextPtr ctxt)
> +{
> + g_autoptr(virSGXCapability) sgx = NULL;
> + g_autofree char *flc = NULL;
> +
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGX_EPC))
> + return 0;
> +
> + if (virXPathBoolean("boolean(./sgx)", ctxt) == 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing SGX platform data in QEMU capabilities cache"));
> + return -1;
> + }
> +
> + sgx = g_new0(virSGXCapability, 1);
> +
> + if ((!(flc = virXPathString("string(./sgx/flc)", ctxt))) ||
> + virStringParseYesNo(flc, &sgx->flc) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing or invalid SGX platform flc in QEMU capabilities cache"));
> + return -1;
> + }
> +
> + if (virXPathUInt("string(./sgx/epc_size)", ctxt, &sgx->epc_size) < 0) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing or malformed SGX platform epc_size in QEMU capabilities cache"));
> + return -1;
> + }
> +
> + qemuCaps->sgxCapabilities = g_steal_pointer(&sgx);
> + return 0;
> +}
> +
> +
> static int
> virQEMUCapsParseFlags(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)
> {
> @@ -4524,6 +4618,9 @@ virQEMUCapsLoadCache(virArch hostArch,
> if (virQEMUCapsParseSEVInfo(qemuCaps, ctxt) < 0)
> return -1;
>
> + if (virQEMUCapsParseSGXInfo(qemuCaps, ctxt) < 0)
> + return -1;
> +
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_KVM);
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_HVF))
> @@ -4709,6 +4806,25 @@ virQEMUCapsFormatSEVInfo(virQEMUCaps *qemuCaps, virBuffer *buf)
> }
>
>
> +static void
> +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps,
> + virBuffer *buf)
> +{
> + virSGXCapabilityPtr sgx = virQEMUCapsGetSGXCapabilities(qemuCaps);
> +
> + virBufferAddLit(buf, "<sgx>\n");
> + virBufferAdjustIndent(buf, 2);
> + if (sgx->flc) {
> + virBufferAsprintf(buf, "<flc>%s</flc>\n", "yes");
> + } else {
> + virBufferAsprintf(buf, "<flc>%s</flc>\n", "no");
> + }
> + virBufferAsprintf(buf, "<epc_size>%u</epc_size>\n", sgx->epc_size);
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</sgx>\n");
> +}
> +
> +
> char *
> virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
> {
> @@ -4790,6 +4906,9 @@ virQEMUCapsFormatCache(virQEMUCaps *qemuCaps)
> if (qemuCaps->sevCapabilities)
> virQEMUCapsFormatSEVInfo(qemuCaps, &buf);
>
> + if (qemuCaps->sgxCapabilities)
> + virQEMUCapsFormatSGXInfo(qemuCaps, &buf);
> +
> if (qemuCaps->kvmSupportsNesting)
> virBufferAddLit(&buf, "<kvmSupportsNesting/>\n");
>
> @@ -5457,6 +5576,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCaps *qemuCaps,
> return -1;
> if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0)
> return -1;
> + if (virQEMUCapsProbeQMPSGXCapabilities(qemuCaps, mon) < 0)
> + return -1;
>
> virQEMUCapsInitProcessCaps(qemuCaps);
>
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 59c09903f3..38ec3222dd 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -650,6 +650,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
>
> /* 430 */
> QEMU_CAPS_CHARDEV_QEMU_VDAGENT, /* -chardev qemu-vdagent */
> + QEMU_CAPS_SGX_EPC, /* -object sgx-epc,... */
>
> QEMU_CAPS_LAST /* this must always be the last item */
> } virQEMUCapsFlags;
> @@ -843,6 +844,9 @@ virQEMUCapsCPUFeatureFromQEMU(virQEMUCaps *qemuCaps,
> virSEVCapability *
> virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps);
>
> +virSGXCapabilityPtr
> +virQEMUCapsGetSGXCapabilities(virQEMUCaps *qemuCaps);
> +
> bool
> virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_GNUC_NO_INLINE;
>
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index f4f4a99d32..c632647a74 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -101,6 +101,10 @@ void
> virQEMUCapsSetSEVCapabilities(virQEMUCaps *qemuCaps,
> virSEVCapability *capabilities);
>
> +void
> +virQEMUCapsSetSGXCapabilities(virQEMUCaps *qemuCaps,
> + virSGXCapability *capabilities);
> +
This is useless. This function is never introduced and nothing calls it.
In fact, virQEMUCapsSetSEVCapabilities() shouldn't be there either.
> int
> virQEMUCapsProbeCPUDefinitionsTest(virQEMUCaps *qemuCaps,
> qemuMonitor *mon);
Michal
