2020-08-05 17:18:33

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 00/38] virtio: config space endian-ness cleanup

Config space endian-ness is currently a mess: fields are
not tagged with the correct endian-ness so it's easy
to make mistakes like instanciating config space in
native endian-ness.

Further, LE-only devices (e.g. modern-style) add unnecessary overhead
by using generic transitional-style config space accesses.

The following patches adding sparse tagging and then using that
to correctly access config space as either transitional or LE
are my tree.
Lightly tested.

I also start with a version using gcc extensions, then switch
to _Generic. This is helpful for backports to older kernels/older
distros: _Generic patch can just be skipped there.

changes from v2:
- convert a couple of missing devices
- add APIs for accessing config space as LE, use that
in modern devices and in balloon.
- code comments in vdpa_sim explaining use of transitional APIs


Michael S. Tsirkin (38):
virtio_balloon: fix sparse warning
virtio_ring: sparse warning fixup
virtio: allow __virtioXX, __leXX in config space
virtio_9p: correct tags for config space fields
virtio_balloon: correct tags for config space fields
virtio_blk: correct tags for config space fields
virtio_console: correct tags for config space fields
virtio_crypto: correct tags for config space fields
virtio_fs: correct tags for config space fields
virtio_gpu: correct tags for config space fields
virtio_input: correct tags for config space fields
virtio_iommu: correct tags for config space fields
virtio_mem: correct tags for config space fields
virtio_net: correct tags for config space fields
virtio_pmem: correct tags for config space fields
virtio_scsi: correct tags for config space fields
virtio_config: disallow native type fields
mlxbf-tmfifo: sparse tags for config access
vdpa: make sure set_features is invoked for legacy
vhost/vdpa: switch to new helpers
virtio_vdpa: legacy features handling
vdpa_sim: fix endian-ness of config space
virtio_config: cread/write cleanup
virtio_config: rewrite using _Generic
virtio_config: disallow native type fields (again)
virtio_config: LE config space accessors
virtio_caif: correct tags for config space fields
virtio_config: add virtio_cread_le_feature
virtio_balloon: use LE config space accesses
virtio_input: convert to LE accessors
virtio_fs: convert to LE accessors
virtio_crypto: convert to LE accessors
virtio_pmem: convert to LE accessors
drm/virtio: convert to LE accessors
virtio_mem: convert to LE accessors
virtio-iommu: convert to LE accessors
virtio_config: drop LE option from config space
virtio_net: use LE accessors for speed/duplex

drivers/crypto/virtio/virtio_crypto_core.c | 46 ++---
drivers/gpu/drm/virtio/virtgpu_kms.c | 16 +-
drivers/iommu/virtio-iommu.c | 34 ++--
drivers/net/virtio_net.c | 9 +-
drivers/nvdimm/virtio_pmem.c | 4 +-
drivers/platform/mellanox/mlxbf-tmfifo.c | 13 +-
drivers/scsi/virtio_scsi.c | 4 +-
drivers/vdpa/vdpa.c | 1 +
drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++-
drivers/vhost/vdpa.c | 8 +-
drivers/virtio/virtio_balloon.c | 28 ++--
drivers/virtio/virtio_input.c | 32 ++--
drivers/virtio/virtio_mem.c | 30 ++--
drivers/virtio/virtio_vdpa.c | 9 +-
fs/fuse/virtio_fs.c | 4 +-
include/linux/vdpa.h | 34 ++++
include/linux/virtio_caif.h | 6 +-
include/linux/virtio_config.h | 186 +++++++++++++++------
include/linux/virtio_ring.h | 19 +--
include/uapi/linux/virtio_9p.h | 4 +-
include/uapi/linux/virtio_balloon.h | 10 +-
include/uapi/linux/virtio_blk.h | 26 +--
include/uapi/linux/virtio_console.h | 8 +-
include/uapi/linux/virtio_crypto.h | 26 +--
include/uapi/linux/virtio_fs.h | 2 +-
include/uapi/linux/virtio_gpu.h | 8 +-
include/uapi/linux/virtio_input.h | 18 +-
include/uapi/linux/virtio_iommu.h | 12 +-
include/uapi/linux/virtio_mem.h | 14 +-
include/uapi/linux/virtio_net.h | 8 +-
include/uapi/linux/virtio_pmem.h | 4 +-
include/uapi/linux/virtio_scsi.h | 20 +--
32 files changed, 405 insertions(+), 271 deletions(-)

--
MST


2020-08-05 17:19:01

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 24/38] virtio_config: rewrite using _Generic

Min compiler version has been raised, so that's ok now.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/linux/virtio_config.h | 163 ++++++++++++++++------------------
1 file changed, 77 insertions(+), 86 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 5c3b02245ecd..7fa000f02721 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -288,112 +288,103 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
}

-/*
- * Only the checker differentiates between __virtioXX and __uXX types. But we
- * try to share as much code as we can with the regular GCC build.
- */
-#if !defined(CONFIG_CC_IS_GCC) && !defined(__CHECKER__)
+#define virtio_to_cpu(vdev, x) \
+ _Generic((x), \
+ __u8: (x), \
+ __virtio16: virtio16_to_cpu((vdev), (x)), \
+ __virtio32: virtio32_to_cpu((vdev), (x)), \
+ __virtio64: virtio64_to_cpu((vdev), (x)), \
+ /*
+ * Why define a default? checker can distinguish between
+ * e.g. __u16, __le16 and __virtio16, but GCC can't so
+ * attempts to define variants for both look like a duplicate
+ * variant to it.
+ */ \
+ default: _Generic((x), \
+ __u8: (x), \
+ __le16: virtio16_to_cpu((vdev), (__force __virtio16)(x)), \
+ __le32: virtio32_to_cpu((vdev), (__force __virtio32)(x)), \
+ __le64: virtio64_to_cpu((vdev), (__force __virtio64)(x)), \
+ default: _Generic((x), \
+ __u8: (x), \
+ __u16: virtio16_to_cpu((vdev), (__force __virtio16)(x)), \
+ __u32: virtio32_to_cpu((vdev), (__force __virtio32)(x)), \
+ __u64: virtio64_to_cpu((vdev), (__force __virtio64)(x)) \
+ ) \
+ ) \
+ )

-/* Not a checker - we can keep things simple */
-#define __virtio_native_typeof(x) typeof(x)
-
-#else
-
-/*
- * We build this out of a couple of helper macros in a vain attempt to
- * help you keep your lunch down while reading it.
- */
-#define __virtio_pick_value(x, type, then, otherwise) \
- __builtin_choose_expr(__same_type(x, type), then, otherwise)
-
-#define __virtio_pick_type(x, type, then, otherwise) \
- __virtio_pick_value(x, type, (then)0, otherwise)
-
-#define __virtio_pick_endian(x, x16, x32, x64, otherwise) \
- __virtio_pick_type(x, x16, __u16, \
- __virtio_pick_type(x, x32, __u32, \
- __virtio_pick_type(x, x64, __u64, \
- otherwise)))
-
-#define __virtio_native_typeof(x) typeof( \
- __virtio_pick_type(x, __u8, __u8, \
- __virtio_pick_endian(x, __virtio16, __virtio32, __virtio64, \
- __virtio_pick_endian(x, __le16, __le32, __le64, \
- /* No other type allowed */ \
- (void)0))))
-
-#endif
+#define cpu_to_virtio(vdev, x, m) \
+ _Generic((m), \
+ __u8: (x), \
+ __virtio16: cpu_to_virtio16((vdev), (x)), \
+ __virtio32: cpu_to_virtio32((vdev), (x)), \
+ __virtio64: cpu_to_virtio64((vdev), (x)), \
+ /*
+ * Why define a default? checker can distinguish between
+ * e.g. __u16, __le16 and __virtio16, but GCC can't so
+ * attempts to define variants for both look like a duplicate
+ * variant to it.
+ */ \
+ default: _Generic((m), \
+ __u8: (x), \
+ __le16: (__force __le16)cpu_to_virtio16((vdev), (x)), \
+ __le32: (__force __le32)cpu_to_virtio32((vdev), (x)), \
+ __le64: (__force __le64)cpu_to_virtio64((vdev), (x)), \
+ default: _Generic((m), \
+ __u8: (x), \
+ __u16: (__force __u16)cpu_to_virtio16((vdev), (x)), \
+ __u32: (__force __u32)cpu_to_virtio32((vdev), (x)), \
+ __u64: (__force __u64)cpu_to_virtio64((vdev), (x)) \
+ ) \
+ ) \
+ )

