[PATCH v6 1/7] dt-bindings: virtio-mmio: Add IOMMU description
The nature of a virtio-mmio node is discovered by the virtio driver at
probe time. However the DMA relation between devices must be described
statically. When a virtio-mmio node is a virtio-iommu device, it needs an
"#iommu-cells" property as specified by bindings/iommu/iommu.txt.
Otherwise, the virtio-mmio device may perform DMA through an IOMMU, which
requires an "iommus" property. Describe these requirements in the
device-tree bindings documentation.
Reviewed-by: Rob Herring
Reviewed-by: Eric Auger
Signed-off-by: Jean-Philippe Brucker
---
.../devicetree/bindings/virtio/mmio.txt | 30 +++
1 file changed, 30 insertions(+)
diff --git a/Documentation/devicetree/bindings/virtio/mmio.txt
b/Documentation/devicetree/bindings/virtio/mmio.txt
index 5069c1b8e193..21af30fbb81f 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.txt
+++ b/Documentation/devicetree/bindings/virtio/mmio.txt
@@ -8,10 +8,40 @@ Required properties:
- reg: control registers base address and size including configuration
space
- interrupts: interrupt generated by the device
+Required properties for virtio-iommu:
+
+- #iommu-cells:When the node corresponds to a virtio-iommu device, it
is
+ linked to DMA masters using the "iommus" or "iommu-map"
+ properties [1][2]. #iommu-cells specifies the size of the
+ "iommus" property. For virtio-iommu #iommu-cells must be
+ 1, each cell describing a single endpoint ID.
+
+Optional properties:
+
+- iommus: If the device accesses memory through an IOMMU, it should
+ have an "iommus" property [1]. Since virtio-iommu itself
+ does not access memory through an IOMMU, the "virtio,mmio"
+ node cannot have both an "#iommu-cells" and an "iommus"
+ property.
+
Example:
virtio_block@3000 {
compatible = "virtio,mmio";
reg = <0x3000 0x100>;
interrupts = <41>;
+
+ /* Device has endpoint ID 23 */
+ iommus = <&viommu 23>
}
+
+ viommu: iommu@3100 {
+ compatible = "virtio,mmio";
+ reg = <0x3100 0x100>;
+ interrupts = <42>;
+
+ #iommu-cells = <1>
+ }
+
+[1] Documentation/devicetree/bindings/iommu/iommu.txt
+[2] Documentation/devicetree/bindings/pci/pci-iommu.txt
--
2.19.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 0/7] Add virtio-iommu driver
Implement the virtio-iommu driver, following specification v0.9 [1]. Only minor changes since v5 [2]. I fixed issues reported by Michael and added tags from Eric and Bharat. Thanks! You can find Linux driver and kvmtool device on v0.9 branches [3], module and x86 support on virtio-iommu/devel. Also tested with Eric's QEMU device [4]. [1] Virtio-iommu specification v0.9, sources and pdf git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9 http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf [2] [PATCH v5 0/7] Add virtio-iommu driver https://www.spinics.net/lists/linux-pci/msg78158.html [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.1 git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9 [4] [RFC v9 00/17] VIRTIO-IOMMU device https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html Jean-Philippe Brucker (7): dt-bindings: virtio-mmio: Add IOMMU description dt-bindings: virtio: Add virtio-pci-iommu node of: Allow the iommu-map property to omit untranslated devices PCI: OF: Initialize dev->fwnode appropriately iommu: Add virtio-iommu driver iommu/virtio: Add probe request iommu/virtio: Add event queue .../devicetree/bindings/virtio/iommu.txt | 66 + .../devicetree/bindings/virtio/mmio.txt | 30 + MAINTAINERS |7 + drivers/iommu/Kconfig | 11 + drivers/iommu/Makefile|1 + drivers/iommu/virtio-iommu.c | 1157 + drivers/of/base.c | 10 +- drivers/pci/of.c |7 + include/uapi/linux/virtio_ids.h |1 + include/uapi/linux/virtio_iommu.h | 161 +++ 10 files changed, 1448 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt create mode 100644 drivers/iommu/virtio-iommu.c create mode 100644 include/uapi/linux/virtio_iommu.h -- 2.19.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 5/7] iommu: Add virtio-iommu driver
On 10/12/2018 22:53, Michael S. Tsirkin wrote:
> On Mon, Dec 10, 2018 at 03:06:47PM +, Jean-Philippe Brucker wrote:
>> On 27/11/2018 18:53, Michael S. Tsirkin wrote:
>>> On Tue, Nov 27, 2018 at 06:10:46PM +, Jean-Philippe Brucker wrote:
On 27/11/2018 18:04, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2018 at 05:50:50PM +, Jean-Philippe Brucker wrote:
>> On 23/11/2018 22:02, Michael S. Tsirkin wrote:
+/*
+ * __viommu_sync_req - Complete all in-flight requests
+ *
+ * Wait for all added requests to complete. When this function
returns, all
+ * requests that were in-flight at the time of the call have
completed.
+ */
+static int __viommu_sync_req(struct viommu_dev *viommu)
+{
+ int ret = 0;
+ unsigned int len;
+ size_t write_len;
+ struct viommu_request *req;
+ struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
+
+ assert_spin_locked(&viommu->request_lock);
+
+ virtqueue_kick(vq);
+
+ while (!list_empty(&viommu->requests)) {
+ len = 0;
+ req = virtqueue_get_buf(vq, &len);
+ if (!req)
+ continue;
+
+ if (!len)
+ viommu_set_req_status(req->buf, req->len,
+VIRTIO_IOMMU_S_IOERR);
+
+ write_len = req->len - req->write_offset;
+ if (req->writeback && len == write_len)
+ memcpy(req->writeback, req->buf +
req->write_offset,
+ write_len);
+
+ list_del(&req->list);
+ kfree(req);
+ }
>>>
>>> I didn't notice this in the past but it seems this will spin
>>> with interrupts disabled until host handles the request.
>>> Please do not do this - host execution can be another
>>> task that needs the same host CPU. This will then disable
>>> interrupts for a very very long time.
>>
>> In the guest yes, but that doesn't prevent the host from running another
>> task right?
>
> Doesn't prevent it but it will delay it significantly
> until scheduler decides to kick the VCPU task out.
>
>> My tests run fine when QEMU is bound to a single CPU, even
>> though vcpu and viommu run in different threads
>>
>>> What to do then? Queue in software and wake up task.
>>
>> Unfortunately I can't do anything here, because IOMMU drivers can't
>> sleep in the iommu_map() or iommu_unmap() path.
>>
>> The problem is the same
>> for all IOMMU drivers. That's because the DMA API allows drivers to call
>> some functions with interrupts disabled. For example
>> Documentation/DMA-API-HOWTO.txt allows dma_alloc_coherent() and
>> dma_unmap_single() to be called in interrupt context.
>
> In fact I don't really understand how it's supposed to
> work at all: you only sync when ring is full.
> So host may not have seen your map request if ring
> is not full.
> Why is it safe to use the address with a device then?
viommu_map() calls viommu_send_req_sync(), which does the sync
immediately after adding the MAP request.
Thanks,
Jean
>>>
>>> I see. So it happens on every request. Maybe you should clear
>>> event index then. This way if exits are disabled you know that
>>> host is processing the ring. Event index is good for when
>>> you don't care when it will be processed, you just want
>>> to reduce number of exits as much as possible.
>>>
>>
>> I think that's already the case: since we don't attach a callback to the
>> request queue, VRING_AVAIL_F_NO_INTERRUPT is set in avail_flags_shadow,
>> which causes the used event index to stay clear.
>>
>> Thanks,
>> Jean
>
> VRING_AVAIL_F_NO_INTERRUPT has no effect when the event index
> feature has been negotiated. In any case, it also does not
> affect kick notifications from guest - it affects
> device interrupts.
Ok, I thought we were talking about the used event. Then this is a
device-side optimization right?
In QEMU, disabling notifications while processing the queue didn't show
any difference in netperf. Kvmtool already masks events when processing
the ring - if the host is still handling requests while the guest adds
more, then the avail event is at least one behind the new avail index,
and the guest doesn't kick the host.
Anyway I think we can look at optimizations later, since I don't think
there is any trivial one that we can squash into the initial driver.
I'll resend this series with the Kconfig and header change.
Thanks,
Jean
___
Virtualization mailin
Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
On Tue, Dec 11, 2018 at 11:06:43AM +0800, Jason Wang wrote:
>
> On 2018/12/11 上午9:34, Michael S. Tsirkin wrote:
> > On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
> > > When we try to do rx busy polling in tx path in commit 441abde4cd84
> > > ("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
> > > after tx vq mutex is held. This may lead deadlock so we try to lock vq
> > > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
> > > one"). With this commit, we avoid the deadlock with the assumption
> > > that handle_rx() and handle_tx() run in a same process. But this
> > > commit remove the protection for IOTLB updating which requires the
> > > mutex of each vq to be held.
> > >
> > > To solve this issue, the first step is to have a exact same lock
> > > ordering for vhost_net. This is done through:
> > >
> > > - For handle_rx(), if busy polling is enabled, lock tx vq immediately.
> > > - For handle_tx(), always lock rx vq before tx vq, and unlock it if
> > >busy polling is not enabled.
> > > - Remove the tricky locking codes in busy polling.
> > >
> > > With this, we can have a exact same lock ordering for vhost_net, this
> > > allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
> > > vqs one by one") in next patch.
> > >
> > > The patch will add two more atomic operations on the tx path during
> > > each round of handle_tx(). 1 byte TCP_RR does not notice such
> > > overhead.
> > >
> > > Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
> > > Cc: Tonghao Zhang
> > > Signed-off-by: Jason Wang
> > > ---
> > > drivers/vhost/net.c | 18 +++---
> > > 1 file changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index ab11b2bee273..5f272ab4d5b4 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > struct socket *sock;
> > > struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> > > - mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX:
> > > VHOST_NET_VQ_RX);
> > > vhost_disable_notify(&net->dev, vq);
> > > sock = rvq->private_data;
> > > @@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > vhost_net_busy_poll_try_queue(net, vq);
> > > else if (!poll_rx) /* On tx here, sock has no rx data. */
> > > vhost_enable_notify(&net->dev, rvq);
> > > -
> > > - mutex_unlock(&vq->mutex);
> > > }
> > > static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> > > @@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net
> > > *net, struct socket *sock)
> > > static void handle_tx(struct vhost_net *net)
> > > {
> > > struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> > > + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
> > > struct vhost_virtqueue *vq = &nvq->vq;
> > > + struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
> > > struct socket *sock;
> > > + mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
> > > mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> > > + if (!vq->busyloop_timeout)
> > > + mutex_unlock(&vq_rx->mutex);
> > > +
> > > sock = vq->private_data;
> > > if (!sock)
> > > goto out;
> > > @@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
> > > handle_tx_copy(net, sock);
> > > out:
> > > + if (vq->busyloop_timeout)
> > > + mutex_unlock(&vq_rx->mutex);
> > > mutex_unlock(&vq->mutex);
> > > }
> > So rx mutex taken on tx path now. And tx mutex is on rc path ... This
> > is just messed up. Why can't tx polling drop rx lock before
> > getting the tx lock and vice versa?
>
>
> Because we want to poll both tx and rx virtqueue at the same time
> (vhost_net_busy_poll()).
>
> while (vhost_can_busy_poll(endtime)) {
> if (vhost_has_work(&net->dev)) {
> *busyloop_intr = true;
> break;
> }
>
> if ((sock_has_rx_data(sock) &&
> !vhost_vq_avail_empty(&net->dev, rvq)) ||
> !vhost_vq_avail_empty(&net->dev, tvq))
> break;
>
> cpu_relax();
>
> }
>
>
> And we disable kicks and notification for better performance.
Right but it's all slow path - it happens when queue is
otherwise empty. So this is what I am saying: let's drop the locks
we hold around this.
>
> >
> > Or if we really wanted to force everything to be locked at
> > all times, let's just use a single mutex.
> >
> >
> >
>
> We could, but it might requires more changes which could be done for -next I
> believe.
>
>
> Thanks
I'd rather we kept the fine grained locking. E.g. people are
looking at splitting the tx and rx threads. But if not possible
let's fix it cleanly with a coarse-grain
Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
On 2018/12/11 上午9:34, Michael S. Tsirkin wrote:
On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
When we try to do rx busy polling in tx path in commit 441abde4cd84
("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
after tx vq mutex is held. This may lead deadlock so we try to lock vq
one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
one"). With this commit, we avoid the deadlock with the assumption
that handle_rx() and handle_tx() run in a same process. But this
commit remove the protection for IOTLB updating which requires the
mutex of each vq to be held.
To solve this issue, the first step is to have a exact same lock
ordering for vhost_net. This is done through:
- For handle_rx(), if busy polling is enabled, lock tx vq immediately.
- For handle_tx(), always lock rx vq before tx vq, and unlock it if
busy polling is not enabled.
- Remove the tricky locking codes in busy polling.
With this, we can have a exact same lock ordering for vhost_net, this
allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
vqs one by one") in next patch.
The patch will add two more atomic operations on the tx path during
each round of handle_tx(). 1 byte TCP_RR does not notice such
overhead.
Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
Cc: Tonghao Zhang
Signed-off-by: Jason Wang
---
drivers/vhost/net.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ab11b2bee273..5f272ab4d5b4 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
struct socket *sock;
struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
- mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
vhost_disable_notify(&net->dev, vq);
sock = rvq->private_data;
@@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
vhost_net_busy_poll_try_queue(net, vq);
else if (!poll_rx) /* On tx here, sock has no rx data. */
vhost_enable_notify(&net->dev, rvq);
-
- mutex_unlock(&vq->mutex);
}
static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
@@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net *net,
struct socket *sock)
static void handle_tx(struct vhost_net *net)
{
struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
+ struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
struct vhost_virtqueue *vq = &nvq->vq;
+ struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
struct socket *sock;
+ mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
+ if (!vq->busyloop_timeout)
+ mutex_unlock(&vq_rx->mutex);
+
sock = vq->private_data;
if (!sock)
goto out;
@@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
handle_tx_copy(net, sock);
out:
+ if (vq->busyloop_timeout)
+ mutex_unlock(&vq_rx->mutex);
mutex_unlock(&vq->mutex);
}
So rx mutex taken on tx path now. And tx mutex is on rc path ... This
is just messed up. Why can't tx polling drop rx lock before
getting the tx lock and vice versa?
Because we want to poll both tx and rx virtqueue at the same time
(vhost_net_busy_poll()).
while (vhost_can_busy_poll(endtime)) {
if (vhost_has_work(&net->dev)) {
*busyloop_intr = true;
break;
}
if ((sock_has_rx_data(sock) &&
!vhost_vq_avail_empty(&net->dev, rvq)) ||
!vhost_vq_avail_empty(&net->dev, tvq))
break;
cpu_relax();
}
And we disable kicks and notification for better performance.
Or if we really wanted to force everything to be locked at
all times, let's just use a single mutex.
We could, but it might requires more changes which could be done for
-next I believe.
Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
> When we try to do rx busy polling in tx path in commit 441abde4cd84
> ("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
> after tx vq mutex is held. This may lead deadlock so we try to lock vq
> one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
> one"). With this commit, we avoid the deadlock with the assumption
> that handle_rx() and handle_tx() run in a same process. But this
> commit remove the protection for IOTLB updating which requires the
> mutex of each vq to be held.
>
> To solve this issue, the first step is to have a exact same lock
> ordering for vhost_net. This is done through:
>
> - For handle_rx(), if busy polling is enabled, lock tx vq immediately.
> - For handle_tx(), always lock rx vq before tx vq, and unlock it if
> busy polling is not enabled.
> - Remove the tricky locking codes in busy polling.
>
> With this, we can have a exact same lock ordering for vhost_net, this
> allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
> vqs one by one") in next patch.
>
> The patch will add two more atomic operations on the tx path during
> each round of handle_tx(). 1 byte TCP_RR does not notice such
> overhead.
>
> Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
> Cc: Tonghao Zhang
> Signed-off-by: Jason Wang
> ---
> drivers/vhost/net.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index ab11b2bee273..5f272ab4d5b4 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> struct socket *sock;
> struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
>
> - mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX:
> VHOST_NET_VQ_RX);
> vhost_disable_notify(&net->dev, vq);
> sock = rvq->private_data;
>
> @@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> vhost_net_busy_poll_try_queue(net, vq);
> else if (!poll_rx) /* On tx here, sock has no rx data. */
> vhost_enable_notify(&net->dev, rvq);
> -
> - mutex_unlock(&vq->mutex);
> }
>
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> @@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net *net,
> struct socket *sock)
> static void handle_tx(struct vhost_net *net)
> {
> struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
> struct vhost_virtqueue *vq = &nvq->vq;
> + struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
> struct socket *sock;
>
> + mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
> mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> + if (!vq->busyloop_timeout)
> + mutex_unlock(&vq_rx->mutex);
> +
> sock = vq->private_data;
> if (!sock)
> goto out;
> @@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
> handle_tx_copy(net, sock);
>
> out:
> + if (vq->busyloop_timeout)
> + mutex_unlock(&vq_rx->mutex);
> mutex_unlock(&vq->mutex);
> }
>
So rx mutex taken on tx path now. And tx mutex is on rc path ... This
is just messed up. Why can't tx polling drop rx lock before
getting the tx lock and vice versa?
Or if we really wanted to force everything to be locked at
all times, let's just use a single mutex.
> @@ -1060,7 +1065,9 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
> static void handle_rx(struct vhost_net *net)
> {
> struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_RX];
> + struct vhost_net_virtqueue *nvq_tx = &net->vqs[VHOST_NET_VQ_TX];
> struct vhost_virtqueue *vq = &nvq->vq;
> + struct vhost_virtqueue *vq_tx = &nvq_tx->vq;
> unsigned uninitialized_var(in), log;
> struct vhost_log *vq_log;
> struct msghdr msg = {
> @@ -1086,6 +1093,9 @@ static void handle_rx(struct vhost_net *net)
> int recv_pkts = 0;
>
> mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_RX);
> + if (vq->busyloop_timeout)
> + mutex_lock_nested(&vq_tx->mutex, VHOST_NET_VQ_TX);
> +
> sock = vq->private_data;
> if (!sock)
> goto out;
> @@ -1200,6 +1210,8 @@ static void handle_rx(struct vhost_net *net)
> out:
> vhost_net_signal_used(nvq);
> mutex_unlock(&vq->mutex);
> + if (vq->busyloop_timeout)
> + mutex_unlock(&vq_tx->mutex);
> }
>
> static void handle_tx_kick(struct vhost_work *work)
> --
> 2.17.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net 4/4] vhost: log dirty page correctly
On Mon, Dec 10, 2018 at 11:14:41PM +0800, kbuild test robot wrote: > Hi Jason, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on net/master] > > url: > https://github.com/0day-ci/linux/commits/Jason-Wang/Fix-various-issue-of-vhost/20181210-223236 > config: i386-randconfig-x072-201849 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All warnings (new ones prefixed by >>): > >drivers//vhost/vhost.c: In function 'log_used': > >> drivers//vhost/vhost.c:1771:27: warning: cast from pointer to integer of > >> different size [-Wpointer-to-int-cast] > ret = translate_desc(vq, (u64)vq->used + used_offset, len, iov, 64, > ^ >drivers//vhost/vhost.c:1776:27: warning: cast from pointer to integer of > different size [-Wpointer-to-int-cast] > ret = log_write_hva(vq, (u64)iov[i].iov_base, iov[i].iov_len); > ^ >drivers//vhost/vhost.c: In function 'vhost_log_write': >drivers//vhost/vhost.c:1788:26: warning: cast from pointer to integer of > different size [-Wpointer-to-int-cast] >r = log_write_hva(vq, (u64)iov[i].iov_base, > ^ It's a technicality, cast to unsigned long and the warning will go away. Donnu why does gcc bother with these warnings. Nothing is wrong unless size of pointer is > size of int. >Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size >Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size >Cyclomatic Complexity 1 > arch/x86/include/asm/barrier.h:array_index_mask_nospec >Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read >Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write >Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:set_bit >Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:clear_bit >Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:test_and_set_bit >Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit >Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit >Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls >Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32 >Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD >Cyclomatic Complexity 1 include/linux/list.h:__list_del >Cyclomatic Complexity 1 include/linux/list.h:list_empty >Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current >Cyclomatic Complexity 3 include/linux/string.h:memset >Cyclomatic Complexity 5 include/linux/string.h:memcpy >Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order >Cyclomatic Complexity 1 > arch/x86/include/asm/atomic.h:arch_atomic_dec_and_test >Cyclomatic Complexity 1 > include/asm-generic/atomic-instrumented.h:atomic_dec_and_test >Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR >Cyclomatic Complexity 1 include/linux/thread_info.h:set_ti_thread_flag >Cyclomatic Complexity 1 include/linux/thread_info.h:check_object_size >Cyclomatic Complexity 5 include/linux/thread_info.h:check_copy_size >Cyclomatic Complexity 1 arch/x86/include/asm/preempt.h:preempt_count >Cyclomatic Complexity 1 include/linux/spinlock.h:spinlock_check >Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock >Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock >Cyclomatic Complexity 1 include/linux/wait.h:init_waitqueue_func_entry >Cyclomatic Complexity 1 include/linux/llist.h:init_llist_head >Cyclomatic Complexity 1 include/linux/llist.h:llist_empty >Cyclomatic Complexity 1 include/linux/llist.h:llist_del_all >Cyclomatic Complexity 1 include/linux/rbtree.h:rb_link_node >Cyclomatic Complexity 3 include/linux/overflow.h:__ab_c_size >Cyclomatic Complexity 1 include/linux/page_ref.h:page_ref_dec_and_test >Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info >Cyclomatic Complexity 1 include/linux/sched.h:need_resched >Cyclomatic Complexity 1 include/linux/mm.h:put_page_testzero >Cyclomatic Complexity 1 include/linux/mm.h:put_devmap_managed_page >Cyclomatic Complexity 1 include/uapi/linux/virtio_ring.h:vring_need_event >Cyclomatic Complexity 1 > include/linux/virtio_byteorder.h:virtio_legacy_is_little_endian >Cyclomatic Complexity 2 include/linux/uio.h:copy_to_iter >Cyclomatic Complexity 2 include/linux/uio.h:copy_from_iter >Cyclomatic Complexity 2 include/linux/uio.h:copy_from_iter_full >Cyclomatic Complexity 1 include/linux/uio.h:iov_iter_count >Cyclomatic Complexity 1 include/linux/slab.h:kmalloc_type >Cyclomatic Complexity 28 include/linux/slab.h:kmalloc_index >Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large >Cyclomatic Complexity 4 include/linux/slab.h:kmalloc >Cyclomatic Com
Re: [PATCH net 0/4] Fix various issue of vhost
From: Jason Wang Date: Mon, 10 Dec 2018 17:44:50 +0800 > This series tries to fix various issues of vhost: > > - Patch 1 adds a missing write barrier between used idx updating and > logging. > - Patch 2-3 brings back the protection of device IOTLB through vq > mutex, this fixes possible use after free in device IOTLB entries. > - Patch 4 fixes the diry page logging when device IOTLB is > enabled. We should done through GPA instead of GIOVA, this was done > through logging through iovec and traversing GPA->HPA list for the > GPA. > > Please consider them for -stable. Looks like the kbuild robot found some problems. ->used is a pointer (which might be 32-bit) and you're casting it to a u64 in the translate_desc() calls of patch #4. Please make sure that you don't actually require the full domain of a u64 in these values, as obviously if vq->used is a pointer you will only get a 32-bit domain on 32-bit architectures. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
