Re: [dpdk-dev] [PATCH 2/2] virtio: fix PCI config err handling

[Luca Boccassi <bluca@debian.org>]

On Thu, 2018-08-16 at 19:47 +0100, Luca Boccassi wrote:
> From: Brian Russell <brussell@brocade.com>
> 
> 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 <brussell@brocade.com>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: handle additional rte_pci_read_config incomplete reads
> 
>  drivers/net/virtio/virtio_pci.c | 35 +++++++++++++++++++++--------
> ----
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> 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)
>  			hw->common_cfg = get_cfg_addr(dev, &cap);
>  			break;
>  		case VIRTIO_PCI_CAP_NOTIFY_CFG:
> -			rte_pci_read_config(dev, &hw-
> >notify_off_multiplier,
> -					4, pos + sizeof(cap));
> -			hw->notify_base = get_cfg_addr(dev, &cap);
> +			/* Avoid half-reads into hw */
> +			ret = rte_pci_read_config(dev, &multiplier,
> 4,
> +					pos + sizeof(cap));
> +			if (ret == 4) {
> +				hw->notify_off_multiplier =
> multiplier;
> +				hw->notify_base = get_cfg_addr(dev,
> &cap);
> +			}
>  			break;
>  		case VIRTIO_PCI_CAP_DEVICE_CFG:
>  			hw->dev_cfg = 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.

-- 
Kind regards,
Luca Boccassi