#define __virtio_native_type(structname, member) \
- __virtio_native_typeof(((structname*)0)->member)
-
-#define __virtio_typecheck(structname, member, val) \
- /* Must match the member's type, and be integer */ \
- typecheck(__virtio_native_type(structname, member), (val))
-
+ typeof(virtio_to_cpu(NULL, ((structname*)0)->member))

/* Config space accessors. */
#define virtio_cread(vdev, structname, member, ptr) \
do { \
- might_sleep(); \
- /* Must match the member's type, and be integer */ \
- if (!__virtio_typecheck(structname, member, *(ptr))) \
- (*ptr) = 1; \
+ typeof(((structname*)0)->member) virtio_cread_v; \
\
- switch (sizeof(*ptr)) { \
+ might_sleep(); \
+ /* Sanity check: must match the member's type */ \
+ typecheck(typeof(virtio_to_cpu((vdev), virtio_cread_v)), *(ptr)); \
+ \
+ switch (sizeof(virtio_cread_v)) { \
case 1: \
- *(ptr) = virtio_cread8(vdev, \
- offsetof(structname, member)); \
- break; \
case 2: \
- *(ptr) = virtio_cread16(vdev, \
- offsetof(structname, member)); \
- break; \
case 4: \
- *(ptr) = virtio_cread32(vdev, \
- offsetof(structname, member)); \
- break; \
- case 8: \
- *(ptr) = virtio_cread64(vdev, \
- offsetof(structname, member)); \
+ vdev->config->get((vdev), \
+ offsetof(structname, member), \
+ &virtio_cread_v, \
+ sizeof(virtio_cread_v)); \
break; \
default: \
- BUG(); \
+ __virtio_cread_many((vdev), \
+ offsetof(structname, member), \
+ &virtio_cread_v, \
+ 1, \
+ sizeof(virtio_cread_v)); \
+ break; \
} \
+ *(ptr) = virtio_to_cpu(vdev, virtio_cread_v); \
} while(0)

/* Config space accessors. */
#define virtio_cwrite(vdev, structname, member, ptr) \
do { \
- might_sleep(); \
- /* Must match the member's type, and be integer */ \
- if (!__virtio_typecheck(structname, member, *(ptr))) \
- BUG_ON((*ptr) == 1); \
+ typeof(((structname*)0)->member) virtio_cwrite_v = \
+ cpu_to_virtio(vdev, *(ptr), ((structname*)0)->member); \
\
- switch (sizeof(*ptr)) { \
- case 1: \
- virtio_cwrite8(vdev, \
- offsetof(structname, member), \
- *(ptr)); \
- break; \
- case 2: \
- virtio_cwrite16(vdev, \
- offsetof(structname, member), \
- *(ptr)); \
- break; \
- case 4: \
- virtio_cwrite32(vdev, \
- offsetof(structname, member), \
- *(ptr)); \
- break; \
- case 8: \
- virtio_cwrite64(vdev, \
- offsetof(structname, member), \
- *(ptr)); \
- break; \
- default: \
- BUG(); \
- } \
+ might_sleep(); \
+ /* Sanity check: must match the member's type */ \
+ typecheck(typeof(virtio_to_cpu((vdev), virtio_cwrite_v)), *(ptr)); \
+ \
+ vdev->config->set((vdev), offsetof(structname, member), \
+ &virtio_cwrite_v, \
+ sizeof(virtio_cwrite_v)); \
} while(0)

/* Read @count fields, @bytes each. */
--
MST

2020-08-05 17:19:20

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 09/38] virtio_fs: correct tags for config space fields

Since fs is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Acked-by: Vivek Goyal <[email protected]>
Acked-by: Vivek Goyal <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_fs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_fs.h b/include/uapi/linux/virtio_fs.h
index b02eb2ac3d99..3056b6e9f8ce 100644
--- a/include/uapi/linux/virtio_fs.h
+++ b/include/uapi/linux/virtio_fs.h
@@ -13,7 +13,7 @@ struct virtio_fs_config {
__u8 tag[36];

/* Number of request queues */
- __u32 num_request_queues;
+ __le32 num_request_queues;
} __attribute__((packed));

#endif /* _UAPI_LINUX_VIRTIO_FS_H */
--
MST

2020-08-05 17:20:04

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 30/38] virtio_input: convert to LE accessors

Virtio input is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/virtio/virtio_input.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index efaf65b0f42d..877b2ea3ed05 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -113,9 +113,9 @@ static u8 virtinput_cfg_select(struct virtio_input *vi,
{
u8 size;

- virtio_cwrite(vi->vdev, struct virtio_input_config, select, &select);
- virtio_cwrite(vi->vdev, struct virtio_input_config, subsel, &subsel);
- virtio_cread(vi->vdev, struct virtio_input_config, size, &size);
+ virtio_cwrite_le(vi->vdev, struct virtio_input_config, select, &select);
+ virtio_cwrite_le(vi->vdev, struct virtio_input_config, subsel, &subsel);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, size, &size);
return size;
}

@@ -158,11 +158,11 @@ static void virtinput_cfg_abs(struct virtio_input *vi, int abs)
u32 mi, ma, re, fu, fl;

virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
- virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
- virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
- virtio_cread(vi->vdev, struct virtio_input_config, u.abs.res, &re);
- virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
- virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.min, &mi);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.max, &ma);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.res, &re);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.fuzz, &fu);
+ virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.flat, &fl);
input_set_abs_params(vi->idev, abs, mi, ma, fu, fl);
input_abs_set_res(vi->idev, abs, re);
}
@@ -244,14 +244,14 @@ static int virtinput_probe(struct virtio_device *vdev)

size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_DEVIDS, 0);
if (size >= sizeof(struct virtio_input_devids)) {
- virtio_cread(vi->vdev, struct virtio_input_config,
- u.ids.bustype, &vi->idev->id.bustype);
- virtio_cread(vi->vdev, struct virtio_input_config,
- u.ids.vendor, &vi->idev->id.vendor);
- virtio_cread(vi->vdev, struct virtio_input_config,
- u.ids.product, &vi->idev->id.product);
- virtio_cread(vi->vdev, struct virtio_input_config,
- u.ids.version, &vi->idev->id.version);
+ virtio_cread_le(vi->vdev, struct virtio_input_config,
+ u.ids.bustype, &vi->idev->id.bustype);
+ virtio_cread_le(vi->vdev, struct virtio_input_config,
+ u.ids.vendor, &vi->idev->id.vendor);
+ virtio_cread_le(vi->vdev, struct virtio_input_config,
+ u.ids.product, &vi->idev->id.product);
+ virtio_cread_le(vi->vdev, struct virtio_input_config,
+ u.ids.version, &vi->idev->id.version);
} else {
vi->idev->id.bustype = BUS_VIRTUAL;
}
--
MST

