All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tiwei Bie <tiwei.bie@intel.com>
To: Jason Wang <jasowang@redhat.com>
Cc: kvm@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	zhihong.wang@intel.com, maxime.coquelin@redhat.com,
	lingshan.zhu@intel.com
Subject: Re: [PATCH v5] vhost: introduce mdev based hardware backend
Date: Thu, 7 Nov 2019 13:27:05 +0800	[thread overview]
Message-ID: <20191107052705.GA28713@___> (raw)
In-Reply-To: <914081d6-40ee-f184-ff43-c3d4cd885fba@redhat.com>

On Thu, Nov 07, 2019 at 12:08:08PM +0800, Jason Wang wrote:
> On 2019/11/6 下午10:49, Tiwei Bie wrote:
> > > > > > > +	default:
> > > > > > > +		/*
> > > > > > > +		 * VHOST_SET_MEM_TABLE, VHOST_SET_LOG_BASE, and
> > > > > > > +		 * VHOST_SET_LOG_FD are not used yet.
> > > > > > > +		 */
> > > > > > If we don't even use them, there's probably no need to call
> > > > > > vhost_dev_ioctl(). This may help to avoid confusion when we want to develop
> > > > > > new API for e.g dirty page tracking.
> > > > > Good point. It's better to reject these ioctls for now.
> > > > > 
> > > > > PS. One thing I may need to clarify is that, we need the
> > > > > VHOST_SET_OWNER ioctl to get the vq->handle_kick to work.
> > > > > So if we don't call vhost_dev_ioctl(), we will need to
> > > > > call vhost_dev_set_owner() directly.
> > > I may miss something, it looks to me the there's no owner check in
> > > vhost_vring_ioctl() and the vhost_poll_start() can make sure handle_kick
> > > works?
> > Yeah, there is no owner check in vhost_vring_ioctl().
> > IIUC, vhost_poll_start() will start polling the file. And when
> > event arrives, vhost_poll_wakeup() will be called, and it will
> > queue work to work_list and wakeup worker to finish the work.
> > And the worker is created by vhost_dev_set_owner().
> > 
> 
> Right, rethink about this. It looks to me we need:
> 
> - Keep VHOST_SET_OWNER, this could be used for future control vq where it
> needs a kthread to access the userspace memory
> 
> - Temporarily filter  SET_LOG_BASE and SET_LOG_FD until we finalize the API
> for dirty page tracking.
> 
> - For kick through kthread, it looks sub-optimal but we can address this in
> the future, e.g call handle_vq_kick directly in vhost_poll_queue (probably a
> flag for vhost_poll) and deal with the synchronization in vhost_poll_flush
> carefully.

OK.

Thanks,
Tiwei

> 
> Thanks
> 
> 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: Tiwei Bie <tiwei.bie@intel.com>
To: Jason Wang <jasowang@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	alex.williamson@redhat.com, maxime.coquelin@redhat.com,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org, dan.daly@intel.com,
	cunming.liang@intel.com, zhihong.wang@intel.com,
	lingshan.zhu@intel.com
Subject: Re: [PATCH v5] vhost: introduce mdev based hardware backend
Date: Thu, 7 Nov 2019 13:27:05 +0800	[thread overview]
Message-ID: <20191107052705.GA28713@___> (raw)
In-Reply-To: <914081d6-40ee-f184-ff43-c3d4cd885fba@redhat.com>

On Thu, Nov 07, 2019 at 12:08:08PM +0800, Jason Wang wrote:
> On 2019/11/6 下午10:49, Tiwei Bie wrote:
> > > > > > > +	default:
> > > > > > > +		/*
> > > > > > > +		 * VHOST_SET_MEM_TABLE, VHOST_SET_LOG_BASE, and
> > > > > > > +		 * VHOST_SET_LOG_FD are not used yet.
> > > > > > > +		 */
> > > > > > If we don't even use them, there's probably no need to call
> > > > > > vhost_dev_ioctl(). This may help to avoid confusion when we want to develop
> > > > > > new API for e.g dirty page tracking.
> > > > > Good point. It's better to reject these ioctls for now.
> > > > > 
> > > > > PS. One thing I may need to clarify is that, we need the
> > > > > VHOST_SET_OWNER ioctl to get the vq->handle_kick to work.
> > > > > So if we don't call vhost_dev_ioctl(), we will need to
> > > > > call vhost_dev_set_owner() directly.
> > > I may miss something, it looks to me the there's no owner check in
> > > vhost_vring_ioctl() and the vhost_poll_start() can make sure handle_kick
> > > works?
> > Yeah, there is no owner check in vhost_vring_ioctl().
> > IIUC, vhost_poll_start() will start polling the file. And when
> > event arrives, vhost_poll_wakeup() will be called, and it will
> > queue work to work_list and wakeup worker to finish the work.
> > And the worker is created by vhost_dev_set_owner().
> > 
> 
> Right, rethink about this. It looks to me we need:
> 
> - Keep VHOST_SET_OWNER, this could be used for future control vq where it
> needs a kthread to access the userspace memory
> 
> - Temporarily filter  SET_LOG_BASE and SET_LOG_FD until we finalize the API
> for dirty page tracking.
> 
> - For kick through kthread, it looks sub-optimal but we can address this in
> the future, e.g call handle_vq_kick directly in vhost_poll_queue (probably a
> flag for vhost_poll) and deal with the synchronization in vhost_poll_flush
> carefully.

OK.

Thanks,
Tiwei

> 
> Thanks
> 
> 

  reply	other threads:[~2019-11-07  5:27 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-05 11:53 [PATCH v5] vhost: introduce mdev based hardware backend Tiwei Bie
2019-11-05 11:53 ` Tiwei Bie
2019-11-06  7:54 ` Jason Wang
2019-11-06  7:54 ` Jason Wang
2019-11-06 12:22   ` Tiwei Bie
2019-11-06 12:22   ` Tiwei Bie
2019-11-06 12:57     ` Michael S. Tsirkin
2019-11-06 12:57     ` Michael S. Tsirkin
2019-11-06 13:20       ` Jason Wang
2019-11-06 14:49         ` Tiwei Bie
2019-11-07  4:08           ` Jason Wang
2019-11-07  5:27             ` Tiwei Bie [this message]
2019-11-07  5:27               ` Tiwei Bie
2019-11-07  4:08           ` Jason Wang
2019-11-06 14:49         ` Tiwei Bie
2019-11-06 13:20       ` Jason Wang
2019-11-06 12:59 ` Michael S. Tsirkin
2019-11-06 12:59 ` Michael S. Tsirkin
2019-11-06 14:39   ` Tiwei Bie
2019-11-06 14:39   ` Tiwei Bie
2019-11-07  4:16     ` Jason Wang
2019-11-07  4:16     ` Jason Wang
2019-11-07  5:25       ` Tiwei Bie
2019-11-07  5:25         ` Tiwei Bie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191107052705.GA28713@___ \
    --to=tiwei.bie@intel.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=lingshan.zhu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.coquelin@redhat.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.