Message-ID: <bad92641-e10e-cb06-179d-2f4bc11c721d@nvidia.com>
Date:   Thu, 2 Feb 2023 15:31:26 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH v2 6/7] vdpa/mlx5: conditionally show MTU and STATUS in
 config space
Content-Language: en-US
To:     Si-Wei Liu <si-wei.liu@oracle.com>, mst@redhat.com,
        jasowang@redhat.com, parav@nvidia.com
Cc:     virtualization@lists.linux-foundation.org,
        linux-kernel@vger.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>
From:   Eli Cohen <elic@nvidia.com>
In-Reply-To: <1675207345-22328-7-git-send-email-si-wei.liu@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WHp5TjdOQ0U0L2g3eUNpdnQxMmJSc0h4Vk42bXNmdUMvc3J3NW1sNnVDK09E?=
 =?utf-8?B?RjA2bGYxMjVHL1hYVHE5NWNWdHpLOUxvUk96SmQ2MVh5ME51SU13U3RMVWVB?=
 =?utf-8?B?YUZlYW42RDhKaHZ5VzRBc29kZ1JidXIvT213MEN0eHhXY0pZSk5IcSt0L2sy?=
 =?utf-8?B?SGEvWDZ3eE1ocDFXM2Q1RStlUGFqZ2wwRHRjRmY2b3lnZXVTTUsrYisxNnNG?=
 =?utf-8?B?UFBtenFEREZXR3VBclc5QUpRUUtVUmEzK1hOblRBbG1HOUZjdW5SK0hOcE5p?=
 =?utf-8?B?dG01YVhiajhGYXZzcDJxU0t4b1V1Ymt0RFVvZ3RzaWpIUGhkVDF1TWlWY3Fq?=
 =?utf-8?B?TUtQb3lSem1VVWlIUW9vUU8zaGJrK0tnM1M1bFhDMjlRSThETzFSTWJzSmtF?=
 =?utf-8?B?TVY5L3k4SzA5R2hKZU1jUDIyZ0lteXNFajVkREdPY09nODZqaXY5NEJiVTA1?=
 =?utf-8?B?QWVPRFBsUm5rVWFpRi9xcTBJdnJuYmtJQ0xsM2IycUZTdGRqcUZJU0ZnQ1V2?=
 =?utf-8?B?WUFEQjJhMmhjMzZmcS9MUk5JZFVyN3pRd1d1Z3N1VWphOHRpd1FadDlsTllp?=
 =?utf-8?B?cERHcnpsLy9BOVRSN1lJa2IvNjZCaGFIemNSOGc5RGhjRmZIQmpVc1VCbjha?=
 =?utf-8?B?QVVRZG8rQWh3L1lUMEFnQWpxSXRDRFF3TmJkTGZtNlFSQngxVHhGZ0NjMzlB?=
 =?utf-8?B?bTJvZlVYMkI0d0p4TUlpQ3QyR2loK1lzbTdWdzE5SHpDUmMzaHNhc204Mmh1?=
 =?utf-8?B?RUlFdDM4ZDVjNk9lbTQ3VGRadVZXMDBrcUNESTR2Z0ZTYUx2cXQwQzQrVWQw?=
 =?utf-8?B?TUVwREU1dldHWWVqUEF6NE9nVGM5akFVYVQxTXlTaHcvV0k0RzBERGl3TW5T?=
 =?utf-8?B?YU1aZ3Azc1RxbzdKclJOMUlZRytERDh2VU9qNjBGc2o1ZkJqb0RQUmI4NlFF?=
 =?utf-8?B?dFZVN2ZMUmF0aUZpN3Y2UWRiUXJJeUdOSWFWZTQ1NW9ZUzY1WWFuSDUvM09x?=
 =?utf-8?B?Ykc4US81SDRMc1VHd2xBMjlvL1NOVDlNYnRTY1d1S1BGOUxFYXM3bUloMnJ3?=
 =?utf-8?B?TWNiOVZmRDV2eTdoc2dqVCt0aXA3TWNlZGNxWlAwSklGRHR0cndZa0g5Rk8r?=
 =?utf-8?B?c3N4TEg0ZXgrWmlVb01vandNZ09sUGNDVGplZGJaa3N1MU93ZklLVmpFR2tH?=
 =?utf-8?B?RkhVWjFmUVlUaElUcmZkb0pMUzB6MUNrOFAwNVZDRkFrVWtVbEEvWHg3TmRa?=
 =?utf-8?B?V0w2Y0lmVTMram95VkFpZXdXbnZQTXVSbkxEclV1QzloU1Z3S1N2VWpPQmFZ?=
 =?utf-8?B?NTJqeWtvSlFIeU56ZUY1cGJJaC82a0xOU2xkcWRVMWVBZE0vdnNtdXBJME5Y?=
 =?utf-8?B?emtVdHRKY1Bwck1ia0x5UTZ5SUZnRUMwSDBVQmJNUk0zYmNYS3ZQMjlEaXNs?=
 =?utf-8?B?Z1lIaGZVZTJ3QzRrSTlhMnNmMU5FSFlKRWZIS2NlcDVHcitnVzFUemx1RTg2?=
 =?utf-8?B?bmNlaHd3L1JzVHpiQmxmVS9lWGJNRnZlL0xUdXpJZ3VnOW14a1JCMC9WYVI1?=
 =?utf-8?B?M3ZHa0c0WStoQlAwTUI2SW5EZEVvT0JydVpuOC9yMldWS2NPZ2RnTTVoa2Vj?=
 =?utf-8?B?VmtEN3pOamoxbWdEWTUyR1ZKZkVuencvOTV0dmlQaFFON2o0bFpSNEJvemVU?=
 =?utf-8?B?RHFKcUZ3RUhTWTdVOFBEWlVuUzc5Vjl6azBmUzdScXNHYUNMTVlUT1VkcFQx?=
 =?utf-8?B?RFNkSm9xWVhoMExySURoUVRmeHFoVDlsSjNlRURtR2wxRStDNGE1M3pOdHFv?=
 =?utf-8?B?ekFFUjdrcXpyM2hVN0Vqdjh2Y0lDWUY4Y29lZFJoZDhXYXpzS3loaUxudzJ0?=
 =?utf-8?B?UjNGUTdTYko3anFEMzlmWHFIVW10N05zRnlOL3FIb3JwRVo5djdvVHQyU25q?=
 =?utf-8?B?RlBBamlBaDlPd1pncGdML3hxODdLa3ZMZW53NWUzNWRKeGhvK3BPUlVkN3Iv?=
 =?utf-8?B?R3ZlM3RmenRVekxVZ09DT2NrMGUzR1hRSThORzVxYzR5VFZrRFg4UldFQTkv?=
 =?utf-8?B?eDhycTg0a0FvWHNZbWRmelVyVWZmckJycGtodUY0OHNJZnNHcEhCM0UxYk92?=
 =?utf-8?Q?+FZNXFF8xMcKsP7x+Ve5Xk3E7?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2bd0db1c-175e-42f7-af4c-08db0521cd16
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5400.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 13:31:33.8432
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: M5h3EH2rIEnFF9K5CFZ3qwXDQlVBM/gVEHcNrxIpd0Av7VlpE/NdbA15JvnHekbm
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB7245
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 01/02/2023 1:22, 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>
> ---
>   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;
device_features is not defined as a local variable. It is defined in the 
next patch.
>   
> -	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);