2020-08-05 17:21:18

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 10/38] virtio_gpu: correct tags for config space fields

Since gpu is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_gpu.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index 0c85914d9369..ccbd174ef321 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -320,10 +320,10 @@ struct virtio_gpu_resp_edid {
#define VIRTIO_GPU_EVENT_DISPLAY (1 << 0)

struct virtio_gpu_config {
- __u32 events_read;
- __u32 events_clear;
- __u32 num_scanouts;
- __u32 num_capsets;
+ __le32 events_read;
+ __le32 events_clear;
+ __le32 num_scanouts;
+ __le32 num_capsets;
};

/* simple formats for fbcon/X use */
--
MST

2020-08-05 17:27:46

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 02/38] virtio_ring: sparse warning fixup

virtio_store_mb was built with split ring in mind so it accepts
__virtio16 arguments. Packed ring uses __le16 values, so sparse
complains. It's just a store with some barriers so let's convert it to
a macro, we don't loose too much type safety by doing that.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Acked-by: Cornelia Huck <[email protected]>
---
include/linux/virtio_ring.h | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..b485b13fa50b 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -46,16 +46,15 @@ static inline void virtio_wmb(bool weak_barriers)
dma_wmb();
}

-static inline void virtio_store_mb(bool weak_barriers,
- __virtio16 *p, __virtio16 v)
-{
- if (weak_barriers) {
- virt_store_mb(*p, v);
- } else {
- WRITE_ONCE(*p, v);
- mb();
- }
-}
+#define virtio_store_mb(weak_barriers, p, v) \
+do { \
+ if (weak_barriers) { \
+ virt_store_mb(*p, v); \
+ } else { \
+ WRITE_ONCE(*p, v); \
+ mb(); \
+ } \
+} while (0) \

struct virtio_device;
struct virtqueue;
--
MST

2020-08-05 17:30:31

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 06/38] virtio_blk: correct tags for config space fields

Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_blk.h | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 0f99d7b49ede..d888f013d9ff 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -57,20 +57,20 @@

struct virtio_blk_config {
/* The capacity (in 512-byte sectors). */
- __u64 capacity;
+ __virtio64 capacity;
/* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
- __u32 size_max;
+ __virtio32 size_max;
/* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
- __u32 seg_max;
+ __virtio32 seg_max;
/* geometry of the device (if VIRTIO_BLK_F_GEOMETRY) */
struct virtio_blk_geometry {
- __u16 cylinders;
+ __virtio16 cylinders;
__u8 heads;
__u8 sectors;
} geometry;

/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
- __u32 blk_size;
+ __virtio32 blk_size;

/* the next 4 entries are guarded by VIRTIO_BLK_F_TOPOLOGY */
/* exponent for physical block per logical block. */
@@ -78,42 +78,42 @@ struct virtio_blk_config {
/* alignment offset in logical blocks. */
__u8 alignment_offset;
/* minimum I/O size without performance penalty in logical blocks. */
- __u16 min_io_size;
+ __virtio16 min_io_size;
/* optimal sustained I/O size in logical blocks. */
- __u32 opt_io_size;
+ __virtio32 opt_io_size;

/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
__u8 wce;
__u8 unused;

/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
- __u16 num_queues;
+ __virtio16 num_queues;

/* the next 3 entries are guarded by VIRTIO_BLK_F_DISCARD */
/*
* The maximum discard sectors (in 512-byte sectors) for
* one segment.
*/
- __u32 max_discard_sectors;
+ __virtio32 max_discard_sectors;
/*
* The maximum number of discard segments in a
* discard command.
*/
- __u32 max_discard_seg;
+ __virtio32 max_discard_seg;
/* Discard commands must be aligned to this number of sectors. */
- __u32 discard_sector_alignment;
+ __virtio32 discard_sector_alignment;

/* the next 3 entries are guarded by VIRTIO_BLK_F_WRITE_ZEROES */
/*
* The maximum number of write zeroes sectors (in 512-byte sectors) in
* one segment.
*/
- __u32 max_write_zeroes_sectors;
+ __virtio32 max_write_zeroes_sectors;
/*
* The maximum number of segments in a write zeroes
* command.
*/
- __u32 max_write_zeroes_seg;
+ __virtio32 max_write_zeroes_seg;
/*
* Set if a VIRTIO_BLK_T_WRITE_ZEROES request may result in the
* deallocation of one or more of the sectors.
--
MST

2020-08-05 19:27:32

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 35/38] virtio_mem: convert to LE accessors

Virtio mem is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/virtio/virtio_mem.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index f26f5f64ae82..c08512fcea90 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1530,21 +1530,21 @@ static void virtio_mem_refresh_config(struct virtio_mem *vm)
uint64_t new_plugged_size, usable_region_size, end_addr;

/* the plugged_size is just a reflection of what _we_ did previously */
- virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
- &new_plugged_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
+ &new_plugged_size);
if (WARN_ON_ONCE(new_plugged_size != vm->plugged_size))
vm->plugged_size = new_plugged_size;

/* calculate the last usable memory block id */
- virtio_cread(vm->vdev, struct virtio_mem_config,
- usable_region_size, &usable_region_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config,
+ usable_region_size, &usable_region_size);
end_addr = vm->addr + usable_region_size;
end_addr = min(end_addr, phys_limit);
vm->last_usable_mb_id = virtio_mem_phys_to_mb_id(end_addr) - 1;

/* see if there is a request to change the size */
- virtio_cread(vm->vdev, struct virtio_mem_config, requested_size,
- &vm->requested_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, requested_size,
+ &vm->requested_size);

dev_info(&vm->vdev->dev, "plugged size: 0x%llx", vm->plugged_size);
dev_info(&vm->vdev->dev, "requested size: 0x%llx", vm->requested_size);
@@ -1677,16 +1677,16 @@ static int virtio_mem_init(struct virtio_mem *vm)
}

