Re: [Qemu-devel] [PATCH 2/6] x86_iommu/amd: Prepare for interrupt remap support

[Suravee Suthikulpanit]

Brijesh / Peter,

On 9/13/18 10:15 AM, Peter Xu wrote:
> On Wed, Sep 12, 2018 at 01:59:06PM -0500, Brijesh Singh wrote:
>
> [...]
>
> > > >        }
> > > >        return &iommu_as[devfn]->as;
> > > >    }
> > > > @@ -1172,6 +1274,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
> > > >            return;
> > > >        }
> > > > +    /* Pseudo address space under root PCI bus. */
> > > > +    pcms->ioapic_as = amdvi_host_dma_iommu(bus, s, AMDVI_SB_IOAPIC_ID);
> > > > +    s->intr_enabled = x86_iommu->intr_supported;
> > >
> > > So does this mean that AMD IR cannot be disabled if declared support?
> > > For VT-d, IR needs to be explicitly enabled otherwise disabled (even
> > > supported).
> >
> >
> > Yes, once its declared as supported then it can not disabled. Its
> > upto the guest OS to decide whether it want to use the intr remapping
> > feature by parsing the IVRS. This also brings question, should we
> > just enable it by default because its guest OS decision whether it
> > wants to use it or not.
>
> It's by default off?  I mean:
>
>      DEFINE_PROP_BOOL("intremap", X86IOMMUState, intr_supported, false),
>
> Then it's good to me, thanks.  You could add a comment there
> explicitly mentioning that "IR will be enabled as long as declared
> support for AMD" since it might confuse some of the people like me...
> but it's optional.
>
> Regards,

Actually, there are two separate knobs here:

* This option basically means the interrupt remapping support is 
available/unavailable,
  which should default to available unless specified to unavailable at the QEMU 
command line.
  For example, the AMD IOMMU v1 does not have interrupt remapping support, 
which means
  the interrupt remapping bit fields in DTE (e.g. IV, IntCtl, EIntPass, 
INITPass, NMIPass,
  Lint0Pass, Lint1Pass) are ignored.

* If interrupt remapping support is available, the guest OS can decide to 
enable or disable the feature
  using DTE bit fileds (e.g. Linux option intremap=off would disable interrupt 
remapping
  by not setting DTE[IV] bit). For Linux AMD IOMMU driver, the default is to 
enable the interrupt remapping.

In facts, we should not need this option. However, if you prefer to keep this 
option,
we probably should rename this to "intremap_sup", in which if the default value 
should be 1.

Regards,
Suravee