From patchwork Tue Aug 28 10:12:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Boccassi X-Patchwork-Id: 43922 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AC20B2C72; Tue, 28 Aug 2018 12:13:05 +0200 (CEST) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id B6CCE1D8A for ; Tue, 28 Aug 2018 12:13:04 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id 20-v6so1005258wrb.12 for ; Tue, 28 Aug 2018 03:13:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=8jA1rguQNQsWxOIHsHF/vAjDwGqJikL8hM7ycAKTsa4=; b=Ou21WytRzwLp3SUofY8JE1Pi1dYmHn7rxFj0nTc9I4y7dZKRspbgHFHibGWgOWPGiw BOSVT0LOHSwASW0v2vqs4AA0k/B3rZiwQKphX/af33TjfF2d7cgNNZFYP8BgYiMUScMI r38vWpL8VE8FAS6wUblwyCLjNZzp87vPQ5TQTSNGWTxltUdp/D8wx1M8DoMdCScG2bwQ ISNLZX7E6jwdfn5lmOJSTBm8mwQGIrisGXkQhB9NSW2odSfThXIAd1WEHMuO3OIaUhcx +R26iPVo2ho1OKNbheHHZoG0MRnZP1KzIvAj2aC+S2Cdka/HcdMbomwvmlw/1ioT62ti 5nwA== X-Gm-Message-State: APzg51CXhl/vk5AQXgzJhGFLbNRt0KIXua0cXR/p4/f255gDwCG8677n DNC1FlESreST7n8CFr8+c1L5DxiJMOo= X-Google-Smtp-Source: ANB0Vdak+DhXboqw5KVyQVJiONsAK0bsI32os4+kiqpEAvvvbxQDhpFYaukE+6nBYH7U8LSddcI3rA== X-Received: by 2002:adf:a41c:: with SMTP id d28-v6mr637551wra.121.1535451184008; Tue, 28 Aug 2018 03:13:04 -0700 (PDT) Received: from localhost ([2001:1be0:110d:fcfe:41aa:5bfa:6cf3:7531]) by smtp.gmail.com with ESMTPSA id 1-v6sm1611732wmf.47.2018.08.28.03.13.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Aug 2018 03:13:02 -0700 (PDT) From: Luca Boccassi To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, tiwei.bie@intel.com, bruce.richardson@intel.com, brian.russell@intl.att.com Date: Tue, 28 Aug 2018 11:12:39 +0100 Message-Id: <20180828101240.12597-1-bluca@debian.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180827165240.28322-1-bluca@debian.org> References: <20180827165240.28322-1-bluca@debian.org> Subject: [dpdk-dev] [PATCH v6 1/2] bus/pci: harmonize and document rte_pci_read_config return value X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Linux, rte_pci_read_config on success returns the number of read bytes, but on BSD it returns 0. Document the return values, and have BSD behave as Linux does. At least one case (bnx2x PMD) treats 0 as an error, so the change makes sense also for that. Signed-off-by: Luca Boccassi Acked-by: Bruce Richardson --- drivers/bus/pci/bsd/pci.c | 4 +++- drivers/bus/pci/rte_bus_pci.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c index 655b34b7e4..175d83cf1b 100644 --- a/drivers/bus/pci/bsd/pci.c +++ b/drivers/bus/pci/bsd/pci.c @@ -439,6 +439,8 @@ int rte_pci_read_config(const struct rte_pci_device *dev, { int fd = -1; int size; + /* Copy Linux implementation's behaviour */ + const int return_len = len; struct pci_io pi = { .pi_sel = { .pc_domain = dev->addr.domain, @@ -469,7 +471,7 @@ int rte_pci_read_config(const struct rte_pci_device *dev, } close(fd); - return 0; + return return_len; error: if (fd >= 0) diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h index 0d1955ffe0..df8f64798d 100644 --- a/drivers/bus/pci/rte_bus_pci.h +++ b/drivers/bus/pci/rte_bus_pci.h @@ -219,6 +219,8 @@ void rte_pci_unregister(struct rte_pci_driver *driver); * The length of the data buffer. * @param offset * The offset into PCI config space + * @return + * Number of bytes read on success, negative on error. */ int rte_pci_read_config(const struct rte_pci_device *device, void *buf, size_t len, off_t offset); From patchwork Tue Aug 28 10:12:40 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Boccassi X-Patchwork-Id: 43923 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 881BF4C9C; Tue, 28 Aug 2018 12:13:08 +0200 (CEST) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id D15274C99 for ; Tue, 28 Aug 2018 12:13:07 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id o37-v6so1017496wrf.6 for ; Tue, 28 Aug 2018 03:13:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=cSGVISiRIG2hk5dy+M5/DHquw92f06nMjW6X7nH3pkg=; b=JqXoe8Te9tSrkGLb5tS/V2G3JZxWrKXg9pOC3kBAx9mjWGSvdlJ46eNRwXeOwyJaVn vZnv1YqNj+7Tu0Xl6y1n1NGSp5LtclKGKwgWCAzvqaJSB+ahSFwBjf0Nxi1NB+dpa9Or 2U5TVJvPjUxECgbHkK1bQofQKz7AAVuDtJF6R7xGuq6STj5wNOVU7gh6/xmtDWCd2Iq/ 61tWpiBlhpvPn3rytVrcZ+DtNKQHTl6lkWFyjwzO6LgrVVmRbL2M7uOhUocDHEn2AwTS mxD+TgkT9cSjpUZ0XZ/mSThhUpREr+DVCnFAUDdybEVMKcNsACK1iHyD8VJphpzZ6ruy lxzw== X-Gm-Message-State: APzg51Bk0khI5sLl2X4Q+/VtduyUgwJKfCYyU2su3iU4lwF8mAZJa/3A 9otymMSYcx8SvJDj0MZxlsawO2KH7EY= X-Google-Smtp-Source: ANB0VdZFNr88R9v+i0qgYqwUdR0UTrZQk2GJLxIZkHKa6TNMH0nkeszTrUqwPQAkQLeeOuQXw9QWPg== X-Received: by 2002:adf:f906:: with SMTP id b6-v6mr658531wrr.28.1535451187273; Tue, 28 Aug 2018 03:13:07 -0700 (PDT) Received: from localhost ([2001:1be0:110d:fcfe:41aa:5bfa:6cf3:7531]) by smtp.gmail.com with ESMTPSA id g126-v6sm2338103wmg.5.2018.08.28.03.13.06 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Aug 2018 03:13:06 -0700 (PDT) From: Luca Boccassi To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, tiwei.bie@intel.com, bruce.richardson@intel.com, brian.russell@intl.att.com Date: Tue, 28 Aug 2018 11:12:40 +0100 Message-Id: <20180828101240.12597-2-bluca@debian.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180828101240.12597-1-bluca@debian.org> References: <20180827165240.28322-1-bluca@debian.org> <20180828101240.12597-1-bluca@debian.org> Subject: [dpdk-dev] [PATCH v6 2/2] net/virtio: fix PCI config err handling X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" From: Brian Russell In virtio_read_caps and vtpci_msix_detect, rte_pci_read_config returns the number of bytes read from PCI config or < 0 on error. If less than the expected number of bytes are read then log the failure and return rather than carrying on with garbage. Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0") Signed-off-by: Brian Russell Signed-off-by: Luca Boccassi Reviewed-by: Tiwei Bie --- v2: handle additional rte_pci_read_config incomplete reads v3: do not handle rte_pci_read_config of virtio cap, added in v2, as it's less clear what the right thing to do there is v4: do a more robust check - first check what the vendor is, and skip the cap entirely if it's not what we are looking for. v5: fetch only 2 flags bytes if the vndr is PCI_CAP_ID_MSIX v6: fix 32bit build by changing the printf format specifier, fix patch title drivers/net/virtio/virtio_pci.c | 65 ++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c index 6bd22e54a6..b6a3c80b4d 100644 --- a/drivers/net/virtio/virtio_pci.c +++ b/drivers/net/virtio/virtio_pci.c @@ -567,16 +567,18 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw) } ret = rte_pci_read_config(dev, &pos, 1, PCI_CAPABILITY_LIST); - if (ret < 0) { - PMD_INIT_LOG(DEBUG, "failed to read pci capability list"); + if (ret != 1) { + PMD_INIT_LOG(DEBUG, + "failed to read pci capability list, ret %d", ret); return -1; } while (pos) { - ret = rte_pci_read_config(dev, &cap, sizeof(cap), pos); - if (ret < 0) { - PMD_INIT_LOG(ERR, - "failed to read pci cap at pos: %x", pos); + ret = rte_pci_read_config(dev, &cap, 2, pos); + if (ret != 2) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos: %x ret %d", + pos, ret); break; } @@ -586,7 +588,16 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw) * 1st byte is cap ID; 2nd byte is the position of next * cap; next two bytes are the flags. */ - uint16_t flags = ((uint16_t *)&cap)[1]; + uint16_t flags; + + ret = rte_pci_read_config(dev, &flags, sizeof(flags), + pos + 2); + if (ret != sizeof(flags)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos:" + " %x ret %d", pos + 2, ret); + break; + } if (flags & PCI_MSIX_ENABLE) hw->use_msix = VIRTIO_MSIX_ENABLED; @@ -601,6 +612,14 @@ virtio_read_caps(struct rte_pci_device *dev, struct virtio_hw *hw) goto next; } + ret = rte_pci_read_config(dev, &cap, sizeof(cap), pos); + if (ret != sizeof(cap)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos: %x ret %d", + pos, ret); + break; + } + PMD_INIT_LOG(DEBUG, "[%2x] cfg type: %u, bar: %u, offset: %04x, len: %u", pos, cap.cfg_type, cap.bar, cap.offset, cap.length); @@ -689,25 +708,37 @@ enum virtio_msix_status vtpci_msix_detect(struct rte_pci_device *dev) { uint8_t pos; - struct virtio_pci_cap cap; int ret; ret = rte_pci_read_config(dev, &pos, 1, PCI_CAPABILITY_LIST); - if (ret < 0) { - PMD_INIT_LOG(DEBUG, "failed to read pci capability list"); + if (ret != 1) { + PMD_INIT_LOG(DEBUG, + "failed to read pci capability list, ret %d", ret); return VIRTIO_MSIX_NONE; } while (pos) { - ret = rte_pci_read_config(dev, &cap, sizeof(cap), pos); - if (ret < 0) { - PMD_INIT_LOG(ERR, - "failed to read pci cap at pos: %x", pos); + uint8_t cap[2]; + + ret = rte_pci_read_config(dev, cap, sizeof(cap), pos); + if (ret != sizeof(cap)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos: %x ret %d", + pos, ret); break; } - if (cap.cap_vndr == PCI_CAP_ID_MSIX) { - uint16_t flags = ((uint16_t *)&cap)[1]; + if (cap[0] == PCI_CAP_ID_MSIX) { + uint16_t flags; + + ret = rte_pci_read_config(dev, &flags, sizeof(flags), + pos + sizeof(cap)); + if (ret != sizeof(flags)) { + PMD_INIT_LOG(DEBUG, + "failed to read pci cap at pos:" + " %x ret %d", pos + 2, ret); + break; + } if (flags & PCI_MSIX_ENABLE) return VIRTIO_MSIX_ENABLED; @@ -715,7 +746,7 @@ vtpci_msix_detect(struct rte_pci_device *dev) return VIRTIO_MSIX_DISABLED; } - pos = cap.cap_next; + pos = cap[1]; } return VIRTIO_MSIX_NONE;