/* Fetch all properties that can't change. */
- virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
- &vm->plugged_size);
- virtio_cread(vm->vdev, struct virtio_mem_config, block_size,
- &vm->device_block_size);
- virtio_cread(vm->vdev, struct virtio_mem_config, node_id,
- &node_id);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
+ &vm->plugged_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
+ &vm->device_block_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
+ &node_id);
vm->nid = virtio_mem_translate_node_id(vm, node_id);
- virtio_cread(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
- virtio_cread(vm->vdev, struct virtio_mem_config, region_size,
- &vm->region_size);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
+ virtio_cread_le(vm->vdev, struct virtio_mem_config, region_size,
+ &vm->region_size);

/*
* We always hotplug memory in memory block granularity. This way,
--
MST

2020-08-05 19:27:51

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 01/38] virtio_balloon: fix sparse warning

balloon uses virtio32_to_cpu instead of cpu_to_virtio32
to convert a native endian number to virtio.
No practical difference but makes sparse warn.
Fix it up.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
drivers/virtio/virtio_balloon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 54fd989f9353..8bc1704ffdf3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -600,7 +600,7 @@ static int send_cmd_id_start(struct virtio_balloon *vb)
while (virtqueue_get_buf(vq, &unused))
;

- vb->cmd_id_active = virtio32_to_cpu(vb->vdev,
+ vb->cmd_id_active = cpu_to_virtio32(vb->vdev,
virtio_balloon_cmd_id_received(vb));
sg_init_one(&sg, &vb->cmd_id_active, sizeof(vb->cmd_id_active));
err = virtqueue_add_outbuf(vq, &sg, 1, &vb->cmd_id_active, GFP_KERNEL);
--
MST

2020-08-05 19:27:54

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 20/38] vhost/vdpa: switch to new helpers

For new helpers handling legacy features to be effective,
vhost needs to invoke them. Tie them in.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/vhost/vdpa.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 18869a35d408..3674404688f5 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -118,9 +118,8 @@ static irqreturn_t vhost_vdpa_config_cb(void *private)
static void vhost_vdpa_reset(struct vhost_vdpa *v)
{
struct vdpa_device *vdpa = v->vdpa;
- const struct vdpa_config_ops *ops = vdpa->config;

- ops->set_status(vdpa, 0);
+ vdpa_reset(vdpa);
}

static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
@@ -196,7 +195,6 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
struct vhost_vdpa_config __user *c)
{
struct vdpa_device *vdpa = v->vdpa;
- const struct vdpa_config_ops *ops = vdpa->config;
struct vhost_vdpa_config config;
unsigned long size = offsetof(struct vhost_vdpa_config, buf);
u8 *buf;
@@ -209,7 +207,7 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
if (!buf)
return -ENOMEM;

- ops->get_config(vdpa, config.off, buf, config.len);
+ vdpa_get_config(vdpa, config.off, buf, config.len);

if (copy_to_user(c->buf, buf, config.len)) {
kvfree(buf);
@@ -282,7 +280,7 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v, u64 __user *featurep)
if (features & ~vhost_vdpa_features[v->virtio_id])
return -EINVAL;

- if (ops->set_features(vdpa, features))
+ if (vdpa_set_features(vdpa, features))
return -EINVAL;

return 0;
--
MST

2020-08-05 19:29:06

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 12/38] virtio_iommu: correct tags for config space fields

Since this is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Jean-Philippe Brucker <[email protected]>
Reviewed-by: Jean-Philippe Brucker <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_iommu.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index 48e3c29223b5..237e36a280cb 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -18,24 +18,24 @@
#define VIRTIO_IOMMU_F_MMIO 5

struct virtio_iommu_range_64 {
- __u64 start;
- __u64 end;
+ __le64 start;
+ __le64 end;
};

struct virtio_iommu_range_32 {
- __u32 start;
- __u32 end;
+ __le32 start;
+ __le32 end;
};

struct virtio_iommu_config {
/* Supported page sizes */
- __u64 page_size_mask;
+ __le64 page_size_mask;
/* Supported IOVA range */
struct virtio_iommu_range_64 input_range;
/* Max domain ID size */
struct virtio_iommu_range_32 domain_range;
/* Probe buffer size */
- __u32 probe_size;
+ __le32 probe_size;
};

/* Request types */
--
MST

2020-08-05 19:29:11

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 22/38] vdpa_sim: fix endian-ness of config space

VDPA sim accesses config space as native endian - this is
wrong since it's a modern device and actually uses LE.

It only supports modern guests so we could punt and
just force LE, but let's use the full virtio APIs since people
tend to copy/paste code, and this is not data path anyway.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index a9bc5e0fb353..b7d5727fde4c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -24,6 +24,7 @@
#include <linux/etherdevice.h>
#include <linux/vringh.h>
#include <linux/vdpa.h>
+#include <linux/virtio_byteorder.h>
#include <linux/vhost_iotlb.h>
#include <uapi/linux/virtio_config.h>
#include <uapi/linux/virtio_net.h>
@@ -72,6 +73,23 @@ struct vdpasim {
u64 features;
};

+/* TODO: cross-endian support */
+static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim)
+{
+ return virtio_legacy_is_little_endian() ||
+ (vdpasim->features & (1ULL << VIRTIO_F_VERSION_1));
+}
+
+static inline u16 vdpasim16_to_cpu(struct vdpasim *vdpasim, __virtio16 val)
+{
+ return __virtio16_to_cpu(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline __virtio16 cpu_to_vdpasim16(struct vdpasim *vdpasim, u16 val)
+{
+ return __cpu_to_virtio16(vdpasim_is_little_endian(vdpasim), val);
+}
+
static struct vdpasim *vdpasim_dev;

static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
@@ -306,7 +324,6 @@ static const struct vdpa_config_ops vdpasim_net_config_ops;

static struct vdpasim *vdpasim_create(void)
{
- struct virtio_net_config *config;
struct vdpasim *vdpasim;
struct device *dev;
int ret = -ENOMEM;
@@ -331,10 +348,7 @@ static struct vdpasim *vdpasim_create(void)
if (!vdpasim->buffer)
goto err_iommu;

- config = &vdpasim->config;
- config->mtu = 1500;
- config->status = VIRTIO_NET_S_LINK_UP;
- eth_random_addr(config->mac);
+ eth_random_addr(vdpasim->config.mac);

vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
@@ -448,6 +462,7 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
{
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+ struct virtio_net_config *config = &vdpasim->config;

/* DMA mapping must be done by driver */
if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -455,6 +470,14 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)

vdpasim->features = features & vdpasim_features;

+ /* We generally only know whether guest is using the legacy interface
+ * here, so generally that's the earliest we can set config fields.
+ * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
+ * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
+ */
+
+ config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
+ config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
return 0;
}

--
MST

2020-08-05 19:29:12

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 38/38] virtio_net: use LE accessors for speed/duplex

Speed and duplex config fields depend on VIRTIO_NET_F_SPEED_DUPLEX
which being 63>31 depends on VIRTIO_F_VERSION_1.

Accordingly, use LE accessors for these fields.

Reported-by: Cornelia Huck <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/net/virtio_net.c | 9 +++++----
include/uapi/linux/virtio_net.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba38765dc490..0934b1ec5320 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2264,12 +2264,13 @@ static void virtnet_update_settings(struct virtnet_info *vi)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
return;

- speed = virtio_cread32(vi->vdev, offsetof(struct virtio_net_config,
- speed));
+ virtio_cread_le(vi->vdev, struct virtio_net_config, speed, &speed);
+
if (ethtool_validate_speed(speed))
vi->speed = speed;
- duplex = virtio_cread8(vi->vdev, offsetof(struct virtio_net_config,
- duplex));
+
+ virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, &duplex);
+
if (ethtool_validate_duplex(duplex))
vi->duplex = duplex;
}
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 27d996f29dd1..3f55a4215f11 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -99,7 +99,7 @@ struct virtio_net_config {
* speed, in units of 1Mb. All values 0 to INT_MAX are legal.
* Any other value stands for unknown.
*/
- __virtio32 speed;
+ __le32 speed;
/*
* 0x00 - half duplex
* 0x01 - full duplex
--
MST

2020-08-05 19:29:20

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 29/38] virtio_balloon: use LE config space accesses

Balloon is LE, it's cleaner to access it as such directly.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/virtio/virtio_balloon.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8bc1704ffdf3..31cc97f2f515 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -398,12 +398,9 @@ static inline s64 towards_target(struct virtio_balloon *vb)
s64 target;
u32 num_pages;

- virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages,
- &num_pages);
-
/* Legacy balloon config space is LE, unlike all other devices. */
- if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
- num_pages = le32_to_cpu((__force __le32)num_pages);
+ virtio_cread_le(vb->vdev, struct virtio_balloon_config, num_pages,
+ &num_pages);

target = num_pages;
return target - vb->num_pages;
@@ -462,11 +459,8 @@ static void update_balloon_size(struct virtio_balloon *vb)
u32 actual = vb->num_pages;

/* Legacy balloon config space is LE, unlike all other devices. */
- if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
- actual = (__force u32)cpu_to_le32(actual);
-
- virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual,
- &actual);
+ virtio_cwrite_le(vb->vdev, struct virtio_balloon_config, actual,
+ &actual);
}

