Virtio 1.0 doesn't include a modern balloon device. At some point we'll likely define an incompatible interface with a different ID. But for now, it's not a big change to support a transitional balloon device: this has the advantage of supporting existing drivers, transparently. The only issue is with the stats buffer, which has misaligned fields. Seems easy to fix by prepending a 6 byte reserved field. I'll post spec patch and qemu patches shortly. Michael S. Tsirkin (6): virtio_balloon: transitional interface virtio: balloon might not be a legacy device virtio_ccw: support non-legacy balloon devices virtio_mmio: support non-legacy balloon devices virtio_pci: support non-legacy balloon devices virtio: drop virtio_device_is_legacy_only include/linux/virtio.h | 2 -- include/uapi/linux/virtio_balloon.h | 6 ++++++ drivers/s390/kvm/virtio_ccw.c | 10 +++------- drivers/virtio/virtio.c | 6 ------ drivers/virtio/virtio_balloon.c | 8 ++++++-- drivers/virtio/virtio_mmio.c | 8 -------- drivers/virtio/virtio_pci_modern.c | 3 --- 7 files changed, 15 insertions(+), 28 deletions(-) -- MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 1/6] virtio_balloon: transitional interface
Virtio 1.0 doesn't include a modern balloon device.
But it's not a big change to support a transitional
balloon device: this has the advantage of supporting
existing drivers, transparently.
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
include/uapi/linux/virtio_balloon.h | 6 ++++++
drivers/virtio/virtio_balloon.c | 8 ++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4b0488f..5d83902 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -56,4 +56,10 @@ struct virtio_balloon_stat {
__u64 val;
} __attribute__((packed));
+struct virtio_balloon_stat_modern {
+ __u8 reserved[6];
+ __u16 tag;
+ __u64 val;
+};
+
#endif /* _LINUX_VIRTIO_BALLOON_H */
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 6a356e3..68e937f 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -77,7 +77,7 @@ struct virtio_balloon {
/* Memory statistics */
int need_stats_update;
- struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+ struct virtio_balloon_stat_modern stats[VIRTIO_BALLOON_S_NR];
/* To register callback in oom notifier call chain */
struct notifier_block nb;
@@ -269,7 +269,11 @@ static void stats_handle_request(struct virtio_balloon *vb)
vq = vb->stats_vq;
if (!virtqueue_get_buf(vq, &len))
return;
- sg_init_one(&sg, vb->stats, sizeof(vb->stats));
+ if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+ sg_init_one(&sg, vb->stats, sizeof(vb->stats));
+ else
+ sg_init_one(&sg, &vb->stats->tag, sizeof(vb->stats) -
+ offsetof(typeof(*vb->stats, tag);
virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
virtqueue_kick(vq);
}
--
MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 2/6] virtio: balloon might not be a legacy device
We added transitional device support to balloon driver,
so we don't need to black-list it in core anymore.
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
drivers/virtio/virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 5ce2aa4..5fa67b5 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -280,7 +280,7 @@ static struct bus_type virtio_bus = {
bool virtio_device_is_legacy_only(struct virtio_device_id id)
{
- return id.device == VIRTIO_ID_BALLOON;
+ return false;
}
EXPORT_SYMBOL_GPL(virtio_device_is_legacy_only);
--
MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 3/6] virtio_ccw: support non-legacy balloon devices
virtio_device_is_legacy_only is always false now,
drop the test from virtio ccw.
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
drivers/s390/kvm/virtio_ccw.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index 71d7802..6f1fa17 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -1201,13 +1201,9 @@ static int virtio_ccw_online(struct ccw_device *cdev)
vcdev->vdev.id.vendor = cdev->id.cu_type;
vcdev->vdev.id.device = cdev->id.cu_model;
- if (virtio_device_is_legacy_only(vcdev->vdev.id)) {
- vcdev->revision = 0;
- } else {
- ret = virtio_ccw_set_transport_rev(vcdev);
- if (ret)
- goto out_free;
- }
+ ret = virtio_ccw_set_transport_rev(vcdev);
+ if (ret)
+ goto out_free;
ret = register_virtio_device(&vcdev->vdev);
if (ret) {
--
MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 4/6] virtio_mmio: support non-legacy balloon devices
virtio_device_is_legacy_only is always false now,
drop the test from virtio mmio.
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
drivers/virtio/virtio_mmio.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 6010d7e..7a5e60d 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -581,14 +581,6 @@ static int virtio_mmio_probe(struct platform_device *pdev)
}
vm_dev->vdev.id.vendor = readl(vm_dev->base + VIRTIO_MMIO_VENDOR_ID);
- /* Reject legacy-only IDs for version 2 devices */
- if (vm_dev->version == 2 &&
- virtio_device_is_legacy_only(vm_dev->vdev.id)) {
- dev_err(&pdev->dev, "Version 2 not supported for devices
%u!\n",
- vm_dev->vdev.id.device);
- return -ENODEV;
- }
-
if (vm_dev->version == 1)
writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
--
MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 5/6] virtio_pci: support non-legacy balloon devices
virtio_device_is_legacy_only is always false now, drop the test from virtio pci. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/virtio/virtio_pci_modern.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 2aa38e5..dfea17a 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -577,9 +577,6 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev) } vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor; - if (virtio_device_is_legacy_only(vp_dev->vdev.id)) - return -ENODEV; - /* check for a common config: if not, use legacy mode (bar 0). */ common = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_COMMON_CFG, IORESOURCE_IO | IORESOURCE_MEM); -- MST
Michael S. Tsirkin
2015-Mar-30 17:37 UTC
[PATCH 6/6] virtio: drop virtio_device_is_legacy_only
virtio_device_is_legacy_only is now unused, drop
it from core.
Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
include/linux/virtio.h | 2 --
drivers/virtio/virtio.c | 6 ------
2 files changed, 8 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 28f0e65..8f4d4bf 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -108,8 +108,6 @@ struct virtio_device {
void *priv;
};
-bool virtio_device_is_legacy_only(struct virtio_device_id id);
-
static inline struct virtio_device *dev_to_virtio(struct device *_dev)
{
return container_of(_dev, struct virtio_device, dev);
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 5fa67b5..b1877d7 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -278,12 +278,6 @@ static struct bus_type virtio_bus = {
.remove = virtio_dev_remove,
};
-bool virtio_device_is_legacy_only(struct virtio_device_id id)
-{
- return false;
-}
-EXPORT_SYMBOL_GPL(virtio_device_is_legacy_only);
-
int register_virtio_driver(struct virtio_driver *driver)
{
/* Catch this early. */
--
MST
Michael S. Tsirkin
2015-Mar-30 17:55 UTC
[PATCH 1/6] virtio_balloon: transitional interface
On Mon, Mar 30, 2015 at 07:37:12PM +0200, Michael S. Tsirkin wrote:> Virtio 1.0 doesn't include a modern balloon device. > But it's not a big change to support a transitional > balloon device: this has the advantage of supporting > existing drivers, transparently. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > ---Hmm, re-reading the code, this won't be enough for BE guests. We are doing one extra byteswap there. I'll fix up and repost.> include/uapi/linux/virtio_balloon.h | 6 ++++++ > drivers/virtio/virtio_balloon.c | 8 ++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h > index 4b0488f..5d83902 100644 > --- a/include/uapi/linux/virtio_balloon.h > +++ b/include/uapi/linux/virtio_balloon.h > @@ -56,4 +56,10 @@ struct virtio_balloon_stat { > __u64 val; > } __attribute__((packed)); > > +struct virtio_balloon_stat_modern { > + __u8 reserved[6]; > + __u16 tag; > + __u64 val; > +}; > + > #endif /* _LINUX_VIRTIO_BALLOON_H */ > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 6a356e3..68e937f 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -77,7 +77,7 @@ struct virtio_balloon { > > /* Memory statistics */ > int need_stats_update; > - struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; > + struct virtio_balloon_stat_modern stats[VIRTIO_BALLOON_S_NR]; > > /* To register callback in oom notifier call chain */ > struct notifier_block nb; > @@ -269,7 +269,11 @@ static void stats_handle_request(struct virtio_balloon *vb) > vq = vb->stats_vq; > if (!virtqueue_get_buf(vq, &len)) > return; > - sg_init_one(&sg, vb->stats, sizeof(vb->stats)); > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) > + sg_init_one(&sg, vb->stats, sizeof(vb->stats)); > + else > + sg_init_one(&sg, &vb->stats->tag, sizeof(vb->stats) - > + offsetof(typeof(*vb->stats, tag); > virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); > virtqueue_kick(vq); > } > -- > MST >
On Mon, 30 Mar 2015 19:37:08 +0200 "Michael S. Tsirkin" <mst at redhat.com> wrote:> Virtio 1.0 doesn't include a modern balloon device. > At some point we'll likely define an incompatible interface with a different > ID. But for now, it's not a big change to support a transitional balloon > device: this has the advantage of supporting existing drivers, transparently.This is needed to support a ballooner on transports that don't support mixing legacy and 1.0, right?> > The only issue is with the stats buffer, which has misaligned fields. > Seems easy to fix by prepending a 6 byte reserved field.I assume you also define the stats fields to be le for 1.0?> > I'll post spec patch and qemu patches shortly.I'll defer review until then.> > Michael S. Tsirkin (6): > virtio_balloon: transitional interface > virtio: balloon might not be a legacy device > virtio_ccw: support non-legacy balloon devices > virtio_mmio: support non-legacy balloon devices > virtio_pci: support non-legacy balloon devices > virtio: drop virtio_device_is_legacy_only > > include/linux/virtio.h | 2 -- > include/uapi/linux/virtio_balloon.h | 6 ++++++ > drivers/s390/kvm/virtio_ccw.c | 10 +++------- > drivers/virtio/virtio.c | 6 ------ > drivers/virtio/virtio_balloon.c | 8 ++++++-- > drivers/virtio/virtio_mmio.c | 8 -------- > drivers/virtio/virtio_pci_modern.c | 3 --- > 7 files changed, 15 insertions(+), 28 deletions(-) >
On Tue, Mar 31, 2015 at 10:46:18AM +0200, Cornelia Huck wrote:> On Mon, 30 Mar 2015 19:37:08 +0200 > "Michael S. Tsirkin" <mst at redhat.com> wrote: > > > Virtio 1.0 doesn't include a modern balloon device. > > At some point we'll likely define an incompatible interface with a different > > ID. But for now, it's not a big change to support a transitional balloon > > device: this has the advantage of supporting existing drivers, transparently. > > This is needed to support a ballooner on transports that don't support > mixing legacy and 1.0, right?That too. But it's also useful if you don't know whether your guest supports virtio 1 or not - which is the point of using a transitional device.> > > > The only issue is with the stats buffer, which has misaligned fields. > > Seems easy to fix by prepending a 6 byte reserved field. > > I assume you also define the stats fields to be le for 1.0?They were always le for the balloon.> > > > I'll post spec patch and qemu patches shortly. > > I'll defer review until then. > > > > > Michael S. Tsirkin (6): > > virtio_balloon: transitional interface > > virtio: balloon might not be a legacy device > > virtio_ccw: support non-legacy balloon devices > > virtio_mmio: support non-legacy balloon devices > > virtio_pci: support non-legacy balloon devices > > virtio: drop virtio_device_is_legacy_only > > > > include/linux/virtio.h | 2 -- > > include/uapi/linux/virtio_balloon.h | 6 ++++++ > > drivers/s390/kvm/virtio_ccw.c | 10 +++------- > > drivers/virtio/virtio.c | 6 ------ > > drivers/virtio/virtio_balloon.c | 8 ++++++-- > > drivers/virtio/virtio_mmio.c | 8 -------- > > drivers/virtio/virtio_pci_modern.c | 3 --- > > 7 files changed, 15 insertions(+), 28 deletions(-) > >
Pawel Moll
2015-Mar-31 12:31 UTC
[PATCH 4/6] virtio_mmio: support non-legacy balloon devices
On Mon, 2015-03-30 at 18:37 +0100, Michael S. Tsirkin wrote:> virtio_device_is_legacy_only is always false now, > drop the test from virtio mmio. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>Slightly ironic ack ;-) after all the battle you fought for this: Acked-by: Pawel Moll <pawel.moll at arm.com> Thanks! Pawel