On 2019/1/10 下午10:07, Michael S. Tsirkin wrote:
On Thu, Jan 10, 2019 at 08:37:17PM +0800, Jason Wang wrote:
On 2019/1/9 下午10:25, Michael S. Tsirkin wrote:
On Wed, Jan 09, 2019 at 03:29:47PM +0800, Jason Wang wrote:
Vhost dirty page logging API is designed to sync through GPA. But we
try to log GIOVA when device IOTLB is enabled. This is wrong and may
lead to missing data after migration.
To solve this issue, when logging with device IOTLB enabled, we will:
1) reuse the device IOTLB translation result of GIOVA->HVA mapping to
get HVA, for writable descriptor, get HVA through iovec. For used
ring update, translate its GIOVA to HVA
2) traverse the GPA->HVA mapping to get the possible GPA and log
through GPA. Pay attention this reverse mapping is not guaranteed
to be unique, so we should log each possible GPA in this case.
This fix the failure of scp to guest during migration. In -next, we
will probably support passing GIOVA->GPA instead of GIOVA->HVA.
Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Reported-by: Jintack Lim<jintack@xxxxxxxxxxxxxxx>
Cc: Jintack Lim<jintack@xxxxxxxxxxxxxxx>
Signed-off-by: Jason Wang<jasowang@xxxxxxxxxx>
---
The patch is needed for stable.
Changes from V1:
- return error instead of warn
---
drivers/vhost/net.c | 3 +-
drivers/vhost/vhost.c | 82 +++++++++++++++++++++++++++++++++++--------
drivers/vhost/vhost.h | 3 +-
3 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 36f3d0f49e60..bca86bf7189f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1236,7 +1236,8 @@ static void handle_rx(struct vhost_net *net)
if (nvq->done_idx > VHOST_NET_BATCH)
vhost_net_signal_used(nvq);
if (unlikely(vq_log))
- vhost_log_write(vq, vq_log, log, vhost_len);
+ vhost_log_write(vq, vq_log, log, vhost_len,
+ vq->iov, in);
total_len += vhost_len;
if (unlikely(vhost_exceeds_weight(++recv_pkts, total_len))) {
vhost_poll_queue(&vq->poll);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9f7942cbcbb2..ee095f08ffd4 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1733,11 +1733,70 @@ static int log_write(void __user *log_base,
return r;
}
+static int log_write_hva(struct vhost_virtqueue *vq, u64 hva, u64 len)
+{
+ struct vhost_umem *umem = vq->umem;
+ struct vhost_umem_node *u;
+ u64 gpa;
+ int r;
+ bool hit = false;
+
+ list_for_each_entry(u, &umem->umem_list, link) {
+ if (u->userspace_addr < hva &&
+ u->userspace_addr + u->size >=
+ hva + len) {
So this tries to see that the GPA range is completely within
the GVA region. Does this have to be the case?
You mean e.g a buffer that crosses the boundary of two memory regions?
Yes - where hva and gva could be contigious.
Ok, let me add the overlap range logging in v3.
And if yes why not return 0 below instead of hit = true?
I think it's safe but not sure for the case like two GPAs can map to same
HVA?
Oh I see. Yes that's possible. Document the motivation?
Ok.
Thanks
