From mboxrd@z Thu Jan  1 00:00:00 1970
From: Luca Boccassi <bluca@debian.org>
Subject: Re: [PATCH 2/2] virtio: fix PCI config err handling
Date: Thu, 16 Aug 2018 19:49:43 +0100
Message-ID: <1534445383.5764.56.camel@debian.org>
References: <20180814143035.19640-1-bluca@debian.org>
 <20180816184750.30843-1-bluca@debian.org>
 <20180816184750.30843-2-bluca@debian.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Cc: maxime.coquelin@redhat.com, zhihong.wang@intel.com, tiwei.bie@intel.com,
 bruce.richardson@intel.com, brian.russell@intl.att.com
To: dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
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)
In-Reply-To: <20180816184750.30843-2-bluca@debian.org>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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