Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 10/21] vhost: support ASID in IOTLB API
To:     Eli Cohen <elic@nvidia.com>
Cc:     mst@redhat.com, eperezma@redhat.com, kvm@vger.kernel.org,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, lulu@redhat.com, eli@mellanox.com,
        lingshan.zhu@intel.com, rob.miller@broadcom.com,
        stefanha@redhat.com, sgarzare@redhat.com
References: <20201216064818.48239-1-jasowang@redhat.com>
 <20201216064818.48239-11-jasowang@redhat.com>
 <20201229102059.GB195479@mtl-vdi-166.wap.labs.mlnx>
From:   Jason Wang <jasowang@redhat.com>
Message-ID: <0fc761c7-6053-c098-a251-ae2f7380f95e@redhat.com>
Date:   Wed, 30 Dec 2020 12:27:47 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20201229102059.GB195479@mtl-vdi-166.wap.labs.mlnx>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 2020/12/29 下午6:20, Eli Cohen wrote:
>> -static int vhost_process_iotlb_msg(struct vhost_dev *dev,
>> +static int vhost_process_iotlb_msg(struct vhost_dev *dev, u16 asid,
>>   				   struct vhost_iotlb_msg *msg)
>>   {
>>   	int ret = 0;
>>   
>> +	if (asid != 0)
>> +		return -EINVAL;
>> +
>>   	mutex_lock(&dev->mutex);
>>   	vhost_dev_lock_vqs(dev);
>>   	switch (msg->type) {
>> @@ -1135,6 +1138,7 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
>>   	struct vhost_iotlb_msg msg;
>>   	size_t offset;
>>   	int type, ret;
>> +	u16 asid = 0;
> You assume asid occupies just 16 bits. So maybe you should reserve the
> other 16 bits for future extension:
>
> struct vhost_msg_v2 {
>          __u32 type;
> -       __u32 reserved;
> +       __u16 asid;
> +       __u16 reserved;
>          union {
>
> Moreover, maybe this should be reflected in previous patches that use
> the asid:
>
> -static int mlx5_vdpa_set_map(struct vdpa_device *vdev, struct vhost_iotlb *iotlb)
> +static int mlx5_vdpa_set_map(struct vdpa_device *vdev, u16 asid,
> +                            struct vhost_iotlb *iotlb)
>
> -static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev,
> +static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, u16 asid,
>                                          struct vhost_iotlb_msg *msg)
>
> etc.


Good catch.

This is a bug of the code actually. Since I want to stick to 32bit to be 
large enough for e.g PASID.

Will fix.

Thanks


>