Re: [Qemu-devel] [PATCH v2 4/8] x86_iommu/amd: Prepare for interrupt remap support

[Brijesh Singh]

On 09/17/2018 01:06 PM, Eduardo Habkost wrote:
...>>>>    #define TYPE_AMD_IOMMU_DEVICE "amd-iommu"
> > > >    #define AMD_IOMMU_DEVICE(obj)\
> > > > @@ -278,6 +288,9 @@ typedef struct AMDVIState {
> > > >        /* IOTLB */
> > > >        GHashTable *iotlb;
> > > > +
> > > > +    /* Interrupt remapping */
> > > > +    bool intr_enabled;
> > >
> > > Why do you need this field if the same info is already available
> > > at AMDVIState::iommu::intr_supported?
> >
> >
> > Again this is to be consistent with intel-iommu structure which has this
> > fields. Having said that I should be able to access the
> > AMDVIState::iommu::intr_supported and remove this new field.
>
> intel-iommu seems to need the field because intr_enabled can
> change at runtime at vtd_handle_gcmd_ire().  Does amd-iommu have
> anything equivalent?

Actually there is no MMIO control register writes to detect whether
the guest OS enabled the interrupt remap feature. I believe in case of
intel we are able to intercept some register writes which gives info
whether the IR is enabled by guest. In amd-iommu, only way to check if
whether guest OS has enabled the IR is by looking at DTE IV bit - this
lookup is done when we get request to remap the interrupt.

In summary, intr_enabled = iommu->intr_supported hence I also do
not see much value for having this field in AMDVIState.

-Brijesh