> -----Original Message-----
> From: Peter Krempa <pkrempa@xxxxxxxxxx>
> Sent: Friday, June 18, 2021 7:22 PM
> To: Duan, Zhenzhong <zhenzhong.duan@xxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx; Yamahata, Isaku <isaku.yamahata@xxxxxxxxx>;
> Tian, Jun J <jun.j.tian@xxxxxxxxx>; Qiang, Chenyi <chenyi.qiang@xxxxxxxxx>
> 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@xxxxxxxxx>
> > ---
> > 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
