[PATCH] virtio_net: Unregister and re-register xdp_rxq across freeze/restore

2020-06-05 Thread Sean Christopherson
Unregister each queue's xdp_rxq during freeze, and re-register the new
instance during restore.  All queues are released during free and
recreated during restore, i.e. the pre-freeze xdp_rxq will be lost.

The bug is detected by WARNs in xdp_rxq_info_unreg() and
xdp_rxq_info_unreg_mem_model() that fire after a suspend/resume cycle as
virtnet_close() attempts to unregister an uninitialized xdp_rxq object.

  [ cut here ]
  Driver BUG
  WARNING: CPU: 0 PID: 880 at net/core/xdp.c:163 xdp_rxq_info_unreg+0x48/0x50
  Modules linked in:
  CPU: 0 PID: 880 Comm: ip Not tainted 5.7.0-rc5+ #80
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:xdp_rxq_info_unreg+0x48/0x50
  Code: <0f> 0b eb ca 0f 1f 40 00 0f 1f 44 00 00 53 48 83 ec 10 8b 47 0c 83
  RSP: 0018:c91ab540 EFLAGS: 00010286
  RAX:  RBX: 88827f83ac80 RCX: 
  RDX: 000a RSI: 8253bc2a RDI: 825397ec
  RBP:  R08: 8253bc20 R09: 000a
  R10: c91ab548 R11: 0370 R12: 88817a89c000
  R13:  R14: c91abbc8 R15: 0001
  FS:  7f48b70e70c0() GS:88817bc0() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2: 7f48b6634950 CR3: 000277f1d002 CR4: 00160eb0
  Call Trace:
   virtnet_close+0x6a/0xb0
   __dev_close_many+0x91/0x100
   __dev_change_flags+0xc1/0x1c0
   dev_change_flags+0x23/0x60
   do_setlink+0x350/0xdf0
   __rtnl_newlink+0x553/0x860
   rtnl_newlink+0x43/0x60
   rtnetlink_rcv_msg+0x289/0x340
   netlink_rcv_skb+0xd1/0x110
   netlink_unicast+0x203/0x310
   netlink_sendmsg+0x32b/0x460
   sock_sendmsg+0x5b/0x60
   sys_sendmsg+0x23e/0x260
   ___sys_sendmsg+0x88/0xd0
   __sys_sendmsg+0x63/0xa0
   do_syscall_64+0x4c/0x170
   entry_SYSCALL_64_after_hwframe+0x44/0xa9
  [ cut here ]

Cc: Jesper Dangaard Brouer 
Fixes: 754b8a21a96d5 ("virtio_net: setup xdp_rxq_info")
Signed-off-by: Sean Christopherson 
---

Disclaimer: I am not remotely confident that this patch is 100% correct
or complete, my VirtIO knowledge is poor and my networking knowledge is
downright abysmal.

 drivers/net/virtio_net.c | 37 +
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba38765dc490..61055be3615e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1469,6 +1469,21 @@ static int virtnet_poll(struct napi_struct *napi, int 
budget)
return received;
 }
 
+static int virtnet_reg_xdp(struct xdp_rxq_info *xdp_rxq,
+  struct net_device *dev, u32 queue_index)
+{
+   int err;
+
+   err = xdp_rxq_info_reg(xdp_rxq, dev, queue_index);
+   if (err < 0)
+   return err;
+
+   err = xdp_rxq_info_reg_mem_model(xdp_rxq, MEM_TYPE_PAGE_SHARED, NULL);
+   if (err < 0)
+   xdp_rxq_info_unreg(xdp_rxq);
+   return err;
+}
+
 static int virtnet_open(struct net_device *dev)
 {
struct virtnet_info *vi = netdev_priv(dev);
@@ -1480,17 +1495,10 @@ static int virtnet_open(struct net_device *dev)
if (!try_fill_recv(vi, >rq[i], GFP_KERNEL))
schedule_delayed_work(>refill, 0);
 
-   err = xdp_rxq_info_reg(>rq[i].xdp_rxq, dev, i);
+   err = virtnet_reg_xdp(>rq[i].xdp_rxq, dev, i);
if (err < 0)
return err;
 
-   err = xdp_rxq_info_reg_mem_model(>rq[i].xdp_rxq,
-MEM_TYPE_PAGE_SHARED, NULL);
-   if (err < 0) {
-   xdp_rxq_info_unreg(>rq[i].xdp_rxq);
-   return err;
-   }
-
virtnet_napi_enable(vi->rq[i].vq, >rq[i].napi);
virtnet_napi_tx_enable(vi, vi->sq[i].vq, >sq[i].napi);
}
@@ -2306,6 +2314,7 @@ static void virtnet_freeze_down(struct virtio_device 
*vdev)
 
if (netif_running(vi->dev)) {
for (i = 0; i < vi->max_queue_pairs; i++) {
+   xdp_rxq_info_unreg(>rq[i].xdp_rxq);
napi_disable(>rq[i].napi);
virtnet_napi_tx_disable(>sq[i].napi);
}
@@ -2313,6 +2322,8 @@ static void virtnet_freeze_down(struct virtio_device 
*vdev)
 }
 
 static int init_vqs(struct virtnet_info *vi);
