I run latest sparse from git on virtio drivers (turns out the version I had was rather outdated). This patchset fixes a couple of bugs this uncovered, and adds some annotations to make it sparse-clean. In particular, endian-ness is often tricky, so this patchset enabled endian-ness checks for sparse builds. Michael S. Tsirkin (10): virtio_console: drop unused config fields drm/virtio: fix endianness in primary_plane_update drm/virtio: fix lock context imbalance drm/virtio: annotate virtio_gpu_queue_ctrl_buffer_locked vhost: make interval tree static inline vhost: add missing __user annotations vsock/virtio: add a missing __le annotation vsock/virtio: mark an internal function static vsock/virtio: fix src/dst cid format virtio: enable endian checks for sparse builds drivers/char/virtio_console.c | 14 +++++++------- drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 6 +++++- drivers/vhost/vhost.c | 12 ++++++------ net/vmw_vsock/virtio_transport.c | 2 +- net/vmw_vsock/virtio_transport_common.c | 16 ++++++++-------- drivers/block/Makefile | 1 + drivers/char/Makefile | 1 + drivers/char/hw_random/Makefile | 2 ++ drivers/gpu/drm/virtio/Makefile | 1 + drivers/net/Makefile | 3 +++ drivers/net/caif/Makefile | 1 + drivers/rpmsg/Makefile | 1 + drivers/s390/virtio/Makefile | 2 ++ drivers/scsi/Makefile | 1 + drivers/vhost/Makefile | 1 + drivers/virtio/Makefile | 3 +++ net/9p/Makefile | 1 + net/packet/Makefile | 1 + net/vmw_vsock/Makefile | 2 ++ 20 files changed, 50 insertions(+), 25 deletions(-) -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 01/10] virtio_console: drop unused config fields
struct ports_device includes a config field including the whole virtio_console_config, but only max_nr_ports in there is ever updated or used. The rest is unused and in fact does not even mirror the device config. Drop everything except max_nr_ports, saving some memory. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/char/virtio_console.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 5649234..8b00e79 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -152,8 +152,8 @@ struct ports_device { spinlock_t c_ivq_lock; spinlock_t c_ovq_lock; - /* The current config space is stored here */ - struct virtio_console_config config; + /* max. number of ports this device can hold */ + u32 max_nr_ports; /* The virtio device we're associated with */ struct virtio_device *vdev; @@ -1649,11 +1649,11 @@ static void handle_control_message(struct virtio_device *vdev, break; } if (virtio32_to_cpu(vdev, cpkt->id) >- portdev->config.max_nr_ports) { + portdev->max_nr_ports) { dev_warn(&portdev->vdev->dev, "Request for adding port with " "out-of-bound id %u, max. supported id: %u\n", - cpkt->id, portdev->config.max_nr_ports - 1); + cpkt->id, portdev->max_nr_ports - 1); break; } add_port(portdev, virtio32_to_cpu(vdev, cpkt->id)); @@ -1894,7 +1894,7 @@ static int init_vqs(struct ports_device *portdev) u32 i, j, nr_ports, nr_queues; int err; - nr_ports = portdev->config.max_nr_ports; + nr_ports = portdev->max_nr_ports; nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL); @@ -2047,13 +2047,13 @@ static int virtcons_probe(struct virtio_device *vdev) } multiport = false; - portdev->config.max_nr_ports = 1; + portdev->max_nr_ports = 1; /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */ if (!is_rproc_serial(vdev) && virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT, struct virtio_console_config, max_nr_ports, - &portdev->config.max_nr_ports) == 0) { + &portdev->max_nr_ports) == 0) { multiport = true; } -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 02/10] drm/virtio: fix endianness in primary_plane_update
virtio_gpu_cmd_transfer_to_host_2d expects x and y parameters in LE, but virtio_gpu_primary_plane_update passes in the CPU format instead. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index ba28c0f..1fda965 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -88,8 +88,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, (vgdev, handle, 0, cpu_to_le32(plane->state->src_w >> 16), cpu_to_le32(plane->state->src_h >> 16), - plane->state->src_x >> 16, - plane->state->src_y >> 16, NULL); + cpu_to_le32(plane->state->src_x >> 16), + cpu_to_le32(plane->state->src_y >> 16), NULL); } } else { handle = 0; -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 03/10] drm/virtio: fix lock context imbalance
When virtio_gpu_free_vbufs exits due to list empty, it does not drop the free_vbufs lock that it took. list empty is not expected to happen anyway, but it can't hurt to fix this and drop the lock. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_vq.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 5a0f8a7..2f0c2f9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -109,8 +109,10 @@ void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev) spin_lock(&vgdev->free_vbufs_lock); for (i = 0; i < count; i++) { - if (WARN_ON(list_empty(&vgdev->free_vbufs))) + if (WARN_ON(list_empty(&vgdev->free_vbufs))) { + spin_unlock(&vgdev->free_vbufs_lock); return; + } vbuf = list_first_entry(&vgdev->free_vbufs, struct virtio_gpu_vbuffer, list); list_del(&vbuf->list); -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 04/10] drm/virtio: annotate virtio_gpu_queue_ctrl_buffer_locked
virtio_gpu_queue_ctrl_buffer_locked is called with ctrlq.qlock taken, it releases and acquires this lock. This causes a sparse warning. Add appropriate annotations for sparse context checking. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_vq.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 2f0c2f9..a6e2ce4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -297,6 +297,8 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work) static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev, struct virtio_gpu_vbuffer *vbuf) + __releases(&vgdev->ctrlq.qlock) + __acquires(&vgdev->ctrlq.qlock) { struct virtqueue *vq = vgdev->ctrlq.vq; struct scatterlist *sgs[3], vcmd, vout, vresp; -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 05/10] vhost: make interval tree static inline
vhost_umem_interval_tree is only used locally within vhost.c, mark it static. As some functions generated go unused, this triggers warnings unless we also mark it inline. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/vhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index c6f2d89..7331ef3 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -49,7 +49,7 @@ enum { INTERVAL_TREE_DEFINE(struct vhost_umem_node, rb, __u64, __subtree_last, - START, LAST, , vhost_umem_interval_tree); + START, LAST, static inline, vhost_umem_interval_tree); #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY static void vhost_disable_cross_endian(struct vhost_virtqueue *vq) -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 06/10] vhost: add missing __user annotations
Several vhost functions were missing __user annotations on pointers, causing sparse warnings. Fix this up. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/vhost.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 7331ef3..ba7db68 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -719,7 +719,7 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_umem *umem, static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, struct iovec iov[], int iov_size, int access); -static int vhost_copy_to_user(struct vhost_virtqueue *vq, void *to, +static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to, const void *from, unsigned size) { int ret; @@ -749,7 +749,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void *to, } static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to, - void *from, unsigned size) + void __user *from, unsigned size) { int ret; @@ -783,7 +783,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to, } static void __user *__vhost_get_user(struct vhost_virtqueue *vq, - void *addr, unsigned size) + void __user *addr, unsigned size) { int ret; @@ -934,8 +934,8 @@ static int umem_access_ok(u64 uaddr, u64 size, int access) return 0; } -int vhost_process_iotlb_msg(struct vhost_dev *dev, - struct vhost_iotlb_msg *msg) +static int vhost_process_iotlb_msg(struct vhost_dev *dev, + struct vhost_iotlb_msg *msg) { int ret = 0; -- MST
Michael S. Tsirkin
2016-Dec-06 15:40 UTC
[PATCH 07/10] vsock/virtio: add a missing __le annotation
guest cid is read from config space, therefore it's in little endian format and is treated as such, annotate it accordingly. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- net/vmw_vsock/virtio_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c index 936d7ee..90096b9 100644 --- a/net/vmw_vsock/virtio_transport.c +++ b/net/vmw_vsock/virtio_transport.c @@ -336,7 +336,7 @@ static void virtio_vsock_reset_sock(struct sock *sk) static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock) { struct virtio_device *vdev = vsock->vdev; - u64 guest_cid; + __le64 guest_cid; vdev->config->get(vdev, offsetof(struct virtio_vsock_config, guest_cid), &guest_cid, sizeof(guest_cid)); -- MST
Michael S. Tsirkin
2016-Dec-06 15:41 UTC
[PATCH 08/10] vsock/virtio: mark an internal function static
virtio_transport_alloc_pkt is only used locally, make it static. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- net/vmw_vsock/virtio_transport_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index a53b3a1..6120384 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -32,7 +32,7 @@ static const struct virtio_transport *virtio_transport_get_ops(void) return container_of(t, struct virtio_transport, transport); } -struct virtio_vsock_pkt * +static struct virtio_vsock_pkt * virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info, size_t len, u32 src_cid, -- MST
Michael S. Tsirkin
2016-Dec-06 15:41 UTC
[PATCH 09/10] vsock/virtio: fix src/dst cid format
These fields are 64 bit, using le32_to_cpu and friends on these will not do the right thing. Fix this up. Cc: stable at vger.kernel.org Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- net/vmw_vsock/virtio_transport_common.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 6120384..22e99c4 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -606,9 +606,9 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) return 0; pkt = virtio_transport_alloc_pkt(&info, 0, - le32_to_cpu(pkt->hdr.dst_cid), + le64_to_cpu(pkt->hdr.dst_cid), le32_to_cpu(pkt->hdr.dst_port), - le32_to_cpu(pkt->hdr.src_cid), + le64_to_cpu(pkt->hdr.src_cid), le32_to_cpu(pkt->hdr.src_port)); if (!pkt) return -ENOMEM; @@ -823,7 +823,7 @@ virtio_transport_send_response(struct vsock_sock *vsk, struct virtio_vsock_pkt_info info = { .op = VIRTIO_VSOCK_OP_RESPONSE, .type = VIRTIO_VSOCK_TYPE_STREAM, - .remote_cid = le32_to_cpu(pkt->hdr.src_cid), + .remote_cid = le64_to_cpu(pkt->hdr.src_cid), .remote_port = le32_to_cpu(pkt->hdr.src_port), .reply = true, }; @@ -863,9 +863,9 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt) child->sk_state = SS_CONNECTED; vchild = vsock_sk(child); - vsock_addr_init(&vchild->local_addr, le32_to_cpu(pkt->hdr.dst_cid), + vsock_addr_init(&vchild->local_addr, le64_to_cpu(pkt->hdr.dst_cid), le32_to_cpu(pkt->hdr.dst_port)); - vsock_addr_init(&vchild->remote_addr, le32_to_cpu(pkt->hdr.src_cid), + vsock_addr_init(&vchild->remote_addr, le64_to_cpu(pkt->hdr.src_cid), le32_to_cpu(pkt->hdr.src_port)); vsock_insert_connected(vchild); @@ -904,9 +904,9 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt) struct sock *sk; bool space_available; - vsock_addr_init(&src, le32_to_cpu(pkt->hdr.src_cid), + vsock_addr_init(&src, le64_to_cpu(pkt->hdr.src_cid), le32_to_cpu(pkt->hdr.src_port)); - vsock_addr_init(&dst, le32_to_cpu(pkt->hdr.dst_cid), + vsock_addr_init(&dst, le64_to_cpu(pkt->hdr.dst_cid), le32_to_cpu(pkt->hdr.dst_port)); trace_virtio_transport_recv_pkt(src.svm_cid, src.svm_port, -- MST
Michael S. Tsirkin
2016-Dec-06 15:41 UTC
[PATCH 10/10] virtio: enable endian checks for sparse builds
__CHECK_ENDIAN__ isn't on by default presumably because it triggers too many sparse warnings for correct code. But virtio is now clean of these warnings, and we want to keep it this way - enable this for sparse builds. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- It seems that there should be a better way to do it, but this works too. drivers/block/Makefile | 1 + drivers/char/Makefile | 1 + drivers/char/hw_random/Makefile | 2 ++ drivers/gpu/drm/virtio/Makefile | 1 + drivers/net/Makefile | 3 +++ drivers/net/caif/Makefile | 1 + drivers/rpmsg/Makefile | 1 + drivers/s390/virtio/Makefile | 2 ++ drivers/scsi/Makefile | 1 + drivers/vhost/Makefile | 1 + drivers/virtio/Makefile | 3 +++ net/9p/Makefile | 1 + net/packet/Makefile | 1 + net/vmw_vsock/Makefile | 2 ++ 14 files changed, 21 insertions(+) diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 1e9661e..597481c 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_BLK_DEV_OSD) += osdblk.o obj-$(CONFIG_BLK_DEV_UMEM) += umem.o obj-$(CONFIG_BLK_DEV_NBD) += nbd.o obj-$(CONFIG_BLK_DEV_CRYPTOLOOP) += cryptoloop.o +CFLAGS_virtio_blk.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o obj-$(CONFIG_BLK_DEV_SX8) += sx8.o diff --git a/drivers/char/Makefile b/drivers/char/Makefile index 6e6c244..a99467d 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -6,6 +6,7 @@ obj-y += mem.o random.o obj-$(CONFIG_TTY_PRINTK) += ttyprintk.o obj-y += misc.o obj-$(CONFIG_ATARI_DSP56K) += dsp56k.o +CFLAGS_virtio_console.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_CONSOLE) += virtio_console.o obj-$(CONFIG_RAW_DRIVER) += raw.o obj-$(CONFIG_SGI_SNSC) += snsc.o snsc_event.o diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile index 5f52b1e..a2b0931 100644 --- a/drivers/char/hw_random/Makefile +++ b/drivers/char/hw_random/Makefile @@ -17,6 +17,8 @@ obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o +CFLAGS_virtio_transport.o += -D__CHECK_ENDIAN__ +CFLAGS_virtio-rng.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_HW_RANDOM_VIRTIO) += virtio-rng.o obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile index 3fb8eac..1162366 100644 --- a/drivers/gpu/drm/virtio/Makefile +++ b/drivers/gpu/drm/virtio/Makefile @@ -3,6 +3,7 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. ccflags-y := -Iinclude/drm +ccflags-y += -D__CHECK_ENDIAN__ virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_drm_bus.o virtgpu_gem.o \ virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 7336cbd..3f587de 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_EQUALIZER) += eql.o obj-$(CONFIG_IFB) += ifb.o obj-$(CONFIG_MACSEC) += macsec.o obj-$(CONFIG_MACVLAN) += macvlan.o +CFLAGS_macvtap.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_MACVTAP) += macvtap.o obj-$(CONFIG_MII) += mii.o obj-$(CONFIG_MDIO) += mdio.o @@ -20,8 +21,10 @@ obj-$(CONFIG_NETCONSOLE) += netconsole.o obj-$(CONFIG_PHYLIB) += phy/ obj-$(CONFIG_RIONET) += rionet.o obj-$(CONFIG_NET_TEAM) += team/ +CFLAGS_tun.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_TUN) += tun.o obj-$(CONFIG_VETH) += veth.o +CFLAGS_virtio_net.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_NET) += virtio_net.o obj-$(CONFIG_VXLAN) += vxlan.o obj-$(CONFIG_GENEVE) += geneve.o diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile index 9bbd453..d1a922c 100644 --- a/drivers/net/caif/Makefile +++ b/drivers/net/caif/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_CAIF_HSI) += caif_hsi.o # Virtio interface obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o +CFLAGS_caif_virtio.o += -D__CHECK_ENDIAN__ diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile index ae9c913..23c8b66 100644 --- a/drivers/rpmsg/Makefile +++ b/drivers/rpmsg/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_RPMSG) += rpmsg_core.o obj-$(CONFIG_RPMSG_QCOM_SMD) += qcom_smd.o obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o +CFLAGS_virtio_rpmsg_bus.o += -D__CHECK_ENDIAN__ diff --git a/drivers/s390/virtio/Makefile b/drivers/s390/virtio/Makefile index df40692..270ada5 100644 --- a/drivers/s390/virtio/Makefile +++ b/drivers/s390/virtio/Makefile @@ -6,6 +6,8 @@ # it under the terms of the GNU General Public License (version 2 only) # as published by the Free Software Foundation. +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__ +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__ s390-virtio-objs := virtio_ccw.o ifdef CONFIG_S390_GUEST_OLD_TRANSPORT s390-virtio-objs += kvm_virtio.o diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 38d938d..9f70d46 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -135,6 +135,7 @@ obj-$(CONFIG_SCSI_BNX2_ISCSI) += libiscsi.o bnx2i/ obj-$(CONFIG_BE2ISCSI) += libiscsi.o be2iscsi/ obj-$(CONFIG_SCSI_ESAS2R) += esas2r/ obj-$(CONFIG_SCSI_PMCRAID) += pmcraid.o +CFLAGS_virtio_scsi.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_SCSI_VIRTIO) += virtio_scsi.o obj-$(CONFIG_VMWARE_PVSCSI) += vmw_pvscsi.o obj-$(CONFIG_XEN_SCSI_FRONTEND) += xen-scsifront.o diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile index 6b012b9..619e2cd 100644 --- a/drivers/vhost/Makefile +++ b/drivers/vhost/Makefile @@ -1,3 +1,4 @@ +ccflags-y := -D__CHECK_ENDIAN__ obj-$(CONFIG_VHOST_NET) += vhost_net.o vhost_net-y := net.o diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index 41e30e3..d331f19 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -1,3 +1,6 @@ +#virtio must be kept clean wrt endian tags, +#otherwise we'll get to maintain broken host/guest ABIs +ccflags-y := -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o diff --git a/net/9p/Makefile b/net/9p/Makefile index a0874cc..acf1225 100644 --- a/net/9p/Makefile +++ b/net/9p/Makefile @@ -11,6 +11,7 @@ obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o trans_fd.o \ trans_common.o \ +CFLAGS_trans_virtio.o += -D__CHECK_ENDIAN__ 9pnet_virtio-objs := \ trans_virtio.o \ diff --git a/net/packet/Makefile b/net/packet/Makefile index 9df6134..a13bcb3 100644 --- a/net/packet/Makefile +++ b/net/packet/Makefile @@ -2,6 +2,7 @@ # Makefile for the packet AF. # +ccflags-y := -D__CHECK_ENDIAN__ obj-$(CONFIG_PACKET) += af_packet.o obj-$(CONFIG_PACKET_DIAG) += af_packet_diag.o af_packet_diag-y += diag.o diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile index bc27c70..a61eccb 100644 --- a/net/vmw_vsock/Makefile +++ b/net/vmw_vsock/Makefile @@ -8,6 +8,8 @@ vsock-y += af_vsock.o vsock_addr.o vmw_vsock_vmci_transport-y += vmci_transport.o vmci_transport_notify.o \ vmci_transport_notify_qstate.o +CFLAGS_virtio_transport.o += -D__CHECK_ENDIAN__ +CFLAGS_virtio_transport_common.o += -D__CHECK_ENDIAN__ vmw_vsock_virtio_transport-y += virtio_transport.o vmw_vsock_virtio_transport_common-y += virtio_transport_common.o -- MST
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> struct ports_device includes a config field including the whole > virtio_console_config, but only max_nr_ports in there is ever updated or > used. The rest is unused and in fact does not even mirror the > device config. Drop everything except max_nr_ports, > saving some memory. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/char/virtio_console.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-)Reviewed-by: Jason Wang <jasowang at redhat.com>> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 5649234..8b00e79 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -152,8 +152,8 @@ struct ports_device { > spinlock_t c_ivq_lock; > spinlock_t c_ovq_lock; > > - /* The current config space is stored here */ > - struct virtio_console_config config; > + /* max. number of ports this device can hold */ > + u32 max_nr_ports; > > /* The virtio device we're associated with */ > struct virtio_device *vdev; > @@ -1649,11 +1649,11 @@ static void handle_control_message(struct virtio_device *vdev, > break; > } > if (virtio32_to_cpu(vdev, cpkt->id) >> - portdev->config.max_nr_ports) { > + portdev->max_nr_ports) { > dev_warn(&portdev->vdev->dev, > "Request for adding port with " > "out-of-bound id %u, max. supported id: %u\n", > - cpkt->id, portdev->config.max_nr_ports - 1); > + cpkt->id, portdev->max_nr_ports - 1); > break; > } > add_port(portdev, virtio32_to_cpu(vdev, cpkt->id)); > @@ -1894,7 +1894,7 @@ static int init_vqs(struct ports_device *portdev) > u32 i, j, nr_ports, nr_queues; > int err; > > - nr_ports = portdev->config.max_nr_ports; > + nr_ports = portdev->max_nr_ports; > nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; > > vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL); > @@ -2047,13 +2047,13 @@ static int virtcons_probe(struct virtio_device *vdev) > } > > multiport = false; > - portdev->config.max_nr_ports = 1; > + portdev->max_nr_ports = 1; > > /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */ > if (!is_rproc_serial(vdev) && > virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT, > struct virtio_console_config, max_nr_ports, > - &portdev->config.max_nr_ports) == 0) { > + &portdev->max_nr_ports) == 0) { > multiport = true; > } >
Jason Wang
2016-Dec-07 04:13 UTC
[PATCH 02/10] drm/virtio: fix endianness in primary_plane_update
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> virtio_gpu_cmd_transfer_to_host_2d expects x and y > parameters in LE, but virtio_gpu_primary_plane_update > passes in the CPU format instead. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-)Reviewed-by: Jason Wang <jasowang at redhat.com>> > diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c > index ba28c0f..1fda965 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_plane.c > +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c > @@ -88,8 +88,8 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, > (vgdev, handle, 0, > cpu_to_le32(plane->state->src_w >> 16), > cpu_to_le32(plane->state->src_h >> 16), > - plane->state->src_x >> 16, > - plane->state->src_y >> 16, NULL); > + cpu_to_le32(plane->state->src_x >> 16), > + cpu_to_le32(plane->state->src_y >> 16), NULL); > } > } else { > handle = 0;
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> When virtio_gpu_free_vbufs exits due to list empty, it does not > drop the free_vbufs lock that it took. > list empty is not expected to happen anyway, but it can't hurt to fix > this and drop the lock. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_vq.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-)Reviewed-by: Jason Wang <jasowang at redhat.com>> > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 5a0f8a7..2f0c2f9 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -109,8 +109,10 @@ void virtio_gpu_free_vbufs(struct virtio_gpu_device *vgdev) > > spin_lock(&vgdev->free_vbufs_lock); > for (i = 0; i < count; i++) { > - if (WARN_ON(list_empty(&vgdev->free_vbufs))) > + if (WARN_ON(list_empty(&vgdev->free_vbufs))) { > + spin_unlock(&vgdev->free_vbufs_lock); > return; > + } > vbuf = list_first_entry(&vgdev->free_vbufs, > struct virtio_gpu_vbuffer, list); > list_del(&vbuf->list);
Jason Wang
2016-Dec-07 04:15 UTC
[PATCH 04/10] drm/virtio: annotate virtio_gpu_queue_ctrl_buffer_locked
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> virtio_gpu_queue_ctrl_buffer_locked is called with ctrlq.qlock taken, it > releases and acquires this lock. This causes a sparse warning. Add > appropriate annotations for sparse context checking. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_vq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 2f0c2f9..a6e2ce4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -297,6 +297,8 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work) > > static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev, > struct virtio_gpu_vbuffer *vbuf) > + __releases(&vgdev->ctrlq.qlock) > + __acquires(&vgdev->ctrlq.qlock) > { > struct virtqueue *vq = vgdev->ctrlq.vq; > struct scatterlist *sgs[3], vcmd, vout, vresp;Reviewed-by: Jason Wang <jasowang at redhat.com>
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> vhost_umem_interval_tree is only used locally within vhost.c, mark it > static. As some functions generated go unused, this triggers warnings > unless we also mark it inline. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/vhost/vhost.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index c6f2d89..7331ef3 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -49,7 +49,7 @@ enum { > > INTERVAL_TREE_DEFINE(struct vhost_umem_node, > rb, __u64, __subtree_last, > - START, LAST, , vhost_umem_interval_tree); > + START, LAST, static inline, vhost_umem_interval_tree); > > #ifdef CONFIG_VHOST_CROSS_ENDIAN_LEGACY > static void vhost_disable_cross_endian(struct vhost_virtqueue *vq)Reviewed-by: Jason Wang <jasowang at redhat.com>
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> Several vhost functions were missing __user annotations > on pointers, causing sparse warnings. Fix this up. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > drivers/vhost/vhost.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 7331ef3..ba7db68 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -719,7 +719,7 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_umem *umem, > static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, > struct iovec iov[], int iov_size, int access); > > -static int vhost_copy_to_user(struct vhost_virtqueue *vq, void *to, > +static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to, > const void *from, unsigned size) > { > int ret; > @@ -749,7 +749,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void *to, > } > > static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to, > - void *from, unsigned size) > + void __user *from, unsigned size) > { > int ret; > > @@ -783,7 +783,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to, > } > > static void __user *__vhost_get_user(struct vhost_virtqueue *vq, > - void *addr, unsigned size) > + void __user *addr, unsigned size) > { > int ret; > > @@ -934,8 +934,8 @@ static int umem_access_ok(u64 uaddr, u64 size, int access) > return 0; > } > > -int vhost_process_iotlb_msg(struct vhost_dev *dev, > - struct vhost_iotlb_msg *msg) > +static int vhost_process_iotlb_msg(struct vhost_dev *dev, > + struct vhost_iotlb_msg *msg) > { > int ret = 0; >Patch looks good but this looks like another static conversion not __user annotations.
Jason Wang
2016-Dec-07 04:19 UTC
[PATCH 07/10] vsock/virtio: add a missing __le annotation
On 2016?12?06? 23:40, Michael S. Tsirkin wrote:> guest cid is read from config space, therefore it's in little endian > format and is treated as such, annotate it accordingly. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c > index 936d7ee..90096b9 100644 > --- a/net/vmw_vsock/virtio_transport.c > +++ b/net/vmw_vsock/virtio_transport.c > @@ -336,7 +336,7 @@ static void virtio_vsock_reset_sock(struct sock *sk) > static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock) > { > struct virtio_device *vdev = vsock->vdev; > - u64 guest_cid; > + __le64 guest_cid; > > vdev->config->get(vdev, offsetof(struct virtio_vsock_config, guest_cid), > &guest_cid, sizeof(guest_cid));Reviewed-by: Jason Wang <jasowang at redhat.com>
Jason Wang
2016-Dec-07 04:21 UTC
[PATCH 08/10] vsock/virtio: mark an internal function static
On 2016?12?06? 23:41, Michael S. Tsirkin wrote:> virtio_transport_alloc_pkt is only used locally, make it static. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index a53b3a1..6120384 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -32,7 +32,7 @@ static const struct virtio_transport *virtio_transport_get_ops(void) > return container_of(t, struct virtio_transport, transport); > } > > -struct virtio_vsock_pkt * > +static struct virtio_vsock_pkt * > virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info, > size_t len, > u32 src_cid,Git grep shows it was used by tracing.
On 2016?12?06? 23:41, Michael S. Tsirkin wrote:> These fields are 64 bit, using le32_to_cpu and friends > on these will not do the right thing. > Fix this up. > > Cc: stable at vger.kernel.org > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport_common.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 6120384..22e99c4 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -606,9 +606,9 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > return 0; > > pkt = virtio_transport_alloc_pkt(&info, 0, > - le32_to_cpu(pkt->hdr.dst_cid), > + le64_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port), > - le32_to_cpu(pkt->hdr.src_cid), > + le64_to_cpu(pkt->hdr.src_cid), > le32_to_cpu(pkt->hdr.src_port));Looking at sockaddr_vm, svm_cid is "unsigned int", do we really want 64 bit here?> if (!pkt) > return -ENOMEM; > @@ -823,7 +823,7 @@ virtio_transport_send_response(struct vsock_sock *vsk, > struct virtio_vsock_pkt_info info = { > .op = VIRTIO_VSOCK_OP_RESPONSE, > .type = VIRTIO_VSOCK_TYPE_STREAM, > - .remote_cid = le32_to_cpu(pkt->hdr.src_cid), > + .remote_cid = le64_to_cpu(pkt->hdr.src_cid), > .remote_port = le32_to_cpu(pkt->hdr.src_port), > .reply = true, > }; > @@ -863,9 +863,9 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt) > child->sk_state = SS_CONNECTED; > > vchild = vsock_sk(child); > - vsock_addr_init(&vchild->local_addr, le32_to_cpu(pkt->hdr.dst_cid), > + vsock_addr_init(&vchild->local_addr, le64_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port)); > - vsock_addr_init(&vchild->remote_addr, le32_to_cpu(pkt->hdr.src_cid), > + vsock_addr_init(&vchild->remote_addr, le64_to_cpu(pkt->hdr.src_cid), > le32_to_cpu(pkt->hdr.src_port)); > > vsock_insert_connected(vchild); > @@ -904,9 +904,9 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt) > struct sock *sk; > bool space_available; > > - vsock_addr_init(&src, le32_to_cpu(pkt->hdr.src_cid), > + vsock_addr_init(&src, le64_to_cpu(pkt->hdr.src_cid), > le32_to_cpu(pkt->hdr.src_port)); > - vsock_addr_init(&dst, le32_to_cpu(pkt->hdr.dst_cid), > + vsock_addr_init(&dst, le64_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port)); > > trace_virtio_transport_recv_pkt(src.svm_cid, src.svm_port,
Jason Wang
2016-Dec-07 05:27 UTC
[PATCH 10/10] virtio: enable endian checks for sparse builds
On 2016?12?06? 23:41, Michael S. Tsirkin wrote:> __CHECK_ENDIAN__ isn't on by default presumably because > it triggers too many sparse warnings for correct code. > But virtio is now clean of these warnings, and > we want to keep it this way - enable this for > sparse builds. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > > It seems that there should be a better way to do it, > but this works too.Reviewed-by: Jason Wang <jasowang at redhat.com>> > drivers/block/Makefile | 1 + > drivers/char/Makefile | 1 + > drivers/char/hw_random/Makefile | 2 ++ > drivers/gpu/drm/virtio/Makefile | 1 + > drivers/net/Makefile | 3 +++ > drivers/net/caif/Makefile | 1 + > drivers/rpmsg/Makefile | 1 + > drivers/s390/virtio/Makefile | 2 ++ > drivers/scsi/Makefile | 1 + > drivers/vhost/Makefile | 1 + > drivers/virtio/Makefile | 3 +++ > net/9p/Makefile | 1 + > net/packet/Makefile | 1 + > net/vmw_vsock/Makefile | 2 ++ > 14 files changed, 21 insertions(+) > > diff --git a/drivers/block/Makefile b/drivers/block/Makefile > index 1e9661e..597481c 100644 > --- a/drivers/block/Makefile > +++ b/drivers/block/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_BLK_DEV_OSD) += osdblk.o > obj-$(CONFIG_BLK_DEV_UMEM) += umem.o > obj-$(CONFIG_BLK_DEV_NBD) += nbd.o > obj-$(CONFIG_BLK_DEV_CRYPTOLOOP) += cryptoloop.o > +CFLAGS_virtio_blk.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o > > obj-$(CONFIG_BLK_DEV_SX8) += sx8.o > diff --git a/drivers/char/Makefile b/drivers/char/Makefile > index 6e6c244..a99467d 100644 > --- a/drivers/char/Makefile > +++ b/drivers/char/Makefile > @@ -6,6 +6,7 @@ obj-y += mem.o random.o > obj-$(CONFIG_TTY_PRINTK) += ttyprintk.o > obj-y += misc.o > obj-$(CONFIG_ATARI_DSP56K) += dsp56k.o > +CFLAGS_virtio_console.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_VIRTIO_CONSOLE) += virtio_console.o > obj-$(CONFIG_RAW_DRIVER) += raw.o > obj-$(CONFIG_SGI_SNSC) += snsc.o snsc_event.o > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile > index 5f52b1e..a2b0931 100644 > --- a/drivers/char/hw_random/Makefile > +++ b/drivers/char/hw_random/Makefile > @@ -17,6 +17,8 @@ obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o > obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o > obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o > obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o > +CFLAGS_virtio_transport.o += -D__CHECK_ENDIAN__ > +CFLAGS_virtio-rng.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_HW_RANDOM_VIRTIO) += virtio-rng.o > obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o > obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o > diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile > index 3fb8eac..1162366 100644 > --- a/drivers/gpu/drm/virtio/Makefile > +++ b/drivers/gpu/drm/virtio/Makefile > @@ -3,6 +3,7 @@ > # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. > > ccflags-y := -Iinclude/drm > +ccflags-y += -D__CHECK_ENDIAN__ > > virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_drm_bus.o virtgpu_gem.o \ > virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index 7336cbd..3f587de 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_EQUALIZER) += eql.o > obj-$(CONFIG_IFB) += ifb.o > obj-$(CONFIG_MACSEC) += macsec.o > obj-$(CONFIG_MACVLAN) += macvlan.o > +CFLAGS_macvtap.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_MACVTAP) += macvtap.o > obj-$(CONFIG_MII) += mii.o > obj-$(CONFIG_MDIO) += mdio.o > @@ -20,8 +21,10 @@ obj-$(CONFIG_NETCONSOLE) += netconsole.o > obj-$(CONFIG_PHYLIB) += phy/ > obj-$(CONFIG_RIONET) += rionet.o > obj-$(CONFIG_NET_TEAM) += team/ > +CFLAGS_tun.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_TUN) += tun.o > obj-$(CONFIG_VETH) += veth.o > +CFLAGS_virtio_net.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_VIRTIO_NET) += virtio_net.o > obj-$(CONFIG_VXLAN) += vxlan.o > obj-$(CONFIG_GENEVE) += geneve.o > diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile > index 9bbd453..d1a922c 100644 > --- a/drivers/net/caif/Makefile > +++ b/drivers/net/caif/Makefile > @@ -12,3 +12,4 @@ obj-$(CONFIG_CAIF_HSI) += caif_hsi.o > > # Virtio interface > obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o > +CFLAGS_caif_virtio.o += -D__CHECK_ENDIAN__ > diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile > index ae9c913..23c8b66 100644 > --- a/drivers/rpmsg/Makefile > +++ b/drivers/rpmsg/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_RPMSG) += rpmsg_core.o > obj-$(CONFIG_RPMSG_QCOM_SMD) += qcom_smd.o > obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o > +CFLAGS_virtio_rpmsg_bus.o += -D__CHECK_ENDIAN__ > diff --git a/drivers/s390/virtio/Makefile b/drivers/s390/virtio/Makefile > index df40692..270ada5 100644 > --- a/drivers/s390/virtio/Makefile > +++ b/drivers/s390/virtio/Makefile > @@ -6,6 +6,8 @@ > # it under the terms of the GNU General Public License (version 2 only) > # as published by the Free Software Foundation. > > +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__ > +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__ > s390-virtio-objs := virtio_ccw.o > ifdef CONFIG_S390_GUEST_OLD_TRANSPORT > s390-virtio-objs += kvm_virtio.o > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile > index 38d938d..9f70d46 100644 > --- a/drivers/scsi/Makefile > +++ b/drivers/scsi/Makefile > @@ -135,6 +135,7 @@ obj-$(CONFIG_SCSI_BNX2_ISCSI) += libiscsi.o bnx2i/ > obj-$(CONFIG_BE2ISCSI) += libiscsi.o be2iscsi/ > obj-$(CONFIG_SCSI_ESAS2R) += esas2r/ > obj-$(CONFIG_SCSI_PMCRAID) += pmcraid.o > +CFLAGS_virtio_scsi.o += -D__CHECK_ENDIAN__ > obj-$(CONFIG_SCSI_VIRTIO) += virtio_scsi.o > obj-$(CONFIG_VMWARE_PVSCSI) += vmw_pvscsi.o > obj-$(CONFIG_XEN_SCSI_FRONTEND) += xen-scsifront.o > diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile > index 6b012b9..619e2cd 100644 > --- a/drivers/vhost/Makefile > +++ b/drivers/vhost/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y := -D__CHECK_ENDIAN__ > obj-$(CONFIG_VHOST_NET) += vhost_net.o > vhost_net-y := net.o > > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile > index 41e30e3..d331f19 100644 > --- a/drivers/virtio/Makefile > +++ b/drivers/virtio/Makefile > @@ -1,3 +1,6 @@ > +#virtio must be kept clean wrt endian tags, > +#otherwise we'll get to maintain broken host/guest ABIs > +ccflags-y := -D__CHECK_ENDIAN__ > obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o > obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o > obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o > diff --git a/net/9p/Makefile b/net/9p/Makefile > index a0874cc..acf1225 100644 > --- a/net/9p/Makefile > +++ b/net/9p/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o > trans_fd.o \ > trans_common.o \ > > +CFLAGS_trans_virtio.o += -D__CHECK_ENDIAN__ > 9pnet_virtio-objs := \ > trans_virtio.o \ > > diff --git a/net/packet/Makefile b/net/packet/Makefile > index 9df6134..a13bcb3 100644 > --- a/net/packet/Makefile > +++ b/net/packet/Makefile > @@ -2,6 +2,7 @@ > # Makefile for the packet AF. > # > > +ccflags-y := -D__CHECK_ENDIAN__ > obj-$(CONFIG_PACKET) += af_packet.o > obj-$(CONFIG_PACKET_DIAG) += af_packet_diag.o > af_packet_diag-y += diag.o > diff --git a/net/vmw_vsock/Makefile b/net/vmw_vsock/Makefile > index bc27c70..a61eccb 100644 > --- a/net/vmw_vsock/Makefile > +++ b/net/vmw_vsock/Makefile > @@ -8,6 +8,8 @@ vsock-y += af_vsock.o vsock_addr.o > vmw_vsock_vmci_transport-y += vmci_transport.o vmci_transport_notify.o \ > vmci_transport_notify_qstate.o > > +CFLAGS_virtio_transport.o += -D__CHECK_ENDIAN__ > +CFLAGS_virtio_transport_common.o += -D__CHECK_ENDIAN__ > vmw_vsock_virtio_transport-y += virtio_transport.o > > vmw_vsock_virtio_transport_common-y += virtio_transport_common.o
Johannes Berg
2016-Dec-07 06:25 UTC
[PATCH 10/10] virtio: enable endian checks for sparse builds
On Tue, 2016-12-06 at 17:41 +0200, Michael S. Tsirkin wrote:> It seems that there should be a better way to do it, > but this works too.In some cases there might be:> --- a/drivers/s390/virtio/Makefile > +++ b/drivers/s390/virtio/Makefile > @@ -6,6 +6,8 @@ > ?# it under the terms of the GNU General Public License (version 2 > only) > ?# as published by the Free Software Foundation. > ? > +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__ > +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__ > ?s390-virtio-objs := virtio_ccw.o > ?ifdef CONFIG_S390_GUEST_OLD_TRANSPORT > ?s390-virtio-objs += kvm_virtio.oHere you could use ccflags-y += -D__CHECK_ENDIAN__ for example, or even subdir-ccflags-y += -D__CHECK_ENDIAN__ (in case any subdirs ever get added here)> --- a/drivers/vhost/Makefile > +++ b/drivers/vhost/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y := -D__CHECK_ENDIAN__Looks like you did that here and in some other places though - so perhaps the s390 one was intentionally different?> --- a/net/packet/Makefile > +++ b/net/packet/Makefile > @@ -2,6 +2,7 @@ > ?# Makefile for the packet AF. > ?# > ? > +ccflags-y := -D__CHECK_ENDIAN__Technically this is slightly more than advertised, but I guess that still makes sense if it's clean now. johannes
Christoph Hellwig
2016-Dec-07 07:30 UTC
[PATCH 10/10] virtio: enable endian checks for sparse builds
On Tue, Dec 06, 2016 at 05:41:05PM +0200, Michael S. Tsirkin wrote:> __CHECK_ENDIAN__ isn't on by default presumably because > it triggers too many sparse warnings for correct code. > But virtio is now clean of these warnings, and > we want to keep it this way - enable this for > sparse builds. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>Nah. Please just enable it globally when using sparse. I actually had a chat with Linus about that a while ago and he seemed generally fine with it, I just didn't manage to actually do it..
Stefan Hajnoczi
2016-Dec-07 14:09 UTC
[PATCH 07/10] vsock/virtio: add a missing __le annotation
On Tue, Dec 06, 2016 at 05:40:57PM +0200, Michael S. Tsirkin wrote:> guest cid is read from config space, therefore it's in little endian > format and is treated as such, annotate it accordingly. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20161207/c43425dc/attachment.sig>
Stefan Hajnoczi
2016-Dec-07 14:09 UTC
[PATCH 08/10] vsock/virtio: mark an internal function static
On Tue, Dec 06, 2016 at 05:41:00PM +0200, Michael S. Tsirkin wrote:> virtio_transport_alloc_pkt is only used locally, make it static. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20161207/591e032f/attachment.sig>
On Tue, Dec 06, 2016 at 05:41:02PM +0200, Michael S. Tsirkin wrote:> These fields are 64 bit, using le32_to_cpu and friends > on these will not do the right thing. > Fix this up. > > Cc: stable at vger.kernel.org > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > net/vmw_vsock/virtio_transport_common.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20161207/ac33adf4/attachment.sig>
Stefan Hajnoczi
2016-Dec-07 14:09 UTC
[PATCH 10/10] virtio: enable endian checks for sparse builds
On Tue, Dec 06, 2016 at 05:41:05PM +0200, Michael S. Tsirkin wrote:> __CHECK_ENDIAN__ isn't on by default presumably because > it triggers too many sparse warnings for correct code. > But virtio is now clean of these warnings, and > we want to keep it this way - enable this for > sparse builds. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > > It seems that there should be a better way to do it, > but this works too. > > drivers/block/Makefile | 1 + > drivers/char/Makefile | 1 + > drivers/char/hw_random/Makefile | 2 ++ > drivers/gpu/drm/virtio/Makefile | 1 + > drivers/net/Makefile | 3 +++ > drivers/net/caif/Makefile | 1 + > drivers/rpmsg/Makefile | 1 + > drivers/s390/virtio/Makefile | 2 ++ > drivers/scsi/Makefile | 1 + > drivers/vhost/Makefile | 1 + > drivers/virtio/Makefile | 3 +++ > net/9p/Makefile | 1 + > net/packet/Makefile | 1 + > net/vmw_vsock/Makefile | 2 ++ > 14 files changed, 21 insertions(+)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20161207/d7e15474/attachment.sig>