Re: [Qemu-devel] Re: [RFC PATCH 4/7] ide: IOMMU support

[Anthony Liguori <anthony@codemonkey.ws>]

On 07/14/2010 03:13 PM, Paul Brook wrote:
>> On Wed, Jul 14, 2010 at 02:53:03PM +0100, Paul Brook wrote:
>>      
>>>> Memory accesses must go through the IOMMU layer.
>>>>          
>>> No. Devices should not know or care whether an IOMMU is present.
>>>        
>> There are real devices that care very much about an IOMMU. Basically all
>> devices supporting ATS care about that. So I don't see a problem if the
>> device emulation code of qemu also cares about present IOMMUs.
>>
>>      
>>> You should be adding a DeviceState argument to
>>> cpu_physical_memory_{rw,map}. This should then handle IOMMU translation
>>> transparently.
>>>        
>> That's not a good idea imho. With an IOMMU the device no longer accesses
>> cpu physical memory. It accesses device virtual memory. Using
>> cpu_physical_memory* functions in device code becomes misleading when
>> the device virtual address space differs from cpu physical.
>>      
> Well, ok, the function name needs fixing too.  However I think the only thing
> missing from the current API is that it does not provide a way to determine
> which device is performing the access.
>    

I agree with Paul.

The right approach IMHO is to convert devices to use bus-specific 
functions to access memory.  The bus specific functions should have a 
device argument as the first parameter.

For PCI-based IOMMUs, the implementation exists solely within the PCI 
bus.  For platforms (like SPARC) that have lower level IOMMUs, we would 
need to probably introduce a sysbus memory access layer and then provide 
a hook to implement an IOMMU there.

> Depending how the we decide to handle IOMMU invalidation, it may also be
> necessary to augment the memory_map API to allow the system to request a
> mapping be revoked.  However this issue is not specific to the IOMMU
> implementation. Such bugs are already present on any system that allows
> dynamic reconfiguration of the address space, e.g. by changing PCI BARs.
>    

That's why the memory_map API today does not allow mappings to persist 
after trips back to the main loop.

Regards,

Anthony Liguori

>> So different
>> functions for devices make a lot of sense here. Another reason for
>> seperate functions is that we can extend them later to support emulation
>> of ATS devices.
>>      
> I disagree. ATS should be an independent feature, and is inherently bus
> specific.  As usual the PCI spec is not publicly available, but based on the
> AMD IOMMU docs I'd say that ATS is completely independent of memory accesses -
> the convention being that you trust an ATS capable device to DTRT, and
> configure the bus IOMMU to apply a flat mapping for accesses from such
> devices.
>
>    
>>> You also need to accomodate the the case where multiple IOMMU are
>>> present.
>>>        
>> This, indeed, is something transparent to the device. This should be
>> handled inside the iommu emulation code.
>>      
> I think you've got the abstraction boundaries all wrong.
>
> A device performs a memory access on its local bus. It has no knowledge of how
> that access is routed to its destination.  The device should not be aware of
> any IOMMUs, in the same way that it doesn't know whether it happens to be
> accessing RAM or memory mapped peripherals on another device.
>
> Each IOMMU is fundamentally part of a bus bridge. For example the bridge
> between a PCI bus and the system bus. It provides a address mapping from one
> bus to another.
>
> There should be no direct interaction between an IOMMU and a device (ignoring
> ATS, which is effectively a separate data channel).  Everything should be done
> via the cpu_phsycial_memory_* code.  Likewise on a system with multiple nested
> IOMMUs there should be no direct interatcion between these.
> cpu_physical_memory_* should walk the device/bus tree to determine where the
> access terminates, applying mappings appropriately.
>
> Paul
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>