-----Original Message----- From: Peter Krempa <pkrempa@redhat.com> Sent: Friday, June 18, 2021 7:22 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com> Cc: libvir-list@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; Tian, Jun J <jun.j.tian@intel.com>; Qiang, Chenyi <chenyi.qiang@intel.com> Subject: Re: [RFC PATCH 6/7] qemu: force special features enabled for TDX guest
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@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. Thanks for your suggestion, we will drop this chunk.
Also it feels very unjustified, it's bad for performance, but is it guaranteed to always be like this?
No, we are optimizing the performance continuously, I believe the performance will be better and better.
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.
Will fix. Thanks Zhenzhong