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;
Date:   Tue, 17 Nov 2020 16:05:23 +0100
From:   Stefano Garzarella <sgarzare@redhat.com>
To:     Stefan Hajnoczi <stefanha@redhat.com>,
        Jason Wang <jasowang@redhat.com>
Cc:     virtualization@lists.linux-foundation.org,
        Laurent Vivier <lvivier@redhat.com>,
        linux-kernel@vger.kernel.org, Eli Cohen <elic@nvidia.com>,
        Jason Wang <jasowang@redhat.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>
Subject: Re: [PATCH RFC 01/12] vhost-vdpa: add support for vDPA blk devices
Message-ID: <20201117150523.berhqwtgqjov3i63@steredhat>
References: <20201113134712.69744-1-sgarzare@redhat.com>
 <20201113134712.69744-2-sgarzare@redhat.com>
 <20201117105709.GC131917@stefanha-x1.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20201117105709.GC131917@stefanha-x1.localdomain>
Precedence: bulk

On Tue, Nov 17, 2020 at 10:57:09AM +0000, Stefan Hajnoczi wrote:
>On Fri, Nov 13, 2020 at 02:47:01PM +0100, Stefano Garzarella wrote:
>> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
>> index 2754f3069738..fb0411594963 100644
>> --- a/drivers/vhost/vdpa.c
>> +++ b/drivers/vhost/vdpa.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/nospec.h>
>>  #include <linux/vhost.h>
>>  #include <linux/virtio_net.h>
>> +#include <linux/virtio_blk.h>
>>
>>  #include "vhost.h"
>>
>> @@ -194,6 +195,9 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa *v,
>>  	case VIRTIO_ID_NET:
>>  		size = sizeof(struct virtio_net_config);
>>  		break;
>> +	case VIRTIO_ID_BLOCK:
>> +		size = sizeof(struct virtio_blk_config);
>> +		break;
>>  	}
>>
>>  	if (c->len == 0)
>
>Can vdpa_config_ops->get/set_config() handle the size check instead of
>hardcoding device-specific knowledge into drivers/vhost/vdpa.c?

I agree that this should be better. For example we already check if the 
buffer is large enough in the simulator callbacks, we only need to 
return an error in case it is not true.

@Jason, do you think it's okay to add a return value to 
vdpa_config_ops->get/set_config() to handle the size check?

Thanks,
Stefano