Michael S. Tsirkin
2014-Nov-25 16:41 UTC
[PATCH v4 02/42] virtio: add support for 64 bit features.
From: Rusty Russell <rusty at rustcorp.com.au>
Change the u32 to a u64, and make sure to use 1ULL everywhere!
Cc: Brian Swetland <swetland at google.com>
Cc: Christian Borntraeger <borntraeger at de.ibm.com>
[Thomas Huth: fix up virtio-ccw get_features]
Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>
Acked-by: Pawel Moll <pawel.moll at arm.com>
Acked-by: Ohad Ben-Cohen <ohad at wizery.com>
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
include/linux/virtio.h | 2 +-
include/linux/virtio_config.h | 8 ++++----
tools/virtio/linux/virtio.h | 2 +-
tools/virtio/linux/virtio_config.h | 2 +-
drivers/char/virtio_console.c | 2 +-
drivers/lguest/lguest_device.c | 10 +++++-----
drivers/misc/mic/card/mic_virtio.c | 10 +++++-----
drivers/remoteproc/remoteproc_virtio.c | 5 ++++-
drivers/s390/kvm/kvm_virtio.c | 10 +++++-----
drivers/s390/kvm/virtio_ccw.c | 29 ++++++++++++++++++++++++-----
drivers/virtio/virtio.c | 12 ++++++------
drivers/virtio/virtio_mmio.c | 14 +++++++++-----
drivers/virtio/virtio_pci.c | 5 ++---
drivers/virtio/virtio_ring.c | 2 +-
14 files changed, 69 insertions(+), 44 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7828a7f..149284e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,7 +101,7 @@ struct virtio_device {
const struct virtio_config_ops *config;
const struct vringh_config_ops *vringh_config;
struct list_head vqs;
- u32 features;
+ u64 features;
void *priv;
};
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index aa84d0e..022d904 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -66,7 +66,7 @@ struct virtio_config_ops {
vq_callback_t *callbacks[],
const char *names[]);
void (*del_vqs)(struct virtio_device *);
- u32 (*get_features)(struct virtio_device *vdev);
+ u64 (*get_features)(struct virtio_device *vdev);
void (*finalize_features)(struct virtio_device *vdev);
const char *(*bus_name)(struct virtio_device *vdev);
int (*set_vq_affinity)(struct virtqueue *vq, int cpu);
@@ -86,14 +86,14 @@ static inline bool virtio_has_feature(const struct
virtio_device *vdev,
{
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
- BUILD_BUG_ON(fbit >= 32);
+ BUILD_BUG_ON(fbit >= 64);
else
- BUG_ON(fbit >= 32);
+ BUG_ON(fbit >= 64);
if (fbit < VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
- return vdev->features & (1 << fbit);
+ return vdev->features & (1ULL << fbit);
}
static inline
diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 72bff70..8eb6421 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -10,7 +10,7 @@
struct virtio_device {
void *dev;
- u32 features;
+ u64 features;
};
struct virtqueue {
diff --git a/tools/virtio/linux/virtio_config.h
b/tools/virtio/linux/virtio_config.h
index 1f1636b..a254c2b 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
#define VIRTIO_TRANSPORT_F_END 32
#define virtio_has_feature(dev, feature) \
- ((dev)->features & (1 << feature))
+ ((dev)->features & (1ULL << feature))
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 0074f9b..fda9a75 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device
*portdev)
*/
if (!portdev->vdev)
return 0;
- return portdev->vdev->features & (1 <<
VIRTIO_CONSOLE_F_MULTIPORT);
+ return portdev->vdev->features & (1ULL <<
VIRTIO_CONSOLE_F_MULTIPORT);
}
static DEFINE_SPINLOCK(dma_bufs_lock);
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index c831c47..4d29bcd 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -94,17 +94,17 @@ static unsigned desc_size(const struct lguest_device_desc
*desc)
}
/* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
{
unsigned int i;
- u32 features = 0;
+ u64 features = 0;
struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
u8 *in_features = lg_features(desc);
/* We do this the slow but generic way. */
- for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+ for (i = 0; i < min(desc->feature_len * 8, 64); i++)
if (in_features[i / 8] & (1 << (i % 8)))
- features |= (1 << i);
+ features |= (1ULL << i);
return features;
}
@@ -144,7 +144,7 @@ static void lg_finalize_features(struct virtio_device *vdev)
memset(out_features, 0, desc->feature_len);
bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8;
for (i = 0; i < bits; i++) {
- if (vdev->features & (1 << i))
+ if (vdev->features & (1ULL << i))
out_features[i / 8] |= (1 << (i % 8));
}
diff --git a/drivers/misc/mic/card/mic_virtio.c
b/drivers/misc/mic/card/mic_virtio.c
index 0acd564..6d94f04 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -68,10 +68,10 @@ static inline struct device *mic_dev(struct mic_vdev *mvdev)
}
/* This gets the device's feature bits. */
-static u32 mic_get_features(struct virtio_device *vdev)
+static u64 mic_get_features(struct virtio_device *vdev)
{
unsigned int i, bits;
- u32 features = 0;
+ u64 features = 0;
struct mic_device_desc __iomem *desc = to_micvdev(vdev)->desc;
u8 __iomem *in_features = mic_vq_features(desc);
int feature_len = ioread8(&desc->feature_len);
@@ -79,8 +79,8 @@ static u32 mic_get_features(struct virtio_device *vdev)
bits = min_t(unsigned, feature_len,
sizeof(vdev->features)) * 8;
for (i = 0; i < bits; i++)
- if (ioread8(&in_features[i / 8]) & (BIT(i % 8)))
- features |= BIT(i);
+ if (ioread8(&in_features[i / 8]) & (BIT_ULL(i % 8)))
+ features |= BIT_ULL(i);
return features;
}
@@ -101,7 +101,7 @@ static void mic_finalize_features(struct virtio_device
*vdev)
bits = min_t(unsigned, feature_len,
sizeof(vdev->features)) * 8;
for (i = 0; i < bits; i++) {
- if (vdev->features & BIT(bits))
+ if (vdev->features & BIT_ULL(bits))
iowrite8(ioread8(&out_features[i / 8]) | (1 << (i % 8)),
&out_features[i / 8]);
}
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index dafaf38..627737e 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -207,7 +207,7 @@ static void rproc_virtio_reset(struct virtio_device *vdev)
}
/* provide the vdev features as retrieved from the firmware */
-static u32 rproc_virtio_get_features(struct virtio_device *vdev)
+static u64 rproc_virtio_get_features(struct virtio_device *vdev)
{
struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
struct fw_rsc_vdev *rsc;
@@ -227,6 +227,9 @@ static void rproc_virtio_finalize_features(struct
virtio_device *vdev)
/* Give virtio_ring a chance to accept features */
vring_transport_features(vdev);
+ /* Make sure we don't have any features > 32 bits! */
+ BUG_ON((u32)vdev->features != vdev->features);
+
/*
* Remember the finalized features of our vdev, and provide it
* to the remote processor once it is powered on.
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index ac79a09..c78251d 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -80,16 +80,16 @@ static unsigned desc_size(const struct kvm_device_desc
*desc)
}
/* This gets the device's feature bits. */
-static u32 kvm_get_features(struct virtio_device *vdev)
+static u64 kvm_get_features(struct virtio_device *vdev)
{
unsigned int i;
- u32 features = 0;
+ u64 features = 0;
struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
u8 *in_features = kvm_vq_features(desc);
- for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+ for (i = 0; i < min(desc->feature_len * 8, 64); i++)
if (in_features[i / 8] & (1 << (i % 8)))
- features |= (1 << i);
+ features |= (1ULL << i);
return features;
}
@@ -106,7 +106,7 @@ static void kvm_finalize_features(struct virtio_device
*vdev)
memset(out_features, 0, desc->feature_len);
bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8;
for (i = 0; i < bits; i++) {
- if (vdev->features & (1 << i))
+ if (vdev->features & (1ULL << i))
out_features[i / 8] |= (1 << (i % 8));
}
}
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 1dbee95..abba04d 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -660,11 +660,12 @@ static void virtio_ccw_reset(struct virtio_device *vdev)
kfree(ccw);
}
-static u32 virtio_ccw_get_features(struct virtio_device *vdev)
+static u64 virtio_ccw_get_features(struct virtio_device *vdev)
{
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
struct virtio_feature_desc *features;
- int ret, rc;
+ int ret;
+ u64 rc;
struct ccw1 *ccw;
ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
@@ -677,7 +678,6 @@ static u32 virtio_ccw_get_features(struct virtio_device
*vdev)
goto out_free;
}
/* Read the feature bits from the host. */
- /* TODO: Features > 32 bits */
features->index = 0;
ccw->cmd_code = CCW_CMD_READ_FEAT;
ccw->flags = 0;
@@ -691,6 +691,16 @@ static u32 virtio_ccw_get_features(struct virtio_device
*vdev)
rc = le32_to_cpu(features->features);
+ /* Read second half feature bits from the host. */
+ features->index = 1;
+ ccw->cmd_code = CCW_CMD_READ_FEAT;
+ ccw->flags = 0;
+ ccw->count = sizeof(*features);
+ ccw->cda = (__u32)(unsigned long)features;
+ ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_FEAT);
+ if (ret == 0)
+ rc |= (u64)le32_to_cpu(features->features) << 32;
+
out_free:
kfree(features);
kfree(ccw);
@@ -715,8 +725,17 @@ static void virtio_ccw_finalize_features(struct
virtio_device *vdev)
vring_transport_features(vdev);
features->index = 0;
- features->features = cpu_to_le32(vdev->features);
- /* Write the feature bits to the host. */
+ features->features = cpu_to_le32((u32)vdev->features);
+ /* Write the first half of the feature bits to the host. */
+ ccw->cmd_code = CCW_CMD_WRITE_FEAT;
+ ccw->flags = 0;
+ ccw->count = sizeof(*features);
+ ccw->cda = (__u32)(unsigned long)features;
+ ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT);
+
+ features->index = 1;
+ features->features = cpu_to_le32(vdev->features >> 32);
+ /* Write the second half of the feature bits to the host. */
ccw->cmd_code = CCW_CMD_WRITE_FEAT;
ccw->flags = 0;
ccw->count = sizeof(*features);
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 7814b1f..d213567 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -159,7 +159,7 @@ static int virtio_dev_probe(struct device *_d)
int err, i;
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
- u32 device_features;
+ u64 device_features;
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -171,15 +171,15 @@ static int virtio_dev_probe(struct device *_d)
dev->features = 0;
for (i = 0; i < drv->feature_table_size; i++) {
unsigned int f = drv->feature_table[i];
- BUG_ON(f >= 32);
- if (device_features & (1 << f))
- dev->features |= (1 << f);
+ BUG_ON(f >= 64);
+ if (device_features & (1ULL << f))
+ dev->features |= (1ULL << f);
}
/* Transport features always preserved to pass to finalize_features. */
for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
- if (device_features & (1 << i))
- dev->features |= (1 << i);
+ if (device_features & (1ULL << i))
+ dev->features |= (1ULL << i);
dev->config->finalize_features(dev);
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index eb5b0e2..fd01c6d 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -142,14 +142,16 @@ struct virtio_mmio_vq_info {
/* Configuration interface */
-static u32 vm_get_features(struct virtio_device *vdev)
+static u64 vm_get_features(struct virtio_device *vdev)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ u64 features;
- /* TODO: Features > 32 bits */
writel(0, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL);
-
- return readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES);
+ features = readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES);
+ writel(1, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL);
+ features |= ((u64)readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES) <<
32);
+ return features;
}
static void vm_finalize_features(struct virtio_device *vdev)
@@ -160,7 +162,9 @@ static void vm_finalize_features(struct virtio_device *vdev)
vring_transport_features(vdev);
writel(0, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL);
- writel(vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES);
+ writel((u32)vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES);
+ writel(1, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL);
+ writel(vdev->features >> 32, vm_dev->base +
VIRTIO_MMIO_GUEST_FEATURES);
}
static void vm_get(struct virtio_device *vdev, unsigned offset,
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index ab95a4c..68c0711 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -102,12 +102,11 @@ static struct virtio_pci_device *to_vp_device(struct
virtio_device *vdev)
}
/* virtio config->get_features() implementation */
-static u32 vp_get_features(struct virtio_device *vdev)
+static u64 vp_get_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- /* When someone needs more than 32 feature bits, we'll need to
- * steal a bit to indicate that the rest are somewhere else. */
+ /* We only support 32 feature bits. */
return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
}
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 15a8a05..61a1fe1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -781,7 +781,7 @@ void vring_transport_features(struct virtio_device *vdev)
break;
default:
/* We don't understand this bit. */
- vdev->features &= ~(1 << i);
+ vdev->features &= ~(1ULL << i);
}
}
}
--
MST
On Tue, 25 Nov 2014 18:41:22 +0200 "Michael S. Tsirkin" <mst at redhat.com> wrote:> From: Rusty Russell <rusty at rustcorp.com.au> > > Change the u32 to a u64, and make sure to use 1ULL everywhere! > > Cc: Brian Swetland <swetland at google.com> > Cc: Christian Borntraeger <borntraeger at de.ibm.com> > [Thomas Huth: fix up virtio-ccw get_features] > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > Acked-by: Pawel Moll <pawel.moll at arm.com> > Acked-by: Ohad Ben-Cohen <ohad at wizery.com> > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > include/linux/virtio.h | 2 +- > include/linux/virtio_config.h | 8 ++++---- > tools/virtio/linux/virtio.h | 2 +- > tools/virtio/linux/virtio_config.h | 2 +- > drivers/char/virtio_console.c | 2 +- > drivers/lguest/lguest_device.c | 10 +++++----- > drivers/misc/mic/card/mic_virtio.c | 10 +++++----- > drivers/remoteproc/remoteproc_virtio.c | 5 ++++- > drivers/s390/kvm/kvm_virtio.c | 10 +++++----- > drivers/s390/kvm/virtio_ccw.c | 29 ++++++++++++++++++++++++----- > drivers/virtio/virtio.c | 12 ++++++------ > drivers/virtio/virtio_mmio.c | 14 +++++++++----- > drivers/virtio/virtio_pci.c | 5 ++--- > drivers/virtio/virtio_ring.c | 2 +- > 14 files changed, 69 insertions(+), 44 deletions(-) > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index 7828a7f..149284e 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -101,7 +101,7 @@ struct virtio_device { > const struct virtio_config_ops *config; > const struct vringh_config_ops *vringh_config; > struct list_head vqs; > - u32 features; > + u64 features; > void *priv; > }; > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > index aa84d0e..022d904 100644 > --- a/include/linux/virtio_config.h > +++ b/include/linux/virtio_config.h > @@ -66,7 +66,7 @@ struct virtio_config_ops { > vq_callback_t *callbacks[], > const char *names[]); > void (*del_vqs)(struct virtio_device *); > - u32 (*get_features)(struct virtio_device *vdev); > + u64 (*get_features)(struct virtio_device *vdev); > void (*finalize_features)(struct virtio_device *vdev); > const char *(*bus_name)(struct virtio_device *vdev); > int (*set_vq_affinity)(struct virtqueue *vq, int cpu); > @@ -86,14 +86,14 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, > { > /* Did you forget to fix assumptions on max features? */ > if (__builtin_constant_p(fbit)) > - BUILD_BUG_ON(fbit >= 32); > + BUILD_BUG_ON(fbit >= 64);While you're here, maybe you could derive the max value from the .features field ? - BUILD_BUG_ON(fbit >= 64); + BUILD_BUG_ON(fbit >= (sizeof(vdev->features) << 3)); Two other locations below.> else > - BUG_ON(fbit >= 32); > + BUG_ON(fbit >= 64); >Here...> if (fbit < VIRTIO_TRANSPORT_F_START) > virtio_check_driver_offered_feature(vdev, fbit); > > - return vdev->features & (1 << fbit); > + return vdev->features & (1ULL << fbit); > } > > static inline > diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h > index 72bff70..8eb6421 100644 > --- a/tools/virtio/linux/virtio.h > +++ b/tools/virtio/linux/virtio.h > @@ -10,7 +10,7 @@ > > struct virtio_device { > void *dev; > - u32 features; > + u64 features; > }; > > struct virtqueue { > diff --git a/tools/virtio/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h > index 1f1636b..a254c2b 100644 > --- a/tools/virtio/linux/virtio_config.h > +++ b/tools/virtio/linux/virtio_config.h > @@ -2,5 +2,5 @@ > #define VIRTIO_TRANSPORT_F_END 32 > > #define virtio_has_feature(dev, feature) \ > - ((dev)->features & (1 << feature)) > + ((dev)->features & (1ULL << feature)) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 0074f9b..fda9a75 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device *portdev) > */ > if (!portdev->vdev) > return 0; > - return portdev->vdev->features & (1 << VIRTIO_CONSOLE_F_MULTIPORT); > + return portdev->vdev->features & (1ULL << VIRTIO_CONSOLE_F_MULTIPORT); > } > > static DEFINE_SPINLOCK(dma_bufs_lock); > diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c > index c831c47..4d29bcd 100644 > --- a/drivers/lguest/lguest_device.c > +++ b/drivers/lguest/lguest_device.c > @@ -94,17 +94,17 @@ static unsigned desc_size(const struct lguest_device_desc *desc) > } > > /* This gets the device's feature bits. */ > -static u32 lg_get_features(struct virtio_device *vdev) > +static u64 lg_get_features(struct virtio_device *vdev) > { > unsigned int i; > - u32 features = 0; > + u64 features = 0; > struct lguest_device_desc *desc = to_lgdev(vdev)->desc; > u8 *in_features = lg_features(desc); > > /* We do this the slow but generic way. */ > - for (i = 0; i < min(desc->feature_len * 8, 32); i++) > + for (i = 0; i < min(desc->feature_len * 8, 64); i++) > if (in_features[i / 8] & (1 << (i % 8))) > - features |= (1 << i); > + features |= (1ULL << i); > > return features; > } > @@ -144,7 +144,7 @@ static void lg_finalize_features(struct virtio_device *vdev) > memset(out_features, 0, desc->feature_len); > bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8; > for (i = 0; i < bits; i++) { > - if (vdev->features & (1 << i)) > + if (vdev->features & (1ULL << i)) > out_features[i / 8] |= (1 << (i % 8)); > } > > diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c > index 0acd564..6d94f04 100644 > --- a/drivers/misc/mic/card/mic_virtio.c > +++ b/drivers/misc/mic/card/mic_virtio.c > @@ -68,10 +68,10 @@ static inline struct device *mic_dev(struct mic_vdev *mvdev) > } > > /* This gets the device's feature bits. */ > -static u32 mic_get_features(struct virtio_device *vdev) > +static u64 mic_get_features(struct virtio_device *vdev) > { > unsigned int i, bits; > - u32 features = 0; > + u64 features = 0; > struct mic_device_desc __iomem *desc = to_micvdev(vdev)->desc; > u8 __iomem *in_features = mic_vq_features(desc); > int feature_len = ioread8(&desc->feature_len); > @@ -79,8 +79,8 @@ static u32 mic_get_features(struct virtio_device *vdev) > bits = min_t(unsigned, feature_len, > sizeof(vdev->features)) * 8; > for (i = 0; i < bits; i++) > - if (ioread8(&in_features[i / 8]) & (BIT(i % 8))) > - features |= BIT(i); > + if (ioread8(&in_features[i / 8]) & (BIT_ULL(i % 8))) > + features |= BIT_ULL(i); > > return features; > } > @@ -101,7 +101,7 @@ static void mic_finalize_features(struct virtio_device *vdev) > bits = min_t(unsigned, feature_len, > sizeof(vdev->features)) * 8; > for (i = 0; i < bits; i++) { > - if (vdev->features & BIT(bits)) > + if (vdev->features & BIT_ULL(bits)) > iowrite8(ioread8(&out_features[i / 8]) | (1 << (i % 8)), > &out_features[i / 8]); > } > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c > index dafaf38..627737e 100644 > --- a/drivers/remoteproc/remoteproc_virtio.c > +++ b/drivers/remoteproc/remoteproc_virtio.c > @@ -207,7 +207,7 @@ static void rproc_virtio_reset(struct virtio_device *vdev) > } > > /* provide the vdev features as retrieved from the firmware */ > -static u32 rproc_virtio_get_features(struct virtio_device *vdev) > +static u64 rproc_virtio_get_features(struct virtio_device *vdev) > { > struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); > struct fw_rsc_vdev *rsc; > @@ -227,6 +227,9 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) > /* Give virtio_ring a chance to accept features */ > vring_transport_features(vdev); > > + /* Make sure we don't have any features > 32 bits! */ > + BUG_ON((u32)vdev->features != vdev->features); > + > /* > * Remember the finalized features of our vdev, and provide it > * to the remote processor once it is powered on. > diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c > index ac79a09..c78251d 100644 > --- a/drivers/s390/kvm/kvm_virtio.c > +++ b/drivers/s390/kvm/kvm_virtio.c > @@ -80,16 +80,16 @@ static unsigned desc_size(const struct kvm_device_desc *desc) > } > > /* This gets the device's feature bits. */ > -static u32 kvm_get_features(struct virtio_device *vdev) > +static u64 kvm_get_features(struct virtio_device *vdev) > { > unsigned int i; > - u32 features = 0; > + u64 features = 0; > struct kvm_device_desc *desc = to_kvmdev(vdev)->desc; > u8 *in_features = kvm_vq_features(desc); > > - for (i = 0; i < min(desc->feature_len * 8, 32); i++) > + for (i = 0; i < min(desc->feature_len * 8, 64); i++) > if (in_features[i / 8] & (1 << (i % 8))) > - features |= (1 << i); > + features |= (1ULL << i); > return features; > } > > @@ -106,7 +106,7 @@ static void kvm_finalize_features(struct virtio_device *vdev) > memset(out_features, 0, desc->feature_len); > bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8; > for (i = 0; i < bits; i++) { > - if (vdev->features & (1 << i)) > + if (vdev->features & (1ULL << i)) > out_features[i / 8] |= (1 << (i % 8)); > } > } > diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c > index 1dbee95..abba04d 100644 > --- a/drivers/s390/kvm/virtio_ccw.c > +++ b/drivers/s390/kvm/virtio_ccw.c > @@ -660,11 +660,12 @@ static void virtio_ccw_reset(struct virtio_device *vdev) > kfree(ccw); > } > > -static u32 virtio_ccw_get_features(struct virtio_device *vdev) > +static u64 virtio_ccw_get_features(struct virtio_device *vdev) > { > struct virtio_ccw_device *vcdev = to_vc_device(vdev); > struct virtio_feature_desc *features; > - int ret, rc; > + int ret; > + u64 rc; > struct ccw1 *ccw; > > ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL); > @@ -677,7 +678,6 @@ static u32 virtio_ccw_get_features(struct virtio_device *vdev) > goto out_free; > } > /* Read the feature bits from the host. */ > - /* TODO: Features > 32 bits */ > features->index = 0; > ccw->cmd_code = CCW_CMD_READ_FEAT; > ccw->flags = 0; > @@ -691,6 +691,16 @@ static u32 virtio_ccw_get_features(struct virtio_device *vdev) > > rc = le32_to_cpu(features->features); > > + /* Read second half feature bits from the host. */ > + features->index = 1; > + ccw->cmd_code = CCW_CMD_READ_FEAT; > + ccw->flags = 0; > + ccw->count = sizeof(*features); > + ccw->cda = (__u32)(unsigned long)features; > + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_FEAT); > + if (ret == 0) > + rc |= (u64)le32_to_cpu(features->features) << 32; > + > out_free: > kfree(features); > kfree(ccw); > @@ -715,8 +725,17 @@ static void virtio_ccw_finalize_features(struct virtio_device *vdev) > vring_transport_features(vdev); > > features->index = 0; > - features->features = cpu_to_le32(vdev->features); > - /* Write the feature bits to the host. */ > + features->features = cpu_to_le32((u32)vdev->features); > + /* Write the first half of the feature bits to the host. */ > + ccw->cmd_code = CCW_CMD_WRITE_FEAT; > + ccw->flags = 0; > + ccw->count = sizeof(*features); > + ccw->cda = (__u32)(unsigned long)features; > + ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT); > + > + features->index = 1; > + features->features = cpu_to_le32(vdev->features >> 32); > + /* Write the second half of the feature bits to the host. */ > ccw->cmd_code = CCW_CMD_WRITE_FEAT; > ccw->flags = 0; > ccw->count = sizeof(*features); > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > index 7814b1f..d213567 100644 > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -159,7 +159,7 @@ static int virtio_dev_probe(struct device *_d) > int err, i; > struct virtio_device *dev = dev_to_virtio(_d); > struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); > - u32 device_features; > + u64 device_features; > > /* We have a driver! */ > add_status(dev, VIRTIO_CONFIG_S_DRIVER); > @@ -171,15 +171,15 @@ static int virtio_dev_probe(struct device *_d) > dev->features = 0; > for (i = 0; i < drv->feature_table_size; i++) { > unsigned int f = drv->feature_table[i]; > - BUG_ON(f >= 32); > - if (device_features & (1 << f)) > - dev->features |= (1 << f); > + BUG_ON(f >= 64);and here.> + if (device_features & (1ULL << f)) > + dev->features |= (1ULL << f); > } > > /* Transport features always preserved to pass to finalize_features. */ > for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) > - if (device_features & (1 << i)) > - dev->features |= (1 << i); > + if (device_features & (1ULL << i)) > + dev->features |= (1ULL << i); > > dev->config->finalize_features(dev); > > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index eb5b0e2..fd01c6d 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -142,14 +142,16 @@ struct virtio_mmio_vq_info { > > /* Configuration interface */ > > -static u32 vm_get_features(struct virtio_device *vdev) > +static u64 vm_get_features(struct virtio_device *vdev) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > + u64 features; > > - /* TODO: Features > 32 bits */ > writel(0, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL); > - > - return readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES); > + features = readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES); > + writel(1, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL); > + features |= ((u64)readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES) << 32); > + return features; > } > > static void vm_finalize_features(struct virtio_device *vdev) > @@ -160,7 +162,9 @@ static void vm_finalize_features(struct virtio_device *vdev) > vring_transport_features(vdev); > > writel(0, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL); > - writel(vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); > + writel((u32)vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); > + writel(1, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL); > + writel(vdev->features >> 32, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); > } > > static void vm_get(struct virtio_device *vdev, unsigned offset, > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index ab95a4c..68c0711 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -102,12 +102,11 @@ static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev) > } > > /* virtio config->get_features() implementation */ > -static u32 vp_get_features(struct virtio_device *vdev) > +static u64 vp_get_features(struct virtio_device *vdev) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > - /* When someone needs more than 32 feature bits, we'll need to > - * steal a bit to indicate that the rest are somewhere else. */ > + /* We only support 32 feature bits. */ > return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES); > } > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 15a8a05..61a1fe1 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -781,7 +781,7 @@ void vring_transport_features(struct virtio_device *vdev) > break; > default: > /* We don't understand this bit. */ > - vdev->features &= ~(1 << i); > + vdev->features &= ~(1ULL << i); > } > } > }
Michael S. Tsirkin
2014-Nov-26 16:56 UTC
[PATCH v4 02/42] virtio: add support for 64 bit features.
On Wed, Nov 26, 2014 at 05:48:09PM +0100, Greg Kurz wrote:> On Tue, 25 Nov 2014 18:41:22 +0200 > "Michael S. Tsirkin" <mst at redhat.com> wrote: > > > From: Rusty Russell <rusty at rustcorp.com.au> > > > > Change the u32 to a u64, and make sure to use 1ULL everywhere! > > > > Cc: Brian Swetland <swetland at google.com> > > Cc: Christian Borntraeger <borntraeger at de.ibm.com> > > [Thomas Huth: fix up virtio-ccw get_features] > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > > Acked-by: Pawel Moll <pawel.moll at arm.com> > > Acked-by: Ohad Ben-Cohen <ohad at wizery.com> > > > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>...> > @@ -86,14 +86,14 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, > > { > > /* Did you forget to fix assumptions on max features? */ > > if (__builtin_constant_p(fbit)) > > - BUILD_BUG_ON(fbit >= 32); > > + BUILD_BUG_ON(fbit >= 64); > > > While you're here, maybe you could derive the max value from the .features field ? > > - BUILD_BUG_ON(fbit >= 64); > + BUILD_BUG_ON(fbit >= (sizeof(vdev->features) << 3));I don't see how that will help. All that 1ULL math only works up to 64 bit. So this only makes it look like we support any size, but doesn't really. No?
Possibly Parallel Threads
- [PATCH v4 02/42] virtio: add support for 64 bit features.
- [PATCH v4 02/42] virtio: add support for 64 bit features.
- [PATCH RFC 02/11] virtio: add support for 64 bit features.
- [PATCH v3 02/41] virtio: add support for 64 bit features.
- [PATCH v3 02/41] virtio: add support for 64 bit features.