static void update_balloon_stats_func(struct work_struct *work)
@@ -579,12 +573,10 @@ static u32 virtio_balloon_cmd_id_received(struct virtio_balloon *vb)
{
if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID,
&vb->config_read_bitmap)) {
- virtio_cread(vb->vdev, struct virtio_balloon_config,
- free_page_hint_cmd_id,
- &vb->cmd_id_received_cache);
/* Legacy balloon config space is LE, unlike all other devices. */
- if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
- vb->cmd_id_received_cache = le32_to_cpu((__force __le32)vb->cmd_id_received_cache);
+ virtio_cread_le(vb->vdev, struct virtio_balloon_config,
+ free_page_hint_cmd_id,
+ &vb->cmd_id_received_cache);
}

return vb->cmd_id_received_cache;
@@ -987,8 +979,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
if (!want_init_on_free())
memset(&poison_val, PAGE_POISON, sizeof(poison_val));

- virtio_cwrite(vb->vdev, struct virtio_balloon_config,
- poison_val, &poison_val);
+ virtio_cwrite_le(vb->vdev, struct virtio_balloon_config,
+ poison_val, &poison_val);
}

vb->pr_dev_info.report = virtballoon_free_page_report;
--
MST

2020-08-05 19:29:36

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 33/38] virtio_pmem: convert to LE accessors

Virtio pmem is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/nvdimm/virtio_pmem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 5e3d07b47e0c..726c7354d465 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -58,9 +58,9 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
goto out_err;
}

- virtio_cread(vpmem->vdev, struct virtio_pmem_config,
+ virtio_cread_le(vpmem->vdev, struct virtio_pmem_config,
start, &vpmem->start);
- virtio_cread(vpmem->vdev, struct virtio_pmem_config,
+ virtio_cread_le(vpmem->vdev, struct virtio_pmem_config,
size, &vpmem->size);

res.start = vpmem->start;
--
MST

2020-08-05 19:30:03

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 15/38] virtio_pmem: correct tags for config space fields

Since this is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_pmem.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_pmem.h b/include/uapi/linux/virtio_pmem.h
index b022787ffb94..d676b3620383 100644
--- a/include/uapi/linux/virtio_pmem.h
+++ b/include/uapi/linux/virtio_pmem.h
@@ -15,8 +15,8 @@
#include <linux/virtio_config.h>

struct virtio_pmem_config {
- __u64 start;
- __u64 size;
+ __le64 start;
+ __le64 size;
};

#define VIRTIO_PMEM_REQ_TYPE_FLUSH 0
--
MST

2020-08-05 19:30:12

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 37/38] virtio_config: drop LE option from config space

All drivers now use virtio_cread/write_le for LE config
space fields. Drop LE option from virtio_cread/write, only leaving
the option to access transitional fields.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/linux/virtio_config.h | 28 ++--------------------------
1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index cc7a2b1fd7b2..ecb166c824bb 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -293,19 +293,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
__u8: (x), \
__virtio16: virtio16_to_cpu((vdev), (x)), \
__virtio32: virtio32_to_cpu((vdev), (x)), \
- __virtio64: virtio64_to_cpu((vdev), (x)), \
- /*
- * Why define a default? checker can distinguish between
- * e.g. __u16, __le16 and __virtio16, but GCC can't so
- * attempts to define variants for both look like a duplicate
- * variant to it.
- */ \
- default: _Generic((x), \
- __u8: (x), \
- __le16: virtio16_to_cpu((vdev), (__force __virtio16)(x)), \
- __le32: virtio32_to_cpu((vdev), (__force __virtio32)(x)), \
- __le64: virtio64_to_cpu((vdev), (__force __virtio64)(x)) \
- ) \
+ __virtio64: virtio64_to_cpu((vdev), (x)) \
)

#define cpu_to_virtio(vdev, x, m) \
@@ -313,19 +301,7 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
__u8: (x), \
__virtio16: cpu_to_virtio16((vdev), (x)), \
__virtio32: cpu_to_virtio32((vdev), (x)), \
- __virtio64: cpu_to_virtio64((vdev), (x)), \
- /*
- * Why define a default? checker can distinguish between
- * e.g. __u16, __le16 and __virtio16, but GCC can't so
- * attempts to define variants for both look like a duplicate
- * variant to it.
- */ \
- default: _Generic((m), \
- __u8: (x), \
- __le16: (__force __le16)cpu_to_virtio16((vdev), (x)), \
- __le32: (__force __le32)cpu_to_virtio32((vdev), (x)), \
- __le64: (__force __le64)cpu_to_virtio64((vdev), (x)) \
- ) \
+ __virtio64: cpu_to_virtio64((vdev), (x)) \
)

#define __virtio_native_type(structname, member) \
--
MST

2020-08-05 19:30:32

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 27/38] virtio_caif: correct tags for config space fields

Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/linux/virtio_caif.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_caif.h b/include/linux/virtio_caif.h
index 5d2d3124ca3d..ea722479510c 100644
--- a/include/linux/virtio_caif.h
+++ b/include/linux/virtio_caif.h
@@ -11,9 +11,9 @@

#include <linux/types.h>
struct virtio_caif_transf_config {
- u16 headroom;
- u16 tailroom;
- u32 mtu;
+ __virtio16 headroom;
+ __virtio16 tailroom;
+ __virtio32 mtu;
u8 reserved[4];
};

--
MST

2020-08-05 19:30:33

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 07/38] virtio_console: correct tags for config space fields

Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
include/uapi/linux/virtio_console.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_console.h b/include/uapi/linux/virtio_console.h
index b7fb108c9310..7e6ec2ff0560 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -45,13 +45,13 @@

struct virtio_console_config {
/* colums of the screens */
- __u16 cols;
+ __virtio16 cols;
/* rows of the screens */
- __u16 rows;
+ __virtio16 rows;
/* max. number of ports this device can hold */
- __u32 max_nr_ports;
+ __virtio32 max_nr_ports;
/* emergency write register */
- __u32 emerg_wr;
+ __virtio32 emerg_wr;
} __attribute__((packed));

/*
--
MST

2020-08-05 19:30:34

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 16/38] virtio_scsi: correct tags for config space fields

Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Reviewed-by: Cornelia Huck <[email protected]>
---
drivers/scsi/virtio_scsi.c | 4 ++--
include/uapi/linux/virtio_scsi.h | 20 ++++++++++----------
2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0e0910c5b942..c36aeb9a1330 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -746,14 +746,14 @@ static struct scsi_host_template virtscsi_host_template = {

#define virtscsi_config_get(vdev, fld) \
({ \
- typeof(((struct virtio_scsi_config *)0)->fld) __val; \
+ __virtio_native_type(struct virtio_scsi_config, fld) __val; \
virtio_cread(vdev, struct virtio_scsi_config, fld, &__val); \
__val; \
})

#define virtscsi_config_set(vdev, fld, val) \
do { \
- typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
+ __virtio_native_type(struct virtio_scsi_config, fld) __val = (val); \
virtio_cwrite(vdev, struct virtio_scsi_config, fld, &__val); \
} while(0)

diff --git a/include/uapi/linux/virtio_scsi.h b/include/uapi/linux/virtio_scsi.h
index cc18ef8825c0..0abaae4027c0 100644
--- a/include/uapi/linux/virtio_scsi.h
+++ b/include/uapi/linux/virtio_scsi.h
@@ -103,16 +103,16 @@ struct virtio_scsi_event {
} __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;
+ __virtio32 num_queues;
+ __virtio32 seg_max;
+ __virtio32 max_sectors;
+ __virtio32 cmd_per_lun;
+ __virtio32 event_info_size;
+ __virtio32 sense_size;
+ __virtio32 cdb_size;
+ __virtio16 max_channel;
+ __virtio16 max_target;
+ __virtio32 max_lun;
} __attribute__((packed));

/* Feature Bits */
--
MST

