Commit d1b4259f authored by Maxime Coquelin's avatar Maxime Coquelin Committed by Michael S. Tsirkin
Browse files

virtio-bus: Plug devices after features are negotiated 



Currently, devices are plugged before features are negotiated.
If the backend doesn't support VIRTIO_F_VERSION_1, the transport
needs to rewind some settings.

This is the case for CCW, for which a post_plugged callback had
been introduced, where max_rev field is just updated if
VIRTIO_F_VERSION_1 is not supported by the backend.
For PCI, implementing post_plugged would be much more
complicated, so it needs to know whether the backend supports
VIRTIO_F_VERSION_1 at plug time.

Currently, nothing is done for PCI. Modern capabilities get
exposed to the guest even if VIRTIO_F_VERSION_1 is not supported
by the backend, which confuses the guest.

This patch replaces existing post_plugged solution with an
approach that fits with both transports.
Features negotiation is performed before ->device_plugged() call.
A pre_plugged callback is introduced so that the transports can
set their supported features.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: qemu-stable@nongnu.org
Tested-by: Cornelia Huck <cornelia.huck@de.ibm.com> [ccw]
Reviewed-by: default avatarCornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: default avatarMarcel Apfelbaum <marcel@redhat.com>
Signed-off-by: default avatarMaxime Coquelin <maxime.coquelin@redhat.com>
parent d1eb8f2a

hw/s390x/virtio-ccw.c

+15 −15
Original line number Diff line number Diff line
@@ -1261,6 +1261,16 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) 
    return 0;

}



static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)

{

   VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);

   VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);



    if (dev->max_rev >= 1) {

        virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);

    }

}



/* This is called by virtio-bus just after the device is plugged. */

static void virtio_ccw_device_plugged(DeviceState *d, Error **errp)

{
@@ -1270,6 +1280,10 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) 
    SubchDev *sch = ccw_dev->sch;

    int n = virtio_get_num_queues(vdev);



    if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {

        dev->max_rev = 0;

    }



    if (virtio_get_num_queues(vdev) > VIRTIO_CCW_QUEUE_MAX) {

        error_setg(errp, "The number of virtqueues %d "

                   "exceeds ccw limit %d", n,
@@ -1283,25 +1297,11 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) 


    sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus);



    if (dev->max_rev >= 1) {

        virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);

    }



    css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,

                          d->hotplugged, 1);

}



static void virtio_ccw_post_plugged(DeviceState *d, Error **errp)

{

   VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);

   VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);



   if (!virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1)) {

        /* A backend didn't support modern virtio. */

       dev->max_rev = 0;

   }

}



static void virtio_ccw_device_unplugged(DeviceState *d)

{

    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
@@ -1593,8 +1593,8 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data) 
    k->load_queue = virtio_ccw_load_queue;

    k->save_config = virtio_ccw_save_config;

    k->load_config = virtio_ccw_load_config;

    k->pre_plugged = virtio_ccw_pre_plugged;

    k->device_plugged = virtio_ccw_device_plugged;

    k->post_plugged = virtio_ccw_post_plugged;

    k->device_unplugged = virtio_ccw_device_unplugged;

    k->ioeventfd_started = virtio_ccw_ioeventfd_started;

    k->ioeventfd_set_started = virtio_ccw_ioeventfd_set_started;

hw/virtio/virtio-bus.c

+5 −4
Original line number Diff line number Diff line
@@ -49,16 +49,17 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) 


    DPRINTF("%s: plug device.\n", qbus->name);



    if (klass->device_plugged != NULL) {

        klass->device_plugged(qbus->parent, errp);

    if (klass->pre_plugged != NULL) {

        klass->pre_plugged(qbus->parent, errp);

    }



    /* Get the features of the plugged device. */

    assert(vdc->get_features != NULL);

    vdev->host_features = vdc->get_features(vdev, vdev->host_features,

                                            errp);

    if (klass->post_plugged != NULL) {

        klass->post_plugged(qbus->parent, errp);



    if (klass->device_plugged != NULL) {

        klass->device_plugged(qbus->parent, errp);

    }

}

hw/virtio/virtio-pci.c

+32 −4
Original line number Diff line number Diff line
@@ -1576,18 +1576,48 @@ static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy, 
                                &region->mr);

}



