[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [qemu-s390x] [PATCH RFC] vfio-ap: flag as compatible with balloon

From: Tony Krowiak
Subject: Re: [qemu-s390x] [PATCH RFC] vfio-ap: flag as compatible with balloon
Date: Thu, 6 Dec 2018 15:51:40 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

On 12/6/18 7:48 AM, Cornelia Huck wrote:
On Thu, 6 Dec 2018 13:32:39 +0100
Halil Pasic <address@hidden> wrote:

On Thu, 6 Dec 2018 09:28:34 +0100
David Hildenbrand <address@hidden> wrote:

On 05.12.18 18:25, Christian Borntraeger wrote:

On 05.12.2018 17:45, Cornelia Huck wrote:
On Wed, 5 Dec 2018 17:38:22 +0100
David Hildenbrand <address@hidden> wrote:
On 05.12.18 15:51, Cornelia Huck wrote:
vfio-ap devices do not pin any pages in the host. Therefore, they
are belived to be compatible with memory ballooning.

Flag them as compatible, so both vfio-ap and a balloon can be
used simultaneously.

Signed-off-by: Cornelia Huck <address@hidden>

As briefly discussed on IRC. RFC as I do not have easy access to
hardware I can test this with.

  hw/vfio/ap.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 65de952f44..3bf48eed28 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -104,6 +104,14 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
      vapdev->vdev.name = g_strdup_printf("%s", mdevid);
      vapdev->vdev.dev = dev;
+ /*
+     * vfio-ap devices are believed to operate in a way compatible with
+     * memory ballooning, as no pages are pinned in the host.
+     * This needs to be set before vfio_get_device() for vfio common to
+     * handle the balloon inhibitor.
+     */
+    vapdev->vdev.balloon_allowed = true;
      ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
      if (ret) {
          goto out_get_dev_err;

What happens if this ever changes? Shouldn't we have an API to at least
check what the vfio device can guarantee?

"are believed to operate" doesn't sound like guarantees to me :)

I would actually remove that comment or fix it. We either know or we dont.
In the way vfio-works I see no reason to disallow balloon. Even if the guest 
something wrong (e.g. crypto I/O on freed pages) the host would handle that the
same as it would for normal page accesses. From a host point of view the crypto
instructions are just CISC instructions with load/store semantics.

As long as vfio-ap does not and will never pin pages (and keep them
pinned), we are fine. I don't know about the details, but if vfio-ap
really just issues a synchronous instruction for us, we are fine.

I agree with Christian. That comment is best removed.

What about s/believed to operate/operate/?

The second part of the comment is still useful, I believe.

@Tony, I guess you should have the most elaborate test setup. Can you give
this some testing just in case?

Actual testing would be great :)

Will do.

It's the same for ccw :)

As a matter of fact, I don't like that comment.

Do you have a suggestion for rewording it?


While such an API definitely sounds like a good idea, it is probably
overkill to introduce it for this case (do we envision changing the way
vfio-ap operates in the future to make that statement non-true?)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]