2020-08-05 19:30:47

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 21/38] virtio_vdpa: legacy features handling

We normally expect vdpa to use the modern interface.
However for consistency, let's use same APIs as vhost
for legacy guests.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/virtio/virtio_vdpa.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index c30eb55030be..4a9ddb44b2a7 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -57,9 +57,8 @@ static void virtio_vdpa_get(struct virtio_device *vdev, unsigned offset,
void *buf, unsigned len)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- const struct vdpa_config_ops *ops = vdpa->config;

- ops->get_config(vdpa, offset, buf, len);
+ vdpa_get_config(vdpa, offset, buf, len);
}

static void virtio_vdpa_set(struct virtio_device *vdev, unsigned offset,
@@ -101,9 +100,8 @@ static void virtio_vdpa_set_status(struct virtio_device *vdev, u8 status)
static void virtio_vdpa_reset(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- const struct vdpa_config_ops *ops = vdpa->config;

- return ops->set_status(vdpa, 0);
+ vdpa_reset(vdpa);
}

static bool virtio_vdpa_notify(struct virtqueue *vq)
@@ -294,12 +292,11 @@ static u64 virtio_vdpa_get_features(struct virtio_device *vdev)
static int virtio_vdpa_finalize_features(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- const struct vdpa_config_ops *ops = vdpa->config;

/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);

- return ops->set_features(vdpa, vdev->features);
+ return vdpa_set_features(vdpa, vdev->features);
}

static const char *virtio_vdpa_bus_name(struct virtio_device *vdev)
--
MST

2020-08-05 19:30:50

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 36/38] virtio-iommu: convert to LE accessors

Virtio iommu is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/iommu/virtio-iommu.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index f6f07489a9aa..b4da396cce60 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -1010,8 +1010,8 @@ static int viommu_probe(struct virtio_device *vdev)
if (ret)
return ret;

- virtio_cread(vdev, struct virtio_iommu_config, page_size_mask,
- &viommu->pgsize_bitmap);
+ virtio_cread_le(vdev, struct virtio_iommu_config, page_size_mask,
+ &viommu->pgsize_bitmap);

if (!viommu->pgsize_bitmap) {
ret = -EINVAL;
@@ -1022,25 +1022,25 @@ static int viommu_probe(struct virtio_device *vdev)
viommu->last_domain = ~0U;

/* Optional features */
- virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
- struct virtio_iommu_config, input_range.start,
- &input_start);
+ virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+ struct virtio_iommu_config, input_range.start,
+ &input_start);

- virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
- struct virtio_iommu_config, input_range.end,
- &input_end);
+ virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+ struct virtio_iommu_config, input_range.end,
+ &input_end);

- virtio_cread_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
- struct virtio_iommu_config, domain_range.start,
- &viommu->first_domain);
+ virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
+ struct virtio_iommu_config, domain_range.start,
+ &viommu->first_domain);

- virtio_cread_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
- struct virtio_iommu_config, domain_range.end,
- &viommu->last_domain);
+ virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
+ struct virtio_iommu_config, domain_range.end,
+ &viommu->last_domain);

- virtio_cread_feature(vdev, VIRTIO_IOMMU_F_PROBE,
- struct virtio_iommu_config, probe_size,
- &viommu->probe_size);
+ virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_PROBE,
+ struct virtio_iommu_config, probe_size,
+ &viommu->probe_size);

viommu->geometry = (struct iommu_domain_geometry) {
.aperture_start = input_start,
--
MST

2020-08-05 19:31:06

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 26/38] virtio_config: LE config space accessors

To be used by modern code, as well as to handle LE only fields such as
balloon.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/linux/virtio_config.h | 65 +++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 441fd6dd42ab..5b5196fec899 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -375,6 +375,71 @@ static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
sizeof(virtio_cwrite_v)); \
} while(0)

+/*
+ * Nothing virtio-specific about these, but let's worry about generalizing
+ * these later.
+ */
+#define virtio_le_to_cpu(x) \
+ _Generic((x), \
+ __u8: (x), \
+ __le16: le16_to_cpu(x), \
+ __le32: le32_to_cpu(x), \
+ __le64: le64_to_cpu(x) \
+ )
+
+#define virtio_cpu_to_le(x, m) \
+ _Generic((m), \
+ __u8: (x), \
+ __le16: cpu_to_le16(x), \
+ __le32: cpu_to_le32(x), \
+ __le64: cpu_to_le64(x) \
+ )
+
+/* LE (e.g. modern) Config space accessors. */
+#define virtio_cread_le(vdev, structname, member, ptr) \
+ do { \
+ typeof(((structname*)0)->member) virtio_cread_v; \
+ \
+ might_sleep(); \
+ /* Sanity check: must match the member's type */ \
+ typecheck(typeof(virtio_le_to_cpu(virtio_cread_v)), *(ptr)); \
+ \
+ switch (sizeof(virtio_cread_v)) { \
+ case 1: \
+ case 2: \
+ case 4: \
+ vdev->config->get((vdev), \
+ offsetof(structname, member), \
+ &virtio_cread_v, \
+ sizeof(virtio_cread_v)); \
+ break; \
+ default: \
+ __virtio_cread_many((vdev), \
+ offsetof(structname, member), \
+ &virtio_cread_v, \
+ 1, \
+ sizeof(virtio_cread_v)); \
+ break; \
+ } \
+ *(ptr) = virtio_le_to_cpu(virtio_cread_v); \
+ } while(0)
+
+/* Config space accessors. */
+#define virtio_cwrite_le(vdev, structname, member, ptr) \
+ do { \
+ typeof(((structname*)0)->member) virtio_cwrite_v = \
+ virtio_cpu_to_le(*(ptr), ((structname*)0)->member); \
+ \
+ might_sleep(); \
+ /* Sanity check: must match the member's type */ \
+ typecheck(typeof(virtio_le_to_cpu(virtio_cwrite_v)), *(ptr)); \
+ \
+ vdev->config->set((vdev), offsetof(structname, member), \
+ &virtio_cwrite_v, \
+ sizeof(virtio_cwrite_v)); \
+ } while(0)
+
+
/* Read @count fields, @bytes each. */
static inline void __virtio_cread_many(struct virtio_device *vdev,
unsigned int offset,
--
MST

2020-08-05 19:31:32

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 28/38] virtio_config: add virtio_cread_le_feature

Mirrors virtio_cread_feature but for LE fields.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/linux/virtio_config.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 5b5196fec899..cc7a2b1fd7b2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -555,4 +555,14 @@ static inline void virtio_cwrite64(struct virtio_device *vdev,
_r; \
})

