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
Apparently Analagous Threads
- [PATCH 4/6] virtio_mmio: support non-legacy balloon devices
- [PATCH v2 4/6] virtio_mmio: support non-legacy balloon devices
- [PATCH v2 4/6] virtio_mmio: support non-legacy balloon devices
- [PATCH v3 4/6] virtio_mmio: support non-legacy balloon devices
- [PATCH v3 4/6] virtio_mmio: support non-legacy balloon devices