Re: [PATCH v22 QEMU 4/5] virtio-balloon: Implement support for page poison reporting feature

[David Hildenbrand]

On 24.04.20 18:50, Alexander Duyck wrote:
> From: Alexander Duyck <address@hidden>
> 
> We need to make certain to advertise support for page poison reporting if
> we want to actually get data on if the guest will be poisoning pages.
> 
> Add a value for reporting the poison value being used if page poisoning is
> enabled in the guest. With this we can determine if we will need to skip
> free page reporting when it is enabled in the future.
> 
> The value currently has no impact on existing balloon interfaces. In the
> case of existing balloon interfaces the onus is on the guest driver to
> reapply whatever poison is in place.
> 
> When we add free page reporting the poison value is used to determine if
> we can perform in-place page reporting. The expectation is that a reported
> page will already contain the value specified by the poison, and the
> reporting of the page should not change that value.
> 
> Signed-off-by: Alexander Duyck <address@hidden>
> ---
>  hw/virtio/virtio-balloon.c         |   29 +++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-balloon.h |    1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index a1d6fb52c876..c1c76ec09c95 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -634,6 +634,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
> uint8_t *config_data)
>  
>      config.num_pages = cpu_to_le32(dev->num_pages);
>      config.actual = cpu_to_le32(dev->actual);
> +    config.poison_val = cpu_to_le32(dev->poison_val);
>  
>      if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) {
>          config.free_page_hint_cmd_id =
> @@ -683,6 +684,14 @@ static ram_addr_t get_current_ram_size(void)
>      return size;
>  }
>  
> +static bool virtio_balloon_page_poison_support(void *opaque)
> +{
> +    VirtIOBalloon *s = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    return virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
> +}
> +
>  static void virtio_balloon_set_config(VirtIODevice *vdev,
>                                        const uint8_t *config_data)
>  {
> @@ -697,6 +706,10 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>          qapi_event_send_balloon_change(vm_ram_size -
>                          ((ram_addr_t) dev->actual << 
> VIRTIO_BALLOON_PFN_SHIFT));
>      }
> +    dev->poison_val = 0;
> +    if (virtio_balloon_page_poison_support(dev)) {
> +        dev->poison_val = le32_to_cpu(config.poison_val);
> +    }
>      trace_virtio_balloon_set_config(dev->actual, oldactual);
>  }
>  
> @@ -755,6 +768,17 @@ static const VMStateDescription 
> vmstate_virtio_balloon_free_page_hint = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_virtio_balloon_page_poison = {
> +    .name = "vitio-balloon-device/page-poison",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = virtio_balloon_page_poison_support,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(poison_val, VirtIOBalloon),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_virtio_balloon_device = {
>      .name = "virtio-balloon-device",
>      .version_id = 1,
> @@ -767,6 +791,7 @@ static const VMStateDescription 
> vmstate_virtio_balloon_device = {
>      },
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_virtio_balloon_free_page_hint,
> +        &vmstate_virtio_balloon_page_poison,
>          NULL
>      }
>  };
> @@ -854,6 +879,8 @@ static void virtio_balloon_device_reset(VirtIODevice 
> *vdev)
>          g_free(s->stats_vq_elem);
>          s->stats_vq_elem = NULL;
>      }
> +
> +    s->poison_val = 0;
>  }
>  
>  static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
> @@ -916,6 +943,8 @@ static Property virtio_balloon_properties[] = {
>                      VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
>      DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
>                      VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
> +    DEFINE_PROP_BIT("page-poison", VirtIOBalloon, host_features,
> +                    VIRTIO_BALLOON_F_PAGE_POISON, true),
>      /* QEMU 4.0 accidentally changed the config size even when free-page-hint
>       * is disabled, resulting in QEMU 3.1 migration incompatibility.  This
>       * property retains this quirk for QEMU 4.1 machine types.
> diff --git a/include/hw/virtio/virtio-balloon.h 
> b/include/hw/virtio/virtio-balloon.h
> index 108cff97e71a..3ca2a78e1aca 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -70,6 +70,7 @@ typedef struct VirtIOBalloon {
>      uint32_t host_features;
>  
>      bool qemu_4_0_config_size;
> +    uint32_t poison_val;
>  } VirtIOBalloon;
>  
>  #endif
> 

Acked-by: David Hildenbrand <address@hidden>

-- 
Thanks,

David / dhildenb