On Thu, Apr 23, 2020 at 1:11 AM David Hildenbrand <da...@redhat.com> wrote: > > On 22.04.20 20:21, Alexander Duyck wrote: > > From: Alexander Duyck <alexander.h.du...@linux.intel.com> > > > > We need to make certain to advertise support for page poison tracking if > > we want to actually get data on if the guest will be poisoning pages. > > > > Add a value for tracking the poison value being used if page poisoning is > > enabled. With this we can determine if we will need to skip page reporting > > when it is enabled in the future. > > Maybe add something about the semantics > > "VIRTIO_BALLOON_F_PAGE_POISON will not change the behavior of free page > hinting or ordinary balloon inflation/deflation." > > I do wonder if we should just unconditionally enable > VIRTIO_BALLOON_F_PAGE_POISON here, gluing it to the QEMU compat machine > (via a property that is default-enabled, and disabled from compat machines). > > Because, as Michael said, knowing that the guest is using page poisoning > might be interesting even if free page reporting is not around.
I could do that. So if that is the case though would I disable page reporting if it isn't enabled, or would I be enabling page poison if page reporting is enabled? > > > > Signed-off-by: Alexander Duyck <alexander.h.du...@linux.intel.com> > > --- > > hw/virtio/virtio-balloon.c | 7 +++++++ > > include/hw/virtio/virtio-balloon.h | 1 + > > 2 files changed, 8 insertions(+) > > > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > > index a1d6fb52c876..5effc8b4653b 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 = > > @@ -697,6 +698,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_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) { > > + dev->poison_val = le32_to_cpu(config.poison_val); > > + } > > trace_virtio_balloon_set_config(dev->actual, oldactual); > > } > > > > @@ -854,6 +859,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) > > 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 > > > > You still have to migrate poison_val if I am not wrong, otherwise you > would lose it during migration if I am not mistaking. So what are the requirements to migrate a value? Would I need to add a property so it can be retrieved/set?