From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <luca.boccassi@gmail.com>
Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66])
 by dpdk.org (Postfix) with ESMTP id DF8704C6F
 for <dev@dpdk.org>; Thu, 16 Aug 2018 20:49:45 +0200 (CEST)
Received: by mail-wm0-f66.google.com with SMTP id o18-v6so5272266wmc.0
 for <dev@dpdk.org>; Thu, 16 Aug 2018 11:49:45 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to
 :references:content-transfer-encoding:mime-version;
 bh=BT2waaijRbIqUiRiEgO5TLihSIdOnMNQ9Ea2uzlfF2w=;
 b=hjoLJRBZD0lHc2AxtzsAFmoUxP6vUB/cR/FWImCHNfFQINMYw/Um4Negqr4J0ZyYmQ
 PnQtYD/NSif3Jo9fGV8Q9gAJQKhVhGP6M2jSfTwRsAEOVMKI4cl+PZZC10M22onhHviu
 kRChInqBANKYQDJmQABMLazPestMx7PoGPmdCV0ZpooR+uyj6/uTZ/Dpjt3PP8ltZ/Br
 yX+yfEfjElPdDYX6pls6DdGTzPEFGeKZqFgPXUsK4q+IaA9KxoEkOpxx0FUhGQTEJFdW
 iXeVrhyL/xo7NuttniMO8Vbq2/RvsI2yHqkig5omX5yOg6V1CVq/4Q3OObJ3XZVOQkRO
 /KPg==
X-Gm-Message-State: AOUpUlHpGZy32y9wsMArXiRnSjdW1rz+A2IzyMyd9WaXbn69oFOYQ+3s
 txGU1JO+jSCw4HS6hxSr5dVSBC7p
X-Google-Smtp-Source: AA+uWPxTBHewg4pkoA2Ec3lvxUvTGR0eHq02C9mGM+ge/yf3DcFGUgz6momDbYPma/vqwTwSNwv3qQ==
X-Received: by 2002:a1c:b609:: with SMTP id
 g9-v6mr15751477wmf.73.1534445385346; 
 Thu, 16 Aug 2018 11:49:45 -0700 (PDT)
Received: from localhost ([2a01:4b00:f419:6f00:8361:8946:ba2b:d556])
 by smtp.gmail.com with ESMTPSA id e12-v6sm79358wrt.29.2018.08.16.11.49.44
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Thu, 16 Aug 2018 11:49:44 -0700 (PDT)
Message-ID: <1534445383.5764.56.camel@debian.org>
From: Luca Boccassi <bluca@debian.org>
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: Thu, 16 Aug 2018 19:49:43 +0100
In-Reply-To: <20180816184750.30843-2-bluca@debian.org>
References: <20180814143035.19640-1-bluca@debian.org>
 <20180816184750.30843-1-bluca@debian.org>
 <20180816184750.30843-2-bluca@debian.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Evolution 3.22.6-1+deb9u1 
Mime-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 2/2] 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Aug 2018 18:49:46 -0000

On Thu, 2018-08-16 at 19:47 +0100, Luca Boccassi wrote:
> From: Brian Russell <brussell@brocade.com>
>=20
> 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.
>=20
> Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")
>=20
> Signed-off-by: Brian Russell <brussell@brocade.com>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: handle additional rte_pci_read_config incomplete reads
>=20
> =C2=A0drivers/net/virtio/virtio_pci.c | 35 +++++++++++++++++++++--------
> ----
> =C2=A01 file changed, 22 insertions(+), 13 deletions(-)
>=20
> diff --git a/drivers/net/virtio/virtio_pci.c
> b/drivers/net/virtio/virtio_pci.c
> index 6bd22e54a6..ff6f96f361 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
...
> @@ -610,9 +613,13 @@ virtio_read_caps(struct rte_pci_device *dev,
> struct virtio_hw *hw)
> =C2=A0			hw->common_cfg =3D get_cfg_addr(dev, &cap);
> =C2=A0			break;
> =C2=A0		case VIRTIO_PCI_CAP_NOTIFY_CFG:
> -			rte_pci_read_config(dev, &hw-
> >notify_off_multiplier,
> -					4, pos + sizeof(cap));
> -			hw->notify_base =3D get_cfg_addr(dev, &cap);
> +			/* Avoid half-reads into hw */
> +			ret =3D rte_pci_read_config(dev, &multiplier,
> 4,
> +					pos + sizeof(cap));
> +			if (ret =3D=3D 4) {
> +				hw->notify_off_multiplier =3D
> multiplier;
> +				hw->notify_base =3D get_cfg_addr(dev,
> &cap);
> +			}
> =C2=A0			break;
> =C2=A0		case VIRTIO_PCI_CAP_DEVICE_CFG:
> =C2=A0			hw->dev_cfg =3D get_cfg_addr(dev, &cap);

Tiwei: not 100% sure what's the best way to handle a failure here, this
will avoid writing to *hw in case of errors. Let me know if this is OK.

--=20
Kind regards,
Luca Boccassi