On 8/12/19 11:18 AM, Alexander Duyck wrote:
> On Mon, Aug 12, 2019 at 6:14 AM Nitesh Narayan Lal <nitesh@xxxxxxxxxx> wrote:
>> Page reporting is a feature which enables the virtual machine to report
>> chunk of free pages to the hypervisor.
>> This patch enables QEMU to process these reports from the VM and discard the
>> unused memory range.
>>
>> Signed-off-by: Nitesh Narayan Lal <nitesh@xxxxxxxxxx>
>> ---
>> hw/virtio/virtio-balloon.c | 41 ++++++++++++++++++++++++++++++
>> include/hw/virtio/virtio-balloon.h | 2 +-
>> 2 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 25de154307..1132e47ee0 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -320,6 +320,39 @@ static void balloon_stats_set_poll_interval(Object *obj, Visitor *v,
>> balloon_stats_change_timer(s, 0);
>> }
>>
>> +static void virtio_balloon_handle_reporting(VirtIODevice *vdev, VirtQueue *vq)
>> +{
>> + VirtQueueElement *elem;
>> +
>> + while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement)))) {
>> + unsigned int i;
>> +
>> + for (i = 0; i < elem->in_num; i++) {
>> + void *gaddr = elem->in_sg[i].iov_base;
>> + size_t size = elem->in_sg[i].iov_len;
>> + ram_addr_t ram_offset;
>> + size_t rb_page_size;
>> + RAMBlock *rb;
>> +
>> + if (qemu_balloon_is_inhibited())
>> + continue;
>> +
>> + rb = qemu_ram_block_from_host(gaddr, false, &ram_offset);
>> + rb_page_size = qemu_ram_pagesize(rb);
>> +
>> + /* For now we will simply ignore unaligned memory regions */
>> + if ((ram_offset | size) & (rb_page_size - 1))
>> + continue;
>> +
>> + ram_block_discard_range(rb, ram_offset, size);
>> + }
>> +
>> + virtqueue_push(vq, elem, 0);
>> + virtio_notify(vdev, vq);
>> + g_free(elem);
>> + }
>> +}
>> +
> No offense, but I am a bit annoyed.
None taken at all.
> If you are going to copy my code
> you should at least keep up with the fixes.
Yeah I did refer to your code and just because the quality of your code is
better than what I posted earlier and there is quite a lot for me to learn from it.
> stuff to handle the poison value. If you are going to just duplicate
> my setup you might as well have just pulled the QEMU patches from the
> last submission I did. Then this would have at least has the fix for
> the page poisoning.
>
The only reason I didn't include the poison change as I still need to understand
them.
I have this mentioned in my cover-email.
> Also it wouldn't hurt to mention that you are
> basing it off of the patch set I submitted since it hasn't been
> accepted yet.
My bad!! This I will surely do from next time.
>
>> static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>> {
>> VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
>> @@ -792,6 +825,12 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>> s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>> s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
>>
>> + if (virtio_has_feature(s->host_features,
>> + VIRTIO_BALLOON_F_REPORTING)) {
>> + s->reporting_vq = virtio_add_queue(vdev, 16,
>> + virtio_balloon_handle_reporting);
>> + }
>> +
>> if (virtio_has_feature(s->host_features,
>> VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
>> s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
>> @@ -912,6 +951,8 @@ static Property virtio_balloon_properties[] = {
>> * is disabled, resulting in QEMU 3.1 migration incompatibility. This
>> * property retains this quirk for QEMU 4.1 machine types.
>> */
>> + DEFINE_PROP_BIT("free-page-reporting", VirtIOBalloon, host_features,
>> + VIRTIO_BALLOON_F_REPORTING, true),
>> DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
>> qemu_4_0_config_size, false),
>> DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD,
>> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
>> index d1c968d237..15a05e6435 100644
>> --- a/include/hw/virtio/virtio-balloon.h
>> +++ b/include/hw/virtio/virtio-balloon.h
>> @@ -42,7 +42,7 @@ enum virtio_balloon_free_page_report_status {
>>
>> typedef struct VirtIOBalloon {
>> VirtIODevice parent_obj;
>> - VirtQueue *ivq, *dvq, *svq, *free_page_vq;
>> + VirtQueue *ivq, *dvq, *svq, *free_page_vq, *reporting_vq;
>> uint32_t free_page_report_status;
>> uint32_t num_pages;
>> uint32_t actual;
>> --
>> 2.21.0
>> q
--
Thanks
Nitesh
