On 06.05.20 11:49, David Hildenbrand wrote:
> We want to replace qemu_balloon_inhibit() by something more generic.
> Especially, we want to make sure that technologies that really rely on
> RAM block discards to work reliably to run mutual exclusive with
> technologies that break it.
>
> E.g., vfio will usually pin all guest memory, turning the virtio-balloon
> basically useless and make the VM consume more memory than reported via
> the balloon. While the balloon is special already (=> no guarantees, same
> behavior possible afer reboots and with huge pages), this will be
> different, especially, with virtio-mem.
>
> Let's implement a way such that we can make both types of technology run
> mutually exclusive. We'll convert existing balloon inhibitors in successive
> patches and add some new ones. Add the check to
> qemu_balloon_is_inhibited() for now. We might want to make
> virtio-balloon an acutal inhibitor in the future - however, that
> requires more thought to not break existing setups.
>
> Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> Cc: Richard Henderson <rth@xxxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> ---
> balloon.c | 3 ++-
> exec.c | 48 +++++++++++++++++++++++++++++++++++++++++++
> include/exec/memory.h | 41 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/balloon.c b/balloon.c
> index f104b42961..c49f57c27b 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -40,7 +40,8 @@ static int balloon_inhibit_count;
>
> bool qemu_balloon_is_inhibited(void)
> {
> - return atomic_read(&balloon_inhibit_count) > 0;
> + return atomic_read(&balloon_inhibit_count) > 0 ||
> + ram_block_discard_is_broken();
> }
>
> void qemu_balloon_inhibit(bool state)
> diff --git a/exec.c b/exec.c
> index 2874bb5088..52a6e40e99 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -4049,4 +4049,52 @@ void mtree_print_dispatch(AddressSpaceDispatch *d, MemoryRegion *root)
> }
> }
>
> +static int ram_block_discard_broken;
> +
> +int ram_block_discard_set_broken(bool state)
> +{
> + int old;
> +
> + if (!state) {
> + atomic_dec(&ram_block_discard_broken);
> + return 0;
> + }
> +
> + do {
> + old = atomic_read(&ram_block_discard_broken);
> + if (old < 0) {
> + return -EBUSY;
> + }
> + } while (atomic_cmpxchg(&ram_block_discard_broken, old, old + 1) != old);
> + return 0;
> +}
> +
> +int ram_block_discard_set_required(bool state)
> +{
> + int old;
> +
> + if (!state) {
> + atomic_inc(&ram_block_discard_broken);
> + return 0;
> + }
> +
> + do {
> + old = atomic_read(&ram_block_discard_broken);
> + if (old > 0) {
> + return -EBUSY;
> + }
> + } while (atomic_cmpxchg(&ram_block_discard_broken, old, old - 1) != old);
> + return 0;
> +}
> +
> +bool ram_block_discard_is_broken(void)
> +{
> + return atomic_read(&ram_block_discard_broken) > 0;
> +}
> +
> +bool ram_block_discard_is_required(void)
> +{
> + return atomic_read(&ram_block_discard_broken) < 0;
> +}
> +
> #endif
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e000bd2f97..9bb5ced38d 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -2463,6 +2463,47 @@ static inline MemOp devend_memop(enum device_endian end)
> }
> #endif
>
> +/*
> + * Inhibit technologies that rely on discarding of parts of RAM blocks to work
> + * reliably, e.g., to manage the actual amount of memory consumed by the VM
> + * (then, the memory provided by RAM blocks might be bigger than the desired
> + * memory consumption). This *must* be set if:
> + * - Discarding parts of a RAM blocks does not result in the change being
> + * reflected in the VM and the pages getting freed.
> + * - All memory in RAM blocks is pinned or duplicated, invaldiating any previous
> + * discards blindly.
> + * - Discarding parts of a RAM blocks will result in integrity issues (e.g.,
> + * encrypted VMs).
> + * Technologies that only temporarily pin the current working set of a
> + * driver are fine, because we don't expect such pages to be discarded
> + * (esp. based on guest action like balloon inflation).
> + *
> + * This is *not* to be used to protect from concurrent discards (esp.,
> + * postcopy).
> + *
> + * Returns 0 if successful. Returns -EBUSY if a technology that relies on
> + * discards to work reliably is active.
> + */
> +int ram_block_discard_set_broken(bool state);
> +
> +/*
> + * Inhibit technologies that will break discarding of pages in RAM blocks.
> + *
> + * Returns 0 if successful. Returns -EBUSY if discards are already set to
> + * broken.
> + */
> +int ram_block_discard_set_required(bool state);
> +
> +/*
> + * Test if discarding of memory in ram blocks is broken.
> + */
> +bool ram_block_discard_is_broken(void);
> +
> +/*
> + * Test if discarding of memory in ram blocks is required to work reliably.
> + */
> +bool ram_block_discard_is_required(void);
> +
> #endif
>
> #endif
>
I'm wondering if I'll just call these functions
ram_block_discard_disable()
and
ram_block_discard_require()
--
Thanks,
David / dhildenb
