On 16.04.20 10:18, David Hildenbrand wrote: >>> >>> Postcopy is a very good point, bought! >>> >>> But (what you wrote above) it sounds like that this is really what we *have >>> to* do, not an optimization. I‘ll double check the spec tomorrow (hopefully >>> it was documented). We should rephrase the comment then. >> >> Do you have a link to the spec that I could look at? I am not hopeful >> that this will be listed there since the poison side of QEMU was never >> implemented. The flag is only here because it was copied over in the >> kernel header. > > Introducing a feature without > > a) specification what it does > b) implementation (of both sides) showing what has to be done > c) any kind of documentation of what needs to be done > > just horrible. > > The latest-greatest spec lives in > > https://github.com/oasis-tcs/virtio-spec.git > > I can't spot any signs of free page hinting/page poisioning. :( > > We should document our result of page poisoning, free page hinting, and > free page reporting there as well. I hope you'll have time for the latter. > > ------------------------------------------------------------------------- > Semantics of VIRTIO_BALLOON_F_PAGE_POISON > ------------------------------------------------------------------------- > > "The VIRTIO_BALLOON_F_PAGE_POISON feature bit is used to indicate if the > guest is using page poisoning. Guest writes to the poison_val config > field to tell host about the page poisoning value that is in use." > -> Very little information, no signs about what has to be done. > > "Let the hypervisor know that we are expecting a specific value to be > written back in balloon pages." > -> Okay, that talks about "balloon pages", which would include right now > -- pages "inflated" and then "deflated" using free page hinting > -- pages "inflated" and then "deflated" using oridnary inflate/deflate > queue > -- pages "inflated" using free page reporting and automatically > "deflated" on access > > So VIRTIO_BALLOON_F_PAGE_POISON really means "whenever the guest > deflates a page (either explicitly, or implicitly with free page > reporting), it is filled with "poison_val". > > And I would add > > "However, if the inflated page was not filled with "poison_val" when > inflating, it's not predictable if the original page or a page filled > with "poison_val" is returned." > > Which would cover the "we did not discard the page in the hypervisor, so > the original page is still there". > > > We should also document what is expected to happen if "poison_val" is > suddenly changed by the guest at one point in time again. (e.g., not > supported, unexpected things can happen, etc.) Also, we might have to > document that a device reset resets the poison_val to 0. (not sure if > that's really necessary)
Looking at the spec, we will only have to care about "VIRTIO_BALLOON_F_MUST_TELL_HOST". Especially: "If the VIRTIO_BALLOON_F_MUST_TELL_HOST feature is negotiated, the driver MUST NOT use pages from the balloon until the device has acknowledged the deflate request. Otherwise, if the VIRTIO_BALLOON_F_MUST_TELL_HOST feature is not negotiated, the driver MAY begin to re-use pages previously given to the balloon before the device has acknowledged the deflate request." So I guess, in QEMU, if - VIRTIO_BALLOON_F_MUST_TELL_HOST is not set (== currently always) - VIRTIO_BALLOON_F_PAGE_POISON is set - poison_val is != 0 then, don't discard any pages, because we cannot fill the page reliably during a deflation request (== guest could already be reusing the page and expecting a certain value). In QEMU, we should always set VIRTIO_BALLOON_F_MUST_TELL_HOST when setting VIRTIO_BALLOON_F_PAGE_POISON. In the spec, we should document that VIRTIO_BALLOON_F_PAGE_POISON should only be used with VIRTIO_BALLOON_F_MUST_TELL_HOST or sth like that ... confusing stuff. Let me know what you think. -- Thanks, David / dhildenb
