Re: [PATCH v7 13/46] virtio: simplify feature bit handling
Now that we use u64 for bits, we can simply them together. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- drivers/virtio/virtio.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
Based on patch by Cornelia Huck.
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
...
-static unsigned int features[] = {
+static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
+}
+;
+static unsigned int features[] = {
+ VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+ VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+ VIRTIO_BLK_F_TOPOLOGY,
+ VIRTIO_BLK_F_MQ,
+ VIRTIO_F_VERSION_1,
We can fit this into less lines, like done for features_legacy.
I was asking myself if we could do the conversion of the statical values
somehow upfront, to reduce the patch size and avoid cpu_to_virtio.* at those
places.
Otherwise looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 01/46] virtio: add low-level APIs for feature bits
On Sun, 30 Nov 2014 17:09:08 +0200 Michael S. Tsirkin m...@redhat.com wrote: Add low level APIs to test/set/clear feature bits. For use by transports, to make it easier to write code independent of feature bit array format. Note: APIs is prefixed with __ and has _bit suffix to stress its low level nature. It's for use by transports only: drivers should use virtio_has_feature and never need to set/clear features. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/linux/virtio_config.h | 53 --- 1 file changed, 50 insertions(+), 3 deletions(-) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next] vhost: remove unnecessary forward declarations in vhost.h
On Thu, Nov 27, 2014 at 02:41:21PM +0800, Jason Wang wrote:
Signed-off-by: Jason Wang jasow...@redhat.com
Applied, thanks.
---
drivers/vhost/vhost.h | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 3eda654..7d039ef 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -12,8 +12,6 @@
#include linux/virtio_ring.h
#include linux/atomic.h
-struct vhost_device;
-
struct vhost_work;
typedef void (*vhost_work_fn_t)(struct vhost_work *work);
@@ -54,8 +52,6 @@ struct vhost_log {
u64 len;
};
-struct vhost_virtqueue;
-
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
--
1.9.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 28/46] vhost: make features 64 bit
On Mon, Dec 01, 2014 at 04:12:37AM +, Ben Hutchings wrote:
On Sun, 2014-11-30 at 18:44 +0300, Sergei Shtylyov wrote:
Hello.
On 11/30/2014 6:11 PM, Michael S. Tsirkin wrote:
We need to use bit 32 for virtio 1.0
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/vhost.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 3eda654..c624b09 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -106,7 +106,7 @@ struct vhost_virtqueue {
/* Protected by virtqueue mutex. */
struct vhost_memory *memory;
void *private_data;
- unsigned acked_features;
+ u64 acked_features;
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
@@ -174,6 +174,6 @@ enum {
static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
{
- return vq-acked_features (1 bit);
+ return vq-acked_features (1ULL bit);
Erm, wouldn't the high word be just dropped when returning *int*? I
think
you need !!(vq-acked_features (1ULL bit)).
Or change the return type to bool.
Ben.
--
Ben Hutchings
The first rule of tautology club is the first rule of tautology club.
Yes, I'll do that I think.
Thanks!
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 09:16:41AM +0100, David Hildenbrand wrote:
Based on patch by Cornelia Huck.
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
...
-static unsigned int features[] = {
+static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
+}
+;
+static unsigned int features[] = {
+ VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+ VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+ VIRTIO_BLK_F_TOPOLOGY,
+ VIRTIO_BLK_F_MQ,
+ VIRTIO_F_VERSION_1,
We can fit this into less lines, like done for features_legacy.
I was asking myself if we could do the conversion of the statical values
somehow upfront, to reduce the patch size and avoid cpu_to_virtio.* at those
places.
Otherwise looks good to me.
I don't see how we can reduce the patch size.
For BE architectures it's dynamic, so at best the values
will become macros/incline functions taking a flag.
For some places on data path, it might be worth it
to cache the correct value e.g. as part of device
structure. This replaces a branch with a memory load,
so the gain would have to be measured, best done
separately?
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 09:16:41AM +0100, David Hildenbrand wrote:
Based on patch by Cornelia Huck.
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
...
-static unsigned int features[] = {
+static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
+}
+;
+static unsigned int features[] = {
+ VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+ VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+ VIRTIO_BLK_F_TOPOLOGY,
+ VIRTIO_BLK_F_MQ,
+ VIRTIO_F_VERSION_1,
We can fit this into less lines, like done for features_legacy.
Wrt packing code more tightly, I did it like this to
make it easier to compare the arrays.
Each flag is on the same line in original and new array.
I was asking myself if we could do the conversion of the statical values
somehow upfront, to reduce the patch size and avoid cpu_to_virtio.* at those
places.
Otherwise looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net] Revert drivers/net: Disable UFO through virtio in macvtap and tun
On Tue, Nov 11, 2014 at 05:12:58PM +, Ben Hutchings wrote:
This reverts commit 88e0e0e5aa722b193c8758c8b45d041de5316924 for
the tap drivers, but leaves UFO disabled in virtio_net.
libvirt at least assumes that tap features will never be dropped
in new kernel versions, and doing so prevents migration of VMs to
the never kernel version while they are running with virtio net
devices.
Fixes: 88e0e0e5aa7a (drivers/net: Disable UFO through virtio)
Signed-off-by: Ben Hutchings b...@decadent.org.uk
I did some light testing with IPv4, and this seems to migrate fine now.
So let's apply, please
Acked-by: Michael S. Tsirkin m...@redhat.com
---
Compile-tested only.
Ben.
drivers/net/macvtap.c | 13 -
drivers/net/tun.c | 19 ---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 6f226de..aeaeb6d 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -66,7 +66,7 @@ static struct cdev macvtap_cdev;
static const struct proto_ops macvtap_socket_ops;
#define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
- NETIF_F_TSO6)
+ NETIF_F_TSO6 | NETIF_F_UFO)
#define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
#define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
@@ -570,8 +570,6 @@ static int macvtap_skb_from_vnet_hdr(struct sk_buff *skb,
gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- pr_warn_once(macvtap: %s: using disabled UFO feature;
please fix this program\n,
- current-comm);
gso_type = SKB_GSO_UDP;
if (skb-protocol == htons(ETH_P_IPV6))
ipv6_proxy_select_ident(skb);
@@ -619,6 +617,8 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff
*skb,
vnet_hdr-gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo-gso_type SKB_GSO_TCPV6)
vnet_hdr-gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+ else if (sinfo-gso_type SKB_GSO_UDP)
+ vnet_hdr-gso_type = VIRTIO_NET_HDR_GSO_UDP;
else
BUG();
if (sinfo-gso_type SKB_GSO_TCP_ECN)
@@ -953,6 +953,9 @@ static int set_offload(struct macvtap_queue *q, unsigned
long arg)
if (arg TUN_F_TSO6)
feature_mask |= NETIF_F_TSO6;
}
+
+ if (arg TUN_F_UFO)
+ feature_mask |= NETIF_F_UFO;
}
/* tun/tap driver inverts the usage for TSO offloads, where
@@ -963,7 +966,7 @@ static int set_offload(struct macvtap_queue *q, unsigned
long arg)
* When user space turns off TSO, we turn off GSO/LRO so that
* user-space will not receive TSO frames.
*/
- if (feature_mask (NETIF_F_TSO | NETIF_F_TSO6))
+ if (feature_mask (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
features |= RX_OFFLOADS;
else
features = ~RX_OFFLOADS;
@@ -1064,7 +1067,7 @@ static long macvtap_ioctl(struct file *file, unsigned
int cmd,
case TUNSETOFFLOAD:
/* let the user check for future flags */
if (arg ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
- TUN_F_TSO_ECN))
+ TUN_F_TSO_ECN | TUN_F_UFO))
return -EINVAL;
rtnl_lock();
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7302398..a0987d1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -175,7 +175,7 @@ struct tun_struct {
struct net_device *dev;
netdev_features_t set_features;
#define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
- NETIF_F_TSO6)
+ NETIF_F_TSO6|NETIF_F_UFO)
int vnet_hdr_sz;
int sndbuf;
@@ -1152,20 +1152,10 @@ static ssize_t tun_get_user(struct tun_struct *tun,
struct tun_file *tfile,
skb_shinfo(skb)-gso_type = SKB_GSO_TCPV6;
break;
case VIRTIO_NET_HDR_GSO_UDP:
- {
- static bool warned;
-
- if (!warned) {
- warned = true;
- netdev_warn(tun-dev,
- %s: using disabled UFO feature;
please fix this program\n,
- current-comm);
- }
skb_shinfo(skb)-gso_type = SKB_GSO_UDP;
if (skb-protocol == htons(ETH_P_IPV6))
ipv6_proxy_select_ident(skb);
break;
- }
Re: [PATCH v7 08/46] virtio: memory access APIs
On Sun, 30 Nov 2014 17:09:50 +0200 Michael S. Tsirkin m...@redhat.com wrote: virtio 1.0 makes all memory structures LE, so we need APIs to conditionally do a byteswap on BE architectures. To make it easier to check code statically, add virtio specific types for multi-byte integers in memory. Add low level wrappers that do a byteswap conditionally, these will be useful e.g. for vhost. Add high level wrappers that query device endian-ness and act accordingly. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/linux/virtio_byteorder.h | 59 +++ include/linux/virtio_config.h | 32 + include/uapi/linux/virtio_ring.h | 45 ++--- include/uapi/linux/virtio_types.h | 46 ++ include/uapi/linux/Kbuild | 1 + 5 files changed, 161 insertions(+), 22 deletions(-) create mode 100644 include/linux/virtio_byteorder.h create mode 100644 include/uapi/linux/virtio_types.h Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 09:16:41AM +0100, David Hildenbrand wrote:
Based on patch by Cornelia Huck.
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
...
-static unsigned int features[] = {
+static unsigned int features_legacy[] = {
VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_SCSI,
VIRTIO_BLK_F_WCE, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE,
VIRTIO_BLK_F_MQ,
+}
+;
+static unsigned int features[] = {
+ VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_GEOMETRY,
+ VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+ VIRTIO_BLK_F_TOPOLOGY,
+ VIRTIO_BLK_F_MQ,
+ VIRTIO_F_VERSION_1,
We can fit this into less lines, like done for features_legacy.
Wrt packing code more tightly, I did it like this to
make it easier to compare the arrays.
Each flag is on the same line in original and new array.
This just looks inconsistent to me.
1. features_legacy is tightly packed
2. half of features is tightly packed
So either all tightly packed or put every item on a single line. At least
that's what I would do :)
I was asking myself if we could do the conversion of the statical values
somehow upfront, to reduce the patch size and avoid cpu_to_virtio.* at those
places.
Otherwise looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 12/46] virtio: set FEATURES_OK
On Sun, 30 Nov 2014 17:10:17 +0200 Michael S. Tsirkin m...@redhat.com wrote: set FEATURES_OK as per virtio 1.0 spec Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/uapi/linux/virtio_config.h | 2 ++ drivers/virtio/virtio.c| 29 ++--- 2 files changed, 24 insertions(+), 7 deletions(-) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 13/46] virtio: simplify feature bit handling
On Sun, 30 Nov 2014 17:10:22 +0200 Michael S. Tsirkin m...@redhat.com wrote: Now that we use u64 for bits, we can simply them together. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- drivers/virtio/virtio.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH RFC v4 net-next 0/5] virtio_net: enabling tx interrupts
Hello: We used to orphan packets before transmission for virtio-net. This breaks socket accounting and can lead serveral functions won't work, e.g: - Byte Queue Limit depends on tx completion nofication to work. - Packet Generator depends on tx completion nofication for the last transmitted packet to complete. - TCP Small Queue depends on proper accounting of sk_wmem_alloc to work. This series tries to solve the issue by enabling tx interrupts. To minize the performance impacts of this, several optimizations were used: - In guest side, virtqueue_enable_cb_delayed() was used to delay the tx interrupt untile 3/4 pending packets were sent. - In host side, interrupt coalescing were used to reduce tx interrupts. Performance test results[1] (tx-frames 16 tx-usecs 16) shows: - For guest receiving. No obvious regression on throughput were noticed. More cpu utilization were noticed in few cases. - For guest transmission. Very huge improvement on througput for small packet transmission were noticed. This is expected since TSQ and other optimization for small packet transmission work after tx interrupt. But will use more cpu for large packets. - For TCP_RR, regression (10% on transaction rate and cpu utilization) were found. Tx interrupt won't help but cause overhead in this case. Using more aggressive coalescing parameters may help to reduce the regression. Changes from RFC V3: - Don't free tx packets in ndo_start_xmit() - Add interrupt coalescing support for virtio-net Changes from RFC v2: - clean up code, address issues raised by Jason Changes from RFC v1: - address comments by Jason Wang, use delayed cb everywhere - rebased Jason's patch on top of mine and include it (with some tweaks) Please reivew. Comments were more than welcomed. [1] Performance Test result: Environment: - Two Intel(R) Xeon(R) CPU E5620 @ 2.40GHz machines connected back to back with 82599ES cards. - Both host and guest were net-next.git plus the patch - Coalescing parameters for the card: Adaptive RX: off TX: off rx-usecs: 1 rx-frames: 0 tx-usecs: 0 tx-frames: 0 - Vhost_net was enabled and zerocopy was disabled - Tests was done by netperf-2.6 - Guest has 2 vcpus with single queue virtio-net Results: - Numbers of square brackets are whose significance is grater than 95% Guest RX: size/sessions/+throughput/+cpu/+per_cpu_throughput/ 64/1/+2.0326/[+6.2807%]/-3.9970%/ 64/2/-0.2104%/[+3.2012%]/[-3.3058%]/ 64/4/+1.5956%/+2.2451%/-0.6353%/ 64/8/+1.1732%/+3.5123%/-2.2598%/ 256/1/+3.7619%/[+5.8117%]/-1.9372%/ 256/2/-0.0661%/[+3.2511%]/-3.2127%/ 256/4/+1.1435%/[-8.1842%]/[+10.1591%]/ 256/8/[+2.2447%]/[+6.2044%]/[-3.7283%]/ 1024/1/+9.1479%/[+12.0997%]/[-2.6332%]/ 1024/2/[-17.3341%]/[+0.%]/[-17.3341%]/ 1024/4/[-0.6284%]/-1.0376%/+0.4135%/ 1024/8/+1.1444%/-1.6069%/+2.7961%/ 4096/1/+0.0401%/-0.5993%/+0.6433%/ 4096/2/[-0.5894%]/-2.2071%/+1.6542%/ 4096/4/[-0.5560%]/-1.4969%/+0.9553%/ 4096/8/-0.3362%/+2.7086%/-2.9645%/ 16384/1/-0.0285%/+0.7247%/-0.7478%/ 16384/2/-0.5286%/+0.3287%/-0.8545%/ 16384/4/-0.3297%/-2.0543%/+1.7608%/ 16384/8/+1.0932%/+4.0253%/-2.8187%/ 65535/1/+0.0003%/-0.1502%/+0.1508%/ 65535/2/[-0.6065%]/+0.2309%/-0.8355%/ 65535/4/[-0.6861%]/[+3.9451%]/[-4.4554%]/ 65535/8/+1.8359%/+3.1590%/-1.2825%/ Guest RX: size/sessions/+throughput/+cpu/+per_cpu_throughput/ 64/1/[+65.0961%]/[-8.6807%]/[+80.7900%]/ 64/2/[+6.0288%]/[-2.2823%]/[+8.5052%]/ 64/4/[+5.9038%]/[-2.1834%]/[+8.2677%]/ 64/8/[+5.4154%]/[-2.1804%]/[+7.7651%]/ 256/1/[+184.6462%]/[+4.8906%]/[+171.3742%]/ 256/2/[+46.0731%]/[-8.9626%]/[+60.4539%]/ 256/4/[+45.8547%]/[-8.3027%]/[+59.0612%]/ 256/8/[+45.3486%]/[-8.4024%]/[+58.6817%]/ 1024/1/[+432.5372%]/[+3.9566%]/[+412.2689%]/ 1024/2/[-1.4207%]/[-23.6426%]/[+29.1025%]/ 1024/4/-0.1003%/[-13.6416%]/[+15.6804%]/ 1024/8/[+0.2200%]/[+2.0634%]/[-1.8061%]/ 4096/1/[+18.4835%]/[-46.1508%]/[+120.0283%]/ 4096/2/+0.1770%/[-26.2780%]/[+35.8848%]/ 4096/4/-0.1012%/-0.7353%/+0.6388%/ 4096/8/-0.6091%/+1.4159%/-1.9968%/ 16384/1/-0.0424%/[+11.9373%]/[-10.7021%]/ 16384/2/+0.0482%/+2.4685%/-2.3620%/ 16384/4/+0.0840%/[+5.3587%]/[-5.0064%]/ 16384/8/+0.0048%/[+5.0176%]/[-4.7733%]/ 65535/1/-0.0095%/[+10.9408%]/[-9.8705%]/ 65535/2/+0.1515%/[+8.1709%]/[-7.4137%]/ 65535/4/+0.0203%/[+5.4316%]/[-5.1325%]/ 65535/8/+0.1427%/[+6.2753%]/[-5.7705%]/ size/sessions/+trans.rate/+cpu/+per_cpu_trans.rate/ 64/1/+0.2346%/[+11.5080%]/[-10.1099%]/ 64/25/[-10.7893%]/-0.5791%/[-10.2697%]/ 64/50/[-11.5997%]/-0.3429%/[-11.2956%]/ 256/1/+0.7219%/[+13.2374%]/[-11.0524%]/ 256/25/-6.9567%/+0.8887%/[-7.7763%]/ 256/50/[-4.8814%]/-0.0338%/[-4.8492%]/ 4096/1/-1.6061%/-0.7561%/-0.8565%/ 4096/25/[+2.2120%]/[+1.0839%]/+1.1161%/ 4096/50/[+5.6180%]/[+3.2116%]/[+2.3315%]/ Jason Wang (4): virtio_net: enable tx interrupt virtio-net: optimize free_old_xmit_skbs stats virtio-net: add basic interrupt coalescing support vhost_net: interrupt coalescing support Michael S. Tsirkin (1): virtio_net: bql drivers/net/virtio_net.c| 211
[PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
On newer hosts that support delayed tx interrupts,
we probably don't have much to gain from orphaning
packets early.
Note: this might degrade performance for
hosts without event idx support.
Should be addressed by the next patch.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c | 132 +++
1 file changed, 88 insertions(+), 44 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..f68114e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -72,6 +72,8 @@ struct send_queue {
/* Name of the send queue: output.$index */
char name[40];
+
+ struct napi_struct napi;
};
/* Internal representation of a receive virtqueue */
@@ -137,6 +139,9 @@ struct virtnet_info {
/* CPU hot plug notifier */
struct notifier_block nb;
+
+ /* Budget for polling tx completion */
+ u32 tx_work_limit;
};
struct skb_vnet_hdr {
@@ -211,15 +216,41 @@ static struct page *get_a_page(struct receive_queue *rq,
gfp_t gfp_mask)
return p;
}
+static unsigned int free_old_xmit_skbs(struct netdev_queue *txq,
+ struct send_queue *sq, int budget)
+{
+ struct sk_buff *skb;
+ unsigned int len;
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
+ unsigned int packets = 0;
+
+ while (packets budget
+ (skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
+ pr_debug(Sent skb %p\n, skb);
+
+ u64_stats_update_begin(stats-tx_syncp);
+ stats-tx_bytes += skb-len;
+ stats-tx_packets++;
+ u64_stats_update_end(stats-tx_syncp);
+
+ dev_kfree_skb_any(skb);
+ packets++;
+ }
+
+ if (sq-vq-num_free = 2+MAX_SKB_FRAGS)
+ netif_tx_start_queue(txq);
+
+ return packets;
+}
+
static void skb_xmit_done(struct virtqueue *vq)
{
struct virtnet_info *vi = vq-vdev-priv;
+ struct send_queue *sq = vi-sq[vq2txq(vq)];
- /* Suppress further interrupts. */
- virtqueue_disable_cb(vq);
-
- /* We were probably waiting for more output buffers. */
- netif_wake_subqueue(vi-dev, vq2txq(vq));
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
}
static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
@@ -777,6 +808,32 @@ again:
return received;
}
+static int virtnet_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct send_queue *sq =
+ container_of(napi, struct send_queue, napi);
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct netdev_queue *txq = netdev_get_tx_queue(vi-dev, vq2txq(sq-vq));
+ u32 limit = vi-tx_work_limit;
+ unsigned int sent;
+
+ __netif_tx_lock(txq, smp_processor_id());
+ sent = free_old_xmit_skbs(txq, sq, limit);
+ if (sent limit) {
+ napi_complete(napi);
+ /* Note: we must enable cb *after* napi_complete, because
+* napi_schedule calls from callbacks that trigger before
+* napi_complete are ignored.
+*/
+ if (unlikely(!virtqueue_enable_cb_delayed(sq-vq))) {
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
+ }
+ }
+ __netif_tx_unlock(txq);
+ return sent limit ? 0 : budget;
+}
+
#ifdef CONFIG_NET_RX_BUSY_POLL
/* must be called with local_bh_disable()d */
static int virtnet_busy_poll(struct napi_struct *napi)
@@ -825,30 +882,12 @@ static int virtnet_open(struct net_device *dev)
if (!try_fill_recv(vi-rq[i], GFP_KERNEL))
schedule_delayed_work(vi-refill, 0);
virtnet_napi_enable(vi-rq[i]);
+ napi_enable(vi-sq[i].napi);
}
return 0;
}
-static void free_old_xmit_skbs(struct send_queue *sq)
-{
- struct sk_buff *skb;
- unsigned int len;
- struct virtnet_info *vi = sq-vq-vdev-priv;
- struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
-
- while ((skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
- pr_debug(Sent skb %p\n, skb);
-
- u64_stats_update_begin(stats-tx_syncp);
- stats-tx_bytes += skb-len;
- stats-tx_packets++;
- u64_stats_update_end(stats-tx_syncp);
-
- dev_kfree_skb_any(skb);
- }
-}
-
static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
{
struct skb_vnet_hdr *hdr;
@@ -912,7 +951,9 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
sg_set_buf(sq-sg, hdr, hdr_len);
[PATCH RFC v4 net-next 2/5] virtio_net: bql
From: Michael S. Tsirkin m...@redhat.com
Improve tx batching using byte queue limits.
Should be especially effective for MQ.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/net/virtio_net.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f68114e..0ed24ff 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -224,6 +224,7 @@ static unsigned int free_old_xmit_skbs(struct netdev_queue
*txq,
struct virtnet_info *vi = sq-vq-vdev-priv;
struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
unsigned int packets = 0;
+ unsigned int bytes = 0;
while (packets budget
(skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
@@ -231,6 +232,7 @@ static unsigned int free_old_xmit_skbs(struct netdev_queue
*txq,
u64_stats_update_begin(stats-tx_syncp);
stats-tx_bytes += skb-len;
+ bytes += skb-len;
stats-tx_packets++;
u64_stats_update_end(stats-tx_syncp);
@@ -238,6 +240,8 @@ static unsigned int free_old_xmit_skbs(struct netdev_queue
*txq,
packets++;
}
+ netdev_tx_completed_queue(txq, packets, bytes);
+
if (sq-vq-num_free = 2+MAX_SKB_FRAGS)
netif_tx_start_queue(txq);
@@ -964,6 +968,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
int err;
struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
bool kick = !skb-xmit_more;
+ unsigned int bytes = skb-len;
virtqueue_disable_cb(sq-vq);
@@ -981,6 +986,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
return NETDEV_TX_OK;
}
+ netdev_tx_sent_queue(txq, bytes);
+
/* Apparently nice girls don't return TX_BUSY; stop the queue
* before it gets out of hand. Naturally, this wastes entries. */
if (sq-vq-num_free 2+MAX_SKB_FRAGS)
--
1.8.3.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH RFC v4 net-next 3/5] virtio-net: optimize free_old_xmit_skbs stats
We already have counters for sent packets and sent bytes.
Use them to reduce the number of u64_stats_update_begin/end().
Take care not to bother with stats update when called
speculatively.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/net/virtio_net.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0ed24ff..9f420c9 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -230,16 +230,23 @@ static unsigned int free_old_xmit_skbs(struct
netdev_queue *txq,
(skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
pr_debug(Sent skb %p\n, skb);
- u64_stats_update_begin(stats-tx_syncp);
- stats-tx_bytes += skb-len;
bytes += skb-len;
- stats-tx_packets++;
- u64_stats_update_end(stats-tx_syncp);
+ packets++;
dev_kfree_skb_any(skb);
- packets++;
}
+ /* Avoid overhead when no packets have been processed
+* happens when called speculatively from start_xmit.
+*/
+ if (!packets)
+ return 0;
+
+ u64_stats_update_begin(stats-tx_syncp);
+ stats-tx_bytes += bytes;
+ stats-tx_packets += packets;
+ u64_stats_update_end(stats-tx_syncp);
+
netdev_tx_completed_queue(txq, packets, bytes);
if (sq-vq-num_free = 2+MAX_SKB_FRAGS)
--
1.8.3.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH RFC v4 net-next 4/5] virtio-net: add basic interrupt coalescing support
This patch enables the interrupt coalescing setting through ethtool.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 67 +
include/uapi/linux/virtio_net.h | 12
2 files changed, 79 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9f420c9..2a3551a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -142,6 +142,11 @@ struct virtnet_info {
/* Budget for polling tx completion */
u32 tx_work_limit;
+
+ __u32 rx_coalesce_usecs;
+ __u32 rx_max_coalesced_frames;
+ __u32 tx_coalesce_usecs;
+ __u32 tx_max_coalesced_frames;
};
struct skb_vnet_hdr {
@@ -1419,12 +1424,73 @@ static void virtnet_get_channels(struct net_device *dev,
channels-other_count = 0;
}
+static int virtnet_set_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ struct scatterlist sg;
+ struct virtio_net_ctrl_coalesce c;
+
+ if (!vi-has_cvq ||
+ !virtio_has_feature(vi-vdev, VIRTIO_NET_F_CTRL_COALESCE))
+ return -EOPNOTSUPP;
+ if (vi-rx_coalesce_usecs != ec-rx_coalesce_usecs ||
+ vi-rx_max_coalesced_frames != ec-rx_max_coalesced_frames) {
+ c.coalesce_usecs = ec-rx_coalesce_usecs;
+ c.max_coalesced_frames = ec-rx_max_coalesced_frames;
+ sg_init_one(sg, c, sizeof(c));
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
+ VIRTIO_NET_CTRL_COALESCE_RX_SET,
+ sg)) {
+ dev_warn(dev-dev, Fail to set rx coalescing\n);
+ return -EINVAL;
+ }
+ vi-rx_coalesce_usecs = ec-rx_coalesce_usecs;
+ vi-rx_max_coalesced_frames = ec-rx_max_coalesced_frames;
+ }
+
+ if (vi-tx_coalesce_usecs != ec-tx_coalesce_usecs ||
+ vi-tx_max_coalesced_frames != ec-tx_max_coalesced_frames) {
+ c.coalesce_usecs = ec-tx_coalesce_usecs;
+ c.max_coalesced_frames = ec-tx_max_coalesced_frames;
+ sg_init_one(sg, c, sizeof(c));
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_COALESCE,
+ VIRTIO_NET_CTRL_COALESCE_TX_SET,
+ sg)) {
+ dev_warn(dev-dev, Fail to set tx coalescing\n);
+ return -EINVAL;
+ }
+ vi-tx_coalesce_usecs = ec-tx_coalesce_usecs;
+ vi-tx_max_coalesced_frames = ec-tx_max_coalesced_frames;
+ }
+
+ vi-tx_work_limit = ec-tx_max_coalesced_frames_irq;
+
+ return 0;
+}
+
+static int virtnet_get_coalesce(struct net_device *dev,
+ struct ethtool_coalesce *ec)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ ec-rx_coalesce_usecs = vi-rx_coalesce_usecs;
+ ec-rx_max_coalesced_frames = vi-rx_max_coalesced_frames;
+ ec-tx_coalesce_usecs = vi-tx_coalesce_usecs;
+ ec-tx_max_coalesced_frames = vi-tx_max_coalesced_frames;
+ ec-tx_max_coalesced_frames_irq = vi-tx_work_limit;
+
+ return 0;
+}
+
static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
.set_channels = virtnet_set_channels,
.get_channels = virtnet_get_channels,
+ .set_coalesce = virtnet_set_coalesce,
+ .get_coalesce = virtnet_get_coalesce,
};
#define MIN_MTU 68
@@ -2022,6 +2088,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+ VIRTIO_NET_F_CTRL_COALESCE,
};
static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..cdafb57 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -33,6 +33,7 @@
/* The feature bitmap for virtio net */
#define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
#define VIRTIO_NET_F_GUEST_CSUM1 /* Guest handles pkts w/
partial csum */
+#define VIRTIO_NET_F_CTRL_COALESCE 3 /* Set coalescing */
#define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
#define VIRTIO_NET_F_GUEST_TSO47 /* Guest can handle TSOv4 in. */
@@ -201,4 +202,15 @@ struct virtio_net_ctrl_mq {
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN1
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX0x8000
+struct
[PATCH RFC v4 net-next 5/5] vhost_net: interrupt coalescing support
This patch implements interrupt coalescing support for vhost_net. And provides
ioctl()s for userspace to get and set coalescing parameters. Two kinds of
parameters were allowed to be set:
- max_coalesced_frames: which is the maximum numbers of packets were allowed
before issuing an irq.
- coalesced_usecs: which is the maximum number of micro seconds were allowed
before issuing an irq if at least one packet were pending.
A per virtqueue hrtimer were used for coalesced_usecs.
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/net.c| 200 +++--
include/uapi/linux/vhost.h | 12 +++
2 files changed, 203 insertions(+), 9 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8dae2f7..c416aa7 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -18,6 +18,7 @@
#include linux/file.h
#include linux/slab.h
#include linux/vmalloc.h
+#include linux/timer.h
#include linux/net.h
#include linux/if_packet.h
@@ -61,7 +62,8 @@ MODULE_PARM_DESC(experimental_zcopytx, Enable Zero Copy TX;
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
(1ULL VHOST_NET_F_VIRTIO_NET_HDR) |
-(1ULL VIRTIO_NET_F_MRG_RXBUF),
+(1ULL VIRTIO_NET_F_MRG_RXBUF) |
+(1ULL VIRTIO_NET_F_CTRL_COALESCE)
};
enum {
@@ -99,6 +101,15 @@ struct vhost_net_virtqueue {
/* Reference counting for outstanding ubufs.
* Protected by vq mutex. Writers must also take device mutex. */
struct vhost_net_ubuf_ref *ubufs;
+ /* Microseconds after at least 1 paket is processed before
+* generating an interrupt.
+*/
+ __u32 coalesce_usecs;
+ /* Packets are processed before genearting an interrupt. */
+ __u32 max_coalesced_frames;
+ __u32 coalesced;
+ ktime_t last_signal;
+ struct hrtimer c_timer;
};
struct vhost_net {
@@ -196,11 +207,16 @@ static void vhost_net_vq_reset(struct vhost_net *n)
vhost_net_clear_ubuf_info(n);
for (i = 0; i VHOST_NET_VQ_MAX; i++) {
+ hrtimer_cancel(n-vqs[i].c_timer);
n-vqs[i].done_idx = 0;
n-vqs[i].upend_idx = 0;
n-vqs[i].ubufs = NULL;
n-vqs[i].vhost_hlen = 0;
n-vqs[i].sock_hlen = 0;
+ n-vqs[i].max_coalesced_frames = 0;
+ n-vqs[i].coalesce_usecs = 0;
+ n-vqs[i].last_signal = ktime_get();
+ n-vqs[i].coalesced = 0;
}
}
@@ -272,6 +288,56 @@ static void copy_iovec_hdr(const struct iovec *from,
struct iovec *to,
}
}
+static int vhost_net_check_coalesce_and_signal(struct vhost_dev *dev,
+ struct vhost_net_virtqueue *nvq)
+{
+ struct vhost_virtqueue *vq = nvq-vq;
+ int left = 0;
+ ktime_t now;
+
+ if (nvq-coalesced) {
+ now = ktime_get();
+ left = nvq-coalesce_usecs -
+ ktime_to_us(ktime_sub(now, nvq-last_signal));
+ if (left = 0) {
+ vhost_signal(dev, vq);
+ nvq-last_signal = now;
+ nvq-coalesced = 0;
+ }
+ }
+
+ return left;
+}
+
+static bool vhost_net_add_used_and_signal_n(struct vhost_dev *dev,
+ struct vhost_net_virtqueue *nvq,
+ struct vring_used_elem *heads,
+ unsigned count)
+{
+ struct vhost_virtqueue *vq = nvq-vq;
+ bool can_coalesce = nvq-max_coalesced_frames nvq-coalesce_usecs;
+ bool ret = false;
+
+ vhost_add_used_n(vq, heads, count);
+
+ if (can_coalesce) {
+ ktime_t now = ktime_get();
+
+ nvq-coalesced += count;
+ if ((nvq-coalesced = nvq-max_coalesced_frames) ||
+ (ktime_to_us(ktime_sub(now, nvq-last_signal)) =
+nvq-coalesce_usecs)) {
+ vhost_signal(dev, vq);
+ nvq-coalesced = 0;
+ nvq-last_signal = now;
+ ret = true;
+ }
+ } else {
+ vhost_signal(dev, vq);
+ }
+ return ret;
+}
+
/* In case of DMA done not in order in lower device driver for some reason.
* upend_idx is used to track end of used idx, done_idx is used to track head
* of used idx. Once lower device DMA done contiguously, we will signal KVM
@@ -296,8 +362,8 @@ static void vhost_zerocopy_signal_used(struct vhost_net
*net,
}
while (j) {
add = min(UIO_MAXIOV - nvq-done_idx, j);
- vhost_add_used_and_signal_n(vq-dev, vq,
- vq-heads[nvq-done_idx], add);
+
Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
On Mon, Dec 01, 2014 at 06:17:04PM +0800, Jason Wang wrote:
On newer hosts that support delayed tx interrupts,
we probably don't have much to gain from orphaning
packets early.
Note: this might degrade performance for
hosts without event idx support.
Should be addressed by the next patch.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
Could you document the changes from the RFC I sent please?
Are there optimizations?
If yes, it might be easier to review (at least for me), if you refactor this,
e.g. applying the straight-forward rfc patch and then optimizations if
any on top. If it's taking a different approach, pls feel free to
disregard this.
---
drivers/net/virtio_net.c | 132
+++
1 file changed, 88 insertions(+), 44 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..f68114e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -72,6 +72,8 @@ struct send_queue {
/* Name of the send queue: output.$index */
char name[40];
+
+ struct napi_struct napi;
};
/* Internal representation of a receive virtqueue */
@@ -137,6 +139,9 @@ struct virtnet_info {
/* CPU hot plug notifier */
struct notifier_block nb;
+
+ /* Budget for polling tx completion */
+ u32 tx_work_limit;
};
struct skb_vnet_hdr {
@@ -211,15 +216,41 @@ static struct page *get_a_page(struct receive_queue
*rq, gfp_t gfp_mask)
return p;
}
+static unsigned int free_old_xmit_skbs(struct netdev_queue *txq,
+struct send_queue *sq, int budget)
+{
+ struct sk_buff *skb;
+ unsigned int len;
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
+ unsigned int packets = 0;
+
+ while (packets budget
+(skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
+ pr_debug(Sent skb %p\n, skb);
+
+ u64_stats_update_begin(stats-tx_syncp);
+ stats-tx_bytes += skb-len;
+ stats-tx_packets++;
+ u64_stats_update_end(stats-tx_syncp);
+
+ dev_kfree_skb_any(skb);
+ packets++;
+ }
+
+ if (sq-vq-num_free = 2+MAX_SKB_FRAGS)
+ netif_tx_start_queue(txq);
+
+ return packets;
+}
+
static void skb_xmit_done(struct virtqueue *vq)
{
struct virtnet_info *vi = vq-vdev-priv;
+ struct send_queue *sq = vi-sq[vq2txq(vq)];
- /* Suppress further interrupts. */
- virtqueue_disable_cb(vq);
-
- /* We were probably waiting for more output buffers. */
- netif_wake_subqueue(vi-dev, vq2txq(vq));
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
}
static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
@@ -777,6 +808,32 @@ again:
return received;
}
+static int virtnet_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct send_queue *sq =
+ container_of(napi, struct send_queue, napi);
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct netdev_queue *txq = netdev_get_tx_queue(vi-dev, vq2txq(sq-vq));
+ u32 limit = vi-tx_work_limit;
+ unsigned int sent;
+
+ __netif_tx_lock(txq, smp_processor_id());
+ sent = free_old_xmit_skbs(txq, sq, limit);
+ if (sent limit) {
+ napi_complete(napi);
+ /* Note: we must enable cb *after* napi_complete, because
+ * napi_schedule calls from callbacks that trigger before
+ * napi_complete are ignored.
+ */
+ if (unlikely(!virtqueue_enable_cb_delayed(sq-vq))) {
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
+ }
+ }
+ __netif_tx_unlock(txq);
+ return sent limit ? 0 : budget;
+}
+
Unlike the patch I sent, this seems to ignore the budget,
and always poll the full napi_weight.
Seems strange. What is the reason for this?
#ifdef CONFIG_NET_RX_BUSY_POLL
/* must be called with local_bh_disable()d */
static int virtnet_busy_poll(struct napi_struct *napi)
@@ -825,30 +882,12 @@ static int virtnet_open(struct net_device *dev)
if (!try_fill_recv(vi-rq[i], GFP_KERNEL))
schedule_delayed_work(vi-refill, 0);
virtnet_napi_enable(vi-rq[i]);
+ napi_enable(vi-sq[i].napi);
}
return 0;
}
-static void free_old_xmit_skbs(struct send_queue *sq)
-{
- struct sk_buff *skb;
- unsigned int len;
- struct virtnet_info *vi = sq-vq-vdev-priv;
- struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
-
- while ((skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
-
Re: [PATCH v7 27/46] virtio_net: enable v1.0 support
On Mon, Dec 01, 2014 at 12:40:08PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:30 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Now that we have completed 1.0 support, enable it in our driver.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/net/virtio_net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a0e64cf..c6a72d3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2003,6 +2003,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+ VIRTIO_F_VERSION_1,
};
static struct virtio_driver virtio_net_driver = {
Shouldn't you move this after the patch disabling mac address writing?
Probably a good idea.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? We should make sure that e.g. s390 only takes minor hit due to all that swapping that is needed for standard-compliant devices. Caching the value might certainly help in some paths. Well, this is queued in linux-next for 3.19, so now's the time to do it :) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) We should make sure that e.g. s390 only takes minor hit due to all that swapping that is needed for standard-compliant devices. Caching the value might certainly help in some paths. Well, this is queued in linux-next for 3.19, so now's the time to do it :) So much to do, so little time... I'm still feeling a bit uncomfortable with some of the changes (virtio-scsi etc.) as I have not been able to test them yet (as there's no converted qemu for these yet). The virtio-net and virtio-blk changes seem sane, though, and virtio-ccw should be fine as well. OTOH, it's not like we're introducing new external interfaces, so later rework should be fine. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 29/46] vhost: add memory access wrappers
On Sun, 30 Nov 2014 17:11:39 +0200 Michael S. Tsirkin m...@redhat.com wrote: Add guest memory access wrappers to handle virtio endianness conversions. Signed-off-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Jason Wang jasow...@redhat.com --- drivers/vhost/vhost.h | 31 +++ 1 file changed, 31 insertions(+) Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 01:02:58PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) We should make sure that e.g. s390 only takes minor hit due to all that swapping that is needed for standard-compliant devices. Caching the value might certainly help in some paths. Well, this is queued in linux-next for 3.19, so now's the time to do it :) So much to do, so little time... I'm still feeling a bit uncomfortable with some of the changes (virtio-scsi etc.) as I have not been able to test them yet (as there's no converted qemu for these yet). The virtio-net and virtio-blk changes seem sane, though, and virtio-ccw should be fine as well. OTOH, it's not like we're introducing new external interfaces, so later rework should be fine. Right. I'll send a revision with virtio console and the rest of devices shortly. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 30/46] vhost/net: force len for TX to host endian
On Sun, 30 Nov 2014 17:11:44 +0200
Michael S. Tsirkin m...@redhat.com wrote:
vhost/net keeps a copy of some used ring but (ab)uses length
field for internal house-keeping. This works because
for tx used length is always 0.
Suppress sparse errors: we use native endian-ness internally but never
expose it to guest.
I admit that I find this patch description hard to read :)
vhost/net keeps a copy of the used ring in host memory but (ab)uses
the length field for internal house-keeping. This works because the
length in the used ring for tx is always 0. In order to suppress sparse
errors, we need to force native endianness.
?
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/net.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8dae2f7..dce5c58 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, Enable Zero Copy
TX;
* status internally; used for zerocopy tx only.
*/
/* Lower device DMA failed */
-#define VHOST_DMA_FAILED_LEN 3
+#define VHOST_DMA_FAILED_LEN ((__force __virtio32)3)
/* Lower device DMA done */
-#define VHOST_DMA_DONE_LEN 2
+#define VHOST_DMA_DONE_LEN ((__force __virtio32)2)
/* Lower device DMA in progress */
-#define VHOST_DMA_IN_PROGRESS1
+#define VHOST_DMA_IN_PROGRESS((__force __virtio32)1)
/* Buffer unused */
-#define VHOST_DMA_CLEAR_LEN 0
+#define VHOST_DMA_CLEAR_LEN ((__force __virtio32)0)
-#define VHOST_DMA_IS_DONE(len) ((len) = VHOST_DMA_DONE_LEN)
+#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) = (__force
u32)VHOST_DMA_DONE_LEN)
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 30/46] vhost/net: force len for TX to host endian
On Mon, Dec 01, 2014 at 01:20:51PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:44 +0200
Michael S. Tsirkin m...@redhat.com wrote:
vhost/net keeps a copy of some used ring but (ab)uses length
field for internal house-keeping. This works because
for tx used length is always 0.
Suppress sparse errors: we use native endian-ness internally but never
expose it to guest.
I admit that I find this patch description hard to read :)
vhost/net keeps a copy of the used ring in host memory but (ab)uses
the length field for internal house-keeping. This works because the
length in the used ring for tx is always 0. In order to suppress sparse
errors, we need to force native endianness.
?
Yes. Add to this These values are never exposed to guest.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/net.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8dae2f7..dce5c58 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, Enable Zero
Copy TX;
* status internally; used for zerocopy tx only.
*/
/* Lower device DMA failed */
-#define VHOST_DMA_FAILED_LEN 3
+#define VHOST_DMA_FAILED_LEN ((__force __virtio32)3)
/* Lower device DMA done */
-#define VHOST_DMA_DONE_LEN 2
+#define VHOST_DMA_DONE_LEN ((__force __virtio32)2)
/* Lower device DMA in progress */
-#define VHOST_DMA_IN_PROGRESS 1
+#define VHOST_DMA_IN_PROGRESS ((__force __virtio32)1)
/* Buffer unused */
-#define VHOST_DMA_CLEAR_LEN0
+#define VHOST_DMA_CLEAR_LEN((__force __virtio32)0)
-#define VHOST_DMA_IS_DONE(len) ((len) = VHOST_DMA_DONE_LEN)
+#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) = (__force
u32)VHOST_DMA_DONE_LEN)
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 93
+++
1 file changed, 56 insertions(+), 37 deletions(-)
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect-len);
int ret;
/* Sanity check */
- if (unlikely(indirect-len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, Invalid length in indirect descriptor:
len 0x%llx not multiple of 0x%zx\n,
- (unsigned long long)indirect-len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect-len),
Can't you use len here?
Not if I want error message to be readable.
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect-addr, indirect-len, vq-indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect-addr), len,
vq-indirect,
UIO_MAXIOV);
if (unlikely(ret 0)) {
vq_err(vq, Translation failure %d in indirect.\n, ret);
@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq,
struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq-last_used_idx, vq-used-idx)) {
+ if (__put_user(cpu_to_vhost16(vq, vq-last_used_idx), vq-used-idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
We should generally to __ variants since addresses are pre-validated.
But I agree - should be a separate patch.
vq_err(vq, Failed to increment used idx);
return -EFAULT;
}
@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev,
struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
Same. Will split this out, it's unrelated to virtio 1.0.
vq_err(vq, Failed to get used event idx);
return true;
}
- return vring_need_event(event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
On Mon, 1 Dec 2014 14:37:01 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 93
+++
1 file changed, 56 insertions(+), 37 deletions(-)
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue
*vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect-len);
int ret;
/* Sanity check */
- if (unlikely(indirect-len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, Invalid length in indirect descriptor:
len 0x%llx not multiple of 0x%zx\n,
-(unsigned long long)indirect-len,
+(unsigned long long)vhost32_to_cpu(vq, indirect-len),
Can't you use len here?
Not if I want error message to be readable.
Huh? Both have the same value.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
On Mon, Dec 01, 2014 at 01:42:47PM +0100, Cornelia Huck wrote:
On Mon, 1 Dec 2014 14:37:01 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 93
+++
1 file changed, 56 insertions(+), 37 deletions(-)
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue
*vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect-len);
int ret;
/* Sanity check */
- if (unlikely(indirect-len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, Invalid length in indirect descriptor:
len 0x%llx not multiple of 0x%zx\n,
- (unsigned long long)indirect-len,
+ (unsigned long long)vhost32_to_cpu(vq,
indirect-len),
Can't you use len here?
Not if I want error message to be readable.
Huh? Both have the same value.
Ah, good point.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 42/46] virtio_scsi: v1.0 support
On Sun, 30 Nov 2014 17:12:47 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
include/linux/virtio_scsi.h | 32 +++-
drivers/scsi/virtio_scsi.c | 51
-
2 files changed, 49 insertions(+), 34 deletions(-)
@@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi
*vscsi, void *buf)
break;
}
- WARN_ON(resp-sense_len VIRTIO_SCSI_SENSE_SIZE);
+ WARN_ON(virtio32_to_cpu(vscsi-vdev, resp-sense_len)
+ VIRTIO_SCSI_SENSE_SIZE);
Introduce a local variable for this? Might make this statement and the
min_t statement below easier to read.
if (sc-sense_buffer) {
memcpy(sc-sense_buffer, resp-sense,
-min_t(u32, resp-sense_len, VIRTIO_SCSI_SENSE_SIZE));
+min_t(u32,
+ virtio32_to_cpu(vscsi-vdev, resp-sense_len),
+ VIRTIO_SCSI_SENSE_SIZE));
if (resp-sense_len)
set_driver_byte(sc, DRIVER_SENSE);
}
Otherwise looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 01:40:36PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:34:55 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:02:58PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) Also it's ready to the level where benchmarking is possible, right? I don't think you should wait until we finish polishing up commit messages. My point is that I haven't even found time yet to test this thouroughly :( If my experience shows anything, it's unlikely we'll get appropriate testing without code being upstream first. That's why I pushed on with sparse tagging btw. This way we can be reasonably sure we didn't miss some path. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 42/46] virtio_scsi: v1.0 support
On Mon, Dec 01, 2014 at 02:53:05PM +0200, Michael S. Tsirkin wrote:
On Mon, Dec 01, 2014 at 01:50:01PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:12:47 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
include/linux/virtio_scsi.h | 32 +++-
drivers/scsi/virtio_scsi.c | 51
-
2 files changed, 49 insertions(+), 34 deletions(-)
@@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct
virtio_scsi *vscsi, void *buf)
break;
}
- WARN_ON(resp-sense_len VIRTIO_SCSI_SENSE_SIZE);
+ WARN_ON(virtio32_to_cpu(vscsi-vdev, resp-sense_len)
+ VIRTIO_SCSI_SENSE_SIZE);
Introduce a local variable for this? Might make this statement and the
min_t statement below easier to read.
I prefer 1:1 code conversions. We can do refactorings on top.
Since you mention this line as hard to read, I'll just make it 80
chars for now, and trivial line splits can come later.
OK?
Or maybe I'll keep it as is, since Paolo who wrote this code
is happy with it ..
if (sc-sense_buffer) {
memcpy(sc-sense_buffer, resp-sense,
-min_t(u32, resp-sense_len, VIRTIO_SCSI_SENSE_SIZE));
+min_t(u32,
+ virtio32_to_cpu(vscsi-vdev, resp-sense_len),
+ VIRTIO_SCSI_SENSE_SIZE));
if (resp-sense_len)
set_driver_byte(sc, DRIVER_SENSE);
}
Otherwise looks good to me.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 42/46] virtio_scsi: v1.0 support
On Mon, 1 Dec 2014 14:53:05 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:50:01PM +0100, Cornelia Huck wrote: On Sun, 30 Nov 2014 17:12:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: Note: for consistency, and to avoid sparse errors, convert all fields, even those no longer in use for virtio v1.0. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Paolo Bonzini pbonz...@redhat.com --- include/linux/virtio_scsi.h | 32 +++- drivers/scsi/virtio_scsi.c | 51 - 2 files changed, 49 insertions(+), 34 deletions(-) @@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi *vscsi, void *buf) break; } - WARN_ON(resp-sense_len VIRTIO_SCSI_SENSE_SIZE); + WARN_ON(virtio32_to_cpu(vscsi-vdev, resp-sense_len) + VIRTIO_SCSI_SENSE_SIZE); Introduce a local variable for this? Might make this statement and the min_t statement below easier to read. I prefer 1:1 code conversions. We can do refactorings on top. Since you mention this line as hard to read, I'll just make it 80 chars for now, and trivial line splits can come later. OK? I guess I don't care strongly enough about this, so go ahead. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, 1 Dec 2014 14:51:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:40:36PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:34:55 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:02:58PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) Also it's ready to the level where benchmarking is possible, right? I don't think you should wait until we finish polishing up commit messages. My point is that I haven't even found time yet to test this thouroughly :( If my experience shows anything, it's unlikely we'll get appropriate testing without code being upstream first. That's why I pushed on with sparse tagging btw. This way we can be reasonably sure we didn't miss some path. I know that I'm likely the only one to test ccw (unless I manage to get some other also-busy people to try this out). What's the status of virtio-pci, btw? Can people actually test this sanely? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, Dec 01, 2014 at 02:00:04PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:51:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:40:36PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:34:55 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:02:58PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) Also it's ready to the level where benchmarking is possible, right? I don't think you should wait until we finish polishing up commit messages. My point is that I haven't even found time yet to test this thouroughly :( If my experience shows anything, it's unlikely we'll get appropriate testing without code being upstream first. That's why I pushed on with sparse tagging btw. This way we can be reasonably sure we didn't miss some path. I know that I'm likely the only one to test ccw (unless I manage to get some other also-busy people to try this out). What's the status of virtio-pci, btw? Can people actually test this sanely? Sure, I'm testing that it's not broken by these patches. Others can do so, too. Once ccw is done on host and guest (will be complete after I send v8), it will be easier to add virtio 1.0 for more transports. OTOH if we require that everything is ready and perfect before merging anything we'll never get anywhere. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
On Mon, Dec 01, 2014 at 01:37:40PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:12:13 +0200
Michael S. Tsirkin m...@redhat.com wrote:
len is always initialized since function is called with size 0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 984242e..54ffbb0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
- u32 len;
+ u32 uninitialized_var(len);
while (datalen 0 headcount quota) {
if (unlikely(seg = UIO_MAXIOV)) {
Want to merge this with the patch introducing the variable and add a
comment there?
Not really. Warnings in bisect are fine I think.
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 16/46] virtio_blk: v1.0 support
On Mon, 1 Dec 2014 15:47:19 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 02:00:04PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:51:26 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:40:36PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 14:34:55 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 01:02:58PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 13:46:45 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 12:33:15PM +0100, Cornelia Huck wrote: On Mon, 1 Dec 2014 11:26:58 +0200 Michael S. Tsirkin m...@redhat.com wrote: For some places on data path, it might be worth it to cache the correct value e.g. as part of device structure. This replaces a branch with a memory load, so the gain would have to be measured, best done separately? I think we'll want to do some measuring once the basic structure is in place anyway. What's meant by in place here? That this patchset is ready :) Also it's ready to the level where benchmarking is possible, right? I don't think you should wait until we finish polishing up commit messages. My point is that I haven't even found time yet to test this thouroughly :( If my experience shows anything, it's unlikely we'll get appropriate testing without code being upstream first. That's why I pushed on with sparse tagging btw. This way we can be reasonably sure we didn't miss some path. I know that I'm likely the only one to test ccw (unless I manage to get some other also-busy people to try this out). What's the status of virtio-pci, btw? Can people actually test this sanely? Sure, I'm testing that it's not broken by these patches. Others can do so, too. So basically just regression testing, right? Once ccw is done on host and guest (will be complete after I send v8), it will be easier to add virtio 1.0 for more transports. OTOH if we require that everything is ready and perfect before merging anything we'll never get anywhere. I'm not looking for perfect, I'm just trying to juggle testing this + doing qemu changes + various other stuff that is eating my time :) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
On Mon, 1 Dec 2014 15:48:39 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 01:37:40PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:12:13 +0200
Michael S. Tsirkin m...@redhat.com wrote:
len is always initialized since function is called with size 0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 984242e..54ffbb0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
- u32 len;
+ u32 uninitialized_var(len);
while (datalen 0 headcount quota) {
if (unlikely(seg = UIO_MAXIOV)) {
Want to merge this with the patch introducing the variable and add a
comment there?
Not really. Warnings in bisect are fine I think.
I'm not sure what a separate patch buys us, though, as it should be
trivial to merge.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
On Mon, Dec 01, 2014 at 03:21:03PM +0100, Cornelia Huck wrote:
On Mon, 1 Dec 2014 15:48:39 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 01:37:40PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:12:13 +0200
Michael S. Tsirkin m...@redhat.com wrote:
len is always initialized since function is called with size 0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 984242e..54ffbb0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
- u32 len;
+ u32 uninitialized_var(len);
while (datalen 0 headcount quota) {
if (unlikely(seg = UIO_MAXIOV)) {
Want to merge this with the patch introducing the variable and add a
comment there?
Not really. Warnings in bisect are fine I think.
I'm not sure what a separate patch buys us, though, as it should be
trivial to merge.
Easier to document the reason if it's split out.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 36/46] vhost/net: suppress compiler warning
On Mon, 1 Dec 2014 17:12:08 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 03:21:03PM +0100, Cornelia Huck wrote:
On Mon, 1 Dec 2014 15:48:39 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Dec 01, 2014 at 01:37:40PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:12:13 +0200
Michael S. Tsirkin m...@redhat.com wrote:
len is always initialized since function is called with size 0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 984242e..54ffbb0 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -501,7 +501,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
- u32 len;
+ u32 uninitialized_var(len);
while (datalen 0 headcount quota) {
if (unlikely(seg = UIO_MAXIOV)) {
Want to merge this with the patch introducing the variable and add a
comment there?
Not really. Warnings in bisect are fine I think.
I'm not sure what a separate patch buys us, though, as it should be
trivial to merge.
Easier to document the reason if it's split out.
That's why I suggested a comment ;)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 31/46] vhost: virtio 1.0 endian-ness support
On Mon, Dec 01, 2014 at 01:33:53PM +0100, Cornelia Huck wrote:
On Sun, 30 Nov 2014 17:11:49 +0200
Michael S. Tsirkin m...@redhat.com wrote:
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 93
+++
1 file changed, 56 insertions(+), 37 deletions(-)
@@ -1113,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect-len);
int ret;
/* Sanity check */
- if (unlikely(indirect-len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, Invalid length in indirect descriptor:
len 0x%llx not multiple of 0x%zx\n,
- (unsigned long long)indirect-len,
+ (unsigned long long)vhost32_to_cpu(vq, indirect-len),
Can't you use len here?
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect-addr, indirect-len, vq-indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect-addr), len,
vq-indirect,
UIO_MAXIOV);
if (unlikely(ret 0)) {
vq_err(vq, Translation failure %d in indirect.\n, ret);
@@ -1404,7 +1422,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq,
struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq-last_used_idx, vq-used-idx)) {
+ if (__put_user(cpu_to_vhost16(vq, vq-last_used_idx), vq-used-idx)) {
Why s/put_user/__put_user/ - I don't see how endianness conversions
should have an influence there?
vq_err(vq, Failed to increment used idx);
return -EFAULT;
}
@@ -1449,11 +1468,11 @@ static bool vhost_notify(struct vhost_dev *dev,
struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
Dito: why the change?
I remember now.
put_user/get_user macros don't work well
with __virtio tags.
As __ are a good idea anyway, I just switched to that
everywhere.
vq_err(vq, Failed to get used event idx);
return true;
}
- return vring_need_event(event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 04/50] virtio: add support for 64 bit features.
Change u32 to u64, and use BIT_ULL and 1ULL everywhere.
Note: transports are unchanged, and only set low 32 bit.
This guarantees that no transport sets e.g. VERSION_1
by mistake without proper support.
Based on patch by Rusty.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/linux/virtio.h | 2 +-
include/linux/virtio_config.h | 20 ++--
tools/virtio/linux/virtio.h| 2 +-
drivers/lguest/lguest_device.c | 2 +-
drivers/misc/mic/card/mic_virtio.c | 2 +-
drivers/remoteproc/remoteproc_virtio.c | 2 +-
drivers/s390/kvm/kvm_virtio.c | 2 +-
drivers/s390/kvm/virtio_ccw.c | 2 +-
drivers/virtio/virtio.c| 8
drivers/virtio/virtio_mmio.c | 2 +-
drivers/virtio/virtio_pci.c| 2 +-
11 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7828a7f..149284e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,7 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
- u32 features;
+ u64 features;
void *priv;
};
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ffc2ae0..f517884 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -66,7 +66,7 @@ struct virtio_config_ops {
vq_callback_t *callbacks[],
const char *names[]);
void (*del_vqs)(struct virtio_device *);
- u32 (*get_features)(struct virtio_device *vdev);
+ u64 (*get_features)(struct virtio_device *vdev);
void (*finalize_features)(struct virtio_device *vdev);
const char *(*bus_name)(struct virtio_device *vdev);
int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
@@ -88,11 +88,11 @@ static inline bool __virtio_test_bit(const struct
virtio_device *vdev,
{
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
- BUILD_BUG_ON(fbit = 32);
+ BUILD_BUG_ON(fbit = 64);
else
- BUG_ON(fbit = 32);
+ BUG_ON(fbit = 64);
- return vdev-features BIT(fbit);
+ return vdev-features BIT_ULL(fbit);
}
/**
@@ -105,11 +105,11 @@ static inline void __virtio_set_bit(struct virtio_device
*vdev,
{
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
- BUILD_BUG_ON(fbit = 32);
+ BUILD_BUG_ON(fbit = 64);
else
- BUG_ON(fbit = 32);
+ BUG_ON(fbit = 64);
- vdev-features |= BIT(fbit);
+ vdev-features |= BIT_ULL(fbit);
}
/**
@@ -122,11 +122,11 @@ static inline void __virtio_clear_bit(struct
virtio_device *vdev,
{
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
- BUILD_BUG_ON(fbit = 32);
+ BUILD_BUG_ON(fbit = 64);
else
- BUG_ON(fbit = 32);
+ BUG_ON(fbit = 64);
- vdev-features = ~BIT(fbit);
+ vdev-features = ~BIT_ULL(fbit);
}
/**
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 72bff70..8eb6421 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -10,7 +10,7 @@
struct virtio_device {
void *dev;
- u32 features;
+ u64 features;
};
struct virtqueue {
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 97aeb7d..d81170a 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -94,7 +94,7 @@ static unsigned desc_size(const struct lguest_device_desc
*desc)
}
/* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
{
unsigned int i;
u32 features = 0;
diff --git a/drivers/misc/mic/card/mic_virtio.c
b/drivers/misc/mic/card/mic_virtio.c
index d5da9ff..f5e7561 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -68,7 +68,7 @@ static inline struct device *mic_dev(struct mic_vdev *mvdev)
}
/* This gets the device's feature bits. */
-static u32 mic_get_features(struct virtio_device *vdev)
+static u64 mic_get_features(struct virtio_device *vdev)
{
unsigned int i, bits;
u32 features = 0;
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index dafaf38..62897db 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++
[PATCH v8 05/50] virtio: assert 32 bit features in transports
At this point, no transports set any of the high 32 feature bits.
Since transports generally can't (yet) cope with such bits, add BUG_ON
checks to make sure they are not set by mistake.
Based on rproc patch by Rusty.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/lguest/lguest_device.c | 3 +++
drivers/misc/mic/card/mic_virtio.c | 3 +++
drivers/remoteproc/remoteproc_virtio.c | 3 +++
drivers/s390/kvm/kvm_virtio.c | 3 +++
drivers/s390/kvm/virtio_ccw.c | 3 +++
drivers/virtio/virtio_mmio.c | 3 +++
drivers/virtio/virtio_pci.c| 3 +++
7 files changed, 21 insertions(+)
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index d81170a..9b77b66 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -136,6 +136,9 @@ static void lg_finalize_features(struct virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
/*
* Since lguest is currently x86-only, we're little-endian. That
* means we could just memcpy. But it's not time critical, and in
diff --git a/drivers/misc/mic/card/mic_virtio.c
b/drivers/misc/mic/card/mic_virtio.c
index f5e7561..d027d29 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -96,6 +96,9 @@ static void mic_finalize_features(struct virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
memset_io(out_features, 0, feature_len);
bits = min_t(unsigned, feature_len,
sizeof(vdev-features)) * 8;
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 62897db..627737e 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -227,6 +227,9 @@ static void rproc_virtio_finalize_features(struct
virtio_device *vdev)
/* Give virtio_ring a chance to accept features */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
/*
* Remember the finalized features of our vdev, and provide it
* to the remote processor once it is powered on.
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 2336c7e..f5575cc 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -103,6 +103,9 @@ static void kvm_finalize_features(struct virtio_device
*vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
memset(out_features, 0, desc-feature_len);
bits = min_t(unsigned, desc-feature_len, sizeof(vdev-features)) * 8;
for (i = 0; i bits; i++) {
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 56d7895..244d611 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -714,6 +714,9 @@ static void virtio_ccw_finalize_features(struct
virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
features-index = 0;
features-features = cpu_to_le32(vdev-features);
/* Write the feature bits to the host. */
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index c63d0ef..aec1dae 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -159,6 +159,9 @@ static void vm_finalize_features(struct virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't have any features 32 bits! */
+ BUG_ON((u32)vdev-features != vdev-features);
+
writel(0, vm_dev-base + VIRTIO_MMIO_GUEST_FEATURES_SEL);
writel(vdev-features, vm_dev-base + VIRTIO_MMIO_GUEST_FEATURES);
}
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 7e0efa7..dd6df97 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -119,6 +119,9 @@ static void vp_finalize_features(struct virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Make sure we don't
[PATCH v8 07/50] virtio: add virtio 1.0 feature bit
Based on original patches by Rusty Russell, Thomas Huth and Cornelia Huck. Note: at this time, we do not negotiate this feature bit in core, drivers have to declare VERSION_1 support explicitly. For this reason we treat this bit as a device bit and not as a transport bit for now. After all drivers are converted, we will be able to move VERSION_1 to core and drop it from all drivers. Signed-off-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com --- include/uapi/linux/virtio_config.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h index 3ce768c..80e7381 100644 --- a/include/uapi/linux/virtio_config.h +++ b/include/uapi/linux/virtio_config.h @@ -54,4 +54,7 @@ /* Can the device handle any descriptor layout? */ #define VIRTIO_F_ANY_LAYOUT27 +/* v1.0 compliant. */ +#define VIRTIO_F_VERSION_1 32 + #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */ -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 01/50] virtio: add low-level APIs for feature bits
Add low level APIs to test/set/clear feature bits.
For use by transports, to make it easier to
write code independent of feature bit array format.
Note: APIs is prefixed with __ and has _bit suffix
to stress its low level nature. It's for use by transports only:
drivers should use virtio_has_feature and never need to set/clear
features.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/linux/virtio_config.h | 53 ---
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7f4ef66..d8e28a2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -77,11 +77,47 @@ void virtio_check_driver_offered_feature(const struct
virtio_device *vdev,
unsigned int fbit);
/**
- * virtio_has_feature - helper to determine if this device has this feature.
+ * __virtio_test_bit - helper to test feature bits. For use by transports.
+ * Devices should normally use virtio_has_feature,
+ * which includes more checks.
* @vdev: the device
* @fbit: the feature bit
*/
-static inline bool virtio_has_feature(const struct virtio_device *vdev,
+static inline bool __virtio_test_bit(const struct virtio_device *vdev,
+unsigned int fbit)
+{
+ /* Did you forget to fix assumptions on max features? */
+ if (__builtin_constant_p(fbit))
+ BUILD_BUG_ON(fbit = 32);
+ else
+ BUG_ON(fbit = 32);
+
+ return test_bit(fbit, vdev-features);
+}
+
+/**
+ * __virtio_set_bit - helper to set feature bits. For use by transports.
+ * @vdev: the device
+ * @fbit: the feature bit
+ */
+static inline void __virtio_set_bit(struct virtio_device *vdev,
+ unsigned int fbit)
+{
+ /* Did you forget to fix assumptions on max features? */
+ if (__builtin_constant_p(fbit))
+ BUILD_BUG_ON(fbit = 32);
+ else
+ BUG_ON(fbit = 32);
+
+ set_bit(fbit, vdev-features);
+}
+
+/**
+ * __virtio_clear_bit - helper to clear feature bits. For use by transports.
+ * @vdev: the device
+ * @fbit: the feature bit
+ */
+static inline void __virtio_clear_bit(struct virtio_device *vdev,
unsigned int fbit)
{
/* Did you forget to fix assumptions on max features? */
@@ -90,10 +126,21 @@ static inline bool virtio_has_feature(const struct
virtio_device *vdev,
else
BUG_ON(fbit = 32);
+ clear_bit(fbit, vdev-features);
+}
+
+/**
+ * virtio_has_feature - helper to determine if this device has this feature.
+ * @vdev: the device
+ * @fbit: the feature bit
+ */
+static inline bool virtio_has_feature(const struct virtio_device *vdev,
+ unsigned int fbit)
+{
if (fbit VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
- return test_bit(fbit, vdev-features);
+ return __virtio_test_bit(vdev, fbit);
}
static inline
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 10/50] virtio_config: endian conversion for v1.0
We (ab)use virtio conversion functions for device-specific
config space accesses.
Based on original patches by Cornelia and Rusty.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: David Hildenbrand d...@linux.vnet.com
---
include/linux/virtio_config.h | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 02f0acb..1fa5faa 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -318,12 +318,13 @@ static inline u16 virtio_cread16(struct virtio_device
*vdev,
{
u16 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
- return ret;
+ return virtio16_to_cpu(vdev, (__force __virtio16)ret);
}
static inline void virtio_cwrite16(struct virtio_device *vdev,
unsigned int offset, u16 val)
{
+ val = (__force u16)cpu_to_virtio16(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
}
@@ -332,12 +333,13 @@ static inline u32 virtio_cread32(struct virtio_device
*vdev,
{
u32 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
- return ret;
+ return virtio32_to_cpu(vdev, (__force __virtio32)ret);
}
static inline void virtio_cwrite32(struct virtio_device *vdev,
unsigned int offset, u32 val)
{
+ val = (__force u32)cpu_to_virtio32(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
}
@@ -346,12 +348,13 @@ static inline u64 virtio_cread64(struct virtio_device
*vdev,
{
u64 ret;
vdev-config-get(vdev, offset, ret, sizeof(ret));
- return ret;
+ return virtio64_to_cpu(vdev, (__force __virtio64)ret);
}
static inline void virtio_cwrite64(struct virtio_device *vdev,
unsigned int offset, u64 val)
{
+ val = (__force u64)cpu_to_virtio64(vdev, val);
vdev-config-set(vdev, offset, val, sizeof(val));
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 11/50] virtio: allow transports to get avail/used addresses
From: Cornelia Huck cornelia.h...@de.ibm.com
For virtio-1, we can theoretically have a more complex virtqueue
layout with avail and used buffers not on a contiguous memory area
with the descriptor table. For now, it's fine for a transport driver
to stay with the old layout: It needs, however, a way to access
the locations of the avail/used rings so it can register them with
the host.
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/linux/virtio.h | 3 +++
drivers/virtio/virtio_ring.c | 16
2 files changed, 19 insertions(+)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 149284e..d6359a5 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -75,6 +75,9 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *vq);
bool virtqueue_is_broken(struct virtqueue *vq);
+void *virtqueue_get_avail(struct virtqueue *vq);
+void *virtqueue_get_used(struct virtqueue *vq);
+
/**
* virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 0d3c737..55532a4 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -827,4 +827,20 @@ void virtio_break_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(virtio_break_device);
+void *virtqueue_get_avail(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq-vring.avail;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_avail);
+
+void *virtqueue_get_used(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq-vring.used;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_used);
+
MODULE_LICENSE(GPL);
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 25/50] virtio_net: stricter short buffer length checks
Our buffer length check is not strict enough for mergeable
buffers: buffer can still be shorter that header + address
by 2 bytes.
Fix that up.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 516f2cb..098f443 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -437,7 +437,7 @@ static void receive_buf(struct virtnet_info *vi, struct
receive_queue *rq,
struct sk_buff *skb;
struct virtio_net_hdr_mrg_rxbuf *hdr;
- if (unlikely(len sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
+ if (unlikely(len vi-hdr_len + ETH_HLEN)) {
pr_debug(%s: short packet %i\n, dev-name, len);
dev-stats.rx_length_errors++;
if (vi-mergeable_rx_bufs) {
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 13/50] virtio: simplify feature bit handling
Now that we use u64 for bits, we can simply them together.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/virtio/virtio.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 9248125..3e78f4b 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -160,6 +160,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
u64 device_features;
+ u64 driver_features;
unsigned status;
/* We have a driver! */
@@ -168,15 +169,16 @@ static int virtio_dev_probe(struct device *_d)
/* Figure out what features the device supports. */
device_features = dev-config-get_features(dev);
- /* Features supported by both device and driver into dev-features. */
- dev-features = 0;
+ /* Figure out what features the driver supports. */
+ driver_features = 0;
for (i = 0; i drv-feature_table_size; i++) {
unsigned int f = drv-feature_table[i];
BUG_ON(f = 64);
- if (device_features (1ULL f))
- __virtio_set_bit(dev, f);
+ driver_features |= (1ULL f);
}
+ dev-features = driver_features device_features;
+
/* Transport features always preserved to pass to finalize_features. */
for (i = VIRTIO_TRANSPORT_F_START; i VIRTIO_TRANSPORT_F_END; i++)
if (device_features (1ULL i))
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 26/50] virtio_net: bigger header when VERSION_1 is set
With VERSION_1 virtio_net uses same header size whether mergeable buffers are enabled or not. Signed-off-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Reviewed-by: Jason Wang jasow...@redhat.com --- drivers/net/virtio_net.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 098f443..a0e64cf 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1805,7 +1805,8 @@ static int virtnet_probe(struct virtio_device *vdev) if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) vi-mergeable_rx_bufs = true; - if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) + if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) || + virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) vi-hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf); else vi-hdr_len = sizeof(struct virtio_net_hdr); -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 16/50] virtio_blk: v1.0 support
Based on patch by Cornelia Huck.
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/uapi/linux/virtio_blk.h | 15 -
drivers/block/virtio_blk.c | 70 -
2 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 9ad67b2..247c8ba 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -28,6 +28,7 @@
#include linux/types.h
#include linux/virtio_ids.h
#include linux/virtio_config.h
+#include linux/virtio_types.h
/* Feature bits */
#define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? */
@@ -114,18 +115,18 @@ struct virtio_blk_config {
/* This is the first element of the read scatter-gather list. */
struct virtio_blk_outhdr {
/* VIRTIO_BLK_T* */
- __u32 type;
+ __virtio32 type;
/* io priority. */
- __u32 ioprio;
+ __virtio32 ioprio;
/* Sector (ie. 512 byte offset) */
- __u64 sector;
+ __virtio64 sector;
};
struct virtio_scsi_inhdr {
- __u32 errors;
- __u32 data_len;
- __u32 sense_len;
- __u32 residual;
+ __virtio32 errors;
+ __virtio32 data_len;
+ __virtio32 sense_len;
+ __virtio32 residual;
};
/* And this is the final byte of the write scatter-gather list. */
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c6a27d5..f601f16 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -80,7 +80,7 @@ static int __virtblk_add_req(struct virtqueue *vq,
{
struct scatterlist hdr, status, cmd, sense, inhdr, *sgs[6];
unsigned int num_out = 0, num_in = 0;
- int type = vbr-out_hdr.type ~VIRTIO_BLK_T_OUT;
+ __virtio32 type = vbr-out_hdr.type ~cpu_to_virtio32(vq-vdev,
VIRTIO_BLK_T_OUT);
sg_init_one(hdr, vbr-out_hdr, sizeof(vbr-out_hdr));
sgs[num_out++] = hdr;
@@ -91,19 +91,19 @@ static int __virtblk_add_req(struct virtqueue *vq,
* block, and before the normal inhdr we put the sense data and the
* inhdr with additional status information.
*/
- if (type == VIRTIO_BLK_T_SCSI_CMD) {
+ if (type == cpu_to_virtio32(vq-vdev, VIRTIO_BLK_T_SCSI_CMD)) {
sg_init_one(cmd, vbr-req-cmd, vbr-req-cmd_len);
sgs[num_out++] = cmd;
}
if (have_data) {
- if (vbr-out_hdr.type VIRTIO_BLK_T_OUT)
+ if (vbr-out_hdr.type cpu_to_virtio32(vq-vdev,
VIRTIO_BLK_T_OUT))
sgs[num_out++] = data_sg;
else
sgs[num_out + num_in++] = data_sg;
}
- if (type == VIRTIO_BLK_T_SCSI_CMD) {
+ if (type == cpu_to_virtio32(vq-vdev, VIRTIO_BLK_T_SCSI_CMD)) {
sg_init_one(sense, vbr-req-sense, SCSI_SENSE_BUFFERSIZE);
sgs[num_out + num_in++] = sense;
sg_init_one(inhdr, vbr-in_hdr, sizeof(vbr-in_hdr));
@@ -119,12 +119,13 @@ static int __virtblk_add_req(struct virtqueue *vq,
static inline void virtblk_request_done(struct request *req)
{
struct virtblk_req *vbr = blk_mq_rq_to_pdu(req);
+ struct virtio_blk *vblk = req-q-queuedata;
int error = virtblk_result(vbr);
if (req-cmd_type == REQ_TYPE_BLOCK_PC) {
- req-resid_len = vbr-in_hdr.residual;
- req-sense_len = vbr-in_hdr.sense_len;
- req-errors = vbr-in_hdr.errors;
+ req-resid_len = virtio32_to_cpu(vblk-vdev,
vbr-in_hdr.residual);
+ req-sense_len = virtio32_to_cpu(vblk-vdev,
vbr-in_hdr.sense_len);
+ req-errors = virtio32_to_cpu(vblk-vdev, vbr-in_hdr.errors);
} else if (req-cmd_type == REQ_TYPE_SPECIAL) {
req-errors = (error != 0);
}
@@ -173,25 +174,25 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
struct request *req,
vbr-req = req;
if (req-cmd_flags REQ_FLUSH) {
- vbr-out_hdr.type = VIRTIO_BLK_T_FLUSH;
+ vbr-out_hdr.type = cpu_to_virtio32(vblk-vdev,
VIRTIO_BLK_T_FLUSH);
vbr-out_hdr.sector = 0;
- vbr-out_hdr.ioprio = req_get_ioprio(vbr-req);
+ vbr-out_hdr.ioprio = cpu_to_virtio32(vblk-vdev,
req_get_ioprio(vbr-req));
} else {
switch (req-cmd_type) {
case REQ_TYPE_FS:
vbr-out_hdr.type = 0;
- vbr-out_hdr.sector = blk_rq_pos(vbr-req);
- vbr-out_hdr.ioprio = req_get_ioprio(vbr-req);
+ vbr-out_hdr.sector = cpu_to_virtio64(vblk-vdev,
blk_rq_pos(vbr-req));
+ vbr-out_hdr.ioprio = cpu_to_virtio32(vblk-vdev,
[PATCH v8 14/50] virtio: add legacy feature table support
virtio-blk has some legacy feature bits that modern drivers
must not negotiate, but are needed for old legacy hosts
(that e.g. don't support virtio-scsi).
Allow a separate legacy feature table for such cases.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/linux/virtio.h | 4
drivers/virtio/virtio.c | 25 -
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d6359a5..f70411e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -130,6 +130,8 @@ int virtio_device_restore(struct virtio_device *dev);
* @id_table: the ids serviced by this driver.
* @feature_table: an array of feature numbers supported by this driver.
* @feature_table_size: number of entries in the feature table array.
+ * @feature_table_legacy: same as feature_table but when working in legacy
mode.
+ * @feature_table_size_legacy: number of entries in feature table legacy array.
* @probe: the function to call when a device is found. Returns 0 or -errno.
* @remove: the function to call when a device is removed.
* @config_changed: optional function to call when the device configuration
@@ -140,6 +142,8 @@ struct virtio_driver {
const struct virtio_device_id *id_table;
const unsigned int *feature_table;
unsigned int feature_table_size;
+ const unsigned int *feature_table_legacy;
+ unsigned int feature_table_size_legacy;
int (*probe)(struct virtio_device *dev);
void (*scan)(struct virtio_device *dev);
void (*remove)(struct virtio_device *dev);
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 3e78f4b..f9ad99c 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -113,6 +113,13 @@ void virtio_check_driver_offered_feature(const struct
virtio_device *vdev,
for (i = 0; i drv-feature_table_size; i++)
if (drv-feature_table[i] == fbit)
return;
+
+ if (drv-feature_table_legacy) {
+ for (i = 0; i drv-feature_table_size_legacy; i++)
+ if (drv-feature_table_legacy[i] == fbit)
+ return;
+ }
+
BUG();
}
EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
@@ -161,6 +168,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
u64 device_features;
u64 driver_features;
+ u64 driver_features_legacy;
unsigned status;
/* We have a driver! */
@@ -177,7 +185,22 @@ static int virtio_dev_probe(struct device *_d)
driver_features |= (1ULL f);
}
- dev-features = driver_features device_features;
+ /* Some drivers have a separate feature table for virtio v1.0 */
+ if (drv-feature_table_legacy) {
+ driver_features_legacy = 0;
+ for (i = 0; i drv-feature_table_size_legacy; i++) {
+ unsigned int f = drv-feature_table_legacy[i];
+ BUG_ON(f = 64);
+ driver_features_legacy |= (1ULL f);
+ }
+ } else {
+ driver_features_legacy = driver_features;
+ }
+
+ if (driver_features device_features (1ULL VIRTIO_F_VERSION_1))
+ dev-features = driver_features device_features;
+ else
+ dev-features = driver_features_legacy device_features;
/* Transport features always preserved to pass to finalize_features. */
for (i = VIRTIO_TRANSPORT_F_START; i VIRTIO_TRANSPORT_F_END; i++)
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 21/50] virtio_blk: make serial attribute static
It's never declared so no need to make it extern.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/block/virtio_blk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index f601f16..055f3df 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -332,7 +332,8 @@ static ssize_t virtblk_serial_show(struct device *dev,
return err;
}
-DEVICE_ATTR(serial, S_IRUGO, virtblk_serial_show, NULL);
+
+static DEVICE_ATTR(serial, S_IRUGO, virtblk_serial_show, NULL);
static void virtblk_config_changed_work(struct work_struct *work)
{
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 27/50] virtio_net: disable mac write for virtio 1.0
The spec states that mac in config space is only driver-writable in the
legacy case. Fence writing it in virtnet_set_mac_address() in the
virtio 1.0 case.
Suggested-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/net/virtio_net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a0e64cf..b8bd719 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1030,7 +1030,8 @@ static int virtnet_set_mac_address(struct net_device
*dev, void *p)
Failed to set mac address by vq command.\n);
return -EINVAL;
}
- } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
+ } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)
+ !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
unsigned int i;
/* Naturally, this has an atomicity problem. */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 31/50] vhost/net: force len for TX to host endian
vhost/net keeps a copy of the used ring in host memory but (ab)uses
the length field for internal house-keeping. This works because the
length in the used ring for tx is always 0. In order to suppress sparse
warnings, we force native endianness here.
Note that these values are never exposed to guests.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/net.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8dae2f7..dce5c58 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -48,15 +48,15 @@ MODULE_PARM_DESC(experimental_zcopytx, Enable Zero Copy
TX;
* status internally; used for zerocopy tx only.
*/
/* Lower device DMA failed */
-#define VHOST_DMA_FAILED_LEN 3
+#define VHOST_DMA_FAILED_LEN ((__force __virtio32)3)
/* Lower device DMA done */
-#define VHOST_DMA_DONE_LEN 2
+#define VHOST_DMA_DONE_LEN ((__force __virtio32)2)
/* Lower device DMA in progress */
-#define VHOST_DMA_IN_PROGRESS 1
+#define VHOST_DMA_IN_PROGRESS ((__force __virtio32)1)
/* Buffer unused */
-#define VHOST_DMA_CLEAR_LEN0
+#define VHOST_DMA_CLEAR_LEN((__force __virtio32)0)
-#define VHOST_DMA_IS_DONE(len) ((len) = VHOST_DMA_DONE_LEN)
+#define VHOST_DMA_IS_DONE(len) ((__force u32)(len) = (__force
u32)VHOST_DMA_DONE_LEN)
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 30/50] vhost: add memory access wrappers
Add guest memory access wrappers to handle virtio endianness
conversions.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/vhost/vhost.h | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 55a95c9..a0a6c98 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -176,4 +176,35 @@ static inline bool vhost_has_feature(struct
vhost_virtqueue *vq, int bit)
{
return vq-acked_features (1ULL bit);
}
+
+/* Memory accessors */
+static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
+{
+ return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
+{
+ return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
+{
+ return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
+{
+ return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
+{
+ return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
+{
+ return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1),
val);
+}
#endif
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 42/50] virtio_scsi: v1.0 support
Note: for consistency, and to avoid sparse errors,
convert all fields, even those no longer in use
for virtio v1.0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
include/linux/virtio_scsi.h | 32 +++-
drivers/scsi/virtio_scsi.c | 51 -
2 files changed, 49 insertions(+), 34 deletions(-)
diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
index de429d1..af44864 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -27,13 +27,15 @@
#ifndef _LINUX_VIRTIO_SCSI_H
#define _LINUX_VIRTIO_SCSI_H
+#include linux/virtio_types.h
+
#define VIRTIO_SCSI_CDB_SIZE 32
#define VIRTIO_SCSI_SENSE_SIZE 96
/* SCSI command request, followed by data-out */
struct virtio_scsi_cmd_req {
u8 lun[8]; /* Logical Unit Number */
- u64 tag;/* Command identifier */
+ __virtio64 tag; /* Command identifier */
u8 task_attr; /* Task attribute */
u8 prio;/* SAM command priority field */
u8 crn;
@@ -43,20 +45,20 @@ struct virtio_scsi_cmd_req {
/* SCSI command request, followed by protection information */
struct virtio_scsi_cmd_req_pi {
u8 lun[8]; /* Logical Unit Number */
- u64 tag;/* Command identifier */
+ __virtio64 tag; /* Command identifier */
u8 task_attr; /* Task attribute */
u8 prio;/* SAM command priority field */
u8 crn;
- u32 pi_bytesout;/* DataOUT PI Number of bytes */
- u32 pi_bytesin; /* DataIN PI Number of bytes */
+ __virtio32 pi_bytesout; /* DataOUT PI Number of bytes */
+ __virtio32 pi_bytesin; /* DataIN PI Number of bytes */
u8 cdb[VIRTIO_SCSI_CDB_SIZE];
} __packed;
/* Response, followed by sense data and data-in */
struct virtio_scsi_cmd_resp {
- u32 sense_len; /* Sense data length */
- u32 resid; /* Residual bytes in data buffer */
- u16 status_qualifier; /* Status qualifier */
+ __virtio32 sense_len; /* Sense data length */
+ __virtio32 resid; /* Residual bytes in data buffer */
+ __virtio16 status_qualifier;/* Status qualifier */
u8 status; /* Command completion status */
u8 response;/* Response values */
u8 sense[VIRTIO_SCSI_SENSE_SIZE];
@@ -64,10 +66,10 @@ struct virtio_scsi_cmd_resp {
/* Task Management Request */
struct virtio_scsi_ctrl_tmf_req {
- u32 type;
- u32 subtype;
+ __virtio32 type;
+ __virtio32 subtype;
u8 lun[8];
- u64 tag;
+ __virtio64 tag;
} __packed;
struct virtio_scsi_ctrl_tmf_resp {
@@ -76,20 +78,20 @@ struct virtio_scsi_ctrl_tmf_resp {
/* Asynchronous notification query/subscription */
struct virtio_scsi_ctrl_an_req {
- u32 type;
+ __virtio32 type;
u8 lun[8];
- u32 event_requested;
+ __virtio32 event_requested;
} __packed;
struct virtio_scsi_ctrl_an_resp {
- u32 event_actual;
+ __virtio32 event_actual;
u8 response;
} __packed;
struct virtio_scsi_event {
- u32 event;
+ __virtio32 event;
u8 lun[8];
- u32 reason;
+ __virtio32 reason;
} __packed;
struct virtio_scsi_config {
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index b83846f..d9ec806 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -158,7 +158,7 @@ static void virtscsi_complete_cmd(struct virtio_scsi
*vscsi, void *buf)
sc, resp-response, resp-status, resp-sense_len);
sc-result = resp-status;
- virtscsi_compute_resid(sc, resp-resid);
+ virtscsi_compute_resid(sc, virtio32_to_cpu(vscsi-vdev, resp-resid));
switch (resp-response) {
case VIRTIO_SCSI_S_OK:
set_host_byte(sc, DID_OK);
@@ -196,10 +196,13 @@ static void virtscsi_complete_cmd(struct virtio_scsi
*vscsi, void *buf)
break;
}
- WARN_ON(resp-sense_len VIRTIO_SCSI_SENSE_SIZE);
+ WARN_ON(virtio32_to_cpu(vscsi-vdev, resp-sense_len)
+ VIRTIO_SCSI_SENSE_SIZE);
if (sc-sense_buffer) {
memcpy(sc-sense_buffer, resp-sense,
- min_t(u32, resp-sense_len, VIRTIO_SCSI_SENSE_SIZE));
+ min_t(u32,
+virtio32_to_cpu(vscsi-vdev, resp-sense_len),
+VIRTIO_SCSI_SENSE_SIZE));
if (resp-sense_len)
set_driver_byte(sc, DRIVER_SENSE);
}
@@ -323,7 +326,7 @@ static void virtscsi_handle_transport_reset(struct
virtio_scsi *vscsi,
unsigned int target = event-lun[1];
unsigned int lun = (event-lun[2] 8) | event-lun[3];
- switch
[PATCH v8 29/50] vhost: make features 64 bit
We need to use bit 32 for virtio 1.0.
Make vhost_has_feature bool to avoid discarding high bits.
Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
Cc: Ben Hutchings b...@decadent.org.uk
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
---
drivers/vhost/vhost.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 3eda654..55a95c9 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -106,7 +106,7 @@ struct vhost_virtqueue {
/* Protected by virtqueue mutex. */
struct vhost_memory *memory;
void *private_data;
- unsigned acked_features;
+ u64 acked_features;
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
@@ -172,8 +172,8 @@ enum {
(1ULL VHOST_F_LOG_ALL),
};
-static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
+static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
{
- return vq-acked_features (1 bit);
+ return vq-acked_features (1ULL bit);
}
#endif
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 32/50] vhost: switch to __get/__put_user exclusively
Most places in vhost can use __get/__put_user rather than
get/put_user since addresses are pre-validated.
This should be good for performance, but this also
will help make code sparse-clean: get/put_user macros
don't play well with __virtioXX bitwise tags.
Switch to get/put_user to __ variants everywhere in vhost.
There's one exception - for consistency switch that
as well, and add an explicit access_ok check.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c90f437..6a40837 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1038,6 +1038,7 @@ static int vhost_update_avail_event(struct
vhost_virtqueue *vq, u16 avail_event)
int vhost_init_used(struct vhost_virtqueue *vq)
{
+ u16 last_used_idx;
int r;
if (!vq-private_data)
return 0;
@@ -1046,7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq)
if (r)
return r;
vq-signalled_used_valid = false;
- return get_user(vq-last_used_idx, vq-used-idx);
+ if (!access_ok(VERIFY_READ, vq-used-idx, sizeof vq-used-idx))
+ return -EFAULT;
+ r = __get_user(last_used_idx, vq-used-idx);
+ if (r)
+ return r;
+ vq-last_used_idx = last_used_idx;
+ return 0;
}
EXPORT_SYMBOL_GPL(vhost_init_used);
@@ -1404,7 +1411,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct
vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (put_user(vq-last_used_idx, vq-used-idx)) {
+ if (__put_user(vq-last_used_idx, vq-used-idx)) {
vq_err(vq, Failed to increment used idx);
return -EFAULT;
}
@@ -1449,7 +1456,7 @@ static bool vhost_notify(struct vhost_dev *dev, struct
vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (get_user(event, vhost_used_event(vq))) {
+ if (__get_user(event, vhost_used_event(vq))) {
vq_err(vq, Failed to get used event idx);
return true;
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 36/50] vhost/net: enable virtio 1.0
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8ff4a6d..a935c25 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -61,7 +61,8 @@ MODULE_PARM_DESC(experimental_zcopytx, Enable Zero Copy TX;
enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
(1ULL VHOST_NET_F_VIRTIO_NET_HDR) |
-(1ULL VIRTIO_NET_F_MRG_RXBUF),
+(1ULL VIRTIO_NET_F_MRG_RXBUF) |
+(1ULL VIRTIO_F_VERSION_1),
};
enum {
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 48/50] virtio_balloon: add legacy_only flag
We have no plans to support virtio 1.0 in balloon driver. Add an
explicit flag to mark it legacy only.
This will be used by follow-up patches.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/linux/virtio.h | 2 ++
drivers/virtio/virtio_balloon.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index f70411e..2bbf626 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -132,6 +132,7 @@ int virtio_device_restore(struct virtio_device *dev);
* @feature_table_size: number of entries in the feature table array.
* @feature_table_legacy: same as feature_table but when working in legacy
mode.
* @feature_table_size_legacy: number of entries in feature table legacy array.
+ * @legacy_only: driver does not support virtio 1.0.
* @probe: the function to call when a device is found. Returns 0 or -errno.
* @remove: the function to call when a device is removed.
* @config_changed: optional function to call when the device configuration
@@ -144,6 +145,7 @@ struct virtio_driver {
unsigned int feature_table_size;
const unsigned int *feature_table_legacy;
unsigned int feature_table_size_legacy;
+ bool legacy_only;
int (*probe)(struct virtio_device *dev);
void (*scan)(struct virtio_device *dev);
void (*remove)(struct virtio_device *dev);
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index c9703d4..4497def 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -518,6 +518,7 @@ static unsigned int features[] = {
};
static struct virtio_driver virtio_balloon_driver = {
+ .legacy_only = true,
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
.driver.name = KBUILD_MODNAME,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 44/50] virtio_scsi: export to userspace
Replace uXX by __uXX and _packed by __attribute((packed))
as seems to be the norm for userspace headers.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
include/uapi/linux/virtio_scsi.h | 74
include/uapi/linux/Kbuild| 1 +
2 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/include/uapi/linux/virtio_scsi.h b/include/uapi/linux/virtio_scsi.h
index af44864..42b9370 100644
--- a/include/uapi/linux/virtio_scsi.h
+++ b/include/uapi/linux/virtio_scsi.h
@@ -34,78 +34,78 @@
/* SCSI command request, followed by data-out */
struct virtio_scsi_cmd_req {
- u8 lun[8]; /* Logical Unit Number */
+ __u8 lun[8];/* Logical Unit Number */
__virtio64 tag; /* Command identifier */
- u8 task_attr; /* Task attribute */
- u8 prio;/* SAM command priority field */
- u8 crn;
- u8 cdb[VIRTIO_SCSI_CDB_SIZE];
-} __packed;
+ __u8 task_attr; /* Task attribute */
+ __u8 prio; /* SAM command priority field */
+ __u8 crn;
+ __u8 cdb[VIRTIO_SCSI_CDB_SIZE];
+} __attribute__((packed));
/* SCSI command request, followed by protection information */
struct virtio_scsi_cmd_req_pi {
- u8 lun[8]; /* Logical Unit Number */
+ __u8 lun[8];/* Logical Unit Number */
__virtio64 tag; /* Command identifier */
- u8 task_attr; /* Task attribute */
- u8 prio;/* SAM command priority field */
- u8 crn;
+ __u8 task_attr; /* Task attribute */
+ __u8 prio; /* SAM command priority field */
+ __u8 crn;
__virtio32 pi_bytesout; /* DataOUT PI Number of bytes */
__virtio32 pi_bytesin; /* DataIN PI Number of bytes */
- u8 cdb[VIRTIO_SCSI_CDB_SIZE];
-} __packed;
+ __u8 cdb[VIRTIO_SCSI_CDB_SIZE];
+} __attribute__((packed));
/* Response, followed by sense data and data-in */
struct virtio_scsi_cmd_resp {
__virtio32 sense_len; /* Sense data length */
__virtio32 resid; /* Residual bytes in data buffer */
__virtio16 status_qualifier;/* Status qualifier */
- u8 status; /* Command completion status */
- u8 response;/* Response values */
- u8 sense[VIRTIO_SCSI_SENSE_SIZE];
-} __packed;
+ __u8 status;/* Command completion status */
+ __u8 response; /* Response values */
+ __u8 sense[VIRTIO_SCSI_SENSE_SIZE];
+} __attribute__((packed));
/* Task Management Request */
struct virtio_scsi_ctrl_tmf_req {
__virtio32 type;
__virtio32 subtype;
- u8 lun[8];
+ __u8 lun[8];
__virtio64 tag;
-} __packed;
+} __attribute__((packed));
struct virtio_scsi_ctrl_tmf_resp {
- u8 response;
-} __packed;
+ __u8 response;
+} __attribute__((packed));
/* Asynchronous notification query/subscription */
struct virtio_scsi_ctrl_an_req {
__virtio32 type;
- u8 lun[8];
+ __u8 lun[8];
__virtio32 event_requested;
-} __packed;
+} __attribute__((packed));
struct virtio_scsi_ctrl_an_resp {
__virtio32 event_actual;
- u8 response;
-} __packed;
+ __u8 response;
+} __attribute__((packed));
struct virtio_scsi_event {
__virtio32 event;
- u8 lun[8];
+ __u8 lun[8];
__virtio32 reason;
-} __packed;
+} __attribute__((packed));
struct virtio_scsi_config {
- u32 num_queues;
- u32 seg_max;
- u32 max_sectors;
- u32 cmd_per_lun;
- u32 event_info_size;
- u32 sense_size;
- u32 cdb_size;
- u16 max_channel;
- u16 max_target;
- u32 max_lun;
-} __packed;
+ __u32 num_queues;
+ __u32 seg_max;
+ __u32 max_sectors;
+ __u32 cmd_per_lun;
+ __u32 event_info_size;
+ __u32 sense_size;
+ __u32 cdb_size;
+ __u16 max_channel;
+ __u16 max_target;
+ __u32 max_lun;
+} __attribute__((packed));
/* Feature Bits */
#define VIRTIO_SCSI_F_INOUT0
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 44a5581..e61947c 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -428,6 +428,7 @@ header-y += virtio_net.h
header-y += virtio_pci.h
header-y += virtio_ring.h
header-y += virtio_rng.h
+header-y += virtio_scsi.h
header=y += vm_sockets.h
header-y += vt.h
header-y += wait.h
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 45/50] vhost/scsi: partial virtio 1.0 support
Include all endian conversions as required by virtio 1.0.
Don't set virtio 1.0 yet, since that requires ANY_LAYOUT
which we don't yet support.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Acked-by: Paolo Bonzini pbonz...@redhat.com
---
drivers/vhost/scsi.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index a17f118..01c01cb 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -168,6 +168,7 @@ enum {
VHOST_SCSI_VQ_IO = 2,
};
+/* Note: can't set VIRTIO_F_VERSION_1 yet, since that implies ANY_LAYOUT. */
enum {
VHOST_SCSI_FEATURES = VHOST_FEATURES | (1ULL VIRTIO_SCSI_F_HOTPLUG) |
(1ULL VIRTIO_SCSI_F_T10_PI)
@@ -577,8 +578,8 @@ tcm_vhost_allocate_evt(struct vhost_scsi *vs,
return NULL;
}
- evt-event.event = event;
- evt-event.reason = reason;
+ evt-event.event = cpu_to_vhost32(vq, event);
+ evt-event.reason = cpu_to_vhost32(vq, reason);
vs-vs_events_nr++;
return evt;
@@ -636,7 +637,7 @@ again:
}
if (vs-vs_events_missed) {
- event-event |= VIRTIO_SCSI_T_EVENTS_MISSED;
+ event-event |= cpu_to_vhost32(vq, VIRTIO_SCSI_T_EVENTS_MISSED);
vs-vs_events_missed = false;
}
@@ -695,12 +696,13 @@ static void vhost_scsi_complete_cmd_work(struct
vhost_work *work)
cmd, se_cmd-residual_count, se_cmd-scsi_status);
memset(v_rsp, 0, sizeof(v_rsp));
- v_rsp.resid = se_cmd-residual_count;
+ v_rsp.resid = cpu_to_vhost32(cmd-tvc_vq,
se_cmd-residual_count);
/* TODO is status_qualifier field needed? */
v_rsp.status = se_cmd-scsi_status;
- v_rsp.sense_len = se_cmd-scsi_sense_length;
+ v_rsp.sense_len = cpu_to_vhost32(cmd-tvc_vq,
+se_cmd-scsi_sense_length);
memcpy(v_rsp.sense, cmd-tvc_sense_buf,
- v_rsp.sense_len);
+ se_cmd-scsi_sense_length);
ret = copy_to_user(cmd-tvc_resp, v_rsp, sizeof(v_rsp));
if (likely(ret == 0)) {
struct vhost_scsi_virtqueue *q;
@@ -1095,14 +1097,14 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
, but wrong data_direction\n);
goto err_cmd;
}
- prot_bytes = v_req_pi.pi_bytesout;
+ prot_bytes = vhost32_to_cpu(vq,
v_req_pi.pi_bytesout);
} else if (v_req_pi.pi_bytesin) {
if (data_direction != DMA_FROM_DEVICE) {
vq_err(vq, Received non zero
di_pi_niov
, but wrong data_direction\n);
goto err_cmd;
}
- prot_bytes = v_req_pi.pi_bytesin;
+ prot_bytes = vhost32_to_cpu(vq,
v_req_pi.pi_bytesin);
}
if (prot_bytes) {
int tmp = 0;
@@ -1117,12 +1119,12 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
data_first += prot_niov;
data_niov = data_num - prot_niov;
}
- tag = v_req_pi.tag;
+ tag = vhost64_to_cpu(vq, v_req_pi.tag);
task_attr = v_req_pi.task_attr;
cdb = v_req_pi.cdb[0];
lun = ((v_req_pi.lun[2] 8) | v_req_pi.lun[3])
0x3FFF;
} else {
- tag = v_req.tag;
+ tag = vhost64_to_cpu(vq, v_req.tag);
task_attr = v_req.task_attr;
cdb = v_req.cdb[0];
lun = ((v_req.lun[2] 8) | v_req.lun[3]) 0x3FFF;
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 49/50] virtio: make VIRTIO_F_VERSION_1 a transport bit
Activate VIRTIO_F_VERSION_1 automatically unless legacy_only is set. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/uapi/linux/virtio_config.h | 4 ++-- drivers/virtio/virtio.c| 6 +- drivers/virtio/virtio_ring.c | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h index 4d05671..a6d0cde 100644 --- a/include/uapi/linux/virtio_config.h +++ b/include/uapi/linux/virtio_config.h @@ -43,11 +43,11 @@ /* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80 -/* Some virtio feature bits (currently bits 28 through 31) are reserved for the +/* Some virtio feature bits (currently bits 28 through 32) are reserved for the * transport being used (eg. virtio_ring), the rest are per-device feature * bits. */ #define VIRTIO_TRANSPORT_F_START 28 -#define VIRTIO_TRANSPORT_F_END 32 +#define VIRTIO_TRANSPORT_F_END 33 /* Do we get callbacks when the ring is completely used, even if we've * suppressed them? */ diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index f9ad99c..fa6b75d 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -197,7 +197,11 @@ static int virtio_dev_probe(struct device *_d) driver_features_legacy = driver_features; } - if (driver_features device_features (1ULL VIRTIO_F_VERSION_1)) + /* Detect legacy-only drivers and disable VIRTIO_F_VERSION_1. */ + if (drv-legacy_only) + device_features = ~(1ULL VIRTIO_F_VERSION_1); + + if (device_features (1ULL VIRTIO_F_VERSION_1)) dev-features = driver_features device_features; else dev-features = driver_features_legacy device_features; diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 55532a4..00ec6b3 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -780,6 +780,8 @@ void vring_transport_features(struct virtio_device *vdev) break; case VIRTIO_RING_F_EVENT_IDX: break; + case VIRTIO_F_VERSION_1: + break; default: /* We don't understand this bit. */ __virtio_clear_bit(vdev, i); -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 47/50] virtio_console: virtio 1.0 support
Pretty straight-forward, just use accessors for all fields.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/uapi/linux/virtio_console.h | 7 ---
drivers/char/virtio_console.c | 29 +
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/include/uapi/linux/virtio_console.h
b/include/uapi/linux/virtio_console.h
index ba260dd..b7fb108 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -32,6 +32,7 @@
#ifndef _UAPI_LINUX_VIRTIO_CONSOLE_H
#define _UAPI_LINUX_VIRTIO_CONSOLE_H
#include linux/types.h
+#include linux/virtio_types.h
#include linux/virtio_ids.h
#include linux/virtio_config.h
@@ -58,9 +59,9 @@ struct virtio_console_config {
* particular port.
*/
struct virtio_console_control {
- __u32 id; /* Port number */
- __u16 event;/* The kind of control event (see below) */
- __u16 value;/* Extra information for the key */
+ __virtio32 id; /* Port number */
+ __virtio16 event; /* The kind of control event (see below) */
+ __virtio16 value; /* Extra information for the key */
};
/* Some events for control messages */
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8d00aa7..775c898 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -566,9 +566,9 @@ static ssize_t __send_control_msg(struct ports_device
*portdev, u32 port_id,
if (!use_multiport(portdev))
return 0;
- cpkt.id = port_id;
- cpkt.event = event;
- cpkt.value = value;
+ cpkt.id = cpu_to_virtio32(portdev-vdev, port_id);
+ cpkt.event = cpu_to_virtio16(portdev-vdev, event);
+ cpkt.value = cpu_to_virtio16(portdev-vdev, value);
vq = portdev-c_ovq;
@@ -1602,7 +1602,8 @@ static void unplug_port(struct port *port)
}
/* Any private messages that the Host and Guest want to share */
-static void handle_control_message(struct ports_device *portdev,
+static void handle_control_message(struct virtio_device *vdev,
+ struct ports_device *portdev,
struct port_buffer *buf)
{
struct virtio_console_control *cpkt;
@@ -1612,15 +1613,16 @@ static void handle_control_message(struct ports_device
*portdev,
cpkt = (struct virtio_console_control *)(buf-buf + buf-offset);
- port = find_port_by_id(portdev, cpkt-id);
- if (!port cpkt-event != VIRTIO_CONSOLE_PORT_ADD) {
+ port = find_port_by_id(portdev, virtio32_to_cpu(vdev, cpkt-id));
+ if (!port
+ cpkt-event != cpu_to_virtio16(vdev, VIRTIO_CONSOLE_PORT_ADD)) {
/* No valid header at start of buffer. Drop it. */
dev_dbg(portdev-vdev-dev,
Invalid index %u in control packet\n, cpkt-id);
return;
}
- switch (cpkt-event) {
+ switch (virtio16_to_cpu(vdev, cpkt-event)) {
case VIRTIO_CONSOLE_PORT_ADD:
if (port) {
dev_dbg(portdev-vdev-dev,
@@ -1628,13 +1630,15 @@ static void handle_control_message(struct ports_device
*portdev,
send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
break;
}
- if (cpkt-id = portdev-config.max_nr_ports) {
+ if (virtio32_to_cpu(vdev, cpkt-id) =
+ portdev-config.max_nr_ports) {
dev_warn(portdev-vdev-dev,
- Request for adding port with out-of-bound id
%u, max. supported id: %u\n,
+ Request for adding port with
+ out-of-bound id %u, max. supported id: %u\n,
cpkt-id, portdev-config.max_nr_ports - 1);
break;
}
- add_port(portdev, cpkt-id);
+ add_port(portdev, virtio32_to_cpu(vdev, cpkt-id));
break;
case VIRTIO_CONSOLE_PORT_REMOVE:
unplug_port(port);
@@ -1670,7 +1674,7 @@ static void handle_control_message(struct ports_device
*portdev,
break;
}
case VIRTIO_CONSOLE_PORT_OPEN:
- port-host_connected = cpkt-value;
+ port-host_connected = virtio16_to_cpu(vdev, cpkt-value);
wake_up_interruptible(port-waitqueue);
/*
* If the host port got closed and the host had any
@@ -1752,7 +1756,7 @@ static void control_work_handler(struct work_struct *work)
buf-len = len;
buf-offset = 0;
- handle_control_message(portdev, buf);
+ handle_control_message(vq-vdev, portdev, buf);
spin_lock(portdev-c_ivq_lock);
if (add_inbuf(portdev-c_ivq, buf) 0) {
@@ -2126,6
[PATCH v8 50/50] virtio: drop VIRTIO_F_VERSION_1 from drivers
Core activates this bit automatically now,
drop it from drivers that set it explicitly.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/block/virtio_blk.c| 1 -
drivers/char/virtio_console.c | 1 -
drivers/net/virtio_net.c | 1 -
drivers/scsi/virtio_scsi.c| 1 -
4 files changed, 4 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 1f8b111..1fb9e09 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -836,7 +836,6 @@ static unsigned int features[] = {
VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
VIRTIO_BLK_F_TOPOLOGY,
VIRTIO_BLK_F_MQ,
- VIRTIO_F_VERSION_1,
};
static struct virtio_driver virtio_blk = {
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 775c898..6cc832b 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2130,7 +2130,6 @@ static struct virtio_device_id id_table[] = {
static unsigned int features[] = {
VIRTIO_CONSOLE_F_SIZE,
VIRTIO_CONSOLE_F_MULTIPORT,
- VIRTIO_F_VERSION_1,
};
static struct virtio_device_id rproc_serial_id_table[] = {
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9ab3c50..b8bd719 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2004,7 +2004,6 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
- VIRTIO_F_VERSION_1,
};
static struct virtio_driver virtio_net_driver = {
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index d9ec806..2308278 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -1085,7 +1085,6 @@ static unsigned int features[] = {
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_SCSI_F_CHANGE,
VIRTIO_SCSI_F_T10_PI,
- VIRTIO_F_VERSION_1,
};
static struct virtio_driver virtio_scsi_driver = {
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 09/50] virtio_ring: switch to new memory access APIs
Use virtioXX_to_cpu and friends for access to
all multibyte structures in memory.
Note: this is intentionally mechanical.
A follow-up patch will split long lines etc.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/virtio/virtio_ring.c | 89 ++--
1 file changed, 45 insertions(+), 44 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 839247c..0d3c737 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -99,7 +99,8 @@ struct vring_virtqueue
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
-static struct vring_desc *alloc_indirect(unsigned int total_sg, gfp_t gfp)
+static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
+unsigned int total_sg, gfp_t gfp)
{
struct vring_desc *desc;
unsigned int i;
@@ -116,7 +117,7 @@ static struct vring_desc *alloc_indirect(unsigned int
total_sg, gfp_t gfp)
return NULL;
for (i = 0; i total_sg; i++)
- desc[i].next = i+1;
+ desc[i].next = cpu_to_virtio16(_vq-vdev, i + 1);
return desc;
}
@@ -165,17 +166,17 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
if (vq-indirect total_sg 1 vq-vq.num_free)
- desc = alloc_indirect(total_sg, gfp);
+ desc = alloc_indirect(_vq, total_sg, gfp);
else
desc = NULL;
if (desc) {
/* Use a single buffer which doesn't continue */
- vq-vring.desc[head].flags = VRING_DESC_F_INDIRECT;
- vq-vring.desc[head].addr = virt_to_phys(desc);
+ vq-vring.desc[head].flags = cpu_to_virtio16(_vq-vdev,
VRING_DESC_F_INDIRECT);
+ vq-vring.desc[head].addr = cpu_to_virtio64(_vq-vdev,
virt_to_phys(desc));
/* avoid kmemleak false positive (hidden by virt_to_phys) */
kmemleak_ignore(desc);
- vq-vring.desc[head].len = total_sg * sizeof(struct vring_desc);
+ vq-vring.desc[head].len = cpu_to_virtio32(_vq-vdev, total_sg
* sizeof(struct vring_desc));
/* Set up rest to use this indirect table. */
i = 0;
@@ -205,28 +206,28 @@ static inline int virtqueue_add(struct virtqueue *_vq,
for (n = 0; n out_sgs; n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
- desc[i].flags = VRING_DESC_F_NEXT;
- desc[i].addr = sg_phys(sg);
- desc[i].len = sg-length;
+ desc[i].flags = cpu_to_virtio16(_vq-vdev,
VRING_DESC_F_NEXT);
+ desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+ desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
- i = desc[i].next;
+ i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
for (; n (out_sgs + in_sgs); n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
- desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
- desc[i].addr = sg_phys(sg);
- desc[i].len = sg-length;
+ desc[i].flags = cpu_to_virtio16(_vq-vdev,
VRING_DESC_F_NEXT | VRING_DESC_F_WRITE);
+ desc[i].addr = cpu_to_virtio64(_vq-vdev, sg_phys(sg));
+ desc[i].len = cpu_to_virtio32(_vq-vdev, sg-length);
prev = i;
- i = desc[i].next;
+ i = virtio16_to_cpu(_vq-vdev, desc[i].next);
}
}
/* Last one doesn't continue. */
- desc[prev].flags = ~VRING_DESC_F_NEXT;
+ desc[prev].flags = cpu_to_virtio16(_vq-vdev, ~VRING_DESC_F_NEXT);
/* Update free pointer */
if (indirect)
- vq-free_head = vq-vring.desc[head].next;
+ vq-free_head = virtio16_to_cpu(_vq-vdev,
vq-vring.desc[head].next);
else
vq-free_head = i;
@@ -235,13 +236,13 @@ static inline int virtqueue_add(struct virtqueue *_vq,
/* Put entry in available array (but don't update avail-idx until they
* do sync). */
- avail = (vq-vring.avail-idx (vq-vring.num-1));
- vq-vring.avail-ring[avail] = head;
+ avail = virtio16_to_cpu(_vq-vdev, vq-vring.avail-idx)
(vq-vring.num - 1);
+ vq-vring.avail-ring[avail] = cpu_to_virtio16(_vq-vdev, head);
/* Descriptors and available array need to be set before we expose the
* new available array entries. */
virtio_wmb(vq-weak_barriers);
- vq-vring.avail-idx++;
+
[PATCH v8 23/50] virtio_net: pass vi around
Too many places poke at [rs]q-vq-vdev-priv just to get
the vi structure. Let's just pass the pointer around: seems
cleaner, and might even be faster.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/net/virtio_net.c | 38 --
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c07e030..1630c21 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -241,11 +241,11 @@ static unsigned long mergeable_buf_to_ctx(void *buf,
unsigned int truesize)
}
/* Called from bottom half context */
-static struct sk_buff *page_to_skb(struct receive_queue *rq,
+static struct sk_buff *page_to_skb(struct virtnet_info *vi,
+ struct receive_queue *rq,
struct page *page, unsigned int offset,
unsigned int len, unsigned int truesize)
{
- struct virtnet_info *vi = rq-vq-vdev-priv;
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
unsigned int copy, hdr_len, hdr_padded_len;
@@ -328,12 +328,13 @@ static struct sk_buff *receive_small(void *buf, unsigned
int len)
}
static struct sk_buff *receive_big(struct net_device *dev,
+ struct virtnet_info *vi,
struct receive_queue *rq,
void *buf,
unsigned int len)
{
struct page *page = buf;
- struct sk_buff *skb = page_to_skb(rq, page, 0, len, PAGE_SIZE);
+ struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
if (unlikely(!skb))
goto err;
@@ -359,7 +360,8 @@ static struct sk_buff *receive_mergeable(struct net_device
*dev,
int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
- struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, truesize);
+ struct sk_buff *head_skb = page_to_skb(vi, rq, page, offset, len,
+ truesize);
struct sk_buff *curr_skb = head_skb;
if (unlikely(!curr_skb))
@@ -433,9 +435,9 @@ err_buf:
return NULL;
}
-static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
+static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
+ void *buf, unsigned int len)
{
- struct virtnet_info *vi = rq-vq-vdev-priv;
struct net_device *dev = vi-dev;
struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
struct sk_buff *skb;
@@ -459,7 +461,7 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
if (vi-mergeable_rx_bufs)
skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
else if (vi-big_packets)
- skb = receive_big(dev, rq, buf, len);
+ skb = receive_big(dev, vi, rq, buf, len);
else
skb = receive_small(buf, len);
@@ -539,9 +541,9 @@ frame_err:
dev_kfree_skb(skb);
}
-static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp)
+static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq,
+gfp_t gfp)
{
- struct virtnet_info *vi = rq-vq-vdev-priv;
struct sk_buff *skb;
struct skb_vnet_hdr *hdr;
int err;
@@ -664,9 +666,9 @@ static int add_recvbuf_mergeable(struct receive_queue *rq,
gfp_t gfp)
* before we're receiving packets, or from refill_work which is
* careful to disable receiving (using napi_disable).
*/
-static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
+static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
+ gfp_t gfp)
{
- struct virtnet_info *vi = rq-vq-vdev-priv;
int err;
bool oom;
@@ -677,7 +679,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t
gfp)
else if (vi-big_packets)
err = add_recvbuf_big(rq, gfp);
else
- err = add_recvbuf_small(rq, gfp);
+ err = add_recvbuf_small(vi, rq, gfp);
oom = err == -ENOMEM;
if (err)
@@ -726,7 +728,7 @@ static void refill_work(struct work_struct *work)
struct receive_queue *rq = vi-rq[i];
napi_disable(rq-napi);
- still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
virtnet_napi_enable(rq);
/* In theory, this can happen: if we don't get any buffers in
@@ -745,12 +747,12 @@ static int virtnet_receive(struct receive_queue *rq, int
budget)
while (received budget
(buf =
[PATCH v8 28/50] virtio_net: enable v1.0 support
Now that we have completed 1.0 support, enable it in our driver.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/net/virtio_net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b8bd719..9ab3c50 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2004,6 +2004,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+ VIRTIO_F_VERSION_1,
};
static struct virtio_driver virtio_net_driver = {
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 12/50] virtio: set FEATURES_OK
set FEATURES_OK as per virtio 1.0 spec
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/uapi/linux/virtio_config.h | 2 ++
drivers/virtio/virtio.c| 29 ++---
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/virtio_config.h
b/include/uapi/linux/virtio_config.h
index 80e7381..4d05671 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -38,6 +38,8 @@
#define VIRTIO_CONFIG_S_DRIVER 2
/* Driver has used its parts of the config, and is happy */
#define VIRTIO_CONFIG_S_DRIVER_OK 4
+/* Driver has finished configuring features */
+#define VIRTIO_CONFIG_S_FEATURES_OK8
/* We've given up on this device. */
#define VIRTIO_CONFIG_S_FAILED 0x80
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 746d350..9248125 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -160,6 +160,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev-dev.driver);
u64 device_features;
+ unsigned status;
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -183,18 +184,32 @@ static int virtio_dev_probe(struct device *_d)
dev-config-finalize_features(dev);
+ if (virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
+ add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+ status = dev-config-get_status(dev);
+ if (!(status VIRTIO_CONFIG_S_FEATURES_OK)) {
+ dev_err(_d, virtio: device refuses features: %x\n,
+ status);
+ err = -ENODEV;
+ goto err;
+ }
+ }
+
err = drv-probe(dev);
if (err)
- add_status(dev, VIRTIO_CONFIG_S_FAILED);
- else {
- add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
- if (drv-scan)
- drv-scan(dev);
+ goto err;
- virtio_config_enable(dev);
- }
+ add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+ if (drv-scan)
+ drv-scan(dev);
+
+ virtio_config_enable(dev);
+ return 0;
+err:
+ add_status(dev, VIRTIO_CONFIG_S_FAILED);
return err;
+
}
static int virtio_dev_remove(struct device *_d)
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 15/50] virtio_net: v1.0 endianness
Based on patches by Rusty Russell, Cornelia Huck.
Note: more code changes are needed for 1.0 support
(due to different header size).
So we don't advertize support for 1.0 yet.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
include/uapi/linux/virtio_net.h | 15 ---
drivers/net/virtio_net.c| 33 -
2 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..b5f1677 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -28,6 +28,7 @@
#include linux/types.h
#include linux/virtio_ids.h
#include linux/virtio_config.h
+#include linux/virtio_types.h
#include linux/if_ether.h
/* The feature bitmap for virtio net */
@@ -84,17 +85,17 @@ struct virtio_net_hdr {
#define VIRTIO_NET_HDR_GSO_TCPV6 4 // GSO frame, IPv6 TCP
#define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
__u8 gso_type;
- __u16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
- __u16 gso_size; /* Bytes to append to hdr_len per frame */
- __u16 csum_start; /* Position to start checksumming from */
- __u16 csum_offset; /* Offset after that to place checksum */
+ __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __virtio16 gso_size;/* Bytes to append to hdr_len per frame
*/
+ __virtio16 csum_start; /* Position to start checksumming from */
+ __virtio16 csum_offset; /* Offset after that to place checksum */
};
/* This is the version of the header to use when the MRG_RXBUF
* feature has been negotiated. */
struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
- __u16 num_buffers; /* Number of merged rx buffers */
+ __virtio16 num_buffers; /* Number of merged rx buffers */
};
/*
@@ -149,7 +150,7 @@ typedef __u8 virtio_net_ctrl_ack;
* VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
*/
struct virtio_net_ctrl_mac {
- __u32 entries;
+ __virtio32 entries;
__u8 macs[][ETH_ALEN];
} __attribute__((packed));
@@ -193,7 +194,7 @@ struct virtio_net_ctrl_mac {
* specified.
*/
struct virtio_net_ctrl_mq {
- __u16 virtqueue_pairs;
+ __virtio16 virtqueue_pairs;
};
#define VIRTIO_NET_CTRL_MQ 4
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b0bc8ea..c07e030 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -347,13 +347,14 @@ err:
}
static struct sk_buff *receive_mergeable(struct net_device *dev,
+struct virtnet_info *vi,
struct receive_queue *rq,
unsigned long ctx,
unsigned int len)
{
void *buf = mergeable_ctx_to_buf_address(ctx);
struct skb_vnet_hdr *hdr = buf;
- int num_buf = hdr-mhdr.num_buffers;
+ u16 num_buf = virtio16_to_cpu(rq-vq-vdev, hdr-mhdr.num_buffers);
struct page *page = virt_to_head_page(buf);
int offset = buf - page_address(page);
unsigned int truesize = max(len, mergeable_ctx_to_buf_truesize(ctx));
@@ -369,7 +370,9 @@ static struct sk_buff *receive_mergeable(struct net_device
*dev,
ctx = (unsigned long)virtqueue_get_buf(rq-vq, len);
if (unlikely(!ctx)) {
pr_debug(%s: rx error: %d buffers out of %d missing\n,
-dev-name, num_buf, hdr-mhdr.num_buffers);
+dev-name, num_buf,
+virtio16_to_cpu(rq-vq-vdev,
+hdr-mhdr.num_buffers));
dev-stats.rx_length_errors++;
goto err_buf;
}
@@ -454,7 +457,7 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
}
if (vi-mergeable_rx_bufs)
- skb = receive_mergeable(dev, rq, (unsigned long)buf, len);
+ skb = receive_mergeable(dev, vi, rq, (unsigned long)buf, len);
else if (vi-big_packets)
skb = receive_big(dev, rq, buf, len);
else
@@ -473,8 +476,8 @@ static void receive_buf(struct receive_queue *rq, void
*buf, unsigned int len)
if (hdr-hdr.flags VIRTIO_NET_HDR_F_NEEDS_CSUM) {
pr_debug(Needs csum!\n);
if (!skb_partial_csum_set(skb,
- hdr-hdr.csum_start,
- hdr-hdr.csum_offset))
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_start),
+ virtio16_to_cpu(vi-vdev, hdr-hdr.csum_offset)))
goto frame_err;
} else if
[PATCH v8 33/50] vhost: virtio 1.0 endian-ness support
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
drivers/vhost/vhost.c | 86 +--
1 file changed, 49 insertions(+), 37 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6a40837..ed71b53 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -33,8 +33,8 @@ enum {
VHOST_MEMORY_F_LOG = 0x1,
};
-#define vhost_used_event(vq) ((u16 __user *)vq-avail-ring[vq-num])
-#define vhost_avail_event(vq) ((u16 __user *)vq-used-ring[vq-num])
+#define vhost_used_event(vq) ((__virtio16 __user *)vq-avail-ring[vq-num])
+#define vhost_avail_event(vq) ((__virtio16 __user *)vq-used-ring[vq-num])
static void vhost_poll_func(struct file *file, wait_queue_head_t *wqh,
poll_table *pt)
@@ -1001,7 +1001,7 @@ EXPORT_SYMBOL_GPL(vhost_log_write);
static int vhost_update_used_flags(struct vhost_virtqueue *vq)
{
void __user *used;
- if (__put_user(vq-used_flags, vq-used-flags) 0)
+ if (__put_user(cpu_to_vhost16(vq, vq-used_flags), vq-used-flags)
0)
return -EFAULT;
if (unlikely(vq-log_used)) {
/* Make sure the flag is seen before log. */
@@ -1019,7 +1019,7 @@ static int vhost_update_used_flags(struct vhost_virtqueue
*vq)
static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16
avail_event)
{
- if (__put_user(vq-avail_idx, vhost_avail_event(vq)))
+ if (__put_user(cpu_to_vhost16(vq, vq-avail_idx),
vhost_avail_event(vq)))
return -EFAULT;
if (unlikely(vq-log_used)) {
void __user *used;
@@ -1038,7 +1038,7 @@ static int vhost_update_avail_event(struct
vhost_virtqueue *vq, u16 avail_event)
int vhost_init_used(struct vhost_virtqueue *vq)
{
- u16 last_used_idx;
+ __virtio16 last_used_idx;
int r;
if (!vq-private_data)
return 0;
@@ -1052,7 +1052,7 @@ int vhost_init_used(struct vhost_virtqueue *vq)
r = __get_user(last_used_idx, vq-used-idx);
if (r)
return r;
- vq-last_used_idx = last_used_idx;
+ vq-last_used_idx = vhost16_to_cpu(vq, last_used_idx);
return 0;
}
EXPORT_SYMBOL_GPL(vhost_init_used);
@@ -1094,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq,
u64 addr, u32 len,
/* Each buffer in the virtqueues is actually a chain of descriptors. This
* function returns the next descriptor in the chain,
* or -1U if we're at the end. */
-static unsigned next_desc(struct vring_desc *desc)
+static unsigned next_desc(struct vhost_virtqueue *vq, struct vring_desc *desc)
{
unsigned int next;
/* If this descriptor says it doesn't chain, we're done. */
- if (!(desc-flags VRING_DESC_F_NEXT))
+ if (!(desc-flags cpu_to_vhost16(vq, VRING_DESC_F_NEXT)))
return -1U;
/* Check they're not leading us off end of descriptors. */
- next = desc-next;
+ next = vhost16_to_cpu(vq, desc-next);
/* Make sure compiler knows to grab that: we don't want it changing! */
/* We will use the result as an index in an array, so most
* architectures only need a compiler barrier here. */
@@ -1120,18 +1120,19 @@ static int get_indirect(struct vhost_virtqueue *vq,
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
+ u32 len = vhost32_to_cpu(vq, indirect-len);
int ret;
/* Sanity check */
- if (unlikely(indirect-len % sizeof desc)) {
+ if (unlikely(len % sizeof desc)) {
vq_err(vq, Invalid length in indirect descriptor:
len 0x%llx not multiple of 0x%zx\n,
- (unsigned long long)indirect-len,
+ (unsigned long long)len,
sizeof desc);
return -EINVAL;
}
- ret = translate_desc(vq, indirect-addr, indirect-len, vq-indirect,
+ ret = translate_desc(vq, vhost64_to_cpu(vq, indirect-addr), len,
vq-indirect,
UIO_MAXIOV);
if (unlikely(ret 0)) {
vq_err(vq, Translation failure %d in indirect.\n, ret);
@@ -1142,7 +1143,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
* architectures only need a compiler barrier here. */
read_barrier_depends();
- count = indirect-len / sizeof desc;
+ count = len / sizeof desc;
/* Buffers are chained via a 16 bit next field, so
* we can have at most 2^16 of these. */
if (unlikely(count USHRT_MAX + 1)) {
@@ -1162,16 +1163,17 @@ static int get_indirect(struct vhost_virtqueue *vq,
if (unlikely(memcpy_fromiovec((unsigned char *)desc,
vq-indirect, sizeof desc))) {
vq_err(vq, Failed indirect descriptor: idx %d, %zx\n,
- i, (size_t)indirect-addr + i * sizeof desc);
+
[PATCH v8 35/50] vhost/net: larger header for virtio 1.0
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Jason Wang jasow...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/vhost/net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index c218188..8ff4a6d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1030,7 +1030,8 @@ static int vhost_net_set_features(struct vhost_net *n,
u64 features)
size_t vhost_hlen, sock_hlen, hdr_len;
int i;
- hdr_len = (features (1 VIRTIO_NET_F_MRG_RXBUF)) ?
+ hdr_len = (features ((1ULL VIRTIO_NET_F_MRG_RXBUF) |
+ (1ULL VIRTIO_F_VERSION_1))) ?
sizeof(struct virtio_net_hdr_mrg_rxbuf) :
sizeof(struct virtio_net_hdr);
if (features (1 VHOST_NET_F_VIRTIO_NET_HDR)) {
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 02/50] virtio: use u32, not bitmap for features
It seemed like a good idea to use bitmap for features
in struct virtio_device, but it's actually a pain,
and seems to become even more painful when we get more
than 32 feature bits. Just change it to a u32 for now.
Based on patch by Rusty.
Suggested-by: David Hildenbrand d...@linux.vnet.ibm.com
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/linux/virtio.h | 3 +--
include/linux/virtio_config.h | 6 +++---
tools/virtio/linux/virtio.h| 22 +-
tools/virtio/linux/virtio_config.h | 2 +-
drivers/char/virtio_console.c | 2 +-
drivers/lguest/lguest_device.c | 8
drivers/misc/mic/card/mic_virtio.c | 2 +-
drivers/remoteproc/remoteproc_virtio.c | 2 +-
drivers/s390/kvm/kvm_virtio.c | 2 +-
drivers/s390/kvm/virtio_ccw.c | 23 +--
drivers/virtio/virtio.c| 10 +-
drivers/virtio/virtio_mmio.c | 8 ++--
drivers/virtio/virtio_pci.c| 3 +--
drivers/virtio/virtio_ring.c | 2 +-
tools/virtio/virtio_test.c | 5 ++---
tools/virtio/vringh_test.c | 16
16 files changed, 42 insertions(+), 74 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 65261a7..7828a7f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,8 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
- /* Note that this is a Linux set_bit-style bitmap. */
- unsigned long features[1];
+ u32 features;
void *priv;
};
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index d8e28a2..ffc2ae0 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -92,7 +92,7 @@ static inline bool __virtio_test_bit(const struct
virtio_device *vdev,
else
BUG_ON(fbit = 32);
- return test_bit(fbit, vdev-features);
+ return vdev-features BIT(fbit);
}
/**
@@ -109,7 +109,7 @@ static inline void __virtio_set_bit(struct virtio_device
*vdev,
else
BUG_ON(fbit = 32);
- set_bit(fbit, vdev-features);
+ vdev-features |= BIT(fbit);
}
/**
@@ -126,7 +126,7 @@ static inline void __virtio_clear_bit(struct virtio_device
*vdev,
else
BUG_ON(fbit = 32);
- clear_bit(fbit, vdev-features);
+ vdev-features = ~BIT(fbit);
}
/**
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 5a2d1f0..72bff70 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -6,31 +6,11 @@
/* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
#define list_add_tail(a, b) do {} while (0)
#define list_del(a) do {} while (0)
-
-#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
-#define BITS_PER_BYTE 8
-#define BITS_PER_LONG (sizeof(long) * BITS_PER_BYTE)
-#define BIT_MASK(nr) (1UL ((nr) % BITS_PER_LONG))
-
-/* TODO: Not atomic as it should be:
- * we don't use this for anything important. */
-static inline void clear_bit(int nr, volatile unsigned long *addr)
-{
- unsigned long mask = BIT_MASK(nr);
- unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-
- *p = ~mask;
-}
-
-static inline int test_bit(int nr, const volatile unsigned long *addr)
-{
-return 1UL (addr[BIT_WORD(nr)] (nr (BITS_PER_LONG-1)));
-}
/* end of stubs */
struct virtio_device {
void *dev;
- unsigned long features[1];
+ u32 features;
};
struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h
b/tools/virtio/linux/virtio_config.h
index 5049967..83b27e8 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
#define VIRTIO_TRANSPORT_F_END 32
#define virtio_has_feature(dev, feature) \
- test_bit((feature), (dev)-features)
+ (__virtio_test_bit((dev), feature))
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cf7a561..8d00aa7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device
*portdev)
*/
if (!portdev-vdev)
return 0;
- return portdev-vdev-features[0] (1 VIRTIO_CONSOLE_F_MULTIPORT);
+ return __virtio_test_bit(portdev-vdev, VIRTIO_CONSOLE_F_MULTIPORT);
}
static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index d0a1d8a..97aeb7d 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -137,14 +137,14 @@
[PATCH v8 22/50] virtio_blk: fix race at module removal
If a device appears while module is being removed,
driver will get a callback after we've given up
on the major number.
In theory this means this major number can get reused
by something else, resulting in a conflict.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/block/virtio_blk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 055f3df..1f8b111 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -884,8 +884,8 @@ out_destroy_workqueue:
static void __exit fini(void)
{
- unregister_blkdev(major, virtblk);
unregister_virtio_driver(virtio_blk);
+ unregister_blkdev(major, virtblk);
destroy_workqueue(virtblk_wq);
}
module_init(init);
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 34/50] vhost/net: virtio 1.0 byte swap
I had to add an explicit tag to suppress compiler warning:
gcc isn't smart enough to notice that
len is always initialized since function is called with size 0.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
drivers/vhost/net.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index dce5c58..c218188 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -416,7 +416,7 @@ static void handle_tx(struct vhost_net *net)
struct ubuf_info *ubuf;
ubuf = nvq-ubuf_info + nvq-upend_idx;
- vq-heads[nvq-upend_idx].id = head;
+ vq-heads[nvq-upend_idx].id = cpu_to_vhost32(vq, head);
vq-heads[nvq-upend_idx].len = VHOST_DMA_IN_PROGRESS;
ubuf-callback = vhost_zerocopy_callback;
ubuf-ctx = nvq-ubufs;
@@ -500,6 +500,10 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
int headcount = 0;
unsigned d;
int r, nlogs = 0;
+ /* len is always initialized before use since we are always called with
+* datalen 0.
+*/
+ u32 uninitialized_var(len);
while (datalen 0 headcount quota) {
if (unlikely(seg = UIO_MAXIOV)) {
@@ -527,13 +531,14 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
nlogs += *log_num;
log += *log_num;
}
- heads[headcount].id = d;
- heads[headcount].len = iov_length(vq-iov + seg, in);
- datalen -= heads[headcount].len;
+ heads[headcount].id = cpu_to_vhost32(vq, d);
+ len = iov_length(vq-iov + seg, in);
+ heads[headcount].len = cpu_to_vhost32(vq, len);
+ datalen -= len;
++headcount;
seg += in;
}
- heads[headcount - 1].len += datalen;
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
*iovcount = seg;
if (unlikely(log))
*log_num = nlogs;
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 08/50] virtio: memory access APIs
virtio 1.0 makes all memory structures LE, so
we need APIs to conditionally do a byteswap on BE
architectures.
To make it easier to check code statically,
add virtio specific types for multi-byte integers
in memory.
Add low level wrappers that do a byteswap conditionally, these will be
useful e.g. for vhost. Add high level wrappers that
query device endian-ness and act accordingly.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
---
include/linux/virtio_byteorder.h | 59 +++
include/linux/virtio_config.h | 32 +
include/uapi/linux/virtio_ring.h | 45 ++---
include/uapi/linux/virtio_types.h | 46 ++
include/uapi/linux/Kbuild | 1 +
5 files changed, 161 insertions(+), 22 deletions(-)
create mode 100644 include/linux/virtio_byteorder.h
create mode 100644 include/uapi/linux/virtio_types.h
diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
new file mode 100644
index 000..51865d0
--- /dev/null
+++ b/include/linux/virtio_byteorder.h
@@ -0,0 +1,59 @@
+#ifndef _LINUX_VIRTIO_BYTEORDER_H
+#define _LINUX_VIRTIO_BYTEORDER_H
+#include linux/types.h
+#include uapi/linux/virtio_types.h
+
+/*
+ * Low-level memory accessors for handling virtio in modern little endian and
in
+ * compatibility native endian format.
+ */
+
+static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
+{
+ if (little_endian)
+ return le16_to_cpu((__force __le16)val);
+ else
+ return (__force u16)val;
+}
+
+static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
+{
+ if (little_endian)
+ return (__force __virtio16)cpu_to_le16(val);
+ else
+ return (__force __virtio16)val;
+}
+
+static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
+{
+ if (little_endian)
+ return le32_to_cpu((__force __le32)val);
+ else
+ return (__force u32)val;
+}
+
+static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
+{
+ if (little_endian)
+ return (__force __virtio32)cpu_to_le32(val);
+ else
+ return (__force __virtio32)val;
+}
+
+static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
+{
+ if (little_endian)
+ return le64_to_cpu((__force __le64)val);
+ else
+ return (__force u64)val;
+}
+
+static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
+{
+ if (little_endian)
+ return (__force __virtio64)cpu_to_le64(val);
+ else
+ return (__force __virtio64)val;
+}
+
+#endif /* _LINUX_VIRTIO_BYTEORDER */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index f517884..02f0acb 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -4,6 +4,7 @@
#include linux/err.h
#include linux/bug.h
#include linux/virtio.h
+#include linux/virtio_byteorder.h
#include uapi/linux/virtio_config.h
/**
@@ -199,6 +200,37 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
}
+/* Memory accessors */
+static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
+{
+ return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
+{
+ return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
+{
+ return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
+{
+ return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
+{
+ return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
+static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
+{
+ return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1),
val);
+}
+
/* Config space accessors. */
#define virtio_cread(vdev, structname, member, ptr)\
do {\
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index a99f9b7..61c818a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -32,6 +32,7 @@
*
* Copyright Rusty Russell IBM Corporation 2007. */
#include linux/types.h
+#include linux/virtio_types.h
/* This marks a buffer as continuing via the next field. */
#define VRING_DESC_F_NEXT 1
@@ -61,32 +62,32 @@
/* Virtio ring descriptors: 16 bytes. These can
Re: [PATCH v7 44/46] virtio_scsi: export to userspace
Hi Michael, Thanks for the patch. On Sun, Nov 30, 2014 at 3:12 PM, Michael S. Tsirkin m...@redhat.com wrote: Replace uXX by __uXX and _packed by __attribute((packed)) as seems to be the norm for userspace headers. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Paolo Bonzini pbonz...@redhat.com --- include/uapi/linux/virtio_scsi.h | 74 include/uapi/linux/Kbuild| 1 + Probably worth making a separate patch for this ? as it doesnt match with patch description aswel. Apart from that patch looks good. Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar Lad ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Xen-devel] [PATCH v4 04/10] x86: paravirt: Wrap initialization of set_iopl_mask in a macro
On Sun, Nov 02, 2014 at 09:32:20AM -0800, Josh Triplett wrote:
This will allow making set_iopl_mask optional later.
Signed-off-by: Josh Triplett j...@joshtriplett.org
Reviewed-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
---
arch/x86/include/asm/paravirt_types.h | 1 +
arch/x86/kernel/paravirt.c| 2 +-
arch/x86/xen/enlighten.c | 4 ++--
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/paravirt_types.h
b/arch/x86/include/asm/paravirt_types.h
index 7549b8b..3caf2a8 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -143,6 +143,7 @@ struct pv_cpu_ops {
void (*load_sp0)(struct tss_struct *tss, struct thread_struct *t);
void (*set_iopl_mask)(unsigned mask);
+#define INIT_SET_IOPL_MASK(f) .set_iopl_mask = f,
void (*wbinvd)(void);
void (*io_delay)(void);
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 548d25f..e7969d4 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -383,7 +383,7 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
.iret = native_iret,
.swapgs = native_swapgs,
- .set_iopl_mask = native_set_iopl_mask,
+ INIT_SET_IOPL_MASK(native_set_iopl_mask)
.io_delay = native_io_delay,
.start_context_switch = paravirt_nop,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 1a3f044..8ad0778 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -912,7 +912,7 @@ static void xen_load_sp0(struct tss_struct *tss,
xen_mc_issue(PARAVIRT_LAZY_CPU);
}
-static void xen_set_iopl_mask(unsigned mask)
+static void __maybe_unused xen_set_iopl_mask(unsigned mask)
{
struct physdev_set_iopl set_iopl;
@@ -1279,7 +1279,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst
= {
.write_idt_entry = xen_write_idt_entry,
.load_sp0 = xen_load_sp0,
- .set_iopl_mask = xen_set_iopl_mask,
+ INIT_SET_IOPL_MASK(xen_set_iopl_mask)
.io_delay = xen_io_delay,
/* Xen takes care of %gs when switching to usermode for us */
--
2.1.1
___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v12 09/11] pvqspinlock, x86: Add para-virtualization support
On Tue, Nov 25, 2014 at 07:33:58PM -0500, Waiman Long wrote: On 10/27/2014 02:02 PM, Konrad Rzeszutek Wilk wrote: On Mon, Oct 27, 2014 at 01:38:20PM -0400, Waiman Long wrote: My concern is that spin_unlock() can be called in many places, including loadable kernel modules. Can the paravirt_patch_ident_32() function able to patch all of them in reasonable time? How about a kernel module loaded later at run time? It has too. When the modules are loaded the .paravirt symbols are exposed and the module loader patches that. And during bootup time (before modules are loaded) it also patches everything - when it only runs on one CPU. I have been changing the patching code to patch the unlock call sites and it seems to be working now. However, when I manually inserted a kernel module using insmod and run the code in the newly inserted module, I got memory access violation as follows: BUG: unable to handle kernel NULL pointer dereference at (null) IP: [ (null)] (null) PGD 18d62f3067 PUD 18d476f067 PMD 0 Oops: 0010 [#1] SMP Modules linked in: locktest(OE) ebtable_nat ebtables xt_CHECKSUM iptable_mangle bridge autofs4 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 vhost_net macvtap macvlan vhost tun uinput ppdev parport_pc parport sg microcode pcspkr virtio_balloon snd_hda_codec_generic virtio_console snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore virtio_net i2c_piix4 i2c_core ext4(E) jbd2(E) mbcache(E) floppy(E) virtio_blk(E) sr_mod(E) cdrom(E) virtio_pci(E) virtio_ring(E) virtio(E) pata_acpi(E) ata_generic(E) ata_piix(E) dm_mirror(E) dm_region_hash(E) dm_log(E) dm_mod(E) [last unloaded: speedstep_lib] CPU: 1 PID: 3907 Comm: run-locktest Tainted: GW OE 3.17.0-pvqlock #3 Hardware name: Red Hat KVM, BIOS Bochs 01/01/2011 task: 8818cc5baf90 ti: 8818b7094000 task.ti: 8818b7094000 RIP: 0010:[] [ (null)] (null) RSP: 0018:8818b7097db0 EFLAGS: 00010246 RAX: RBX: 004c4b40 RCX: RDX: 0001 RSI: RDI: 8818d3f052c0 RBP: 8818b7097dd8 R08: 80522014 R09: R10: 1000 R11: 0001 R12: 0001 R13: R14: 0001 R15: 8818b7097ea0 FS: 7fb828ece700() GS:88193ec2() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: CR3: 0018cc7e9000 CR4: 06e0 Stack: a06ff395 8818d465e000 8164bec0 0001 0050 8818b7097e18 a06ff785 8818b7097e38 0246 54755e3a 39f8ba72 8818c174f000 Call Trace: [a06ff395] ? test_spinlock+0x65/0x90 [locktest] [a06ff785] etime_show+0xd5/0x120 [locktest] [812a2dc6] kobj_attr_show+0x16/0x20 [8121a7fa] sysfs_kf_seq_show+0xca/0x1b0 [81218a13] kernfs_seq_show+0x23/0x30 [811c82db] seq_read+0xbb/0x400 [812197e5] kernfs_fop_read+0x35/0x40 [811a4223] vfs_read+0xa3/0x110 [811a47e6] SyS_read+0x56/0xd0 [810f3e16] ? __audit_syscall_exit+0x216/0x2c0 [815b3ca9] system_call_fastpath+0x16/0x1b Code: Bad RIP value. RSP 8818b7097db0 CR2: ---[ end trace 69d0e259c9ec632f ]--- It seems like call site patching isn't properly done or the kernel module that I built was missing some critical information necessary for the proper Did the readelf give you the paravirt note section? linking. Anyway, I will include the unlock call patching code as a separate patch as it seems there may be problem under certain circumstance. one way to troubleshoot those is to enable the paravirt patching code to actually print where it is patching the code. That way when you load the module you can confirm it has done its job. Then you can verify that the address where the code is called: a06ff395 is indeed patched. You might as well also do a hexdump in the module loading to confim that the patching had been done correctly. BTW, the kernel panic problem that your team reported had been fixed. The fix will be in the next version of the patch. -Longman ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v13 09/11] pvqspinlock, x86: Add para-virtualization support
On Wed, Oct 29, 2014 at 04:19:09PM -0400, Waiman Long wrote:
This patch adds para-virtualization support to the queue spinlock
code base with minimal impact to the native case. There are some
minor code changes in the generic qspinlock.c file which should be
usable in other architectures. The other code changes are specific
to x86 processors and so are all put under the arch/x86 directory.
On the lock side, the slowpath code is split into 2 separate functions
generated from the same code - one for bare metal and one for PV guest.
The switching is done in the _raw_spin_lock* functions. This makes
sure that the performance impact to the bare metal case is minimal,
just a few NOPs in the _raw_spin_lock* functions. In the PV slowpath
code, there are 2 paravirt callee saved calls that minimize register
pressure.
On the unlock side, however, the disabling of unlock function inlining
does have some slight impact on bare metal performance.
The actual paravirt code comes in 5 parts;
- init_node; this initializes the extra data members required for PV
state. PV state data is kept 1 cacheline ahead of the regular data.
- link_and_wait_node; this replaces the regular MCS queuing code. CPU
halting can happen if the wait is too long.
- wait_head; this waits until the lock is avialable and the CPU will
be halted if the wait is too long.
- wait_check; this is called after acquiring the lock to see if the
next queue head CPU is halted. If this is the case, the lock bit is
changed to indicate the queue head will have to be kicked on unlock.
- queue_unlock; this routine has a jump label to check if paravirt
is enabled. If yes, it has to do an atomic cmpxchg to clear the lock
bit or call the slowpath function to kick the queue head cpu.
Tracking the head is done in two parts, firstly the pv_wait_head will
store its cpu number in whichever node is pointed to by the tail part
of the lock word. Secondly, pv_link_and_wait_node() will propagate the
existing head from the old to the new tail node.
Signed-off-by: Waiman Long waiman.l...@hp.com
---
arch/x86/include/asm/paravirt.h | 19 ++
arch/x86/include/asm/paravirt_types.h | 20 ++
arch/x86/include/asm/pvqspinlock.h| 411
+
arch/x86/include/asm/qspinlock.h | 71 ++-
arch/x86/kernel/paravirt-spinlocks.c |6 +
include/asm-generic/qspinlock.h |2 +
kernel/locking/qspinlock.c| 69 +-
7 files changed, 591 insertions(+), 7 deletions(-)
create mode 100644 arch/x86/include/asm/pvqspinlock.h
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index cd6e161..7e296e6 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -712,6 +712,24 @@ static inline void __set_fixmap(unsigned /* enum
fixed_addresses */ idx,
#if defined(CONFIG_SMP) defined(CONFIG_PARAVIRT_SPINLOCKS)
+#ifdef CONFIG_QUEUE_SPINLOCK
+
+static __always_inline void pv_kick_cpu(int cpu)
+{
+ PVOP_VCALLEE1(pv_lock_ops.kick_cpu, cpu);
+}
+
+static __always_inline void pv_lockwait(u8 *lockbyte)
+{
+ PVOP_VCALLEE1(pv_lock_ops.lockwait, lockbyte);
+}
+
+static __always_inline void pv_lockstat(enum pv_lock_stats type)
+{
+ PVOP_VCALLEE1(pv_lock_ops.lockstat, type);
+}
+
+#else
static __always_inline void __ticket_lock_spinning(struct arch_spinlock
*lock,
__ticket_t ticket)
{
@@ -723,6 +741,7 @@ static __always_inline void __ticket_unlock_kick(struct
arch_spinlock *lock,
{
PVOP_VCALL2(pv_lock_ops.unlock_kick, lock, ticket);
}
+#endif
#endif
diff --git a/arch/x86/include/asm/paravirt_types.h
b/arch/x86/include/asm/paravirt_types.h
index 7549b8b..49e4b76 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -326,6 +326,9 @@ struct pv_mmu_ops {
phys_addr_t phys, pgprot_t flags);
};
+struct mcs_spinlock;
+struct qspinlock;
+
struct arch_spinlock;
#ifdef CONFIG_SMP
#include asm/spinlock_types.h
@@ -333,9 +336,26 @@ struct arch_spinlock;
typedef u16 __ticket_t;
#endif
+#ifdef CONFIG_QUEUE_SPINLOCK
+enum pv_lock_stats {
+ PV_HALT_QHEAD, /* Queue head halting */
+ PV_HALT_QNODE, /* Other queue node halting */
+ PV_HALT_ABORT, /* Halting aborted */
+ PV_WAKE_KICKED, /* Wakeup by kicking*/
+ PV_WAKE_SPURIOUS, /* Spurious wakeup */
+ PV_KICK_NOHALT /* Kick but CPU not halted */
+};
+#endif
+
struct pv_lock_ops {
+#ifdef CONFIG_QUEUE_SPINLOCK
+ struct paravirt_callee_save kick_cpu;
+ struct paravirt_callee_save lockstat;
+ struct paravirt_callee_save lockwait;
+#else
struct paravirt_callee_save lock_spinning;
void (*unlock_kick)(struct
Re: [PATCH RFC v4 net-next 1/5] virtio_net: enable tx interrupt
On Mon, Dec 1, 2014 at 6:35 PM, Michael S. Tsirkin m...@redhat.com
wrote:
On Mon, Dec 01, 2014 at 06:17:04PM +0800, Jason Wang wrote:
On newer hosts that support delayed tx interrupts,
we probably don't have much to gain from orphaning
packets early.
Note: this might degrade performance for
hosts without event idx support.
Should be addressed by the next patch.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Jason Wang jasow...@redhat.com
Could you document the changes from the RFC I sent please?
Okay, I will sent a V5 add document more.
Are there optimizations?
Two optimizations.
- Don't do tx packets free in ndo_start_xmit(), tests shows it reduce
the effect of coalescing.
- Let ethtool can change the number of packets freed in one tx napi run
through tx-frames-irq. This is necessary, since user may coalesce more
than 64 packets per irq.
If yes, it might be easier to review (at least for me), if you
refactor this,
e.g. applying the straight-forward rfc patch and then optimizations if
any on top. If it's taking a different approach, pls feel free to
disregard this.
---
drivers/net/virtio_net.c | 132
+++
1 file changed, 88 insertions(+), 44 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..f68114e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -72,6 +72,8 @@ struct send_queue {
/* Name of the send queue: output.$index */
char name[40];
+
+ struct napi_struct napi;
};
/* Internal representation of a receive virtqueue */
@@ -137,6 +139,9 @@ struct virtnet_info {
/* CPU hot plug notifier */
struct notifier_block nb;
+
+ /* Budget for polling tx completion */
+ u32 tx_work_limit;
};
struct skb_vnet_hdr {
@@ -211,15 +216,41 @@ static struct page *get_a_page(struct
receive_queue *rq, gfp_t gfp_mask)
return p;
}
+static unsigned int free_old_xmit_skbs(struct netdev_queue *txq,
+ struct send_queue *sq, int budget)
+{
+ struct sk_buff *skb;
+ unsigned int len;
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct virtnet_stats *stats = this_cpu_ptr(vi-stats);
+ unsigned int packets = 0;
+
+ while (packets budget
+ (skb = virtqueue_get_buf(sq-vq, len)) != NULL) {
+ pr_debug(Sent skb %p\n, skb);
+
+ u64_stats_update_begin(stats-tx_syncp);
+ stats-tx_bytes += skb-len;
+ stats-tx_packets++;
+ u64_stats_update_end(stats-tx_syncp);
+
+ dev_kfree_skb_any(skb);
+ packets++;
+ }
+
+ if (sq-vq-num_free = 2+MAX_SKB_FRAGS)
+ netif_tx_start_queue(txq);
+
+ return packets;
+}
+
static void skb_xmit_done(struct virtqueue *vq)
{
struct virtnet_info *vi = vq-vdev-priv;
+ struct send_queue *sq = vi-sq[vq2txq(vq)];
- /* Suppress further interrupts. */
- virtqueue_disable_cb(vq);
-
- /* We were probably waiting for more output buffers. */
- netif_wake_subqueue(vi-dev, vq2txq(vq));
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
}
static unsigned int mergeable_ctx_to_buf_truesize(unsigned long
mrg_ctx)
@@ -777,6 +808,32 @@ again:
return received;
}
+static int virtnet_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct send_queue *sq =
+ container_of(napi, struct send_queue, napi);
+ struct virtnet_info *vi = sq-vq-vdev-priv;
+ struct netdev_queue *txq = netdev_get_tx_queue(vi-dev,
vq2txq(sq-vq));
+ u32 limit = vi-tx_work_limit;
+ unsigned int sent;
+
+ __netif_tx_lock(txq, smp_processor_id());
+ sent = free_old_xmit_skbs(txq, sq, limit);
+ if (sent limit) {
+ napi_complete(napi);
+ /* Note: we must enable cb *after* napi_complete, because
+ * napi_schedule calls from callbacks that trigger before
+ * napi_complete are ignored.
+ */
+ if (unlikely(!virtqueue_enable_cb_delayed(sq-vq))) {
+ virtqueue_disable_cb(sq-vq);
+ napi_schedule(sq-napi);
+ }
+ }
+ __netif_tx_unlock(txq);
+ return sent limit ? 0 : budget;
+}
+
Unlike the patch I sent, this seems to ignore the budget,
and always poll the full napi_weight.
Seems strange. What is the reason for this?
The budget were in fact the tx_work_limit (by default 64).
This could be tuned by ethtool tx-frames-irq to control
how many packets at most could be processed by on tx napi.
Use may want to coalesce more than 64 packets per irq, so
something like this is necessary for user.
#ifdef
Re: [PATCH RFC v4 net-next 0/5] virtio_net: enabling tx interrupts
On Mon, Dec 1, 2014 at 6:42 PM, Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 01, 2014 at 06:17:03PM +0800, Jason Wang wrote: Hello: We used to orphan packets before transmission for virtio-net. This breaks socket accounting and can lead serveral functions won't work, e.g: - Byte Queue Limit depends on tx completion nofication to work. - Packet Generator depends on tx completion nofication for the last transmitted packet to complete. - TCP Small Queue depends on proper accounting of sk_wmem_alloc to work. This series tries to solve the issue by enabling tx interrupts. To minize the performance impacts of this, several optimizations were used: - In guest side, virtqueue_enable_cb_delayed() was used to delay the tx interrupt untile 3/4 pending packets were sent. - In host side, interrupt coalescing were used to reduce tx interrupts. Performance test results[1] (tx-frames 16 tx-usecs 16) shows: - For guest receiving. No obvious regression on throughput were noticed. More cpu utilization were noticed in few cases. - For guest transmission. Very huge improvement on througput for small packet transmission were noticed. This is expected since TSQ and other optimization for small packet transmission work after tx interrupt. But will use more cpu for large packets. - For TCP_RR, regression (10% on transaction rate and cpu utilization) were found. Tx interrupt won't help but cause overhead in this case. Using more aggressive coalescing parameters may help to reduce the regression. OK, you do have posted coalescing patches - does it help any? Helps a lot. For RX, it saves about 5% - 10% cpu. (reduce 60%-90% tx intrs) For small packet TX, it increases 33% - 245% throughput. (reduce about 60% inters) For TCP_RR, it increase the 3%-10% trans.rate. (reduce 40%-80% tx intrs) I'm not sure the regression is due to interrupts. It would make sense for CPU but why would it hurt transaction rate? Anyway guest need to take some cycles to handle tx interrupts. And transaction rate does increase if we coalesces more tx interurpts. It's possible that we are deferring kicks too much due to BQL. As an experiment: do we get any of it back if we do -if (kick || netif_xmit_stopped(txq)) -virtqueue_kick(sq-vq); +virtqueue_kick(sq-vq); ? I will try, but during TCP_RR, at most 1 packets were pending, I suspect if BQL can help in this case. If yes, we can just kick e.g. periodically, e.g. after queueing each X bytes. Okay, let me try to see if this help. Changes from RFC V3: - Don't free tx packets in ndo_start_xmit() - Add interrupt coalescing support for virtio-net Changes from RFC v2: - clean up code, address issues raised by Jason Changes from RFC v1: - address comments by Jason Wang, use delayed cb everywhere - rebased Jason's patch on top of mine and include it (with some tweaks) Please reivew. Comments were more than welcomed. [1] Performance Test result: Environment: - Two Intel(R) Xeon(R) CPU E5620 @ 2.40GHz machines connected back to back with 82599ES cards. - Both host and guest were net-next.git plus the patch - Coalescing parameters for the card: Adaptive RX: off TX: off rx-usecs: 1 rx-frames: 0 tx-usecs: 0 tx-frames: 0 - Vhost_net was enabled and zerocopy was disabled - Tests was done by netperf-2.6 - Guest has 2 vcpus with single queue virtio-net Results: - Numbers of square brackets are whose significance is grater than 95% Guest RX: size/sessions/+throughput/+cpu/+per_cpu_throughput/ 64/1/+2.0326/[+6.2807%]/-3.9970%/ 64/2/-0.2104%/[+3.2012%]/[-3.3058%]/ 64/4/+1.5956%/+2.2451%/-0.6353%/ 64/8/+1.1732%/+3.5123%/-2.2598%/ 256/1/+3.7619%/[+5.8117%]/-1.9372%/ 256/2/-0.0661%/[+3.2511%]/-3.2127%/ 256/4/+1.1435%/[-8.1842%]/[+10.1591%]/ 256/8/[+2.2447%]/[+6.2044%]/[-3.7283%]/ 1024/1/+9.1479%/[+12.0997%]/[-2.6332%]/ 1024/2/[-17.3341%]/[+0.%]/[-17.3341%]/ 1024/4/[-0.6284%]/-1.0376%/+0.4135%/ 1024/8/+1.1444%/-1.6069%/+2.7961%/ 4096/1/+0.0401%/-0.5993%/+0.6433%/ 4096/2/[-0.5894%]/-2.2071%/+1.6542%/ 4096/4/[-0.5560%]/-1.4969%/+0.9553%/ 4096/8/-0.3362%/+2.7086%/-2.9645%/ 16384/1/-0.0285%/+0.7247%/-0.7478%/ 16384/2/-0.5286%/+0.3287%/-0.8545%/ 16384/4/-0.3297%/-2.0543%/+1.7608%/ 16384/8/+1.0932%/+4.0253%/-2.8187%/ 65535/1/+0.0003%/-0.1502%/+0.1508%/ 65535/2/[-0.6065%]/+0.2309%/-0.8355%/ 65535/4/[-0.6861%]/[+3.9451%]/[-4.4554%]/ 65535/8/+1.8359%/+3.1590%/-1.2825%/ Guest RX: size/sessions/+throughput/+cpu/+per_cpu_throughput/ 64/1/[+65.0961%]/[-8.6807%]/[+80.7900%]/ 64/2/[+6.0288%]/[-2.2823%]/[+8.5052%]/ 64/4/[+5.9038%]/[-2.1834%]/[+8.2677%]/ 64/8/[+5.4154%]/[-2.1804%]/[+7.7651%]/ 256/1/[+184.6462%]/[+4.8906%]/[+171.3742%]/ 256/2/[+46.0731%]/[-8.9626%]/[+60.4539%]/ 256/4/[+45.8547%]/[-8.3027%]/[+59.0612%]/ 256/8/[+45.3486%]/[-8.4024%]/[+58.6817%]/ 1024/1/[+432.5372%]/[+3.9566%]/[+412.2689%]/
