On Fri, Oct 14, 2016 at 02:23:55PM +0800, Peter Xu wrote: > On Thu, Oct 13, 2016 at 04:16:03PM +0200, Alexander Gordeev wrote: > > On Thu, Oct 13, 2016 at 02:40:35PM +0800, Peter Xu wrote: > > > > > > + return pci_translate_addr(dev, addr); > > > > > > > > > > Raw question: do we need to translate bar addresses as well? > > > > > > > > I believe, yes. > > > > > > > > Unless we always have identity mapping between PCI address space and > > > > CPU physical address space I can not realize how could it be done > > > > otherwise. But even if we were, I would leave the translation routine > > > > for clarity. > > > > > > Sorry I didn't quite catch your point. Are we talking about IOMMU > > > address remapping here? IMHO BAR addresses are from CPU's point of > > > view. It's only used by CPU, not device. In that case, BAR address > > > should not be translated at least by IOMMU (no matter for x86/arm or > > > whatever). > > > > > > Take Linux as example: pci_ioremap_bar() is responsible to be called > > > for any PCI drivers to map device memory bars into kernel virtual > > > address space. Basically it does: > > > > > > void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar) > > > { > > > struct resource *res = &pdev->resource[bar]; > > > return ioremap_nocache(res->start, resource_size(res)); > > > } > > > > > > So as it is written: I believe we don't translate the bar address > > > (which should be res->start). We use it as physical address. > > > > > > Or, do you mean other kinds of translation that I don't aware of? > > > > Yes, I mean translation from PCI bus address space to CPU physical > > address space. These two busses are different and hence need a > > translation. I assume Linux pci_dev::resource[] have translated > > address, but it is not what PCI devices see. Unless I do not terribly > > missing somethig, BAR addresses is what a device sees on its AD[0..31] > > pins. > > I believe pci_dev::resource[] should be assigned by BIOS or something > before Linux. At that time, IOMMU is possibly even not inited. So no > chance for a translation at all. So pci-host-generic.c is "BIOS or something" in this context. There is no contradiction with the your excerpt. Please note "Figure 1-2: PCI System Block Diagram" in the spec. The CPU and PCI busses are two physically different busses interconnected by a bridge. A CPU data access to PCI devices initiates data access on a PCI bus, but adresses seen by the CPU and PCI devices do not necessarily match. So in case of x86 they do (lib/x86/asm/pci.h): static inline phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr) { return addr; } But in case of ARM they might not (lib/asm-generic/pci-host-bridge.h): static inline phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr) { /* * Assume we only have single PCI host bridge in a system. */ return pci_host_bridge_get_paddr(addr); } > If you see PCI Local Bus Spec Rev 3.0, chap 6.2.5.1: > > Power-up software needs to build a consistent address map before > booting the machine to an operating system. This means it has to > determine how much memory is in the system, and how much address > space the I/O controllers in the system require. After determining > this information, power-up software can map the I/O controllers > into reasonable locations and proceed with system boot. In order > to do this mapping in a device independent manner, the base > registers for this mapping are placed in the predefined header > portion of Configuration Space. > > BARs for each PCI devices should be pre-allocated during power-up > software, and a consistent map is built with the knowledge of existing > RAM in the system. Yep, see how pci_probe() / pci_alloc_resource() allocate and assign BARs to each device. It is the case for PPC/ARM but in case of x86 it is BIOS who does it. > If you boot a VM with/without IOMMU, you'll see that BAR addresses > won't change before/after enabling IOMMU. BAR addresses are in PCI bus address space. It sounds quite logical. (I am not experienced with IOMMU address translation, though). > Thanks, > > -- peterx -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html