+/* Conditional config space accessors. */
+#define virtio_cread_le_feature(vdev, fbit, structname, member, ptr) \
+ ({ \
+ int _r = 0; \
+ if (!virtio_has_feature(vdev, fbit)) \
+ _r = -ENOENT; \
+ else \
+ virtio_cread_le((vdev), structname, member, ptr); \
+ _r; \
+ })
#endif /* _LINUX_VIRTIO_CONFIG_H */
--
MST

2020-08-05 19:31:32

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 34/38] drm/virtio: convert to LE accessors

Virtgpu is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 0a5c8cf409fb..4d944a0dff3e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -39,8 +39,8 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
u32 events_read, events_clear = 0;

/* read the config space */
- virtio_cread(vgdev->vdev, struct virtio_gpu_config,
- events_read, &events_read);
+ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+ events_read, &events_read);
if (events_read & VIRTIO_GPU_EVENT_DISPLAY) {
if (vgdev->has_edid)
virtio_gpu_cmd_get_edids(vgdev);
@@ -49,8 +49,8 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
drm_helper_hpd_irq_event(vgdev->ddev);
events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
}
- virtio_cwrite(vgdev->vdev, struct virtio_gpu_config,
- events_clear, &events_clear);
+ virtio_cwrite_le(vgdev->vdev, struct virtio_gpu_config,
+ events_clear, &events_clear);
}

static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
@@ -165,8 +165,8 @@ int virtio_gpu_init(struct drm_device *dev)
}

/* get display info */
- virtio_cread(vgdev->vdev, struct virtio_gpu_config,
- num_scanouts, &num_scanouts);
+ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+ num_scanouts, &num_scanouts);
vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
VIRTIO_GPU_MAX_SCANOUTS);
if (!vgdev->num_scanouts) {
@@ -176,8 +176,8 @@ int virtio_gpu_init(struct drm_device *dev)
}
DRM_INFO("number of scanouts: %d\n", num_scanouts);

- virtio_cread(vgdev->vdev, struct virtio_gpu_config,
- num_capsets, &num_capsets);
+ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+ num_capsets, &num_capsets);
DRM_INFO("number of cap sets: %d\n", num_capsets);

virtio_gpu_modeset_init(vgdev);
--
MST

2020-08-05 19:32:05

by Michael S. Tsirkin

[permalink] [raw]
Subject: [PATCH v3 18/38] mlxbf-tmfifo: sparse tags for config access

mlxbf-tmfifo accesses config space using native types -
which works for it since the legacy virtio native types.

This will break if it ever needs to support modern virtio,
so with new tags previously introduced for virtio net config,
sparse now warns for this in drivers.

Since this is a legacy only device, fix it up using
virtio_legacy_is_little_endian for now.

No functional changes.

Signed-off-by: Michael S. Tsirkin <[email protected]>
Acked-by: Cornelia Huck <[email protected]>
---
drivers/platform/mellanox/mlxbf-tmfifo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c b/drivers/platform/mellanox/mlxbf-tmfifo.c
index 5739a9669b29..bbc4e71a16ff 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -625,7 +625,10 @@ static void mlxbf_tmfifo_rxtx_header(struct mlxbf_tmfifo_vring *vring,
vdev_id = VIRTIO_ID_NET;
hdr_len = sizeof(struct virtio_net_hdr);
config = &fifo->vdev[vdev_id]->config.net;
- if (ntohs(hdr.len) > config->mtu +
+ /* A legacy-only interface for now. */
+ if (ntohs(hdr.len) >
+ __virtio16_to_cpu(virtio_legacy_is_little_endian(),
+ config->mtu) +
MLXBF_TMFIFO_NET_L2_OVERHEAD)
return;
} else {
@@ -1231,8 +1234,12 @@ static int mlxbf_tmfifo_probe(struct platform_device *pdev)

/* Create the network vdev. */
memset(&net_config, 0, sizeof(net_config));
- net_config.mtu = ETH_DATA_LEN;
- net_config.status = VIRTIO_NET_S_LINK_UP;
+
+ /* A legacy-only interface for now. */
+ net_config.mtu = __cpu_to_virtio16(virtio_legacy_is_little_endian(),
+ ETH_DATA_LEN);
+ net_config.status = __cpu_to_virtio16(virtio_legacy_is_little_endian(),
+ VIRTIO_NET_S_LINK_UP);
mlxbf_tmfifo_get_cfg_mac(net_config.mac);
rc = mlxbf_tmfifo_create_vdev(dev, fifo, VIRTIO_ID_NET,
MLXBF_TMFIFO_NET_FEATURES, &net_config,
--
MST

2020-08-05 20:02:42

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH v3 06/38] virtio_blk: correct tags for config space fields

On Wed, Aug 05, 2020 at 09:43:30AM -0400, Michael S. Tsirkin wrote:
> Tag config space fields as having virtio endian-ness.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>
> ---
> include/uapi/linux/virtio_blk.h | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)


Reviewed-by: Stefano Garzarella <[email protected]>


>
> diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
> index 0f99d7b49ede..d888f013d9ff 100644
> --- a/include/uapi/linux/virtio_blk.h
> +++ b/include/uapi/linux/virtio_blk.h
> @@ -57,20 +57,20 @@
>
> struct virtio_blk_config {
> /* The capacity (in 512-byte sectors). */
> - __u64 capacity;
> + __virtio64 capacity;
> /* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
> - __u32 size_max;
> + __virtio32 size_max;
> /* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
> - __u32 seg_max;
> + __virtio32 seg_max;
> /* geometry of the device (if VIRTIO_BLK_F_GEOMETRY) */
> struct virtio_blk_geometry {
> - __u16 cylinders;
> + __virtio16 cylinders;
> __u8 heads;
> __u8 sectors;
> } geometry;
>
> /* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
> - __u32 blk_size;
> + __virtio32 blk_size;
>
> /* the next 4 entries are guarded by VIRTIO_BLK_F_TOPOLOGY */
> /* exponent for physical block per logical block. */
> @@ -78,42 +78,42 @@ struct virtio_blk_config {
> /* alignment offset in logical blocks. */
> __u8 alignment_offset;
> /* minimum I/O size without performance penalty in logical blocks. */
> - __u16 min_io_size;
> + __virtio16 min_io_size;
> /* optimal sustained I/O size in logical blocks. */
> - __u32 opt_io_size;
> + __virtio32 opt_io_size;
>
> /* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
> __u8 wce;
> __u8 unused;
>
> /* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
> - __u16 num_queues;
> + __virtio16 num_queues;
>
> /* the next 3 entries are guarded by VIRTIO_BLK_F_DISCARD */
> /*
> * The maximum discard sectors (in 512-byte sectors) for
> * one segment.
> */
> - __u32 max_discard_sectors;
> + __virtio32 max_discard_sectors;
> /*
> * The maximum number of discard segments in a
> * discard command.
> */
> - __u32 max_discard_seg;
> + __virtio32 max_discard_seg;
> /* Discard commands must be aligned to this number of sectors. */
> - __u32 discard_sector_alignment;
> + __virtio32 discard_sector_alignment;
>
> /* the next 3 entries are guarded by VIRTIO_BLK_F_WRITE_ZEROES */
> /*
> * The maximum number of write zeroes sectors (in 512-byte sectors) in
> * one segment.
> */
> - __u32 max_write_zeroes_sectors;
> + __virtio32 max_write_zeroes_sectors;
> /*
> * The maximum number of segments in a write zeroes
> * command.
> */
> - __u32 max_write_zeroes_seg;
> + __virtio32 max_write_zeroes_seg;
> /*
> * Set if a VIRTIO_BLK_T_WRITE_ZEROES request may result in the
> * deallocation of one or more of the sectors.
> --
> MST
>
> _______________________________________________
> Virtualization mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>

2020-08-05 20:06:50

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 18/38] mlxbf-tmfifo: sparse tags for config access

On Wed, Aug 5, 2020 at 4:44 PM Michael S. Tsirkin <[email protected]> wrote:
>
> mlxbf-tmfifo accesses config space using native types -
> which works for it since the legacy virtio native types.
>
> This will break if it ever needs to support modern virtio,
> so with new tags previously introduced for virtio net config,
> sparse now warns for this in drivers.
>
> Since this is a legacy only device, fix it up using
> virtio_legacy_is_little_endian for now.
>
> No functional changes.
>

Acked-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Michael S. Tsirkin <[email protected]>
> Acked-by: Cornelia Huck <[email protected]>
> ---
> drivers/platform/mellanox/mlxbf-tmfifo.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c b/drivers/platform/mellanox/mlxbf-tmfifo.c
> index 5739a9669b29..bbc4e71a16ff 100644
> --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> @@ -625,7 +625,10 @@ static void mlxbf_tmfifo_rxtx_header(struct mlxbf_tmfifo_vring *vring,
> vdev_id = VIRTIO_ID_NET;
> hdr_len = sizeof(struct virtio_net_hdr);
> config = &fifo->vdev[vdev_id]->config.net;
> - if (ntohs(hdr.len) > config->mtu +
> + /* A legacy-only interface for now. */
> + if (ntohs(hdr.len) >
> + __virtio16_to_cpu(virtio_legacy_is_little_endian(),
> + config->mtu) +
> MLXBF_TMFIFO_NET_L2_OVERHEAD)
> return;
> } else {
> @@ -1231,8 +1234,12 @@ static int mlxbf_tmfifo_probe(struct platform_device *pdev)
>
> /* Create the network vdev. */
> memset(&net_config, 0, sizeof(net_config));
> - net_config.mtu = ETH_DATA_LEN;
> - net_config.status = VIRTIO_NET_S_LINK_UP;
> +
> + /* A legacy-only interface for now. */
> + net_config.mtu = __cpu_to_virtio16(virtio_legacy_is_little_endian(),
> + ETH_DATA_LEN);
> + net_config.status = __cpu_to_virtio16(virtio_legacy_is_little_endian(),
> + VIRTIO_NET_S_LINK_UP);
> mlxbf_tmfifo_get_cfg_mac(net_config.mac);
> rc = mlxbf_tmfifo_create_vdev(dev, fifo, VIRTIO_ID_NET,
> MLXBF_TMFIFO_NET_FEATURES, &net_config,
> --
> MST
>


--
With Best Regards,
Andy Shevchenko

2020-08-07 11:23:34

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH v3 10/38] virtio_gpu: correct tags for config space fields

On Wed, Aug 05, 2020 at 09:43:42AM -0400, Michael S. Tsirkin wrote:
> Since gpu is a modern-only device,
> tag config space fields as having little endian-ness.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> Reviewed-by: Cornelia Huck <[email protected]>

Reviewed-by: Gerd Hoffmann <[email protected]>

2020-08-07 11:24:43

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH v3 34/38] drm/virtio: convert to LE accessors

On Wed, Aug 05, 2020 at 09:44:48AM -0400, Michael S. Tsirkin wrote:
> Virtgpu is modern-only. Use LE accessors for config space.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>

Reviewed-by: Gerd Hoffmann <[email protected]>

2020-08-07 20:06:46

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v3 33/38] virtio_pmem: convert to LE accessors

On Wed, Aug 05, 2020 at 09:44:45AM -0400, Michael S. Tsirkin wrote:
> Virtio pmem is modern-only. Use LE accessors for config space.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> ---
> drivers/nvdimm/virtio_pmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> index 5e3d07b47e0c..726c7354d465 100644
> --- a/drivers/nvdimm/virtio_pmem.c
> +++ b/drivers/nvdimm/virtio_pmem.c
> @@ -58,9 +58,9 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
> goto out_err;
> }
>
> - virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + virtio_cread_le(vpmem->vdev, struct virtio_pmem_config,
> start, &vpmem->start);
> - virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + virtio_cread_le(vpmem->vdev, struct virtio_pmem_config,
> size, &vpmem->size);

