[v10,05/26] acpi: enable INTR for DMAR report structure

In ACPI DMA remapping report structure, enable INTR flag when specified.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/acpi-build.c          | 11 ++++++++++-
 include/hw/i386/intel_iommu.h |  2 ++
 2 files changed, 12 insertions(+), 1 deletion(-)

On Tue, Jun 21, 2016 at 03:47:33PM +0800, Peter Xu wrote:
> In ACPI DMA remapping report structure, enable INTR flag when specified.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  hw/i386/acpi-build.c          | 11 ++++++++++-
>  include/hw/i386/intel_iommu.h |  2 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 161f089..961ccd6a 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -57,6 +57,7 @@
>  
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
> +#include "hw/i386/x86-iommu.h"
>  
>  /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>   * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> @@ -2422,10 +2423,18 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
>  
>      AcpiTableDmar *dmar;
>      AcpiDmarHardwareUnit *drhd;
> +    uint8_t dmar_flags = 0;
> +    X86IOMMUState *iommu = x86_iommu_get_default();
> +
> +    assert(iommu);
> +    if (iommu->intr_supported) {
> +        /* enable INTR for the IOMMU device */
> +        dmar_flags |= DMAR_REPORT_F_INTR;

Please rewrite it: drop DMAR_REPORT_F_INTR macro,
and replace with literal + comment documenting
earliest spec version has it and the exact text
to look for in the spec.


> +    }
>  
>      dmar = acpi_data_push(table_data, sizeof(*dmar));
>      dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
> -    dmar->flags = 0;    /* No intr_remap for now */
> +    dmar->flags = dmar_flags;
>  
>      /* DMAR Remapping Hardware Unit Definition structure */
>      drhd = acpi_data_push(table_data, sizeof(*drhd));
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index e36b896..638d77f 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -44,6 +44,8 @@
>  #define VTD_HOST_ADDRESS_WIDTH      39
>  #define VTD_HAW_MASK                ((1ULL << VTD_HOST_ADDRESS_WIDTH) - 1)
>  
> +#define DMAR_REPORT_F_INTR          (1)
> +
>  typedef struct VTDContextEntry VTDContextEntry;
>  typedef struct VTDContextCacheEntry VTDContextCacheEntry;
>  typedef struct IntelIOMMUState IntelIOMMUState;
> -- 
> 2.4.11

On Mon, Jul 04, 2016 at 06:14:41PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 21, 2016 at 03:47:33PM +0800, Peter Xu wrote:
> > In ACPI DMA remapping report structure, enable INTR flag when specified.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  hw/i386/acpi-build.c          | 11 ++++++++++-
> >  include/hw/i386/intel_iommu.h |  2 ++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 161f089..961ccd6a 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -57,6 +57,7 @@
> >  
> >  #include "qapi/qmp/qint.h"
> >  #include "qom/qom-qobject.h"
> > +#include "hw/i386/x86-iommu.h"
> >  
> >  /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
> >   * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> > @@ -2422,10 +2423,18 @@ build_dmar_q35(GArray *table_data, BIOSLinker *linker)
> >  
> >      AcpiTableDmar *dmar;
> >      AcpiDmarHardwareUnit *drhd;
> > +    uint8_t dmar_flags = 0;
> > +    X86IOMMUState *iommu = x86_iommu_get_default();
> > +
> > +    assert(iommu);
> > +    if (iommu->intr_supported) {
> > +        /* enable INTR for the IOMMU device */
> > +        dmar_flags |= DMAR_REPORT_F_INTR;
> 
> Please rewrite it: drop DMAR_REPORT_F_INTR macro,
> and replace with literal + comment documenting
> earliest spec version has it and the exact text
> to look for in the spec.

For "literal", do you mean this?

   dmar_flags |= 0x1;

Could I ask why we need to drop the macro? There are two possible
flags here, bit 0 is for IR, bit 1 (not used in QEMU yet) for
X2APIC_OPT_OUT. Macros seem to be more clear. Did I miss anything?

Regarding to the comment, maybe:

"enable INTR for the IOMMU device. See VT-d spec 8.1 (any version
 newer than Oct. 2014)"

Thanks,

-- peterx