On Thu, Sep 10, 2026 at 05:58:33 +0000, Duan, Zhenzhong wrote:
>
>
> >-----Original Message-----
> >From: Peter Krempa <[email protected]>
> >Subject: Re: [PATCH v2 02/12] conf: Add scalable_mode attribute to iommu
> >
> >On Wed, Sep 09, 2026 at 16:29:49 +0800, Zhenzhong Duan wrote:
> >> Add scalable_mode attribute to iommu to enable/disable scalable mode
> >> for intel-iommu.
> >>
> >> Signed-off-by: Zhenzhong Duan <[email protected]>
> >> ---
> >> docs/formatdomain.rst | 4 ++
> >> src/conf/domain_conf.c | 16 ++++++++
> >> src/conf/domain_conf.h | 1 +
> >> src/conf/domain_validate.c | 3 ++
> >> src/conf/schemas/domaincommon.rng | 5 +++
> >> src/qemu/qemu_validate.c | 8 +++-
> >> ...tel-iommu-scalable-mode.x86_64-latest.args | 34 +++++++++++++++++
> >> ...ntel-iommu-scalable-mode.x86_64-latest.xml | 37 +++++++++++++++++++
> >> .../intel-iommu-scalable-mode.xml | 37 +++++++++++++++++++
> >> tests/qemuxmlconftest.c | 1 +
> >> 10 files changed, 145 insertions(+), 1 deletion(-)
> >> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-
> >mode.x86_64-latest.args
> >> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-
> >mode.x86_64-latest.xml
> >> create mode 100644 tests/qemuxmlconfdata/intel-iommu-scalable-mode.xml
> >
> >[...]
> >
> >
> >> @@ -22823,6 +22828,13 @@
> >virDomainIOMMUDefCheckABIStability(virDomainIOMMUDef *src,
> >>
> >> virTristateSwitchTypeToString(src->dma_translation));
> >> return false;
> >> }
> >> + if (src->scalable_mode != dst->scalable_mode) {
> >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >> + _("Target domain IOMMU device scalable mode '%1$s'
> >> does not
> >match source '%2$s'"),
> >
> >The error message should mention the attribute name verbatim:
> >
> > _("Target domain IOMMU device 'scalable_mode' '%1$s' does not match source
> >'%2$s'"),
>
> The existing code in virDomainIOMMUDefCheckABIStability() formats the
> attribute without quotes (i.e. ssid_size value rather than 'ssid_size' value).
> Should I follow existing code style or you prefer 'scalable_mode'?
If you want you can fix the existing checks in a separate patch. For the
new code please put them into quotes even if you decide to not change
the existing code.
>
> >
> >
> >Please apply this change to any further patch in the series.
>
> Sure.
>
> >
> >
> >> + virTristateSwitchTypeToString(dst->scalable_mode),
> >> + virTristateSwitchTypeToString(src->scalable_mode));
> >> + return false;
> >> + }
> >> if (src->pt != dst->pt) {
> >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >> _("Target domain IOMMU device passthrough '%1$s'
> >> does not
> >match source '%2$s'"),
> >
> >[...]
> >
> >> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> >> index e946f55351..55513b2691 100644
> >> --- a/src/qemu/qemu_validate.c
> >> +++ b/src/qemu/qemu_validate.c
> >> @@ -5815,7 +5815,7 @@ qemuValidateDomainDeviceDefIOMMU(const
> >virDomainIOMMUDef *iommu,
> >> return -1;
> >> }
> >> if (iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT &&
> >> - !virQEMUCapsGet(qemuCaps,
> >QEMU_CAPS_INTEL_IOMMU_CACHING_MODE)) {
> >> + !virQEMUCapsGet(qemuCaps,
> >QEMU_CAPS_INTEL_IOMMU_CACHING_MODE)) {
> >
> >This hunk is unrelated.
>
> Will delete it.
>
> >
> >
> >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> >> _("iommu: caching mode is not supported with this
> >> QEMU
> >binary"));
> >> return -1;
> >> @@ -5843,6 +5843,12 @@ qemuValidateDomainDeviceDefIOMMU(const
> >virDomainIOMMUDef *iommu,
> >> _("iommu: updating dma translation is not
> >> supported with this
> >QEMU binary"));
> >> return -1;
> >> }
> >> + if (iommu->scalable_mode != VIR_TRISTATE_SWITCH_ABSENT &&
> >
> >Missing newline before if.
>
> This is following existing code style in qemuValidateDomainDeviceDefIOMMU().
> I'm not sure, should I add new line starting from scalable_mode?
I see; keep it as is in your patch.
