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

[David Hildenbrand <david@redhat.com>]

 >>> 2. Can we assume that the guest will always rewrite the page after the
>>> deflate in the case of init_on_free or poison?
>>
>> Depends on what we think is the right way to do - IOW if we think "some
>> other content" as mentioned above is a BUG or not.
> 
> So I wouldn't consider it a but as the zero page probably doesn't
> apply. We are basically just indicating we don't care about the
> contents, we aren't giving it a value. At least that is how I see it
> working based on how it was implemented.
> 
>>> 3. Can we assume that free page hinting will always function as a
>>> balloon setup, so no moving it over to a page reporting type setup?
>>
>> I think we have to define the valid semantics. That implies what would
>> be valid to do with it. Unfortunately, we have to reverse-engineer here.
> 
> So at this point a "hinted" page is a page that can essentially
> transition to uninitialized while it is sitting in the balloon. I
> suspect that is how we are going to have to treat it since that is the
> behavior that it has right now.

At least it's not what Michael initially thought should be done - IIRC.

"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 ...").

So the question remains: Does QEMU have a BUG or do we actually allow to
corrupt guest memory.

That leaves us with either

1. "Pages hinted via VIRTIO_BALLOON_F_FREE_PAGE_HINT might get replaced
by zero page. However, as soon as the page is written by the guest (even
before the hinting request was processed by the host), the modified page
will stay - whereby the unwritten parts might either be from the old, or
from the zero page." - a QEMU BUG.

2. "Pages hinted via VIRTIO_BALLOON_F_FREE_PAGE_HINT are considered
unused and will contain an undefined/uninitialized content once hinted.
As soon as the page is written by the guest (even before the hinting
request was processed by the host), the modified page will stay. The
guest should reinitialized the full page in case it cares about the
actual content (e.g., page poisoning)."


I tend to favor 2 - although it basically leaves no room for future
improvement regarding skipping re-initialization in the guest after
migration.

>>>
>>> If we assume the above 3 items then there isn't any point in worrying
>>> about poison when it comes to free page hinting. It doesn't make sense
>>> to since they essentially negate it. As such I could go through this
>>> patch and the QEMU patches and clean up any associations since the to
>>> are not really tied together in any way.
>>
>> The big question is, if we want to support VIRTIO_BALLOON_F_PAGE_POISON
>> with free page hinting. e.g.,:
>>
>> "Pages hinted via VIRTIO_BALLOON_F_FREE_PAGE_HINT might get replaced by
>> a page full of X. However, as soon as the page is written by the guest
>> (even before the hinting request was processed by the host), the
>> modified page will stay - whereby the unwritten parts might either be
>> from the old, or from a page filled with X. Without
>> VIRTIO_BALLOON_F_PAGE_POISON, X is zero, otherwise it is poison_val."

[...]

> 
> With the VIRTIO_BALLOON_F_PAGE_POISON we could make it so that when
> the page comes out of the balloon it is either unmodified or it is
> poison_val. Without the VIRTIO_BALLOON_F_PAGE_POISON feature present
> you cannot make that guarantee and are stuck with the page being
> treated as either unmodified or uninitialized memory.

While it would be possible, I doubt it will be easy to implement, and I
still wonder if we should really care about optimizing an undocumented
feature that takes three people to figure out the semantics.

> 
>> The current QEMU implementation would be to simply migrate all hinted
>> pages. In the future we could optimize. Not sure if it's worth the trouble.
> 
> So the trick for me is how best to go about sorting this all out.
> There are two ways I see of doing it.
> 
> The first approach would be to just assume that hinting should be
> disassociated from poisoning. If I go that route that would more
> closely match up with what happened in QEMU. The downside is that it
> locks in the unmodified/uninitialized behavior and would require pages
> to be rewritten when they come out of the balloon. However this is the
> behavior we have now so it would only require specification
> documentation changes.

Right now, for simplicity, I prefer this and define
VIRTIO_BALLOON_F_PAGE_POISON only for explicit deflation (balloon
deflation via the deflate queue) and implicit deflation
(VIRTIO_BALLOON_F_REPORTING).

Let's see if Michael has another opinion.

-- 
Thanks,

David / dhildenb