[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] virtio-ccw: enable virtio-1
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] virtio-ccw: enable virtio-1 |
Date: |
Thu, 10 Sep 2015 11:11:20 +0200 |
On Thu, 10 Sep 2015 12:07:18 +0300
"Michael S. Tsirkin" <address@hidden> wrote:
> On Fri, Sep 04, 2015 at 10:54:29AM +0200, Cornelia Huck wrote:
> > Let's enable revision 1 for virtio-ccw devices. We can always offer
> > VERSION_1 as drivers in legacy mode won't be able to see it anyway.
> >
> > We have to introduce a way to set a lower maximum revision for a device
> > to accommodate the following cases:
> > - compat machines (to enforce legacy only)
>
> But you don't actually set this for any compat machines.
> If you don't, this seems a bit pointless.
Huh? The first hunk of this patch does this.
>
> > - virtio-blk with scsi support (version 1 + scsi is fenced by common
> > code, with a user-configured max revision of 0 we can allow scsi
> > via not offering VERSION_1)
>
> For this use, for pci users need to do disable_modern=true.
> I find it unfortunate that for ccw one needs to do max_revision=0.
I don't like the pci concept: much too coarse-grained and not very
future proof.
>
> Revision numbers generally are a ccw specific concept. I'm not sure it
> is wise to expose it to users.
What is wrong about exposing transport-specific concepts?
>
> > Signed-off-by: Cornelia Huck <address@hidden>
> > ---
> > hw/s390x/s390-virtio-ccw.c | 4 ++++
> > hw/s390x/virtio-ccw.c | 30 ++++++++++++++++++++++++++++--
> > hw/s390x/virtio-ccw.h | 6 ++++--
> > 3 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index e2a26e9..fb6e45c 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -287,6 +287,10 @@ static const TypeInfo ccw_machine_info = {
> > .driver = TYPE_S390_SKEYS,\
> > .property = "migration-enabled",\
> > .value = "off",\
> > + },{\
> > + .driver = TYPE_VIRTIO_CCW_DEVICE,\
> > + .property = "max_revision",\
> > + .value = 0,\
> > },
> >
> > static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
> > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> > index eed7b3e..fb103b7 100644
> > --- a/hw/s390x/virtio-ccw.c
> > +++ b/hw/s390x/virtio-ccw.c
> > @@ -467,7 +467,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> > MEMTXATTRS_UNSPECIFIED,
> > NULL);
> > if (features.index == 0) {
> > - features.features = (uint32_t)vdev->host_features;
> > + if (dev->revision >= 1) {
> > + /* Don't offer legacy features for modern devices. */
> > + features.features = (uint32_t)
> > + (vdev->host_features & ~VIRTIO_LEGACY_FEATURES);
> > + } else {
> > + features.features = (uint32_t)vdev->host_features;
> > + }
> > } else if ((features.index == 1) && (dev->revision >= 1)) {
> > /*
> > * Only offer feature bits beyond 31 if the guest has
> > @@ -768,7 +774,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> > * need to fetch it here. Nothing to do for now, though.
> > */
> > if (dev->revision >= 0 ||
> > - revinfo.revision > virtio_ccw_rev_max(vdev)) {
> > + revinfo.revision > virtio_ccw_rev_max(dev)) {
> > ret = -ENOSYS;
> > break;
> > }
> > @@ -1541,6 +1547,10 @@ 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);
> > }
> > @@ -1557,6 +1567,8 @@ static Property virtio_ccw_net_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1584,6 +1596,8 @@ static Property virtio_ccw_blk_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1611,6 +1625,8 @@ static Property virtio_ccw_serial_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1638,6 +1654,8 @@ static Property virtio_ccw_balloon_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1665,6 +1683,8 @@ static Property virtio_ccw_scsi_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1691,6 +1711,8 @@ static const TypeInfo virtio_ccw_scsi = {
> > #ifdef CONFIG_VHOST_SCSI
> > static Property vhost_ccw_scsi_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1729,6 +1751,8 @@ static Property virtio_ccw_rng_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > @@ -1882,6 +1906,8 @@ static Property virtio_ccw_9p_properties[] = {
> > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
> > DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
> > VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
> > + DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
> > + VIRTIO_CCW_MAX_REV),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
> > index 692ddd7..7ab8367 100644
> > --- a/hw/s390x/virtio-ccw.h
> > +++ b/hw/s390x/virtio-ccw.h
> > @@ -88,6 +88,7 @@ struct VirtioCcwDevice {
> > SubchDev *sch;
> > char *bus_id;
> > int revision;
> > + uint32_t max_rev;
> > VirtioBusState bus;
> > bool ioeventfd_started;
> > bool ioeventfd_disabled;
> > @@ -102,9 +103,10 @@ struct VirtioCcwDevice {
> > };
> >
> > /* The maximum virtio revision we support. */
> > -static inline int virtio_ccw_rev_max(VirtIODevice *vdev)
> > +#define VIRTIO_CCW_MAX_REV 1
> > +static inline int virtio_ccw_rev_max(VirtioCcwDevice *dev)
> > {
> > - return 0;
> > + return dev->max_rev;
> > }
> >
> > /* virtual css bus type */
> > --
> > 2.3.8
>
- Re: [Qemu-devel] [PATCH 1/4] virtio: ring sizes vs. reset, (continued)
[Qemu-devel] [PATCH 3/4] virtio-ccw: feature bits > 31 handling, Cornelia Huck, 2015/09/04
[Qemu-devel] [PATCH 2/4] virtio-ccw: support ring size changes, Cornelia Huck, 2015/09/04
[Qemu-devel] [PATCH 4/4] virtio-ccw: enable virtio-1, Cornelia Huck, 2015/09/04
Re: [Qemu-devel] [PATCH 4/4] virtio-ccw: enable virtio-1, Cornelia Huck, 2015/09/11