Re: [Qemu-devel] [PATCH V3] virtio: do not require IOMMU to be created in advance

[Paolo Bonzini]

On 09/03/2017 16:31, Michael S. Tsirkin wrote:
> On Thu, Mar 09, 2017 at 11:05:36AM +0100, Paolo Bonzini wrote:
>>
>>
>> On 09/03/2017 10:58, Jason Wang wrote:
>>>
>>>
>>> On 2017年03月09日 17:28, Igor Mammedov wrote:
>>>> On Thu, 9 Mar 2017 10:32:44 +0800
>>>> Jason Wang<address@hidden>  wrote:
>>>>
>>>>> On 2017年03月09日 00:40, Igor Mammedov wrote:
>>>>>> On Tue, 7 Mar 2017 14:47:30 +0200
>>>>>> Marcel Apfelbaum<address@hidden>   wrote:
>>>>>>  
>>>>>>> On 03/07/2017 11:09 AM, Jason Wang wrote:
>>>>>>>> After commit 96a8821d2141 ("virtio: unbreak virtio-pci with IOMMU
>>>>>>>> after caching ring translations"), IOMMU was required to be
>>>>>>>> created in
>>>>>>>> advance. This is because we can only get the correct dma_as after pci
>>>>>>>> IOMMU (e.g intel_iommu) was initialized. This is suboptimal and
>>>>>>>> inconvenient for user. This patch releases this by:
>>>>>>>>
>>>>>>>> - introduce a bus_master_ready method for PCIDeviceClass and trigger
>>>>>>>>     this during pci_init_bus_master
>>>>>>>> - implement virtio-pci method and 1) reset the dma_as 2) re-register
>>>>>>>>     the memory listener to the new dma_as
>>>> Instead of trying to fix up later it's possible to refuse
>>>> adding iommu device if other devices has been added before
>>>> it with -device/device_add.
>>>> That would match current CLI semantics where device that
>>>> others depend on should be listed on CLI before that others
>>>> are listed.
>>>
>>> Yes, but it works by chance in the past for the device that does not
>>> want bus_master_as in their realize. This change may surprise their users.
>>
>> There is another posssibility.  Create the address space at init time
>> and add a container region instead of the bus_master_enable_region
>> alias.  Then at machine_done time you create the bus_master_enable_region.
>>
>> This removes the need for the callbacks and makes the MemoryListener
>> just work.
>>
>> Paolo
> 
> That's definitely cleaner than a callback, though a bit tricky
> so needs a good comment explaining what is going on.
> And then I think we can revert
> 96a8821d21411f10d77ea994af369c6e5c35a2cc, right?

Yes. 96a8821d21411f10d77ea994af369c6e5c35a2cc can go then (and this is
how I expected it to work all the time---my bad).

Paolo