From patchwork Tue Feb 28 18:08:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Gordeev X-Patchwork-Id: 9596493 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 274F960574 for ; Tue, 28 Feb 2017 18:09:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2069328552 for ; Tue, 28 Feb 2017 18:09:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1563828556; Tue, 28 Feb 2017 18:09:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A410F28552 for ; Tue, 28 Feb 2017 18:09:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751668AbdB1SJL (ORCPT ); Tue, 28 Feb 2017 13:09:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59819 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751344AbdB1SI4 (ORCPT ); Tue, 28 Feb 2017 13:08:56 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (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 AF40081F01 for ; Tue, 28 Feb 2017 18:08:37 +0000 (UTC) Received: from dhcp-27-118.brq.redhat.com (dhcp-27-122.brq.redhat.com [10.34.27.122]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1SI8WNp012601; Tue, 28 Feb 2017 13:08:36 -0500 From: Alexander Gordeev To: kvm@vger.kernel.org Cc: Alexander Gordeev , Thomas Huth , Andrew Jones , Peter Xu Subject: [kvm-unit-tests PATCH v4 2/5] pci: Accomodate 64 bit BARs in pci_dev::resource[] Date: Tue, 28 Feb 2017 19:08:27 +0100 Message-Id: <9184b3fc479b6d6ee41219c7419b4830819e5f29.1488304691.git.agordeev@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 28 Feb 2017 18:08:37 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Array pci_dev::resource[] is ambiguous wrt what is actually stored in its elements and how to interpret the index into the array. It is simple in case a device has only 32-bit BARs - an element pci_dev::resource[bar_num] contains the decoded address of BAR # bar_num. But what if a device has 64-bit BAR starting at bar_num? Curretnly pci_dev::resource[bar_num] contains the decoded address of the BAR, while pci_dev::resource[bar_num + 1] contains 0. That makes meaning of (bar_num + 1) index difficult to understand. On the one hand a testcase should not address a 64-bit BAR using high part BAR number. Particularly, when it tries to obtan BAR size or address. On the other hand a testcase should be able to access a low and high parts separately if it needs to for whatever reason. The rest of the API allows that as well. pci_dev::resource[] contains decoded 32/64-bit BAR addresses, not low/high parts of underlying 32-bit device BARs. Yet, indexes into this array correspond to raw 32-bit BARs in the PCI device configuration space. Thus, a question arises - what should be stored in array elements that correspond to high-parts of 64-bit BARs? Zero is particularly bad choice, because: - it is a valid address in PIO address space, so it can not stand for "no value" or NULL or whatever marker could be used to indicate a high part; - the high part of underlying 64-bit address is (could be) non-zero. So there is inconsistency also; By placing the same 64-bit address in both bar_num and (bar_num + 1) elements the ambiguity is less striking, since: - the meaning of bar_num kept consistent with the rest of the functions (where it refers 32-bit BAR in terms of the device configuration address space); - pci_dev::resource[bar_num + 1] contains a valid address rather than vague value of 0. - both bar_num and (bar_num + 1) indexes refer to the same 64-bit BAR and therefore return the same address; The notion of low and high parts of a 64-bit address is ignored, but that is fine, since pci_dev::resource[] contain only full addresses; Cc: Thomas Huth Cc: Andrew Jones Cc: Peter Xu Reviewed-by: Andrew Jones Signed-off-by: Alexander Gordeev --- lib/pci.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/pci.c b/lib/pci.c index 62b1c6b0b7c5..9acc9652cb25 100644 --- a/lib/pci.c +++ b/lib/pci.c @@ -331,12 +331,15 @@ void pci_scan_bars(struct pci_dev *dev) int i; for (i = 0; i < PCI_BAR_NUM; i++) { - if (!pci_bar_is_valid(dev, i)) - continue; - dev->resource[i] = pci_bar_get_addr(dev, i); - if (pci_bar_is64(dev, i)) { - i++; - dev->resource[i] = (phys_addr_t)0; + if (pci_bar_size(dev, i)) { + dev->resource[i] = pci_bar_get_addr(dev, i); + if (pci_bar_is64(dev, i)) { + assert(i + 1 < PCI_BAR_NUM); + dev->resource[i + 1] = dev->resource[i]; + i++; + } + } else { + dev->resource[i] = INVALID_PHYS_ADDR; } } }