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

[Peter Xu <peterx@redhat.com>]

On Tue, Sep 11, 2018 at 11:49:45AM -0500, Brijesh Singh wrote:
>  static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>  {
>      AMDVIState *s = opaque;
> @@ -1055,6 +1151,12 @@ static AddressSpace *amdvi_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>          address_space_init(&iommu_as[devfn]->as,
>                             MEMORY_REGION(&iommu_as[devfn]->iommu),
>                             "amd-iommu");
> +        memory_region_init_io(&iommu_as[devfn]->iommu_ir, OBJECT(s),
> +                              &amdvi_ir_ops, s, "amd-iommu-ir",
> +                              AMDVI_INT_ADDR_SIZE);
> +        memory_region_add_subregion(MEMORY_REGION(&iommu_as[devfn]->iommu),
> +                              AMDVI_INT_ADDR_FIRST,
> +                              &iommu_as[devfn]->iommu_ir);

A pure question: just to make sure this IR region won't be masked out
by other memory regions.  Asked since VT-d is explicitly setting a
higher priority of the memory region for interrupts with
memory_region_add_subregion_overlap().

>      }
>      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).

Otherwise the patch looks good to me.  Thanks,

> +
>      /* set up MMIO */
>      memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
>                            AMDVI_MMIO_SIZE);
> @@ -1205,6 +1311,7 @@ static void amdvi_class_init(ObjectClass *klass, void* data)
>      dc->vmsd = &vmstate_amdvi;
>      dc->hotpluggable = false;
>      dc_class->realize = amdvi_realize;
> +    dc_class->int_remap = amdvi_int_remap;
>      /* Supported by the pc-q35-* machine types */
>      dc->user_creatable = true;
>  }

Regards,

-- 
Peter Xu