FWIW I think squashing patch 15/38 and this patch would have made more sense.

Acked-by: Ira Weiny <[email protected]>

>
> res.start = vpmem->start;
> --
> MST
>

2020-08-10 07:58:55

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v3 35/38] virtio_mem: convert to LE accessors

On 05.08.20 15:44, Michael S. Tsirkin wrote:
> Virtio mem is modern-only. Use LE accessors for config space.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> ---
> drivers/virtio/virtio_mem.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index f26f5f64ae82..c08512fcea90 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -1530,21 +1530,21 @@ static void virtio_mem_refresh_config(struct virtio_mem *vm)
> uint64_t new_plugged_size, usable_region_size, end_addr;
>
> /* the plugged_size is just a reflection of what _we_ did previously */
> - virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
> - &new_plugged_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
> + &new_plugged_size);
> if (WARN_ON_ONCE(new_plugged_size != vm->plugged_size))
> vm->plugged_size = new_plugged_size;
>
> /* calculate the last usable memory block id */
> - virtio_cread(vm->vdev, struct virtio_mem_config,
> - usable_region_size, &usable_region_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config,
> + usable_region_size, &usable_region_size);
> end_addr = vm->addr + usable_region_size;
> end_addr = min(end_addr, phys_limit);
> vm->last_usable_mb_id = virtio_mem_phys_to_mb_id(end_addr) - 1;
>
> /* see if there is a request to change the size */
> - virtio_cread(vm->vdev, struct virtio_mem_config, requested_size,
> - &vm->requested_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, requested_size,
> + &vm->requested_size);
>
> dev_info(&vm->vdev->dev, "plugged size: 0x%llx", vm->plugged_size);
> dev_info(&vm->vdev->dev, "requested size: 0x%llx", vm->requested_size);
> @@ -1677,16 +1677,16 @@ static int virtio_mem_init(struct virtio_mem *vm)
> }
>
> /* Fetch all properties that can't change. */
> - virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
> - &vm->plugged_size);
> - virtio_cread(vm->vdev, struct virtio_mem_config, block_size,
> - &vm->device_block_size);
> - virtio_cread(vm->vdev, struct virtio_mem_config, node_id,
> - &node_id);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
> + &vm->plugged_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
> + &vm->device_block_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
> + &node_id);
> vm->nid = virtio_mem_translate_node_id(vm, node_id);
> - virtio_cread(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
> - virtio_cread(vm->vdev, struct virtio_mem_config, region_size,
> - &vm->region_size);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, addr, &vm->addr);
> + virtio_cread_le(vm->vdev, struct virtio_mem_config, region_size,
> + &vm->region_size);
>
> /*
> * We always hotplug memory in memory block granularity. This way,
>

Acked-by: David Hildenbrand <[email protected]>

--
Thanks,

David / dhildenb

2020-08-19 06:03:02

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH v3 30/38] virtio_input: convert to LE accessors

On Wed, Aug 05, 2020 at 09:44:36AM -0400, Michael S. Tsirkin wrote:
> Virtio input is modern-only. Use LE accessors for config space.
>
> Signed-off-by: Michael S. Tsirkin <[email protected]>

Reviewed-by: Gerd Hoffmann <[email protected]>