Re: [virtio-dev] Re: [PATCH] virtio-balloon: Disable free page hinting/reporting if page poison is disabled

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Apr 17, 2020 at 09:49:03AM +0200, David Hildenbrand wrote:
> On 17.04.20 08:28, Michael S. Tsirkin wrote:
> > On Thu, Apr 16, 2020 at 04:52:42PM -0700, Alexander Duyck wrote:
> >> On Thu, Apr 16, 2020 at 3:13 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >>>
> >>> On Thu, Apr 16, 2020 at 12:30:38PM -0700, Alexander Duyck wrote:
> >>>> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >>>>
> >>>> If we have free page hinting or page reporting enabled we should disable it
> >>>> if the pages are poisoned or initialized on free and we cannot notify the
> >>>> hypervisor.
> >>>>
> >>>> Fixes: 5d757c8d518d ("virtio-balloon: add support for providing free page reports to host")
> >>>>
> >>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >>>
> >>>
> >>> Why not put this logic in the hypervisor?
> >>
> >> I did that too. This is to cover the case where somebody is running
> >> the code prior to my QEMU changes where the page poison feature wasn't
> >> being enabled.
> > 
> > 
> > Hmm so that one looks like a QEMU bug (does not expose poison flag).  In
> > the past we just said need to fix the bug where it's found unless the
> > issue is very widespread and major.  Let's assume QEMU learns to always
> > expose POISON with HINT.  Then this configuration (HINT && !POISON)
> > allows us to detect old QEMU (pre your changes).
> 
> Don't see why this is a QEMU bug. It's just a feature it does not
> implement. Perfectly valid.

I'll need to think about this.
We need to remember that the whole HINT thing is not a mandate for host to
corrupt memory. It's just some info about page use guest
gives host. If host corrupts memory it's broken ...

> [...]
> >>
> >> The problem is we cannot communicate the full situation to the
> >> hypervisor without the page poison feature being present. As such I
> >> would worry about that backfiring on us due to the hypervisor acting
> >> on incomplete data.
> > 
> > 
> > I'll try to think about VIRTIO_BALLOON_F_FREE_PAGE_HINT situation
> > over the weekend. But for the new page reporting, why not
> 
> I shared my thoughts about VIRTIO_BALLOON_F_FREE_PAGE_HINT in the other
> thread and think we should simply not care at all for now.
> 
> > assume host implementation will be sane?
> 
> I don't think we should enforce having poison support around. See my
> reply to this mail for an alternative.

OK so you basically say leave this to host to handle? That's more or
less what I'm saying too.


> -- 
> Thanks,
> 
> David / dhildenb