From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [kvm-unit-tests PATCH v7 06/13] pci: Rework pci_bar_addr() Date: Fri, 14 Oct 2016 14:23:55 +0800 Message-ID: <20161014062355.GA31091@pxdev.xzpeter.org> References: <38dc0bd6c1350568f4978dba3373249753944a61.1471434672.git.agordeev@redhat.com> <20160923071404.GB15411@pxdev.xzpeter.org> <20161012143754.GD23960@agordeev.lab.eng.brq.redhat.com> <20161013064035.GC21663@pxdev.xzpeter.org> <20161013141603.GE30384@agordeev.lab.eng.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: kvm@vger.kernel.org, Thomas Huth , Andrew Jones To: Alexander Gordeev Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40038 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751645AbcJNGX7 (ORCPT ); Fri, 14 Oct 2016 02:23:59 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EBEEEC05A286 for ; Fri, 14 Oct 2016 06:23:58 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20161013141603.GE30384@agordeev.lab.eng.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. 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. If you boot a VM with/without IOMMU, you'll see that BAR addresses won't change before/after enabling IOMMU. Thanks, -- peterx