[libvirt][PATCH v11 1/4] qemu: provide support to query the SGX capability

[Peter Krempa]

On Tue, May 10, 2022 at 23:11:09 -0700, Lin Yang wrote:
> From: Haibin Huang <haibin.huang at intel.com>
> 
> QEMU version >= 6.2.0 provides support for creating enclave on
> SGX x86 platform using Software Guard Extensions (SGX) feature.
> This patch adds support to query the SGX capability from the qemu.
> 
> Signed-off-by: Haibin Huang <haibin.huang at intel.com>
> ---
>  src/conf/domain_capabilities.c                |  10 ++
>  src/conf/domain_capabilities.h                |  13 ++
>  src/libvirt_private.syms                      |   1 +
>  src/qemu/qemu_capabilities.c                  | 119 ++++++++++++++++++
>  src/qemu/qemu_capabilities.h                  |   6 +
>  src/qemu/qemu_capspriv.h                      |   4 +
>  src/qemu/qemu_monitor.c                       |  10 ++
>  src/qemu/qemu_monitor.h                       |   3 +
>  src/qemu/qemu_monitor_json.c                  | 104 +++++++++++++--
>  src/qemu/qemu_monitor_json.h                  |   9 ++
>  .../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 +
>  14 files changed, 318 insertions(+), 15 deletions(-)

This is not a full review. Couple of points:

1) Do not mix other changes with adding QEMU_CAPS* stuff
    Basically theres waaay too much going on in this patch and it
    definitely can be separated into smaller chunks. The QEMU_CAPS is
    just one of them.
    Separate at least:
        - qemu monitor command introduction
        - domain capabilities data structs for sgx
        - parsing and formatting of the XML
        - adding of the QEMU_CAPS_ flag
2) caps for qemu-7.1 were added very recently
    You'll need to fix that one too since you added an extra query. Make
    sure that you _don't_ add the faking of SXG into that file, but
    rather the error case. My box doesn't support SGX so it will be
    overwritten in my next refresh anyways.

[...]

> @@ -4706,6 +4805,21 @@ virQEMUCapsFormatSEVInfo(virQEMUCaps *qemuCaps, virBuffer *buf)
>  }
>  
>  
> +static void
> +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps,
> +                         virBuffer *buf)
> +{
> +    virSGXCapabilityPtr sgx = virQEMUCapsGetSGXCapabilities(qemuCaps);
> +
> +    virBufferAddLit(buf, "<sgx>\n");
> +    virBufferAdjustIndent(buf, 2);
> +    virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no");

Don't use the ternary operator ('?'), use a full if/else branch instead
or pick a better data structure.

> +    virBufferAsprintf(buf, "<epc_size>%u</epc_size>\n", sgx->epc_size);
> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</sgx>\n");
> +}