Re: [PULL 15/28] hw/i386/x86-iommu: Remove X86IOMMUState::pt_supported field

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jun 18, 2026 at 09:55:48AM +0200, Peter Krempa wrote:
> On Thu, Mar 05, 2026 at 11:52:02 +0100, Thomas Huth wrote:
> > From: Philippe Mathieu-Daudé <philmd@linaro.org>
> > 
> > The X86IOMMUState::pt_supported boolean was only set in
> > the hw_compat_2_9[] array, via the 'pt=off' property. We
> > removed all machines using that array, lets remove that
> > property and all the code around it, always setting the
> > VTD_ECAP_PT capability.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Message-ID: <20250501210456.89071-19-philmd@linaro.org>
> > [thuth: Dropped the hunks that were already merged via commit 31753d5a336f]
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > Message-ID: <20260225092024.794595-16-thuth@redhat.com>
> > ---
> >  include/hw/i386/x86-iommu.h |  1 -
> >  hw/i386/intel_iommu.c       | 13 ++-----------
> >  hw/i386/x86-iommu.c         |  1 -
> >  3 files changed, 2 insertions(+), 13 deletions(-)
> 
> [...]
> 
> > diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> > index 33ac0bfc97e..a57445404ef 100644
> > --- a/hw/i386/x86-iommu.c
> > +++ b/hw/i386/x86-iommu.c
> > @@ -129,7 +129,6 @@ static const Property x86_iommu_properties[] = {
> >      DEFINE_PROP_ON_OFF_AUTO("intremap", X86IOMMUState,
> >                              intr_supported, ON_OFF_AUTO_AUTO),
> >      DEFINE_PROP_BOOL("device-iotlb", X86IOMMUState, dt_supported, false),
> > -    DEFINE_PROP_BOOL("pt", X86IOMMUState, pt_supported, true),
> >      DEFINE_PROP_BOOL("dma-translation", X86IOMMUState, dma_translation, true),
> 
> Sorry for necromancing this 3 months later.
> 
> This removes the 'pt' option from the 'amd-iommu' device too since
> it inherits from  X86IOMMUState. Libvirt does expose the setting for
> that option although I no longer remember why that was added.

IIUC, the 'pt' option was already dead code for the amd-iommu
device before this patch, per Thomas' mention of commit 31753d5a336f
which removed 'pt' logic from amd-iommu.

The AMD IOMMU device was unsupportable prior to f864a3235
because it could not have its PCI adderssing controlled.

IOW, 10.1.0 was the first QEMU release where libvirt could
use amd-iommu, and 'pt' logic was already removed from the
amd-iommu code at that point.

So, while libvirt offered a "pt" setting for amd-iommu, it
never had any effect.

Meanwhile libvirt did NOT offer a "pt" setting for intel-iommu,
despite it existing in QEMU until 11.0.0

> I'm not sure based on the commit message though whether that was an
> intended outcome.
> 
> When updating the libvirt dumps of capability probing I unfortunately
> mis-read the diff and thought that 'pt' was added instead of removed so
> I didn't pay any closer attention:
> 
>  https://gitlab.com/libvirt/libvirt/-/commit/75edc7ae98938ae0548e4c368d9c291d3763289a
> 
> IMO since device properties are visible from outside changes like this
> should be mentioned in the 'deprecated.rst' document.
> 
> Since this is just an optional setting it doesn't cause any problems for
> users who didn't set it though so it's not really too big of a problem.
> Libvirt will likely add a capability to reject it.

We don't need a capability - we can reject it for amd-iommu
unconditionally since it never had any effect AFAICT.

We could wire up "pt" for the intel-iommu though so we the XML
schema for it is not entirely useless :-)

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|