Michael S. Tsirkin
2015-Mar-31 19:52 UTC
[PATCH v2 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 | 11 +++++++++-- drivers/virtio/virtio_balloon.c | 29 +++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h index 4b0488f..71ef93b 100644 --- a/include/uapi/linux/virtio_balloon.h +++ b/include/uapi/linux/virtio_balloon.h @@ -38,9 +38,9 @@ struct virtio_balloon_config { /* Number of pages host wants Guest to give up. */ - __le32 num_pages; + __u32 num_pages; /* Number of pages we've actually got in balloon. */ - __le32 actual; + __u32 actual; }; #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */ @@ -51,9 +51,16 @@ struct virtio_balloon_config { #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */ #define VIRTIO_BALLOON_S_NR 6 +/* Legacy stat structure. We keep it around to avoid breaking old userspace. */ struct virtio_balloon_stat { __u16 tag; __u64 val; } __attribute__((packed)); +struct virtio_balloon_stat_modern { + __u8 reserved[6]; + __virtio16 tag; + __virtio64 val; +}; + #endif /* _LINUX_VIRTIO_BALLOON_H */ diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 6a356e3..574267f 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); } @@ -283,21 +287,30 @@ static void virtballoon_changed(struct virtio_device *vdev) static inline s64 towards_target(struct virtio_balloon *vb) { - __le32 v; s64 target; + u32 num_pages; - virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v); + virtio_cread(vb->vdev, struct virtio_balloon_config, + num_pages, &num_pages); - target = le32_to_cpu(v); + /* 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); + + target = num_pages; return target - vb->num_pages; } static void update_balloon_size(struct virtio_balloon *vb) { - __le32 actual = cpu_to_le32(vb->num_pages); + 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(num_pages); - virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual, - &actual); + virtio_cwrite(vb->vdev, struct virtio_balloon_config, + actual, &actual); } /* -- MST
"Michael S. Tsirkin" <mst at redhat.com> writes:> 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.You decided to fix the packed struct...> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 6a356e3..574267f 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);This makes it compile, but definitely won't work.> virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); > virtqueue_kick(vq); > } > @@ -283,21 +287,30 @@ static void virtballoon_changed(struct virtio_device *vdev) > > static inline s64 towards_target(struct virtio_balloon *vb) > { > - __le32 v; > s64 target; > + u32 num_pages; > > - virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v); > + virtio_cread(vb->vdev, struct virtio_balloon_config, > + num_pages, &num_pages); > > - target = le32_to_cpu(v); > + /* 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); > + > + target = num_pages; > return target - vb->num_pages; > } > > static void update_balloon_size(struct virtio_balloon *vb) > { > - __le32 actual = cpu_to_le32(vb->num_pages); > + 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(num_pages); > > - virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual, > - &actual); > + virtio_cwrite(vb->vdev, struct virtio_balloon_config, > + actual, &actual); > }Final line is gratitous reformatting. I would leave the device *exactly* as is, ugly structure packing and all. Cheers, Rusty.
Michael S. Tsirkin
2015-Apr-01 07:44 UTC
[PATCH v2 1/6] virtio_balloon: transitional interface
On Wed, Apr 01, 2015 at 02:17:23PM +1030, Rusty Russell wrote:> I would leave the device *exactly* as is, ugly structure packing and > all.But why? It's going to be used for years, might as well make it clean? -- MST
Possibly Parallel Threads
- [PATCH v2 1/6] virtio_balloon: transitional interface
- [PATCH v2 1/6] virtio_balloon: transitional interface
- [PATCH v3 1/6] virtio_balloon: transitional interface
- [PATCH v3 1/6] virtio_balloon: transitional interface
- [PATCH v3 0/6] virtio_balloon: virtio 1 support