static void virtio_pci_pre_plugged(DeviceState *d, Error **errp)

{

    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);

    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);



    if (virtio_pci_modern(proxy)) {

        virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);

    }



    virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);

}



/* This is called by virtio-bus just after the device is plugged. */

static void virtio_pci_device_plugged(DeviceState *d, Error **errp)

{

    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);

    VirtioBusState *bus = &proxy->bus;

    bool legacy = virtio_pci_legacy(proxy);

    bool modern = virtio_pci_modern(proxy);

    bool modern;

    bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY;

    uint8_t *config;

    uint32_t size;

    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);



    /*

     * Virtio capabilities present without

     * VIRTIO_F_VERSION_1 confuses guests

     */

    if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {

        virtio_pci_disable_modern(proxy);



        if (!legacy) {

            error_setg(errp, "Device doesn't support modern mode, and legacy"

                             " mode is disabled");

            error_append_hint(errp, "Set disable-legacy to off\n");



            return;

        }

    }



    modern = virtio_pci_modern(proxy);



    config = proxy->pci_dev.config;

    if (proxy->class_code) {

        pci_config_set_class(config, proxy->class_code);
@@ -1629,7 +1659,6 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) 


        struct virtio_pci_cfg_cap *cfg_mask;



        virtio_add_feature(&vdev->host_features, VIRTIO_F_VERSION_1);

        virtio_pci_modern_regions_init(proxy);



        virtio_pci_modern_mem_region_map(proxy, &proxy->common, &cap);
@@ -1694,8 +1723,6 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) 
    if (!kvm_has_many_ioeventfds()) {

        proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;

    }



    virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE);

}



static void virtio_pci_device_unplugged(DeviceState *d)
@@ -2508,6 +2535,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data) 
    k->query_guest_notifiers = virtio_pci_query_guest_notifiers;

    k->set_guest_notifiers = virtio_pci_set_guest_notifiers;

    k->vmstate_change = virtio_pci_vmstate_change;

    k->pre_plugged = virtio_pci_pre_plugged;

    k->device_plugged = virtio_pci_device_plugged;

    k->device_unplugged = virtio_pci_device_unplugged;

    k->query_nvectors = virtio_pci_query_nvectors;

hw/virtio/virtio-pci.h

+5 −0
Original line number Diff line number Diff line
@@ -181,6 +181,11 @@ static inline void virtio_pci_force_virtio_1(VirtIOPCIProxy *proxy) 
    proxy->disable_legacy = ON_OFF_AUTO_ON;

}



static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy)

{

    proxy->disable_modern = true;

}



/*

 * virtio-scsi-pci: This extends VirtioPCIProxy.

 */

include/hw/virtio/virtio-bus.h

+5 −5
Original line number Diff line number Diff line
@@ -53,16 +53,16 @@ typedef struct VirtioBusClass { 
    bool (*query_guest_notifiers)(DeviceState *d);

    int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign);

    void (*vmstate_change)(DeviceState *d, bool running);

    /*

     * Expose the features the transport layer supports before

     * the negotiation takes place.

     */

    void (*pre_plugged)(DeviceState *d, Error **errp);

    /*

     * transport independent init function.

     * This is called by virtio-bus just after the device is plugged.

     */

    void (*device_plugged)(DeviceState *d, Error **errp);

    /*

     * Re-evaluate setup after feature bits have been validated

     * by the device backend.

     */

    void (*post_plugged)(DeviceState *d, Error **errp);

    /*

     * transport independent exit function.

     * This is called by virtio-bus just before the device is unplugged.