On Sat, May 9, 2020 at 10:25 AM Jason Wang <jasow...@redhat.com> wrote:
>
>
> On 2020/5/9 上午12:32, Cindy Lu wrote:
> > Introduce a function to set the state to the vhost driver.
> > vDPA need to sync the driver's state to NIC
>
>
> Let's split this patch into two.
>
> 1) introduce vhost_set_state
> 2) make virtio-net use of vhost_set_state
>
Sure, Will fix this
> >
> > Signed-off-by: Cindy Lu <l...@redhat.com>
> > ---
> > hw/net/vhost_net.c | 9 +++++++++
> > hw/net/virtio-net.c | 9 +++++++++
> > include/hw/virtio/vhost-backend.h | 2 ++
> > include/net/vhost_net.h | 2 +-
> > 4 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index d1d421e3d9..63b2a85d6e 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -465,3 +465,12 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t
> > mtu)
> >
> > return vhost_ops->vhost_net_set_mtu(&net->dev, mtu);
> > }
> > +int vhost_set_state(NetClientState *nc, uint8_t state)
> > +{
> > + struct vhost_net *net = get_vhost_net(nc);
> > + struct vhost_dev *hdev = &net->dev;
> > + if (hdev->vhost_ops->vhost_set_state) {
>
>
> Indentation looks wrong.
>
Will fix this
>
> > + return hdev->vhost_ops->vhost_set_state(hdev, state);
> > + }
> > + return 0;
> > +}
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index db3d7c38e6..1bddb4b4af 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -206,6 +206,9 @@ static void virtio_net_vhost_status(VirtIONet *n,
> > uint8_t status)
> > VirtIODevice *vdev = VIRTIO_DEVICE(n);
> > NetClientState *nc = qemu_get_queue(n->nic);
> > int queues = n->multiqueue ? n->max_queues : 1;
> > + NetClientState *peer = qemu_get_peer(nc, 0);
> > + uint8_t status_set = vdev->status ;
> > + uint8_t vhost_started_pre = n->vhost_started;
> >
> > if (!get_vhost_net(nc->peer)) {
> > return;
> > @@ -245,6 +248,7 @@ static void virtio_net_vhost_status(VirtIONet *n,
> > uint8_t status)
> > return;
> > }
> > }
> > + status_set = status_set | VIRTIO_CONFIG_S_DRIVER_OK;
> >
> > n->vhost_started = 1;
> > r = vhost_net_start(vdev, n->nic->ncs, queues);
> > @@ -252,11 +256,16 @@ static void virtio_net_vhost_status(VirtIONet *n,
> > uint8_t status)
> > error_report("unable to start vhost net: %d: "
> > "falling back on userspace virtio", -r);
> > n->vhost_started = 0;
> > + status_set = status_set & ~VIRTIO_CONFIG_S_DRIVER_OK;
> > }
> > } else {
> > vhost_net_stop(vdev, n->nic->ncs, queues);
> > + status_set = status_set & ~VIRTIO_CONFIG_S_DRIVER_OK;
> > n->vhost_started = 0;
> > }
> > + if (vhost_started_pre != n->vhost_started) {
> > + vhost_set_state(peer, status_set);
>
>
> Any reason why not just passing virtio device status to vhost-vdpa?
>
I will double check and fix this
>
> > + }
> > }
> >
> > static int virtio_net_set_vnet_endian_one(VirtIODevice *vdev,
> > diff --git a/include/hw/virtio/vhost-backend.h
> > b/include/hw/virtio/vhost-backend.h
> > index 6f6670783f..f823055167 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -112,6 +112,7 @@ typedef int (*vhost_get_inflight_fd_op)(struct
> > vhost_dev *dev,
> > typedef int (*vhost_set_inflight_fd_op)(struct vhost_dev *dev,
> > struct vhost_inflight *inflight);
> >
> > +typedef int (*vhost_set_state_op)(struct vhost_dev *dev, uint8_t state);
>
>
> Need document what's the meaning of state here, is it e.g virtio device
> status? If yes, is it better to rename it to vhost_set_status()?
>
> Thanks
>
sure will fix this
>
> > typedef struct VhostOps {
> > VhostBackendType backend_type;
> > vhost_backend_init vhost_backend_init;
> > @@ -152,6 +153,7 @@ typedef struct VhostOps {
> > vhost_backend_mem_section_filter_op vhost_backend_mem_section_filter;
> > vhost_get_inflight_fd_op vhost_get_inflight_fd;
> > vhost_set_inflight_fd_op vhost_set_inflight_fd;
> > + vhost_set_state_op vhost_set_state;
> > } VhostOps;
> >
> > extern const VhostOps user_ops;
> > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> > index 77e47398c4..6548a5a105 100644
> > --- a/include/net/vhost_net.h
> > +++ b/include/net/vhost_net.h
> > @@ -39,5 +39,5 @@ int vhost_set_vring_enable(NetClientState * nc, int
> > enable);
> > uint64_t vhost_net_get_acked_features(VHostNetState *net);
> >
> > int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu);
> > -
> > +int vhost_set_state(NetClientState *nc, uint8_t state);
> > #endif
>
