On Sun, Oct 30, 2011 at 06:20:46PM +0100, Alexander Graf wrote: > > On 14.10.2011, at 11:20, David Gibson wrote: > > > A while back, Eduard - Gabriel Munteanu send a series of patches > > implementing support for emulating the AMD IOMMU in conjunction with > > qemu emulated PCI devices. A revised patch series added support for > > the Intel IOMMU, and I also send a revised version of this series > > which added support for the hypervisor mediated IOMMU on the pseries > > machine. > > > > Richard Henderson also weighed in on the discussion, and there's still > > a cretain amount to be thrashed out in terms of exactly how to set up > > an IOMMU / DMA translation subsystem. > > > > However, really only 2 or 3 patches in any of these series have > > contained anything interesting. The rest of the series has been > > converting existing PCI emulated devices to use the new DMA interface > > which worked through the IOMMU translation, whatever it was. While we > > keep working out what we want for the guts of the IOMMU support, these > > device conversion patches keep bitrotting against updates to the > > various device implementations themselves. > > > > Really, regardless of whether we're actually implementing IOMMU > > translation, it makes sense that qemu code should distinguish between > > when it is really operating in CPU physical addresses and when it is > > operating in bus or DMA addresses which might have some kind of > > translation into physical addresses. > > > > This series, therefore, begins the conversion of existing PCI device > > emulation code to use new (stub) pci dma access functions. These are, > > for now, just defined to be untranslated cpu physical memory accesses, > > as before, but has three advantages: > > > > * It becomes obvious where the code is working with dma addresses, > > so it's easier to grep for what might be affected by an IOMMU or > > other bus address translation. > > > > * The new stubs take the PCIDevice *, from which any of the various > > suggested IOMMU interfaces should be able to locate the correct > > IOMMU translation context. > > > > * The new pci_dma_{read,write}() functions have a return value. > > When we do have IOMMU support, translation failures could lead to > > these functions failing, so we want a way to report it. > > > > This series converts all the easy cases. It doesn't yet handle > > devices which have both PCI and non-PCI variants, such as AHCI, OHCI > > and ne2k. Unlike the earlier version of this series, functions using > > scatter gather _are_ covered, though. > > Michael, ping?
It's unfortunate that v2 of the patchset doesn't include a changelog, which makes one reread all of the patchset before responding. I still think just making stX_phys/ldX_phys do memcpy is wrong: the former guaranteed atomic access, the later doesn't give any access order guarantees. We do need atomic versions IMO. If you think we need the non-ordered unaligned version of stl_XXX as well, the atomic versions could get an _atomic suffix, or the non-atomic versions _nonatomic suffix. I think I showed an example in e1000 which doesn't currently use stX_phys/ldX_phys but really should - otherwise the valid bit might get flipped before data is written (or it could do two cpu_physical_memory_XXX to ensure ordering but that's rather inconvenient IMO). intel-hda does something similar, I don't know whether ordering matters there, but this patch sneaks in a semantic change into what should have been a harmless rework. Also, could the stub functions be made inline? That'd make the intent clearer IMO. > > Alex