Re: [Qemu-devel] [PATCH 2/2] i386: Interrupt remapping support for VT-d

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2016-03-10 06:28, Peter Xu wrote:
> On Wed, Mar 09, 2016 at 12:58:17AM +0530, Rita Sinha wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Still a bit hacky, unconditionally enabled (must become opt-in, not
>> available with in-kernel irqchip), not reporting faults properly - but
>> it works! And revealed a Linux bug [1]
> 
> If the patch is to be merged finally, shall we better add a
> parameter to disable this feature for people do not need this?

Yes, we need a switch. The original IOMMU in the Q35 chipset doesn't
support IR, and already to be emulation-wise precise, we should allow to
user to turn this off (or on).

> Also, shall we make sure:
> 
> - make sure patches' in-reply-to are correct (so that it's in a
>   series, as mentioned by Eric before)
> - remove useless lines like "/* printf(...) */"
> - add one-line subject for each patch (possibly)?
> - ...
> 
> [...]
> 
>> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
>> index 566e3d8..f7adc8e 100644
>> --- a/hw/pci-host/q35.c
>> +++ b/hw/pci-host/q35.c
>> @@ -431,6 +431,17 @@ static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
>>      assert(0 <= devfn && devfn <= VTD_PCI_DEVFN_MAX);
>>  
>>      vtd_as = vtd_find_add_as(s, bus, devfn);
>> +
>> +    memory_region_init_iommu(&vtd_as->iommu, OBJECT(s),
>> +                             &s->iommu_ops, "intel_iommu", UINT64_MAX);
>> +    address_space_init(&vtd_as->as,
>> +                             &vtd_as->iommu, "intel_iommu");
>> +    memory_region_init_io(&vtd_as->int_remap_region, OBJECT(s),
>> +                             &vtd_int_remap_ops, vtd_as,
>> +                             "intel_int_remap", UINT64_MAX);
>> +    address_space_init(&vtd_as->int_remap_as,
>> +                             &vtd_as->int_remap_region,
>> +                             "intel_int_remap");
> 
> One more thing... vtd_as->{as|iommu} should have been inited in
> vtd_find_add_as() already.
> 

Good point. Logical merge artefact.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux