[RFC PATCH 6/7] qemu: force special features enabled for TDX guest

[Peter Krempa]

On Fri, Jun 18, 2021 at 16:50:51 +0800, Zhenzhong Duan wrote:
> TDX guest requires some special parameters in qemu command line.
> They are "pic=no,kernel_irqchip=split" without which guest fails to
> bootup.
> 
> PMU has a big impact to the performance of TDX guest. So always
> disable PMU except it's forcely enabled.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan at intel.com>
> ---
>  src/qemu/qemu_command.c  | 6 +++++-
>  src/qemu/qemu_validate.c | 6 ++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 891d795b02..bffa3fdf10 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6599,6 +6599,10 @@ qemuBuildCpuCommandLine(virCommand *cmd,
>          virTristateSwitch pmu = def->features[VIR_DOMAIN_FEATURE_PMU];
>          virBufferAsprintf(&buf, ",pmu=%s",
>                            virTristateSwitchTypeToString(pmu));
> +    } else if (!def->features[VIR_DOMAIN_FEATURE_PMU] && def->tdx) {
> +        /* PMU lead to performance drop if TDX enabled, disable PMU by default */
> +        virBufferAsprintf(&buf, ",pmu=%s",
> +                          virTristateSwitchTypeToString(VIR_TRISTATE_SWITCH_OFF));
>      }

This must be done in a way that will be visible in the XML rather than
silently hiding it in the command line generator code.

Also it feels very unjustified, it's bad for performance, but is it
guaranteed to always be like this?

>  
>      if (def->cpu && def->cpu->cache) {

[...]

> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 2efd011cc0..3c3a00c7e8 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -202,6 +202,12 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
>                      return -1;
>                  }
>              }
> +            if (def->tdx && (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP)
> +                             || def->features[i] != VIR_DOMAIN_IOAPIC_QEMU)) {

Our coding style usually has boolean operators at the end of the
previous line in multi-line statements.

> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("TDX guest needs split kernel irqchip"));
> +                return -1;
> +            }
>              break;
>  
>          case VIR_DOMAIN_FEATURE_HPT:
> -- 
> 2.25.1
>