Date:   Fri, 3 Feb 2023 03:14:28 -0500
From:   "Michael S. Tsirkin" <mst@redhat.com>
To:     Si-Wei Liu <si-wei.liu@oracle.com>
Cc:     jasowang@redhat.com, parav@nvidia.com, elic@nvidia.com,
        virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 6/7] vdpa/mlx5: conditionally show MTU and STATUS in
 config space
Message-ID: <20230203030944-mutt-send-email-mst@kernel.org>
References: <1675207345-22328-1-git-send-email-si-wei.liu@oracle.com>
 <1675207345-22328-7-git-send-email-si-wei.liu@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1675207345-22328-7-git-send-email-si-wei.liu@oracle.com>
Precedence: bulk

On Tue, Jan 31, 2023 at 03:22:24PM -0800, Si-Wei Liu wrote:
> The spec says:
>     mtu only exists if VIRTIO_NET_F_MTU is set
>     status only exists if VIRTIO_NET_F_STATUS is set
> 
> We should only show MTU and STATUS conditionally depending on
> the feature bits.
> 
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>

so change the subject pls. it seems to say you are showing them
when you previously didn't, what's going on is something like:

	make MTU/status access conditional on feature bits

> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 3a6dbbc6..3d49eae 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -3009,6 +3009,8 @@ static int event_handler(struct notifier_block *nb, unsigned long event, void *p
>  	struct mlx5_vdpa_wq_ent *wqent;
>  
>  	if (event == MLX5_EVENT_TYPE_PORT_CHANGE) {
> +		if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_STATUS)))
> +			return NOTIFY_DONE;
>  		switch (eqe->sub_type) {
>  		case MLX5_PORT_CHANGE_SUBTYPE_DOWN:
>  		case MLX5_PORT_CHANGE_SUBTYPE_ACTIVE:
> @@ -3118,16 +3120,20 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
>  			goto err_alloc;
>  	}
>  
> -	err = query_mtu(mdev, &mtu);
> -	if (err)
> -		goto err_alloc;
> +	if (device_features & BIT_ULL(VIRTIO_NET_F_MTU)) {
> +		err = query_mtu(mdev, &mtu);
> +		if (err)
> +			goto err_alloc;
>  
> -	ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, mtu);
> +		ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, mtu);
> +	}
>  
> -	if (get_link_state(mvdev))
> -		ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP);
> -	else
> -		ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, ~VIRTIO_NET_S_LINK_UP);
> +	if (device_features & BIT_ULL(VIRTIO_NET_F_STATUS)) {
> +		if (get_link_state(mvdev))
> +			ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP);
> +		else
> +			ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, ~VIRTIO_NET_S_LINK_UP);
> +	}
>  
>  	if (add_config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
>  		memcpy(ndev->config.mac, add_config->net.mac, ETH_ALEN);
> -- 
> 1.8.3.1