+static void virtnet_del_vqs(struct virtnet_info *vi);
+static void free_receive_page_frags(struct virtnet_info *vi);
 
 static int virtnet_restore_up(struct virtio_device *vdev)
 {
@@ -2331,6 +2342,10 @@ static int virtnet_restore_up(struct virtio_device *vdev)
schedule_delayed_work(>refill, 0);
 
for (i = 0; i < vi->max_queue_pairs; i++) {
+   err = virtnet_reg_xdp(>rq[i].xdp_rxq, vi->dev, i);
+  

Re: [PATCH v3 3/3] crypto: virtio: Fix dest length calculation in __virtio_crypto_skcipher_do_req()

2020-06-05 Thread Sasha Levin
Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: dbaf0624ffa5 ("crypto: add virtio-crypto driver").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182.

v5.6.15: Build OK!
v5.4.43: Build failed! Errors:
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:408:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
./include/linux/kernel.h:866:2: error: first argument to 
‘__builtin_choose_expr’ not a constant

v4.19.125: Build failed! Errors:
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
drivers/crypto/virtio/virtio_crypto_algs.c:422:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’
./include/linux/kernel.h:870:2: error: first argument to 
‘__builtin_choose_expr’ not a constant

v4.14.182: Build failed! Errors:
drivers/crypto/virtio/virtio_crypto_algs.c:409:35: error: ‘struct 
ablkcipher_request’ has no member named ‘cryptlen’


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

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

Re: [PATCH v3 1/3] crypto: virtio: Fix src/dst scatterlist calculation in __virtio_crypto_skcipher_do_req()

2020-06-05 Thread Sasha Levin
<20200123101000.GB24255@Red>
References: <20200602070501.2023-2-longpe...@huawei.com>
<20200123101000.GB24255@Red>

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: dbaf0624ffa5 ("crypto: add virtio-crypto driver").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182.

v5.6.15: Build OK!
v5.4.43: Failed to apply! Possible dependencies:
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")

v4.19.125: Failed to apply! Possible dependencies:
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")

v4.14.182: Failed to apply! Possible dependencies:
500e6807ce93 ("crypto: virtio - implement missing support for output IVs")
67189375bb3a ("crypto: virtio - convert to new crypto engine API")
d0d859bb87ac ("crypto: virtio - Register an algo only if it's supported")
e02b8b43f55a ("crypto: virtio - pr_err() strings should end with newlines")
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

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


Re: [PATCH v3 2/3] crypto: virtio: Fix use-after-free in virtio_crypto_skcipher_finalize_req()

2020-06-05 Thread Sasha Levin
<20200123101000.GB24255@Red>
References: <20200602070501.2023-3-longpe...@huawei.com>
<20200123101000.GB24255@Red>

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: dbaf0624ffa5 ("crypto: add virtio-crypto driver").

The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182.

v5.6.15: Build OK!
v5.4.43: Failed to apply! Possible dependencies:
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")

v4.19.125: Failed to apply! Possible dependencies:
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")

v4.14.182: Failed to apply! Possible dependencies:
500e6807ce93 ("crypto: virtio - implement missing support for output IVs")
67189375bb3a ("crypto: virtio - convert to new crypto engine API")
d0d859bb87ac ("crypto: virtio - Register an algo only if it's supported")
e02b8b43f55a ("crypto: virtio - pr_err() strings should end with newlines")
eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

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


[PATCH AUTOSEL 4.14 3/8] net: check untrusted gso_size at kernel entry

2020-06-05 Thread Sasha Levin
From: Willem de Bruijn 

[ Upstream commit 6dd912f82680761d8fb6b1bb274a69d4c7010988 ]

Syzkaller again found a path to a kernel crash through bad gso input:
a packet with gso size exceeding len.

These packets are dropped in tcp_gso_segment and udp[46]_ufo_fragment.
But they may affect gso size calculations earlier in the path.

Now that we have thlen as of commit 9274124f023b ("net: stricter
validation of untrusted gso packets"), check gso_size at entry too.

Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: syzbot 
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 include/linux/virtio_net.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 44e20c4b5141..a16e0bdf7751 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -31,6 +31,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 {
unsigned int gso_type = 0;
unsigned int thlen = 0;
+   unsigned int p_off = 0;
unsigned int ip_proto;
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
@@ -68,7 +69,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
 
-   if (skb_transport_offset(skb) + thlen > skb_headlen(skb))
+   p_off = skb_transport_offset(skb) + thlen;
+   if (p_off > skb_headlen(skb))
return -EINVAL;
} else {
/* gso packets without NEEDS_CSUM do not set transport_offset.
@@ -90,17 +92,25 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
return -EINVAL;
}
 
-   if (keys.control.thoff + thlen > skb_headlen(skb) ||
+   p_off = keys.control.thoff + thlen;
+   if (p_off > skb_headlen(skb) ||
keys.basic.ip_proto != ip_proto)
return -EINVAL;
 
skb_set_transport_header(skb, keys.control.thoff);
+   } else if (gso_type) {
+   p_off = thlen;
+   if (p_off > skb_headlen(skb))
+   return -EINVAL;
}
}
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
 
+   if (skb->len - p_off <= gso_size)
+   return -EINVAL;
+
skb_shinfo(skb)->gso_size = gso_size;
skb_shinfo(skb)->gso_type = gso_type;
 
-- 
2.25.1

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


[PATCH AUTOSEL 5.6 06/17] net: check untrusted gso_size at kernel entry

2020-06-05 Thread Sasha Levin
From: Willem de Bruijn 

[ Upstream commit 6dd912f82680761d8fb6b1bb274a69d4c7010988 ]

Syzkaller again found a path to a kernel crash through bad gso input:
a packet with gso size exceeding len.

These packets are dropped in tcp_gso_segment and udp[46]_ufo_fragment.
But they may affect gso size calculations earlier in the path.

Now that we have thlen as of commit 9274124f023b ("net: stricter
validation of untrusted gso packets"), check gso_size at entry too.

Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: syzbot 
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 include/linux/virtio_net.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 6f6ade63b04c..88997022a4b5 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -31,6 +31,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 {
unsigned int gso_type = 0;
unsigned int thlen = 0;
+   unsigned int p_off = 0;
unsigned int ip_proto;
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
@@ -68,7 +69,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
 
-   if (skb_transport_offset(skb) + thlen > skb_headlen(skb))
+   p_off = skb_transport_offset(skb) + thlen;
+   if (p_off > skb_headlen(skb))
return -EINVAL;
} else {
/* gso packets without NEEDS_CSUM do not set transport_offset.
@@ -92,17 +94,25 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
return -EINVAL;
}
 
-   if (keys.control.thoff + thlen > skb_headlen(skb) ||
+   p_off = keys.control.thoff + thlen;
+   if (p_off > skb_headlen(skb) ||
keys.basic.ip_proto != ip_proto)
return -EINVAL;
 
skb_set_transport_header(skb, keys.control.thoff);
+   } else if (gso_type) {
+   p_off = thlen;
+   if (p_off > skb_headlen(skb))
+   return -EINVAL;
}
}
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
 
+   if (skb->len - p_off <= gso_size)
+   return -EINVAL;
+
skb_shinfo(skb)->gso_size = gso_size;
skb_shinfo(skb)->gso_type = gso_type;
 
-- 
2.25.1

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


[PATCH AUTOSEL 5.4 05/14] net: check untrusted gso_size at kernel entry

2020-06-05 Thread Sasha Levin
From: Willem de Bruijn 

[ Upstream commit 6dd912f82680761d8fb6b1bb274a69d4c7010988 ]

Syzkaller again found a path to a kernel crash through bad gso input:
a packet with gso size exceeding len.

These packets are dropped in tcp_gso_segment and udp[46]_ufo_fragment.
But they may affect gso size calculations earlier in the path.

Now that we have thlen as of commit 9274124f023b ("net: stricter
validation of untrusted gso packets"), check gso_size at entry too.

Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: syzbot 
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 include/linux/virtio_net.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 6f6ade63b04c..88997022a4b5 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -31,6 +31,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 {
unsigned int gso_type = 0;
unsigned int thlen = 0;
+   unsigned int p_off = 0;
unsigned int ip_proto;
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
@@ -68,7 +69,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
 
-   if (skb_transport_offset(skb) + thlen > skb_headlen(skb))
+   p_off = skb_transport_offset(skb) + thlen;
+   if (p_off > skb_headlen(skb))
return -EINVAL;
} else {
/* gso packets without NEEDS_CSUM do not set transport_offset.
@@ -92,17 +94,25 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
return -EINVAL;
}
 
-   if (keys.control.thoff + thlen > skb_headlen(skb) ||
+   p_off = keys.control.thoff + thlen;
+   if (p_off > skb_headlen(skb) ||
keys.basic.ip_proto != ip_proto)
return -EINVAL;
 
skb_set_transport_header(skb, keys.control.thoff);
+   } else if (gso_type) {
+   p_off = thlen;
+   if (p_off > skb_headlen(skb))
+   return -EINVAL;
}
}
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
 
+   if (skb->len - p_off <= gso_size)
+   return -EINVAL;
+
skb_shinfo(skb)->gso_size = gso_size;
skb_shinfo(skb)->gso_type = gso_type;
 
-- 
2.25.1

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


[PATCH AUTOSEL 4.19 4/9] net: check untrusted gso_size at kernel entry

2020-06-05 Thread Sasha Levin
From: Willem de Bruijn 

[ Upstream commit 6dd912f82680761d8fb6b1bb274a69d4c7010988 ]

Syzkaller again found a path to a kernel crash through bad gso input:
a packet with gso size exceeding len.

These packets are dropped in tcp_gso_segment and udp[46]_ufo_fragment.
But they may affect gso size calculations earlier in the path.

Now that we have thlen as of commit 9274124f023b ("net: stricter
validation of untrusted gso packets"), check gso_size at entry too.

Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: syzbot 
Signed-off-by: Willem de Bruijn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 include/linux/virtio_net.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index f36727098df8..1c296f370e46 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -31,6 +31,7 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
 {
unsigned int gso_type = 0;
unsigned int thlen = 0;
+   unsigned int p_off = 0;
unsigned int ip_proto;
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
@@ -68,7 +69,8 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
 
-   if (skb_transport_offset(skb) + thlen > skb_headlen(skb))
+   p_off = skb_transport_offset(skb) + thlen;
+   if (p_off > skb_headlen(skb))
return -EINVAL;
} else {
/* gso packets without NEEDS_CSUM do not set transport_offset.
@@ -92,17 +94,25 @@ static inline int virtio_net_hdr_to_skb(struct sk_buff *skb,
return -EINVAL;
}
 
-   if (keys.control.thoff + thlen > skb_headlen(skb) ||
+   p_off = keys.control.thoff + thlen;
+   if (p_off > skb_headlen(skb) ||
keys.basic.ip_proto != ip_proto)
return -EINVAL;
 
skb_set_transport_header(skb, keys.control.thoff);
+   } else if (gso_type) {
+   p_off = thlen;
+   if (p_off > skb_headlen(skb))
+   return -EINVAL;
}
}
 
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
u16 gso_size = __virtio16_to_cpu(little_endian, hdr->gso_size);
 
+   if (skb->len - p_off <= gso_size)
+   return -EINVAL;
+
skb_shinfo(skb)->gso_size = gso_size;
skb_shinfo(skb)->gso_type = gso_type;
 
-- 
2.25.1

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


Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread David Hildenbrand
On 05.06.20 12:46, Alex Shi wrote:
> 
> 
> 在 2020/6/5 下午6:05, David Hildenbrand 写道:
>>> I guess I know what's happening here. In case we only have DMA memory
>>> when booting, we don't reserve swiotlb buffers. Once we hotplug memory
>>> and online ZONE_NORMAL, we don't have any swiotlb DMA bounce buffers to
>>> map such PFNs (total 0 (slots), used 0 (slots)).
>>>
>>> Can you try with "swiotlb=force" on the kernel cmdline?
>> Alternative, looks like you can specify "-m 2G,maxmem=16G,slots=1", to
>> create proper ACPI tables that indicate hotpluggable memory. (I'll have
>> to look into QEMU to figure out to always indicate hotpluggable memory
>> that way).
>>
> 
> 
> That works too. Yes, better resolved in qemu, maybe. :)
> 

You can checkout

g...@github.com:davidhildenbrand/qemu.git virtio-mem-v4

(prone to change before officially sent), which will create srat tables
also if no "slots" parameter was defined (and no -numa config was
specified).

Your original example should work with that.

-- 
Thanks,

David / dhildenb

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

Re: [PATCH] s390/virtio: remove unused pm callbacks

2020-06-05 Thread Heiko Carstens
On Fri, Jun 05, 2020 at 09:39:07AM +0200, Cornelia Huck wrote:
> On Thu, 4 Jun 2020 23:44:21 +0200
> Halil Pasic  wrote:
> 
> > On Tue, 26 May 2020 11:36:29 +0200
> > Cornelia Huck  wrote:
> > 
> > > Support for hibernation on s390 has been recently been removed with
> 
> s/been recently been removed/recently been removed/
> 
> > > commit 394216275c7d ("s390: remove broken hibernate / power management
> > > support"), no need to keep unused code around.
> > > 
> > > Signed-off-by: Cornelia Huck   
> > 
> > Reviewed-by: Halil Pasic 
> 
> Thanks!
> 
> As this is only a single patch, I think a pull request is a bit
> overkill, so it would probably be best for someone to pick this
> directly.
> 
> s390 arch maintainers? Michael?

Applied, thanks!
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread Alex Shi


在 2020/6/5 下午6:05, David Hildenbrand 写道:
>> I guess I know what's happening here. In case we only have DMA memory
>> when booting, we don't reserve swiotlb buffers. Once we hotplug memory
>> and online ZONE_NORMAL, we don't have any swiotlb DMA bounce buffers to
>> map such PFNs (total 0 (slots), used 0 (slots)).
>>
>> Can you try with "swiotlb=force" on the kernel cmdline?
> Alternative, looks like you can specify "-m 2G,maxmem=16G,slots=1", to
> create proper ACPI tables that indicate hotpluggable memory. (I'll have
> to look into QEMU to figure out to always indicate hotpluggable memory
> that way).
> 


That works too. Yes, better resolved in qemu, maybe. :)

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

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread Alex Shi


在 2020/6/5 下午5:08, David Hildenbrand 写道:
> Please use the virtio-mem-v4 branch for now, v5 is still under
> construction (and might be scrapped completely if v4 goes upstream as is).
> 
> Looks like a DMA issue. Your're hotplugging 1GB, which should not really
> eat too much memory. There was a similar issue reported by Hui in [1],
> which boiled down to wrong usage of the swiotlb parameter.

I have no swiotbl=noforce set, and sometime no swiotlb error reported, like
(qemu) [   41.591308] e1000 :00:03.0: dma_direct_map_page: overflow 
0x00011fd470da+54 of device mask 
[   41.592431] e1000 :00:03.0: TX DMA map failed
[   41.593031] e1000 :00:03.0: dma_direct_map_page: overflow 
0x00011fd474da+54 of device mask ff
...
[   63.049464] ata_piix :00:01.1: dma_direct_map_sg: overflow 
0x000107db2000+4096 of device mask 
[   63.068297] ata1.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x0
[   63.069057] ata1.00: failed command: READ DMA
[   63.069580] ata1.00: cmd c8/00:20:40:bd:d2/00:00:00:00:00/e0 tag 0 dma 16384 
in
[   63.069580]  res 50/00:00:3f:30:80/00:00:00:00:00/a0 Emask 0x40 
(internal error) 
> 
> In such cases you should always try to reproduce with hotplug of a
> sam-sized DIMM. E.g., hotplugging a 1GB DIMM should result in the same
> issue.
> 
> What does your .config specify for CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE?

Yes, it's set. 

I had tried the v2/v4 version, which has the same issue.
Is this related with virtio-mem start address too low?

Thanks a lot!
> 
> I'll try to reproduce with v4 briefly.
> 
> [1]
> https://lkml.kernel.org/r/9708f43a-9bd2-4377-8ee8-7fb1d95c6...@linux.alibaba.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread Alex Shi


在 2020/6/5 下午5:36, David Hildenbrand 写道:
> I guess I know what's happening here. In case we only have DMA memory
> when booting, we don't reserve swiotlb buffers. Once we hotplug memory
> and online ZONE_NORMAL, we don't have any swiotlb DMA bounce buffers to
> map such PFNs (total 0 (slots), used 0 (slots)).
> 
> Can you try with "swiotlb=force" on the kernel cmdline?

Yes, it works fine with this cmdline. problems gone,
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread David Hildenbrand
On 05.06.20 11:36, David Hildenbrand wrote:
> On 05.06.20 11:08, David Hildenbrand wrote:
>> On 05.06.20 10:55, Alex Shi wrote:
>>>
>>>
>>> 在 2020/1/9 下午9:48, David Hildenbrand 写道:
 Ping,

 I'd love to get some feedback on

 a) The remaining MM bits from MM folks (especially, patch #6 and #8).
 b) The general virtio infrastructure (esp. uapi in patch #2) from virtio
 folks.

 I'm planning to send a proper v1 (!RFC) once I have all necessary MM
 acks. In the meanwhile, I will do more testing and minor reworks (e.g.,
 fix !CONFIG_NUMA compilation).
>>>
>>>
>>> Hi David,
>>>
>>> Thanks for your work!
>>>
>>> I am trying your https://github.com/davidhildenbrand/linux.git virtio-mem-v5
>>> which works fine for me, but just a 'DMA error' happens when a vm start with
>>> less than 2GB memory, Do I missed sth?
>>
>> Please use the virtio-mem-v4 branch for now, v5 is still under
>> construction (and might be scrapped completely if v4 goes upstream as is).
>>
>> Looks like a DMA issue. Your're hotplugging 1GB, which should not really
>> eat too much memory. There was a similar issue reported by Hui in [1],
>> which boiled down to wrong usage of the swiotlb parameter.
>>
>> In such cases you should always try to reproduce with hotplug of a
>> sam-sized DIMM. E.g., hotplugging a 1GB DIMM should result in the same
>> issue.
>>
>> What does your .config specify for CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE?
>>
>> I'll try to reproduce with v4 briefly.
> 
> I guess I know what's happening here. In case we only have DMA memory
> when booting, we don't reserve swiotlb buffers. Once we hotplug memory
> and online ZONE_NORMAL, we don't have any swiotlb DMA bounce buffers to
> map such PFNs (total 0 (slots), used 0 (slots)).
> 
> Can you try with "swiotlb=force" on the kernel cmdline?

Alternative, looks like you can specify "-m 2G,maxmem=16G,slots=1", to
create proper ACPI tables that indicate hotpluggable memory. (I'll have
to look into QEMU to figure out to always indicate hotpluggable memory
that way).


-- 
Thanks,

David / dhildenb

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

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread David Hildenbrand
On 05.06.20 11:08, David Hildenbrand wrote:
> On 05.06.20 10:55, Alex Shi wrote:
>>
>>
>> 在 2020/1/9 下午9:48, David Hildenbrand 写道:
>>> Ping,
>>>
>>> I'd love to get some feedback on
>>>
>>> a) The remaining MM bits from MM folks (especially, patch #6 and #8).
>>> b) The general virtio infrastructure (esp. uapi in patch #2) from virtio
>>> folks.
>>>
>>> I'm planning to send a proper v1 (!RFC) once I have all necessary MM
>>> acks. In the meanwhile, I will do more testing and minor reworks (e.g.,
>>> fix !CONFIG_NUMA compilation).
>>
>>
>> Hi David,
>>
>> Thanks for your work!
>>
>> I am trying your https://github.com/davidhildenbrand/linux.git virtio-mem-v5
>> which works fine for me, but just a 'DMA error' happens when a vm start with
>> less than 2GB memory, Do I missed sth?
> 
> Please use the virtio-mem-v4 branch for now, v5 is still under
> construction (and might be scrapped completely if v4 goes upstream as is).
> 
> Looks like a DMA issue. Your're hotplugging 1GB, which should not really
> eat too much memory. There was a similar issue reported by Hui in [1],
> which boiled down to wrong usage of the swiotlb parameter.
> 
> In such cases you should always try to reproduce with hotplug of a
> sam-sized DIMM. E.g., hotplugging a 1GB DIMM should result in the same
> issue.
> 
> What does your .config specify for CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE?
> 
> I'll try to reproduce with v4 briefly.

I guess I know what's happening here. In case we only have DMA memory
when booting, we don't reserve swiotlb buffers. Once we hotplug memory
and online ZONE_NORMAL, we don't have any swiotlb DMA bounce buffers to
map such PFNs (total 0 (slots), used 0 (slots)).

Can you try with "swiotlb=force" on the kernel cmdline?

-- 
Thanks,

David / dhildenb

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

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread David Hildenbrand
On 05.06.20 10:55, Alex Shi wrote:
> 
> 
> 在 2020/1/9 下午9:48, David Hildenbrand 写道:
>> Ping,
>>
>> I'd love to get some feedback on
>>
>> a) The remaining MM bits from MM folks (especially, patch #6 and #8).
>> b) The general virtio infrastructure (esp. uapi in patch #2) from virtio
>> folks.
>>
>> I'm planning to send a proper v1 (!RFC) once I have all necessary MM
>> acks. In the meanwhile, I will do more testing and minor reworks (e.g.,
>> fix !CONFIG_NUMA compilation).
> 
> 
> Hi David,
> 
> Thanks for your work!
> 
> I am trying your https://github.com/davidhildenbrand/linux.git virtio-mem-v5
> which works fine for me, but just a 'DMA error' happens when a vm start with
> less than 2GB memory, Do I missed sth?

Please use the virtio-mem-v4 branch for now, v5 is still under
construction (and might be scrapped completely if v4 goes upstream as is).

Looks like a DMA issue. Your're hotplugging 1GB, which should not really
eat too much memory. There was a similar issue reported by Hui in [1],
which boiled down to wrong usage of the swiotlb parameter.

In such cases you should always try to reproduce with hotplug of a
sam-sized DIMM. E.g., hotplugging a 1GB DIMM should result in the same
issue.

What does your .config specify for CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE?

I'll try to reproduce with v4 briefly.

[1]
https://lkml.kernel.org/r/9708f43a-9bd2-4377-8ee8-7fb1d95c6...@linux.alibaba.com

> 
> Thanks
> Alex
> 
> 
> (qemu) qom-set vm0 requested-size 1g
> (qemu) [   26.560026] virtio_mem virtio0: plugged size: 0x0
> [   26.560648] virtio_mem virtio0: requested size: 0x4000
> [   26.561730] systemd-journald[167]: no db file to read 
> /run/udev/data/+virtio:virtio0: No such file or directory
> [   26.563138] systemd-journald[167]: no db file to read 
> /run/udev/data/+virtio:virtio0: No such file or directory
> [   26.569122] Built 1 zonelists, mobility grouping on.  Total pages: 513141
> [   26.570039] Policy zone: Normal
> 
> (qemu) [   32.175838] e1000 :00:03.0: swiotlb buffer is full (sz: 81 
> bytes), total 0 (slots), used 0 (slots)
> [   32.176922] e1000 :00:03.0: TX DMA map failed
> [   32.177488] e1000 :00:03.0: swiotlb buffer is full (sz: 81 bytes), 
> total 0 (slots), used 0 (slots)
> [   32.178535] e1000 :00:03.0: TX DMA map failed
> 
> my qemu command is like this:
> qemu-system-x86_64  --enable-kvm \
>   -m 2G,maxmem=16G -kernel /root/linux-next/$1/arch/x86/boot/bzImage \
>   -smp 4 \
>   -append "earlyprintk=ttyS0 root=/dev/sda1 console=ttyS0 debug psi=1 
> nokaslr ignore_loglevel" \
>   -hda /root/CentOS-7-x86_64-Azure-1703.qcow2 \
>   -net user,hostfwd=tcp::-:22 -net nic -s \
>   -object memory-backend-ram,id=mem0,size=3G \
>   -device virtio-mem-pci,id=vm0,memdev=mem0,node=0,requested-size=0M \
>   --nographic
> 
> 


-- 
Thanks,

David / dhildenb

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

Re: [PATCH RFC v4 00/13] virtio-mem: paravirtualized memory

2020-06-05 Thread Alex Shi


在 2020/1/9 下午9:48, David Hildenbrand 写道:
> Ping,
> 
> I'd love to get some feedback on
> 
> a) The remaining MM bits from MM folks (especially, patch #6 and #8).
> b) The general virtio infrastructure (esp. uapi in patch #2) from virtio
> folks.
> 
> I'm planning to send a proper v1 (!RFC) once I have all necessary MM
> acks. In the meanwhile, I will do more testing and minor reworks (e.g.,
> fix !CONFIG_NUMA compilation).


Hi David,

Thanks for your work!

I am trying your https://github.com/davidhildenbrand/linux.git virtio-mem-v5
which works fine for me, but just a 'DMA error' happens when a vm start with
less than 2GB memory, Do I missed sth?

Thanks
Alex


(qemu) qom-set vm0 requested-size 1g
(qemu) [   26.560026] virtio_mem virtio0: plugged size: 0x0
[   26.560648] virtio_mem virtio0: requested size: 0x4000
[   26.561730] systemd-journald[167]: no db file to read 
/run/udev/data/+virtio:virtio0: No such file or directory
[   26.563138] systemd-journald[167]: no db file to read 
/run/udev/data/+virtio:virtio0: No such file or directory
[   26.569122] Built 1 zonelists, mobility grouping on.  Total pages: 513141
[   26.570039] Policy zone: Normal

(qemu) [   32.175838] e1000 :00:03.0: swiotlb buffer is full (sz: 81 
bytes), total 0 (slots), used 0 (slots)
[   32.176922] e1000 :00:03.0: TX DMA map failed
[   32.177488] e1000 :00:03.0: swiotlb buffer is full (sz: 81 bytes), total 
0 (slots), used 0 (slots)
[   32.178535] e1000 :00:03.0: TX DMA map failed

my qemu command is like this:
qemu-system-x86_64  --enable-kvm \
-m 2G,maxmem=16G -kernel /root/linux-next/$1/arch/x86/boot/bzImage \
-smp 4 \
-append "earlyprintk=ttyS0 root=/dev/sda1 console=ttyS0 debug psi=1 
nokaslr ignore_loglevel" \
-hda /root/CentOS-7-x86_64-Azure-1703.qcow2 \
-net user,hostfwd=tcp::-:22 -net nic -s \
  -object memory-backend-ram,id=mem0,size=3G \
  -device virtio-mem-pci,id=vm0,memdev=mem0,node=0,requested-size=0M \
--nographic

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

Re: [PATCH 5/6] vdpa: introduce virtio pci driver

2020-06-05 Thread Jason Wang


On 2020/6/2 下午3:08, Jason Wang wrote:



+static const struct pci_device_id vp_vdpa_id_table[] = {
+    { PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_ANY_ID) },
+    { 0 }
+};

This looks like it'll create a mess with either virtio pci
or vdpa being loaded at random. Maybe just don't specify
any IDs for now. Down the road we could get a
distinct vendor ID or a range of device IDs for this.



Right, will do.

Thanks 



Rethink about this. If we don't specify any ID, the binding won't work.

How about using a dedicated subsystem vendor id for this?

Thanks

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

Re: [PATCH RFC 11/13] vhost/scsi: switch to buf APIs

2020-06-05 Thread Stefan Hajnoczi
On Tue, Jun 02, 2020 at 09:06:20AM -0400, Michael S. Tsirkin wrote:
> Switch to buf APIs. Doing this exposes a spec violation in vhost scsi:
> all used bufs are marked with length 0.
> Fix that is left for another day.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  drivers/vhost/scsi.c | 73 ++--
>  1 file changed, 44 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index c39952243fd3..c426c4e899c7 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -71,8 +71,8 @@ struct vhost_scsi_inflight {
>  };
>  
>  struct vhost_scsi_cmd {
> - /* Descriptor from vhost_get_vq_desc() for virt_queue segment */
> - int tvc_vq_desc;
> + /* Descriptor from vhost_get_avail_buf() for virt_queue segment */
> + struct vhost_buf tvc_vq_desc;
>   /* virtio-scsi initiator task attribute */
>   int tvc_task_attr;
>   /* virtio-scsi response incoming iovecs */
> @@ -213,7 +213,7 @@ struct vhost_scsi {
>   * Context for processing request and control queue operations.
>   */
>  struct vhost_scsi_ctx {
> - int head;
> + struct vhost_buf buf;
>   unsigned int out, in;
>   size_t req_size, rsp_size;
>   size_t out_size, in_size;
> @@ -443,6 +443,20 @@ static int vhost_scsi_check_stop_free(struct se_cmd 
> *se_cmd)
>   return target_put_sess_cmd(se_cmd);
>  }
>  
> +/* Signal to guest that request finished with no input buffer. */
> +/* TODO calling this when writing into buffer and most likely a bug */
> +static void vhost_scsi_signal_noinput(struct vhost_dev *vdev,
> +   struct vhost_virtqueue *vq,
> +   struct vhost_buf *bufp)
> +{
> + struct vhost_buf buf = *bufp;
> +
> + buf.in_len = 0;
> + vhost_put_used_buf(vq, );

Yes, this behavior differs from the QEMU virtio-scsi device
implementation. I think it's just a quirk that is probably my fault (I
guess I thought the length information is already encoded in the payload
SCSI headers so we have no use for the used descriptor length field).

Whether it's worth changing now or is an interesting question. In theory
it would make vhost-scsi more spec compliant and guest drivers might be
happier (especially drivers for niche OSes that were only tested against
QEMU's virtio-scsi). On the other hand, it's a guest-visible change that
could break similar niche drivers that assume length is always 0.

I'd leave it as-is unless people hit issues that justify the risk of
changing it.

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH RFC 12/13] vhost/vsock: switch to the buf API

2020-06-05 Thread Stefan Hajnoczi
On Tue, Jun 02, 2020 at 09:06:22AM -0400, Michael S. Tsirkin wrote:
> A straight-forward conversion.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  drivers/vhost/vsock.c | 30 ++
>  1 file changed, 18 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] s390/virtio: remove unused pm callbacks

2020-06-05 Thread Cornelia Huck
On Thu, 4 Jun 2020 23:44:21 +0200
Halil Pasic  wrote:

> On Tue, 26 May 2020 11:36:29 +0200
> Cornelia Huck  wrote:
> 
> > Support for hibernation on s390 has been recently been removed with

s/been recently been removed/recently been removed/

> > commit 394216275c7d ("s390: remove broken hibernate / power management
> > support"), no need to keep unused code around.
> > 
> > Signed-off-by: Cornelia Huck   
> 
> Reviewed-by: Halil Pasic 

Thanks!

As this is only a single patch, I think a pull request is a bit
overkill, so it would probably be best for someone to pick this
directly.

s390 arch maintainers? Michael?

> 
> > ---
> >  drivers/s390/virtio/virtio_ccw.c | 26 --
> >  1 file changed, 26 deletions(-)
> > 
> > diff --git a/drivers/s390/virtio/virtio_ccw.c 
> > b/drivers/s390/virtio/virtio_ccw.c
> > index 957889a42d2e..5730572b52cd 100644
> > --- a/drivers/s390/virtio/virtio_ccw.c
> > +++ b/drivers/s390/virtio/virtio_ccw.c
> > @@ -1372,27 +1372,6 @@ static struct ccw_device_id virtio_ids[] = {
> > {},
> >  };
> >  
> > -#ifdef CONFIG_PM_SLEEP
> > -static int virtio_ccw_freeze(struct ccw_device *cdev)
> > -{
> > -   struct virtio_ccw_device *vcdev = dev_get_drvdata(>dev);
> > -
> > -   return virtio_device_freeze(>vdev);
> > -}
> > -
> > -static int virtio_ccw_restore(struct ccw_device *cdev)
> > -{
> > -   struct virtio_ccw_device *vcdev = dev_get_drvdata(>dev);
> > -   int ret;
> > -
> > -   ret = virtio_ccw_set_transport_rev(vcdev);
> > -   if (ret)
> > -   return ret;
> > -
> > -   return virtio_device_restore(>vdev);
> > -}
> > -#endif
> > -
> >  static struct ccw_driver virtio_ccw_driver = {
> > .driver = {
> > .owner = THIS_MODULE,
> > @@ -1405,11 +1384,6 @@ static struct ccw_driver virtio_ccw_driver = {
> > .set_online = virtio_ccw_online,
> > .notify = virtio_ccw_cio_notify,
> > .int_class = IRQIO_VIR,
> > -#ifdef CONFIG_PM_SLEEP
> > -   .freeze = virtio_ccw_freeze,
> > -   .thaw = virtio_ccw_restore,
> > -   .restore = virtio_ccw_restore,
> > -#endif
> >  };
> >  
> >  static int __init pure_hex(char **cp, unsigned int *val, int min_digit,  
> 

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


Re: [RFC 12/12] rpmsg: add a device ID to also bind to the ADSP device

2020-06-05 Thread Guennadi Liakhovetski
Hi Mathieu,

On Thu, Jun 04, 2020 at 02:01:56PM -0600, Mathieu Poirier wrote:
> On Fri, May 29, 2020 at 09:37:22AM +0200, Guennadi Liakhovetski wrote:
> > The ADSP device uses the RPMsg API to connect vhost and VirtIO SOF
> > Audio DSP drivers on KVM host and guest.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> >  drivers/rpmsg/virtio_rpmsg_bus.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c 
> > b/drivers/rpmsg/virtio_rpmsg_bus.c
> > index f3bd050..ebe3f19 100644
> > --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> > @@ -949,6 +949,7 @@ static void rpmsg_remove(struct virtio_device *vdev)
> >  
> >  static struct virtio_device_id id_table[] = {
> > { VIRTIO_ID_RPMSG, VIRTIO_DEV_ANY_ID },
> > +   { VIRTIO_ID_ADSP, VIRTIO_DEV_ANY_ID },
> 
> I am fine with this patch but won't add an RB because of the (many) checkpatch
> errors.  Based on the comment I made on the previous set seeing those was
> unexpected.

Are you using "--strict?" Sorry, I don't see any checkpatch errors, only 
warnings. 
Most of them are "over 80 characters" which as we now know is no more an issue, 
I just haven't updated my tree yet. Most others are really minor IMHO. Maybe 
one 
of them I actually would want to fix - using "help" instead of "---help---" in 
Kconfig. What errors are you seeing in your checks?

Thanks
Guennadi

> Thanks,
> Mathieu
> 
> > { 0 },
> >  };
> >  
> > -- 
> > 1.9.3
> > 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC 11/12] rpmsg: increase buffer size and reduce buffer number

2020-06-05 Thread Guennadi Liakhovetski
Hi Mathieu,

On Thu, Jun 04, 2020 at 01:58:39PM -0600, Mathieu Poirier wrote:
> Hi Guennadi,
> 
> On Fri, May 29, 2020 at 09:37:21AM +0200, Guennadi Liakhovetski wrote:
> > It is hard to imagine use-cases where 512 buffers would really be
> > needed, whereas 512 bytes per buffer might be too little. Change this
> > to use 16 16KiB buffers instead.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> >  include/linux/virtio_rpmsg.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/virtio_rpmsg.h b/include/linux/virtio_rpmsg.h
> > index 679be8b..1add468 100644
> > --- a/include/linux/virtio_rpmsg.h
> > +++ b/include/linux/virtio_rpmsg.h
> > @@ -72,8 +72,8 @@ enum rpmsg_ns_flags {
> >   * can change this without changing anything in the firmware of the remote
> >   * processor.
> >   */
> > -#define MAX_RPMSG_NUM_BUFS 512
> > -#define MAX_RPMSG_BUF_SIZE 512
> > +#define MAX_RPMSG_NUM_BUFS (512 / 32)
> > +#define MAX_RPMSG_BUF_SIZE (512 * 32)
> 
> These have been a standard in the rpmsg protocol since the inception of the
> subsystem 9 years ago and can't be changed without serious impact to existing
> implementations.

Yes, I expected this to raise complaints. I just modified them to be able to 
run my code, but a better solution is needed for sure.

> I suggest to dynamically set the number and size of the buffers to use
> based on the value of virtio_device_id::device.  To do that please spin
> off a new function, something like rpmsg_get_buffer_size(), and in there use
> the device ID to fetch the numbers based on vdev->id->device.  That way the
> rpmsg driver can be used by multiple clients and the specifics of the buffers
> adjusted without impact to other users.

I'll look into this!

Thanks
Guennadi

> Thanks,
> Mathieu
> 
> >  
> >  /* Address 53 is reserved for advertising remote services */
> >  #define RPMSG_NS_ADDR  53
> > -- 
> > 1.9.3
> > 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] Add a vhost RPMsg API

2020-06-05 Thread Guennadi Liakhovetski
Hi Michael,

Thanks for your review.

On Thu, Jun 04, 2020 at 03:23:37PM -0400, Michael S. Tsirkin wrote:
> On Wed, May 27, 2020 at 08:05:36PM +0200, Guennadi Liakhovetski wrote:
> > v3:
> > - address several checkpatch warnings
> > - address comments from Mathieu Poirier
> > 
> > v2:
> > - update patch #5 with a correct vhost_dev_init() prototype
> > - drop patch #6 - it depends on a different patch, that is currently
> >   an RFC
> > - address comments from Pierre-Louis Bossart:
> >   * remove "default n" from Kconfig
> > 
> > Linux supports RPMsg over VirtIO for "remote processor" /AMP use
> > cases. It can however also be used for virtualisation scenarios,
> > e.g. when using KVM to run Linux on both the host and the guests.
> > This patch set adds a wrapper API to facilitate writing vhost
> > drivers for such RPMsg-based solutions. The first use case is an
> > audio DSP virtualisation project, currently under development, ready
> > for review and submission, available at
> > https://github.com/thesofproject/linux/pull/1501/commits
> > A further patch for the ADSP vhost RPMsg driver will be sent
> > separately for review only since it cannot be merged without audio
> > patches being upstreamed first.
> 
> 
> RPMsg over virtio has several problems. One is that it's
> not specced at all. Before we add more stuff, I'd like so
> see at least an attempt at describing what it's supposed to do.

Sure, I can work on this with the original authors of the virtio-rpmsg 
implementation.

> Another it's out of line with 1.0 spec passing guest
> endian data around. Won't work if host and guest
> endian-ness do not match. Should pass eveything in LE and
> convert.

Yes, I have to fix this, thanks.

> It's great to see it's seeing active development finally.
> Do you think you will have time to address these?

Sure, I'll try to take care of them.

Thanks
Guennadi

> > Guennadi Liakhovetski (5):
> >   vhost: convert VHOST_VSOCK_SET_RUNNING to a generic ioctl
> >   vhost: (cosmetic) remove a superfluous variable initialisation
> >   rpmsg: move common structures and defines to headers
> >   rpmsg: update documentation
> >   vhost: add an RPMsg API
> > 
> >  Documentation/rpmsg.txt  |   6 +-
> >  drivers/rpmsg/virtio_rpmsg_bus.c |  78 +---
> >  drivers/vhost/Kconfig|   7 +
> >  drivers/vhost/Makefile   |   3 +
> >  drivers/vhost/rpmsg.c| 382 
> > +++
> >  drivers/vhost/vhost.c|   2 +-
> >  drivers/vhost/vhost_rpmsg.h  |  74 
> >  include/linux/virtio_rpmsg.h |  81 +
> >  include/uapi/linux/rpmsg.h   |   3 +
> >  include/uapi/linux/vhost.h   |   4 +-
> >  10 files changed, 559 insertions(+), 81 deletions(-)
> >  create mode 100644 drivers/vhost/rpmsg.c
> >  create mode 100644 drivers/vhost/vhost_rpmsg.h
> >  create mode 100644 include/linux/virtio_rpmsg.h
> > 
> > -- 
> > 1.9.3
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization