Re: [Qemu-devel] [PATCH 1/9] Add stub functions for PCI device models to do PCI DMA

[Alexander Graf]

On 02.10.2011, at 13:17, Michael S. Tsirkin wrote:

> On Sun, Oct 02, 2011 at 12:58:35PM +0200, Avi Kivity wrote:
>> On 10/02/2011 12:52 PM, Michael S. Tsirkin wrote:
>>> On Sun, Oct 02, 2011 at 12:29:08PM +0200, Avi Kivity wrote:
>>>> On 10/02/2011 12:25 PM, Michael S. Tsirkin wrote:
>>>>> On Mon, Sep 05, 2011 at 02:34:56PM +1000, David Gibson wrote:
>>>>>>  This patch adds functions to pci.[ch] to perform PCI DMA operations.  At
>>>>>>  present, these are just stubs which perform directly cpu physical memory
>>>>>>  accesses.
>>>>>> 
>>>>>>  Using these stubs, however, distinguishes PCI device DMA transactions 
>>>>>> from
>>>>>>  other accesses to physical memory, which will allow PCI IOMMU support to
>>>>>>  be added in one place, rather than updating every PCI driver at that 
>>>>>> time.
>>>>>> 
>>>>>>  That is, it allows us to update individual PCI drivers to support an 
>>>>>> IOMMU
>>>>>>  without having yet determined the details of how the IOMMU emulation 
>>>>>> will
>>>>>>  operate.  This will let us remove the most bitrot-sensitive part of an
>>>>>>  IOMMU patch in advance.
>>>>>> 
>>>>>>  Signed-off-by: David Gibson<address@hidden>
>>>>> 
>>>>> So something I just thought about:
>>>>> 
>>>>> all wrappers now go through cpu_physical_memory_rw.
>>>>> This is a problem as e.g. virtio assumes that
>>>>> accesses such as stw are atomic. cpu_physical_memory_rw
>>>>> is a memcpy which makes no such guarantees.
>>>>> 
>>>> 
>>>> Let's change cpu_physical_memory_rw() to provide that guarantee for
>>>> aligned two and four byte accesses.  Having separate paths just for
>>>> that is not maintainable.
>>> 
>>> Well, we also have stX_phys convert to target native endian-ness
>>> (nop for KVM but not necessarily for qemu).
>>> 
>>> So if we do what you suggest, this patch will become more correct, but
>>> it would still need to duplicate the endian-ness work.
>>> 
>>> For that reason, I think calling stX_phys and friends from pci
>>> makes more sense - we get more simple inline wrappers
>>> but that code duplication worries me much less than tricky
>>> endian-ness hidden within a macro.
>>> 
>> 
>> Good point.  Though this is really a virtio specific issue since
>> other devices have explicit endianness (not guest dependent).
> 
> Hmm, not entirely virtio specific, some devices use stX macros to do the
> conversion.  E.g. stw_be_phys and stl_le_phys are used in several
> places.

Yes, explicit endianness. Virtio is the only device type we support in QEMU 
that changes its endianness depending on the guest CPU. All other devices are 
independent of the guest CPU we're targeting.


Alex