| Message ID | 1465813009-21390-5-git-send-email-liang.z.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 13, 2016 at 06:16:46PM +0800, Liang Li wrote: > Add a new feature to get the free page information from guest, > the free page information is saved in a bitmap. Please note that > 'free page' only means these pages are free before the request, > some of the pages will become no free during the process of > sending the free page bitmap to QEMU. > > Signed-off-by: Liang Li <liang.z.li@intel.com> I don't much like this request interface. The meaning of free page is rather fuzzy too - so at what point are they free? My suggestion would be: report free page request ID to guest include request ID when guest sends free page list the definition is then: page was free sometime after host set this value of request ID and before it received response with the same ID > --- > balloon.c | 24 +++++++++++- > hw/virtio/virtio-balloon.c | 75 +++++++++++++++++++++++++++++++++++++- > include/hw/virtio/virtio-balloon.h | 4 ++ > include/sysemu/balloon.h | 8 ++++ > 4 files changed, 108 insertions(+), 3 deletions(-) > > diff --git a/balloon.c b/balloon.c > index 3d96111..c74c472 100644 > --- a/balloon.c > +++ b/balloon.c > @@ -37,6 +37,7 @@ > static QEMUBalloonEvent *balloon_event_fn; > static QEMUBalloonStatus *balloon_stat_fn; > static QEMUBalloonDropCache *balloon_drop_cache_fn; > +static QEMUBalloonGetFreePage *balloon_get_free_page_fn; > static void *balloon_opaque; > static bool balloon_inhibited; > > @@ -68,10 +69,11 @@ static bool have_balloon(Error **errp) > int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, > QEMUBalloonStatus *stat_func, > QEMUBalloonDropCache *drop_cache_func, > + QEMUBalloonGetFreePage *get_free_page_func, > void *opaque) > { > if (balloon_event_fn || balloon_stat_fn || balloon_drop_cache_fn > - || balloon_opaque) { > + || balloon_get_free_page_fn || balloon_opaque) { > /* We're already registered one balloon handler. How many can > * a guest really have? > */ > @@ -80,6 +82,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, > balloon_event_fn = event_func; > balloon_stat_fn = stat_func; > balloon_drop_cache_fn = drop_cache_func; > + balloon_get_free_page_fn = get_free_page_func; > balloon_opaque = opaque; > return 0; > } > @@ -92,6 +95,7 @@ void qemu_remove_balloon_handler(void *opaque) > balloon_event_fn = NULL; > balloon_stat_fn = NULL; > balloon_drop_cache_fn = NULL; > + balloon_get_free_page_fn = NULL; > balloon_opaque = NULL; > } > > @@ -141,3 +145,21 @@ void qmp_balloon_drop_cache(DropCacheType type, Error **errp) > > balloon_drop_cache_fn(balloon_opaque, type); > } > + > +bool balloon_free_pages_support(void) > +{ > + return balloon_get_free_page_fn ? true : false; > +} > + > +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, unsigned long len) > +{ > + if (!balloon_get_free_page_fn) { > + return REQ_UNSUPPORT; > + } > + > + if (!bitmap) { > + return REQ_INVALID_PARAM; > + } > + > + return balloon_get_free_page_fn(balloon_opaque, bitmap, len); > +} > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index 4757ba5..30ba074 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -38,6 +38,7 @@ > > enum balloon_req_id { > BALLOON_DROP_CACHE, > + BALLOON_GET_FREE_PAGES, > }; > > static void balloon_page(void *addr, int deflate) > @@ -435,7 +436,8 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) > VirtIOBalloon *s = VIRTIO_BALLOON(vdev); > VirtQueueElement *elem; > size_t offset = 0; > - uint32_t tmp32, id = 0; > + uint32_t tmp32, id = 0, page_shift; > + uint64_t base_pfn, tmp64, bmap_len; > > elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > if (!elem) { > @@ -457,6 +459,32 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) > case BALLOON_DROP_CACHE: > s->req_status = REQ_DONE; > break; > + case BALLOON_GET_FREE_PAGES: > + iov_to_buf(elem->out_sg, elem->out_num, offset, > + &tmp32, sizeof(uint32_t)); > + page_shift = virtio_ldl_p(vdev, &tmp32); > + offset += sizeof(uint32_t); > + s->page_shift = page_shift; > + > + iov_to_buf(elem->out_sg, elem->out_num, offset, > + &tmp64, sizeof(uint64_t)); > + base_pfn = virtio_ldq_p(vdev, &tmp64); > + offset += sizeof(uint64_t); > + s->base_pfn = base_pfn; > + > + iov_to_buf(elem->out_sg, elem->out_num, offset, > + &tmp64, sizeof(uint64_t)); > + bmap_len = virtio_ldq_p(vdev, &tmp64); > + offset += sizeof(uint64_t); > + if (s->bmap_len < bmap_len) { > + s->req_status = REQ_INVALID_PARAM; > + return; > + } > + > + iov_to_buf(elem->out_sg, elem->out_num, offset, > + s->free_page_bmap, bmap_len); > + s->req_status = REQ_DONE; > + break; > default: > break; > } > @@ -574,6 +602,48 @@ static int virtio_balloon_drop_cache(void *opaque, unsigned long type) > return REQ_DONE; > } > > +static BalloonReqStatus virtio_balloon_free_pages(void *opaque, > + unsigned long *bitmap, > + unsigned long bmap_len) > +{ > + VirtIOBalloon *s = opaque; > + VirtIODevice *vdev = VIRTIO_DEVICE(s); > + VirtQueueElement *elem = s->misc_vq_elem; > + int len; > + > + if (!balloon_misc_supported(s)) { > + return REQ_UNSUPPORT; > + } > + > + if (s->req_status == REQ_INIT) { > + s->free_page_bmap = bitmap; > + if (elem == NULL || !elem->in_num) { > + elem = virtqueue_pop(s->mvq, sizeof(VirtQueueElement)); > + if (!elem) { > + return REQ_ERROR; > + } > + s->misc_vq_elem = elem; > + } > + s->misc_req.id = BALLOON_GET_FREE_PAGES; > + s->misc_req.param = 0; > + s->bmap_len = bmap_len; > + len = iov_from_buf(elem->in_sg, elem->in_num, 0, &s->misc_req, > + sizeof(s->misc_req)); > + virtqueue_push(s->mvq, elem, len); > + virtio_notify(vdev, s->mvq); > + g_free(s->misc_vq_elem); > + s->misc_vq_elem = NULL; > + s->req_status = REQ_ON_GOING; > + return REQ_ERROR; > + } else if (s->req_status == REQ_ON_GOING) { > + return REQ_ON_GOING; > + } else if (s->req_status == REQ_DONE) { > + s->req_status = REQ_INIT; > + } > + > + return REQ_DONE; > +} > + > static void virtio_balloon_to_target(void *opaque, ram_addr_t target) > { > VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); > @@ -637,7 +707,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) > > ret = qemu_add_balloon_handler(virtio_balloon_to_target, > virtio_balloon_stat, > - virtio_balloon_drop_cache, s); > + virtio_balloon_drop_cache, > + virtio_balloon_free_pages, s); > > if (ret < 0) { > error_setg(errp, "Only one balloon device is supported"); > diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h > index a21bb45..6382bcf 100644 > --- a/include/hw/virtio/virtio-balloon.h > +++ b/include/hw/virtio/virtio-balloon.h > @@ -60,6 +60,10 @@ typedef struct VirtIOBalloon { > uint32_t host_features; > MiscReq misc_req; > BalloonReqStatus req_status; > + uint64_t *free_page_bmap; > + uint64_t bmap_len; > + uint64_t base_pfn; > + uint32_t page_shift; > } VirtIOBalloon; > > #endif > diff --git a/include/sysemu/balloon.h b/include/sysemu/balloon.h > index 0e85f2b..6c362e8 100644 > --- a/include/sysemu/balloon.h > +++ b/include/sysemu/balloon.h > @@ -15,17 +15,25 @@ > #define _QEMU_BALLOON_H > > #include "qapi-types.h" > +#include "hw/virtio/virtio-balloon.h" > > typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target); > typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info); > typedef int (QEMUBalloonDropCache)(void *opaque, unsigned long ctrl); > +typedef BalloonReqStatus (QEMUBalloonGetFreePage)(void *opaque, > + unsigned long *bitmap, > + unsigned long len); > > int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, > QEMUBalloonStatus *stat_func, > QEMUBalloonDropCache *drop_cache_func, > + QEMUBalloonGetFreePage *get_free_page_func, > void *opaque); > void qemu_remove_balloon_handler(void *opaque); > bool qemu_balloon_is_inhibited(void); > void qemu_balloon_inhibit(bool state); > +bool balloon_free_pages_support(void); > +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, > + unsigned long len); > > #endif > -- > 1.9.1
> On Mon, Jun 13, 2016 at 06:16:46PM +0800, Liang Li wrote: > > Add a new feature to get the free page information from guest, the > > free page information is saved in a bitmap. Please note that 'free > > page' only means these pages are free before the request, some of the > > pages will become no free during the process of sending the free page > > bitmap to QEMU. > > > > Signed-off-by: Liang Li <liang.z.li@intel.com> > > > I don't much like this request interface. > The meaning of free page is rather fuzzy too - so at what point are they free? > > > My suggestion would be: > report free page request ID to guest > include request ID when guest sends free page list > > the definition is then: > page was free sometime after host set this value of request > ID and before it received response with the same ID That's better. I will change in next version. And there is another issue similar as we solved to speed up the inflating/deflating process. Should we use a large page bitmap or a small one ? I used a big one in the patch. If we chose to use a small page bitmap, then we have to traverse the free page list for many times, and the meaning of free page will be more fuzzy. But if we use a big map bitmap, people may ask, why a small one here and a big one there? Thanks! Liang
diff --git a/balloon.c b/balloon.c index 3d96111..c74c472 100644 --- a/balloon.c +++ b/balloon.c @@ -37,6 +37,7 @@ static QEMUBalloonEvent *balloon_event_fn; static QEMUBalloonStatus *balloon_stat_fn; static QEMUBalloonDropCache *balloon_drop_cache_fn; +static QEMUBalloonGetFreePage *balloon_get_free_page_fn; static void *balloon_opaque; static bool balloon_inhibited; @@ -68,10 +69,11 @@ static bool have_balloon(Error **errp) int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, QEMUBalloonStatus *stat_func, QEMUBalloonDropCache *drop_cache_func, + QEMUBalloonGetFreePage *get_free_page_func, void *opaque) { if (balloon_event_fn || balloon_stat_fn || balloon_drop_cache_fn - || balloon_opaque) { + || balloon_get_free_page_fn || balloon_opaque) { /* We're already registered one balloon handler. How many can * a guest really have? */ @@ -80,6 +82,7 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, balloon_event_fn = event_func; balloon_stat_fn = stat_func; balloon_drop_cache_fn = drop_cache_func; + balloon_get_free_page_fn = get_free_page_func; balloon_opaque = opaque; return 0; } @@ -92,6 +95,7 @@ void qemu_remove_balloon_handler(void *opaque) balloon_event_fn = NULL; balloon_stat_fn = NULL; balloon_drop_cache_fn = NULL; + balloon_get_free_page_fn = NULL; balloon_opaque = NULL; } @@ -141,3 +145,21 @@ void qmp_balloon_drop_cache(DropCacheType type, Error **errp) balloon_drop_cache_fn(balloon_opaque, type); } + +bool balloon_free_pages_support(void) +{ + return balloon_get_free_page_fn ? true : false; +} + +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, unsigned long len) +{ + if (!balloon_get_free_page_fn) { + return REQ_UNSUPPORT; + } + + if (!bitmap) { + return REQ_INVALID_PARAM; + } + + return balloon_get_free_page_fn(balloon_opaque, bitmap, len); +} diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 4757ba5..30ba074 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -38,6 +38,7 @@ enum balloon_req_id { BALLOON_DROP_CACHE, + BALLOON_GET_FREE_PAGES, }; static void balloon_page(void *addr, int deflate) @@ -435,7 +436,8 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) VirtIOBalloon *s = VIRTIO_BALLOON(vdev); VirtQueueElement *elem; size_t offset = 0; - uint32_t tmp32, id = 0; + uint32_t tmp32, id = 0, page_shift; + uint64_t base_pfn, tmp64, bmap_len; elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); if (!elem) { @@ -457,6 +459,32 @@ static void virtio_balloon_handle_resp(VirtIODevice *vdev, VirtQueue *vq) case BALLOON_DROP_CACHE: s->req_status = REQ_DONE; break; + case BALLOON_GET_FREE_PAGES: + iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp32, sizeof(uint32_t)); + page_shift = virtio_ldl_p(vdev, &tmp32); + offset += sizeof(uint32_t); + s->page_shift = page_shift; + + iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp64, sizeof(uint64_t)); + base_pfn = virtio_ldq_p(vdev, &tmp64); + offset += sizeof(uint64_t); + s->base_pfn = base_pfn; + + iov_to_buf(elem->out_sg, elem->out_num, offset, + &tmp64, sizeof(uint64_t)); + bmap_len = virtio_ldq_p(vdev, &tmp64); + offset += sizeof(uint64_t); + if (s->bmap_len < bmap_len) { + s->req_status = REQ_INVALID_PARAM; + return; + } + + iov_to_buf(elem->out_sg, elem->out_num, offset, + s->free_page_bmap, bmap_len); + s->req_status = REQ_DONE; + break; default: break; } @@ -574,6 +602,48 @@ static int virtio_balloon_drop_cache(void *opaque, unsigned long type) return REQ_DONE; } +static BalloonReqStatus virtio_balloon_free_pages(void *opaque, + unsigned long *bitmap, + unsigned long bmap_len) +{ + VirtIOBalloon *s = opaque; + VirtIODevice *vdev = VIRTIO_DEVICE(s); + VirtQueueElement *elem = s->misc_vq_elem; + int len; + + if (!balloon_misc_supported(s)) { + return REQ_UNSUPPORT; + } + + if (s->req_status == REQ_INIT) { + s->free_page_bmap = bitmap; + if (elem == NULL || !elem->in_num) { + elem = virtqueue_pop(s->mvq, sizeof(VirtQueueElement)); + if (!elem) { + return REQ_ERROR; + } + s->misc_vq_elem = elem; + } + s->misc_req.id = BALLOON_GET_FREE_PAGES; + s->misc_req.param = 0; + s->bmap_len = bmap_len; + len = iov_from_buf(elem->in_sg, elem->in_num, 0, &s->misc_req, + sizeof(s->misc_req)); + virtqueue_push(s->mvq, elem, len); + virtio_notify(vdev, s->mvq); + g_free(s->misc_vq_elem); + s->misc_vq_elem = NULL; + s->req_status = REQ_ON_GOING; + return REQ_ERROR; + } else if (s->req_status == REQ_ON_GOING) { + return REQ_ON_GOING; + } else if (s->req_status == REQ_DONE) { + s->req_status = REQ_INIT; + } + + return REQ_DONE; +} + static void virtio_balloon_to_target(void *opaque, ram_addr_t target) { VirtIOBalloon *dev = VIRTIO_BALLOON(opaque); @@ -637,7 +707,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) ret = qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, - virtio_balloon_drop_cache, s); + virtio_balloon_drop_cache, + virtio_balloon_free_pages, s); if (ret < 0) { error_setg(errp, "Only one balloon device is supported"); diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h index a21bb45..6382bcf 100644 --- a/include/hw/virtio/virtio-balloon.h +++ b/include/hw/virtio/virtio-balloon.h @@ -60,6 +60,10 @@ typedef struct VirtIOBalloon { uint32_t host_features; MiscReq misc_req; BalloonReqStatus req_status; + uint64_t *free_page_bmap; + uint64_t bmap_len; + uint64_t base_pfn; + uint32_t page_shift; } VirtIOBalloon; #endif diff --git a/include/sysemu/balloon.h b/include/sysemu/balloon.h index 0e85f2b..6c362e8 100644 --- a/include/sysemu/balloon.h +++ b/include/sysemu/balloon.h @@ -15,17 +15,25 @@ #define _QEMU_BALLOON_H #include "qapi-types.h" +#include "hw/virtio/virtio-balloon.h" typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target); typedef void (QEMUBalloonStatus)(void *opaque, BalloonInfo *info); typedef int (QEMUBalloonDropCache)(void *opaque, unsigned long ctrl); +typedef BalloonReqStatus (QEMUBalloonGetFreePage)(void *opaque, + unsigned long *bitmap, + unsigned long len); int qemu_add_balloon_handler(QEMUBalloonEvent *event_func, QEMUBalloonStatus *stat_func, QEMUBalloonDropCache *drop_cache_func, + QEMUBalloonGetFreePage *get_free_page_func, void *opaque); void qemu_remove_balloon_handler(void *opaque); bool qemu_balloon_is_inhibited(void); void qemu_balloon_inhibit(bool state); +bool balloon_free_pages_support(void); +BalloonReqStatus balloon_get_free_pages(unsigned long *bitmap, + unsigned long len); #endif
Add a new feature to get the free page information from guest, the free page information is saved in a bitmap. Please note that 'free page' only means these pages are free before the request, some of the pages will become no free during the process of sending the free page bitmap to QEMU. Signed-off-by: Liang Li <liang.z.li@intel.com> --- balloon.c | 24 +++++++++++- hw/virtio/virtio-balloon.c | 75 +++++++++++++++++++++++++++++++++++++- include/hw/virtio/virtio-balloon.h | 4 ++ include/sysemu/balloon.h | 8 ++++ 4 files changed, 108 insertions(+), 3 deletions(-)