lkml.org 
[lkml]   [2020]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 13/19] vdpa_sim: add get_config callback in vdpasim_dev_attr
On Mon, Dec 07, 2020 at 01:29:17PM +0800, Jason Wang wrote:
>
>On 2020/12/4 上午1:05, Stefano Garzarella wrote:
>>The get_config callback can be used by the device to fill the
>>config structure.
>>The callback will be invoked in vdpasim_get_config() before copying
>>bytes into caller buffer.
>>
>>Move vDPA-net config updates from vdpasim_set_features() in the
>>new vdpasim_net_get_config() callback.
>>
>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>---
>>v3:
>>- checked if get_config callback is set before call it
>>---
>> drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
>> 1 file changed, 21 insertions(+), 14 deletions(-)
>>
>>diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>index fe71ed7890e1..f935ade0806b 100644
>>--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>@@ -60,6 +60,8 @@ struct vdpasim_virtqueue {
>> #define VDPASIM_NET_FEATURES (VDPASIM_FEATURES | \
>> (1ULL << VIRTIO_NET_F_MAC))
>>+struct vdpasim;
>>+
>> struct vdpasim_dev_attr {
>> u64 supported_features;
>> size_t config_size;
>>@@ -67,6 +69,7 @@ struct vdpasim_dev_attr {
>> u32 id;
>> work_func_t work_fn;
>>+ void (*get_config)(struct vdpasim *vdpasim, void *config);
>> };
>> /* State of each vdpasim device */
>>@@ -522,8 +525,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
>> static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>> {
>> struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>>- struct virtio_net_config *config =
>>- (struct virtio_net_config *)vdpasim->config;
>> /* DMA mapping must be done by driver */
>> if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
>>@@ -531,16 +532,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
>> vdpasim->features = features & vdpasim->dev_attr.supported_features;
>>- /* We generally only know whether guest is using the legacy interface
>>- * here, so generally that's the earliest we can set config fields.
>>- * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
>>- * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
>>- */
>>-
>>- config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
>>- config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
>>- memcpy(config->mac, macaddr_buf, ETH_ALEN);
>
>
>Patch looks good to me.
>
>But we need Michael to confirm whether doing moving like this is safe.
>I guess what has been done were trying to make sure get_config() fail
>before set_features(), but it's not clear to me whether it's useful.

IIUC, also looking the QEMU code, the set_features() should be called
every time before get_config(), but to be sure, in get_config(), I can
check for example if 'vdpasim->features' is not zero (we require
VIRTIO_F_ACCESS_PLATFORM set).

@Michael any suggestion?

Thanks,
Stefano

>
>Thanks
>
>
>>-
>> return 0;
>> }
>>@@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
>> {
>> struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>>- if (offset + len < vdpasim->dev_attr.config_size)
>>- memcpy(buf, vdpasim->config + offset, len);
>>+ if (offset + len > vdpasim->dev_attr.config_size)
>>+ return;
>>+
>>+ if (vdpasim->dev_attr.get_config)
>>+ vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
>>+
>>+ memcpy(buf, vdpasim->config + offset, len);
>> }
>> static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
>>@@ -739,6 +735,16 @@ static const struct vdpa_config_ops vdpasim_batch_config_ops = {
>> .free = vdpasim_free,
>> };
>>+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
>>+{
>>+ struct virtio_net_config *net_config =
>>+ (struct virtio_net_config *)config;
>>+
>>+ net_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
>>+ net_config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
>>+ memcpy(net_config->mac, macaddr_buf, ETH_ALEN);
>>+}
>>+
>> static int __init vdpasim_dev_init(void)
>> {
>> struct vdpasim_dev_attr dev_attr = {};
>>@@ -747,6 +753,7 @@ static int __init vdpasim_dev_init(void)
>> dev_attr.supported_features = VDPASIM_NET_FEATURES;
>> dev_attr.nvqs = VDPASIM_VQ_NUM;
>> dev_attr.config_size = sizeof(struct virtio_net_config);
>>+ dev_attr.get_config = vdpasim_net_get_config;
>> dev_attr.work_fn = vdpasim_net_work;
>> vdpasim_dev = vdpasim_create(&dev_attr);
>

\
 
 \ /
  Last update: 2020-12-09 12:11    [W:0.193 / U:0.160 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site