[Top][All Lists]

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

Re: [qemu-s390x] [Qemu-devel] [PATCH] s390x/vfio-ap: Implement hot plug/

From: Tony Krowiak
Subject: Re: [qemu-s390x] [Qemu-devel] [PATCH] s390x/vfio-ap: Implement hot plug/unplug of vfio-ap device
Date: Thu, 10 Jan 2019 11:22:48 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

On 1/9/19 12:28 PM, David Hildenbrand wrote:
On 09.01.19 18:13, Halil Pasic wrote:
On Wed, 9 Jan 2019 17:37:49 +0100
David Hildenbrand <address@hidden> wrote:

On 09.01.19 17:27, Tony Krowiak wrote:
On 1/9/19 6:30 AM, Cornelia Huck wrote:
On Tue, 8 Jan 2019 23:13:39 +0100
David Hildenbrand <address@hidden> wrote:

On 08.01.19 20:52, Tony Krowiak wrote:
On 1/8/19 11:09 AM, David Hildenbrand wrote:
On 08.01.19 17:01, Tony Krowiak wrote:
Introduces hot plug/unplug support for the vfio-ap device. Note that only one
vfio-ap device can be attached to the ap-bus, so a vfio-ap device can only be
hot plugged if the '-device vfio-ap,sysfsdev=$path_to_mdev' option is not
specified on the QEMU command line.

Signed-off-by: Tony Krowiak <address@hidden>
Reviewed-by: Pierre Morel<address@hidden>
Tested-by: Pierre Morel<address@hidden>
    hw/s390x/ap-bridge.c | 12 +++++++++++-
    hw/vfio/ap.c         |  2 +-
    2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c
index 3795d30dd7c9..25a03412fcb9 100644
--- a/hw/s390x/ap-bridge.c
+++ b/hw/s390x/ap-bridge.c
@@ -39,6 +39,7 @@ static const TypeInfo ap_bus_info = {
    void s390_init_ap(void)
        DeviceState *dev;
+    BusState *bus;
/* If no AP instructions then no need for AP bridge */
        if (!s390_has_feat(S390_FEAT_AP)) {
@@ -52,13 +53,18 @@ void s390_init_ap(void)
/* Create bus on bridge device */
-    qbus_create(TYPE_AP_BUS, dev, TYPE_AP_BUS);
+    bus = qbus_create(TYPE_AP_BUS, dev, TYPE_AP_BUS);
+    /* Enable hotplugging */
+    qbus_set_hotplug_handler(bus, dev, &error_abort);
static void ap_bridge_class_init(ObjectClass *oc, void *data)
        DeviceClass *dc = DEVICE_CLASS(oc);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
+ hc->unplug = qdev_simple_device_unplug_cb;

confused, why is there no plug action?

You will find this is the case for several devices that are hot

Usually missing hotplug handlers are an indication of legacy code ;)

The plug callback is invoked after the device is
attached to the bus and after the device is realized. Not having
a hot plug callback does not preclude hot plugging of a device.

The hotplug handler is there to

1. Assign resources (e.g. ids etc)
2. Notify the system (e.g. hotplug interrupt)

In legacy code (e.g. PCI) such stuff is usually still located in the
realize function (where it doesn't belong anymore but factoring out is
hard ...)

Looking at hw/vfio/ap.c:realize(), there isn't really anything in there.

So I assume that

1. No resources have to be assigned (for vfio-ap, I guess the IDs and
such are implicit)

That's my understanding as well. The interesting stuff will be
configured on kernel level before the device is even handed to QEMU for

The vfio-ap device represents a VFIO mdev device. AP resources - i.e.,
adapters, domains and control domains - are assigned to the mdev device
via its sysfs interfaces. This is all handled by the vfio_ap kernel
driver before a guest can use the mdev device. As part of vfio-ap device
realization, a file descriptor is opened on the mdev device. When the
mdev device's fd is opened, a callback is invoked on the vfio_ap kernel
device driver. The device driver then updates the guest's AP matrix
configuration based on the configuration specified via the mdev
device's sysfs interfaces.

(Would be nice to hint at that in the patch description.)

2. No notification will happen. How will the guest know that a new AP
adapter is available?

My understanding is that hotplugging the matrix device will make the
guest go from "no adapters/domains are available" to "some
adapters/domains are available" (and unplug will do the reverse). I.e.,
no hot(un)plugging of individual queues (which would need to be done on
the kernel level, and is tricky IIRC.)

I'm not sure what the architectured options for notifying the guest are
(I dimly recall some kind of "AP configuration has changed event").

IIRC, the Linux guest driver scans for new queues periodically. Does it
even do that if no queues are available to start with?

The AP bus - in this case, running in the guest - does a periodic scan
for AP devices. The bus relies on an AP instruction that queries the
AP configuration information. When the guest's AP matrix is updated -
see description of mdev device fd open processing above - the query
will provide the newly configured AP matrix and the bus will create
the adapter and queue devices on the guest. Consequently, there is
nothing to do in a hot plug handler. If you'd like, I'd be more than
happy to include a hot plug handler that does some logging (or nothing
at all) so it doesn't look like legacy code ;)

Hehe, no it's fine for me.

Can you extend this patch description a little bit, including what we
discussed here?

Maybe a short comment that explains why qdev_simple_device_unplug_cb()
is appropriate as well (i.e. hits that closing the mdev's fd is what
triggers the cleanup of the actual resources)? I personally go log
digging only once I get desperate.

I go digging if I can't find a public document on how it works ;)

Which reminds me, I will also need to update the docs/vfio-ap.txt


reply via email to

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