On Sat, Jan 19, 2019 at 05:37:34PM +0100, Stefan Fritsch wrote:
> Add a sc_driver_features field that is automatically used by
> virtio_negotiate_features() and during reinit.
>
> Make virtio_negotiate_features() return an error code. Virtio 1.0 has a
> special status bit for feature negotiation that means that negotiation
> can fail. Make virtio_negotiate_features() return an error code instead
> of the features.
>
> Make virtio_reinit_start() automatically call
> virtio_negotiate_features().
>
> Add a convenience function virtio_has_feature() to make checking bits
> easier.
>
> Add an error check in viomb for virtio_negotiate_features because it has
> some feature bits that may cause negotiation to fail. More error
> checking in the child drivers is still missing.
ok mlarkin
> ---
> sys/dev/fdt/virtio_mmio.c | 38 ++++++++++++++++++++++++++------------
> sys/dev/pci/virtio_pci.c | 39 +++++++++++++++++++++++++++------------
> sys/dev/pv/if_vio.c | 38 +++++++++++++-------------------------
> sys/dev/pv/vioblk.c | 25 +++++++++----------------
> sys/dev/pv/viocon.c | 6 +++---
> sys/dev/pv/viomb.c | 9 +++++----
> sys/dev/pv/viornd.c | 2 +-
> sys/dev/pv/vioscsi.c | 2 +-
> sys/dev/pv/virtio.c | 13 +++++++------
> sys/dev/pv/virtiovar.h | 15 ++++++++++++---
> sys/dev/pv/vmmci.c | 13 ++++++-------
> 11 files changed, 110 insertions(+), 90 deletions(-)
>
> diff --git a/sys/dev/fdt/virtio_mmio.c b/sys/dev/fdt/virtio_mmio.c
> index 2d7a131125d..6e09fb0e76f 100644
> --- a/sys/dev/fdt/virtio_mmio.c
> +++ b/sys/dev/fdt/virtio_mmio.c
> @@ -91,8 +91,8 @@ void
> virtio_mmio_write_device_config_8(struct virtio_softc *, int, uint64_t);
> uint16_t virtio_mmio_read_queue_size(struct virtio_softc *, uint16_t);
> void virtio_mmio_setup_queue(struct virtio_softc *, struct virtqueue
> *, uint64_t);
> void virtio_mmio_set_status(struct virtio_softc *, int);
> -uint64_t virtio_mmio_negotiate_features(struct virtio_softc *, uint64_t,
> - const struct virtio_feature_name
> *);
> +int virtio_mmio_negotiate_features(struct virtio_softc *,
> + const struct virtio_feature_name *);
> int virtio_mmio_intr(void *);
>
> struct virtio_mmio_softc {
> @@ -300,28 +300,42 @@ virtio_mmio_detach(struct device *self, int flags)
> * Prints available / negotiated features if guest_feature_names != NULL and
> * VIRTIO_DEBUG is 1
> */
> -uint64_t
> -virtio_mmio_negotiate_features(struct virtio_softc *vsc, uint64_t
> guest_features,
> - const struct virtio_feature_name *guest_feature_names)
> +int
> +virtio_mmio_negotiate_features(struct virtio_softc *vsc,
> + const struct virtio_feature_name *guest_feature_names)
> {
> struct virtio_mmio_softc *sc = (struct virtio_mmio_softc *)vsc;
> uint64_t host, neg;
>
> + vsc->sc_active_features = 0;
> +
> /*
> - * indirect descriptors can be switched off by setting bit 1 in the
> - * driver flags, see config(8)
> + * We enable indirect descriptors by default. They can be switched
> + * off by setting bit 1 in the driver flags, see config(8).
> */
> if (!(vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_INDIRECT) &&
> !(vsc->sc_child->dv_cfdata->cf_flags & VIRTIO_CF_NO_INDIRECT)) {
> - guest_features |= VIRTIO_F_RING_INDIRECT_DESC;
> - } else {
> + vsc->sc_driver_features |= VIRTIO_F_RING_INDIRECT_DESC;
> + } else if (guest_feature_names != NULL) {
> printf("RingIndirectDesc disabled by UKC\n");
> }
> + /*
> + * The driver must add VIRTIO_F_RING_EVENT_IDX if it supports it.
> + * If it did, check if it is disabled by bit 2 in the driver flags.
> + */
> + if ((vsc->sc_driver_features & VIRTIO_F_RING_EVENT_IDX) &&
> + ((vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX) ||
> + (vsc->sc_child->dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX))) {
> + if (guest_feature_names != NULL)
> + printf(" RingEventIdx disabled by UKC");
> + vsc->sc_driver_features &= ~(VIRTIO_F_RING_EVENT_IDX);
> + }
> +
> bus_space_write_4(sc->sc_iot, sc->sc_ioh,
> VIRTIO_MMIO_HOST_FEATURES_SEL, 0);
> host = bus_space_read_4(sc->sc_iot, sc->sc_ioh,
> VIRTIO_MMIO_HOST_FEATURES);
> - neg = host & guest_features;
> + neg = host & vsc->sc_driver_features;
> #if VIRTIO_DEBUG
> if (guest_feature_names)
> virtio_log_features(host, neg, guest_feature_names);
> @@ -330,13 +344,13 @@ virtio_mmio_negotiate_features(struct virtio_softc
> *vsc, uint64_t guest_features
> VIRTIO_MMIO_GUEST_FEATURES_SEL, 0);
> bus_space_write_4(sc->sc_iot, sc->sc_ioh,
> VIRTIO_MMIO_GUEST_FEATURES, neg);
> - vsc->sc_features = neg;
> + vsc->sc_active_features = neg;
> if (neg & VIRTIO_F_RING_INDIRECT_DESC)
> vsc->sc_indirect = 1;
> else
> vsc->sc_indirect = 0;
>
> - return neg;
> + return 0;
> }
>
> /*
> diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
> index 7be93684a68..f370f513e85 100644
> --- a/sys/dev/pci/virtio_pci.c
> +++ b/sys/dev/pci/virtio_pci.c
> @@ -67,8 +67,8 @@ void virtio_pci_write_device_config_8(struct
> virtio_softc *, int, uint64_t);
> uint16_t virtio_pci_read_queue_size(struct virtio_softc *, uint16_t);
> void virtio_pci_setup_queue(struct virtio_softc *, struct virtqueue
> *, uint64_t);
> void virtio_pci_set_status(struct virtio_softc *, int);
> -uint64_t virtio_pci_negotiate_features(struct virtio_softc *, uint64_t,
> - const struct virtio_feature_name
> *);
> +int virtio_pci_negotiate_features(struct virtio_softc *,
> + const struct virtio_feature_name *);
> void virtio_pci_set_msix_queue_vector(struct virtio_pci_softc *,
> uint32_t, uint16_t);
> void virtio_pci_set_msix_config_vector(struct virtio_pci_softc *,
> uint16_t);
> int virtio_pci_msix_establish(struct virtio_pci_softc *, struct
> pci_attach_args *, int, int (*)(void *), void *);
> @@ -374,39 +374,54 @@ virtio_pci_adjust_config_region(struct virtio_pci_softc
> *sc)
> * Prints available / negotiated features if guest_feature_names != NULL and
> * VIRTIO_DEBUG is 1
> */
> -uint64_t
> -virtio_pci_negotiate_features(struct virtio_softc *vsc, uint64_t
> guest_features,
> +int
> +virtio_pci_negotiate_features(struct virtio_softc *vsc,
> const struct virtio_feature_name *guest_feature_names)
> {
> struct virtio_pci_softc *sc = (struct virtio_pci_softc *)vsc;
> uint64_t host, neg;
>
> + vsc->sc_active_features = 0;
> +
> /*
> - * indirect descriptors can be switched off by setting bit 1 in the
> - * driver flags, see config(8)
> + * We enable indirect descriptors by default. They can be switched
> + * off by setting bit 1 in the driver flags, see config(8)
> */
> if (!(vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_INDIRECT) &&
> !(vsc->sc_child->dv_cfdata->cf_flags & VIRTIO_CF_NO_INDIRECT)) {
> - guest_features |= VIRTIO_F_RING_INDIRECT_DESC;
> - } else {
> - printf("RingIndirectDesc disabled by UKC\n");
> + vsc->sc_driver_features |= VIRTIO_F_RING_INDIRECT_DESC;
> + } else if (guest_feature_names != NULL) {
> + printf(" RingIndirectDesc disabled by UKC");
> + }
> +
> + /*
> + * The driver must add VIRTIO_F_RING_EVENT_IDX if it supports it.
> + * If it did, check if it is disabled by bit 2 in the driver flags.
> + */
> + if ((vsc->sc_driver_features & VIRTIO_F_RING_EVENT_IDX) &&
> + ((vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX) ||
> + (vsc->sc_child->dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX))) {
> + if (guest_feature_names != NULL)
> + printf(" RingEventIdx disabled by UKC");
> + vsc->sc_driver_features &= ~(VIRTIO_F_RING_EVENT_IDX);
> }
> +
> host = bus_space_read_4(sc->sc_iot, sc->sc_ioh,
> VIRTIO_CONFIG_DEVICE_FEATURES);
> - neg = host & guest_features;
> + neg = host & vsc->sc_driver_features;
> #if VIRTIO_DEBUG
> if (guest_feature_names)
> virtio_log_features(host, neg, guest_feature_names);
> #endif
> bus_space_write_4(sc->sc_iot, sc->sc_ioh,
> VIRTIO_CONFIG_GUEST_FEATURES, neg);
> - vsc->sc_features = neg;
> + vsc->sc_active_features = neg;
> if (neg & VIRTIO_F_RING_INDIRECT_DESC)
> vsc->sc_indirect = 1;
> else
> vsc->sc_indirect = 0;
>
> - return neg;
> + return 0;
> }
>
> /*
> diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c
> index bfc7cfd1ddc..252ad2fdf9b 100644
> --- a/sys/dev/pv/if_vio.c
> +++ b/sys/dev/pv/if_vio.c
> @@ -515,7 +515,6 @@ vio_attach(struct device *parent, struct device *self,
> void *aux)
> {
> struct vio_softc *sc = (struct vio_softc *)self;
> struct virtio_softc *vsc = (struct virtio_softc *)parent;
> - uint64_t features;
> int i;
> struct ifnet *ifp = &sc->sc_ac.ac_if;
>
> @@ -531,23 +530,13 @@ vio_attach(struct device *parent, struct device *self,
> void *aux)
> vsc->sc_ipl = IPL_NET;
> vsc->sc_vqs = &sc->sc_vq[0];
> vsc->sc_config_change = 0;
> -
> - features = VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS |
> + vsc->sc_driver_features = VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS |
> VIRTIO_NET_F_CTRL_VQ | VIRTIO_NET_F_CTRL_RX |
> - VIRTIO_NET_F_MRG_RXBUF | VIRTIO_NET_F_CSUM;
> - /*
> - * VIRTIO_F_RING_EVENT_IDX can be switched off by setting bit 2 in the
> - * driver flags, see config(8)
> - */
> - if (!(sc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX) &&
> - !(vsc->sc_dev.dv_cfdata->cf_flags & VIRTIO_CF_NO_EVENT_IDX))
> - features |= VIRTIO_F_RING_EVENT_IDX;
> - else
> - printf(": RingEventIdx disabled by UKC");
> + VIRTIO_NET_F_MRG_RXBUF | VIRTIO_NET_F_CSUM |
> + VIRTIO_F_RING_EVENT_IDX;
>
> - features = virtio_negotiate_features(vsc, features,
> - virtio_net_feature_names);
> - if (features & VIRTIO_NET_F_MAC) {
> + virtio_negotiate_features(vsc, virtio_net_feature_names);
> + if (virtio_has_feature(vsc, VIRTIO_NET_F_MAC)) {
> vio_get_lladr(&sc->sc_ac, vsc);
> } else {
> ether_fakeaddr(ifp);
> @@ -555,7 +544,7 @@ vio_attach(struct device *parent, struct device *self,
> void *aux)
> }
> printf(": address %s\n", ether_sprintf(sc->sc_ac.ac_enaddr));
>
> - if (features & VIRTIO_NET_F_MRG_RXBUF) {
> + if (virtio_has_feature(vsc, VIRTIO_NET_F_MRG_RXBUF)) {
> sc->sc_hdr_size = sizeof(struct virtio_net_hdr);
> ifp->if_hardmtu = 16000; /* arbitrary limit */
> } else {
> @@ -575,12 +564,12 @@ vio_attach(struct device *parent, struct device *self,
> void *aux)
> vsc->sc_nvqs = 2;
> sc->sc_vq[VQTX].vq_done = vio_tx_intr;
> virtio_start_vq_intr(vsc, &sc->sc_vq[VQRX]);
> - if (features & VIRTIO_F_RING_EVENT_IDX)
> + if (virtio_has_feature(vsc, VIRTIO_F_RING_EVENT_IDX))
> virtio_postpone_intr_far(&sc->sc_vq[VQTX]);
> else
> virtio_stop_vq_intr(vsc, &sc->sc_vq[VQTX]);
> - if ((features & VIRTIO_NET_F_CTRL_VQ)
> - && (features & VIRTIO_NET_F_CTRL_RX)) {
> + if (virtio_has_feature(vsc, VIRTIO_NET_F_CTRL_VQ)
> + && virtio_has_feature(vsc, VIRTIO_NET_F_CTRL_RX)) {
> if (virtio_alloc_vq(vsc, &sc->sc_vq[VQCTL], 2, NBPG, 1,
> "control") == 0) {
> sc->sc_vq[VQCTL].vq_done = vio_ctrleof;
> @@ -598,7 +587,7 @@ vio_attach(struct device *parent, struct device *self,
> void *aux)
> ifp->if_start = vio_start;
> ifp->if_ioctl = vio_ioctl;
> ifp->if_capabilities = IFCAP_VLAN_MTU;
> - if (features & VIRTIO_NET_F_CSUM)
> + if (virtio_has_feature(vsc, VIRTIO_NET_F_CSUM))
> ifp->if_capabilities |= IFCAP_CSUM_TCPv4|IFCAP_CSUM_UDPv4;
> IFQ_SET_MAXLEN(&ifp->if_snd, vsc->sc_vqs[1].vq_num - 1);
> ifmedia_init(&sc->sc_media, 0, vio_media_change, vio_media_status);
> @@ -629,7 +618,7 @@ vio_link_state(struct ifnet *ifp)
> struct virtio_softc *vsc = sc->sc_virtio;
> int link_state = LINK_STATE_FULL_DUPLEX;
>
> - if (vsc->sc_features & VIRTIO_NET_F_STATUS) {
> + if (virtio_has_feature(vsc, VIRTIO_NET_F_STATUS)) {
> int status = virtio_read_device_config_2(vsc,
> VIRTIO_NET_CONFIG_STATUS);
> if (!(status & VIRTIO_NET_S_LINK_UP))
> @@ -706,7 +695,6 @@ vio_stop(struct ifnet *ifp, int disable)
> vio_rx_drain(sc);
>
> virtio_reinit_start(vsc);
> - virtio_negotiate_features(vsc, vsc->sc_features, NULL);
> virtio_start_vq_intr(vsc, &sc->sc_vq[VQRX]);
> virtio_stop_vq_intr(vsc, &sc->sc_vq[VQTX]);
> if (vsc->sc_nvqs >= 3)
> @@ -815,7 +803,7 @@ again:
> }
> if (ifq_is_oactive(&ifp->if_snd)) {
> int r;
> - if (vsc->sc_features & VIRTIO_F_RING_EVENT_IDX)
> + if (virtio_has_feature(vsc, VIRTIO_F_RING_EVENT_IDX))
> r = virtio_postpone_intr_smart(&sc->sc_vq[VQTX]);
> else
> r = virtio_start_vq_intr(vsc, &sc->sc_vq[VQTX]);
> @@ -1069,7 +1057,7 @@ again:
> if (r) {
> vio_populate_rx_mbufs(sc);
> /* set used event index to the next slot */
> - if (vsc->sc_features & VIRTIO_F_RING_EVENT_IDX) {
> + if (virtio_has_feature(vsc, VIRTIO_F_RING_EVENT_IDX)) {
> if (virtio_start_vq_intr(vq->vq_owner, vq))
> goto again;
> }
> diff --git a/sys/dev/pv/vioblk.c b/sys/dev/pv/vioblk.c
> index 460c2dfa766..9fc6e82f89e 100644
> --- a/sys/dev/pv/vioblk.c
> +++ b/sys/dev/pv/vioblk.c
> @@ -171,7 +171,6 @@ vioblk_attach(struct device *parent, struct device *self,
> void *aux)
> struct vioblk_softc *sc = (struct vioblk_softc *)self;
> struct virtio_softc *vsc = (struct virtio_softc *)parent;
> struct scsibus_attach_args saa;
> - uint64_t features;
> int qsize;
>
> vsc->sc_vqs = &sc->sc_vq[0];
> @@ -182,15 +181,12 @@ vioblk_attach(struct device *parent, struct device
> *self, void *aux)
> vsc->sc_child = self;
> vsc->sc_ipl = IPL_BIO;
> sc->sc_virtio = vsc;
> + vsc->sc_driver_features = VIRTIO_BLK_F_RO | VIRTIO_F_NOTIFY_ON_EMPTY |
> + VIRTIO_BLK_F_SIZE_MAX | VIRTIO_BLK_F_SEG_MAX | VIRTIO_BLK_F_FLUSH;
>
> - features = virtio_negotiate_features(vsc,
> - (VIRTIO_BLK_F_RO | VIRTIO_F_NOTIFY_ON_EMPTY |
> - VIRTIO_BLK_F_SIZE_MAX | VIRTIO_BLK_F_SEG_MAX |
> - VIRTIO_BLK_F_FLUSH),
> - vioblk_feature_names);
> + virtio_negotiate_features(vsc, vioblk_feature_names);
>
> -
> - if (features & VIRTIO_BLK_F_SIZE_MAX) {
> + if (virtio_has_feature(vsc, VIRTIO_BLK_F_SIZE_MAX)) {
> uint32_t size_max = virtio_read_device_config_4(vsc,
> VIRTIO_BLK_CONFIG_SIZE_MAX);
> if (size_max < PAGE_SIZE) {
> @@ -199,7 +195,7 @@ vioblk_attach(struct device *parent, struct device *self,
> void *aux)
> }
> }
>
> - if (features & VIRTIO_BLK_F_SEG_MAX) {
> + if (virtio_has_feature(vsc, VIRTIO_BLK_F_SEG_MAX)) {
> uint32_t seg_max = virtio_read_device_config_4(vsc,
> VIRTIO_BLK_CONFIG_SEG_MAX);
> if (seg_max < SEG_MAX) {
> @@ -220,7 +216,7 @@ vioblk_attach(struct device *parent, struct device *self,
> void *aux)
> qsize = sc->sc_vq[0].vq_num;
> sc->sc_vq[0].vq_done = vioblk_vq_done;
>
> - if (features & VIRTIO_F_NOTIFY_ON_EMPTY) {
> + if (virtio_has_feature(vsc, VIRTIO_F_NOTIFY_ON_EMPTY)) {
> virtio_stop_vq_intr(vsc, &sc->sc_vq[0]);
> sc->sc_notify_on_empty = 1;
> }
> @@ -252,7 +248,7 @@ vioblk_attach(struct device *parent, struct device *self,
> void *aux)
> sc->sc_link.luns = 1;
> sc->sc_link.adapter_target = 2;
> DNPRINTF(1, "%s: qsize: %d\n", __func__, qsize);
> - if (features & VIRTIO_BLK_F_RO)
> + if (virtio_has_feature(vsc, VIRTIO_BLK_F_RO))
> sc->sc_link.flags |= SDEV_READONLY;
>
> bzero(&saa, sizeof(saa));
> @@ -430,7 +426,7 @@ vioblk_scsi_cmd(struct scsi_xfer *xs)
> break;
>
> case SYNCHRONIZE_CACHE:
> - if ((vsc->sc_features & VIRTIO_BLK_F_FLUSH) == 0) {
> + if (!virtio_has_feature(vsc, VIRTIO_BLK_F_FLUSH)) {
> vioblk_scsi_done(xs, XS_NOERROR);
> return;
> }
> @@ -553,13 +549,10 @@ vioblk_scsi_cmd(struct scsi_xfer *xs)
> delay(1000);
> } while(--timeout > 0);
> if (timeout <= 0) {
> - uint32_t features;
> printf("%s: SCSI_POLL timed out\n", __func__);
> vioblk_reset(sc);
> virtio_reinit_start(vsc);
> - features = virtio_negotiate_features(vsc, vsc->sc_features,
> - NULL);
> - KASSERT(features == vsc->sc_features);
> + virtio_reinit_end(vsc);
> }
> splx(s);
> return;
> diff --git a/sys/dev/pv/viocon.c b/sys/dev/pv/viocon.c
> index ed73152bbdd..a47eae33d39 100644
> --- a/sys/dev/pv/viocon.c
> +++ b/sys/dev/pv/viocon.c
> @@ -193,8 +193,8 @@ viocon_attach(struct device *parent, struct device *self,
> void *aux)
> goto err;
> }
>
> - virtio_negotiate_features(vsc, VIRTIO_CONSOLE_F_SIZE,
> - viocon_feature_names);
> + vsc->sc_driver_features = VIRTIO_CONSOLE_F_SIZE;
> + virtio_negotiate_features(vsc, viocon_feature_names);
>
> printf("\n");
> DPRINTF("%s: softc: %p\n", __func__, sc);
> @@ -277,7 +277,7 @@ viocon_port_create(struct viocon_softc *sc, int portidx)
> */
> vp->vp_tx_buf = vp->vp_rx_buf + vp->vp_rx->vq_num * BUFSIZE;
>
> - if (vsc->sc_features & VIRTIO_CONSOLE_F_SIZE) {
> + if (virtio_has_feature(vsc, VIRTIO_CONSOLE_F_SIZE)) {
> vp->vp_cols = virtio_read_device_config_2(vsc,
> VIRTIO_CONSOLE_COLS);
> vp->vp_rows = virtio_read_device_config_2(vsc,
> diff --git a/sys/dev/pv/viomb.c b/sys/dev/pv/viomb.c
> index 3be83eed562..71e92abf8a4 100644
> --- a/sys/dev/pv/viomb.c
> +++ b/sys/dev/pv/viomb.c
> @@ -158,8 +158,9 @@ viomb_attach(struct device *parent, struct device *self,
> void *aux)
> vsc->sc_ipl = IPL_BIO;
> vsc->sc_config_change = viomb_config_change;
>
> - virtio_negotiate_features(vsc, VIRTIO_BALLOON_F_MUST_TELL_HOST,
> - viomb_feature_names);
> + vsc->sc_driver_features = VIRTIO_BALLOON_F_MUST_TELL_HOST;
> + if (virtio_negotiate_features(vsc, viomb_feature_names) != 0)
> + goto err;
>
> if ((virtio_alloc_vq(vsc, &sc->sc_vq[VQ_INFLATE], VQ_INFLATE,
> sizeof(u_int32_t) * PGS_PER_REQ, 1, "inflate") != 0))
> @@ -365,7 +366,7 @@ viomb_deflate(struct viomb_softc *sc)
> virtio_enqueue_p(vq, slot, b->bl_dmamap, 0,
> sizeof(u_int32_t) * nvpages, VRING_READ);
>
> - if (!(vsc->sc_features & VIRTIO_BALLOON_F_MUST_TELL_HOST))
> + if (!virtio_has_feature(vsc, VIRTIO_BALLOON_F_MUST_TELL_HOST))
> uvm_pglistfree(&b->bl_pglist);
> virtio_enqueue_commit(vsc, vq, slot, VRING_NOTIFY);
> return;
> @@ -463,7 +464,7 @@ viomb_deflate_intr(struct virtqueue *vq)
> sizeof(u_int32_t) * nvpages,
> BUS_DMASYNC_POSTWRITE);
>
> - if (vsc->sc_features & VIRTIO_BALLOON_F_MUST_TELL_HOST)
> + if (virtio_has_feature(vsc, VIRTIO_BALLOON_F_MUST_TELL_HOST))
> uvm_pglistfree(&b->bl_pglist);
>
> VIOMBDEBUG(sc, "updating sc->sc_actual from %u to %llu\n",
> diff --git a/sys/dev/pv/viornd.c b/sys/dev/pv/viornd.c
> index 9d1e8c60938..0886d052310 100644
> --- a/sys/dev/pv/viornd.c
> +++ b/sys/dev/pv/viornd.c
> @@ -96,7 +96,7 @@ viornd_attach(struct device *parent, struct device *self,
> void *aux)
> vsc->sc_ipl = IPL_NET;
> sc->sc_virtio = vsc;
>
> - virtio_negotiate_features(vsc, 0, NULL);
> + virtio_negotiate_features(vsc, NULL);
>
> if (sc->sc_dev.dv_cfdata->cf_flags & VIORND_ONESHOT) {
> sc->sc_interval = 0;
> diff --git a/sys/dev/pv/vioscsi.c b/sys/dev/pv/vioscsi.c
> index a7ccd8913b3..ad18ec151ec 100644
> --- a/sys/dev/pv/vioscsi.c
> +++ b/sys/dev/pv/vioscsi.c
> @@ -126,7 +126,7 @@ vioscsi_attach(struct device *parent, struct device
> *self, void *aux)
> vsc->sc_vqs = sc->sc_vqs;
> vsc->sc_nvqs = nitems(sc->sc_vqs);
>
> - virtio_negotiate_features(vsc, 0, NULL);
> + virtio_negotiate_features(vsc, NULL);
> uint32_t cmd_per_lun = virtio_read_device_config_4(vsc,
> VIRTIO_SCSI_CONFIG_CMD_PER_LUN);
> uint32_t seg_max = virtio_read_device_config_4(vsc,
> diff --git a/sys/dev/pv/virtio.c b/sys/dev/pv/virtio.c
> index 40ea0b8a85e..904943420e0 100644
> --- a/sys/dev/pv/virtio.c
> +++ b/sys/dev/pv/virtio.c
> @@ -126,15 +126,15 @@ virtio_log_features(uint64_t host, uint64_t neg,
> * <dequeue finished requests>; // virtio_dequeue() still can be called
> * <revoke pending requests in the vqs if any>;
> * virtio_reinit_start(sc); // dequeue prohibitted
> - * newfeatures = virtio_negotiate_features(sc, requestedfeatures);
> * <some other initialization>;
> * virtio_reinit_end(sc); // device activated; enqueue allowed
> - * Once attached, feature negotiation can only be allowed after virtio_reset.
> + * Once attached, features are assumed to not change again.
> */
> void
> virtio_reset(struct virtio_softc *sc)
> {
> virtio_device_reset(sc);
> + sc->sc_active_features = 0;
> }
>
> void
> @@ -144,6 +144,7 @@ virtio_reinit_start(struct virtio_softc *sc)
>
> virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_ACK);
> virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER);
> + virtio_negotiate_features(sc, NULL);
> for (i = 0; i < sc->sc_nvqs; i++) {
> int n;
> struct virtqueue *vq = &sc->sc_vqs[i];
> @@ -300,7 +301,7 @@ virtio_alloc_vq(struct virtio_softc *sc, struct virtqueue
> *vq, int index,
> if (((vq_size - 1) & vq_size) != 0)
> panic("vq_size not power of two: %d", vq_size);
>
> - hdrlen = (sc->sc_features & VIRTIO_F_RING_EVENT_IDX) ? 3 : 2;
> + hdrlen = virtio_has_feature(sc, VIRTIO_F_RING_EVENT_IDX) ? 3 : 2;
>
> /* allocsize1: descriptor table + avail ring + pad */
> allocsize1 = VIRTQUEUE_ALIGN(sizeof(struct vring_desc) * vq_size
> @@ -677,7 +678,7 @@ virtio_enqueue_commit(struct virtio_softc *sc, struct
> virtqueue *vq, int slot,
>
> notify:
> if (notifynow) {
> - if (vq->vq_owner->sc_features & VIRTIO_F_RING_EVENT_IDX) {
> + if (virtio_has_feature(vq->vq_owner, VIRTIO_F_RING_EVENT_IDX)) {
> uint16_t o = vq->vq_avail->idx;
> uint16_t n = vq->vq_avail_idx;
> uint16_t t;
> @@ -874,7 +875,7 @@ virtio_postpone_intr_far(struct virtqueue *vq)
> void
> virtio_stop_vq_intr(struct virtio_softc *sc, struct virtqueue *vq)
> {
> - if ((sc->sc_features & VIRTIO_F_RING_EVENT_IDX)) {
> + if (virtio_has_feature(sc, VIRTIO_F_RING_EVENT_IDX)) {
> /*
> * No way to disable the interrupt completely with
> * RingEventIdx. Instead advance used_event by half
> @@ -898,7 +899,7 @@ virtio_start_vq_intr(struct virtio_softc *sc, struct
> virtqueue *vq)
> * interrupts is done through setting the latest
> * consumed index in the used_event field
> */
> - if (sc->sc_features & VIRTIO_F_RING_EVENT_IDX)
> + if (virtio_has_feature(sc, VIRTIO_F_RING_EVENT_IDX))
> VQ_USED_EVENT(vq) = vq->vq_used_idx;
> else
> vq->vq_avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
> diff --git a/sys/dev/pv/virtiovar.h b/sys/dev/pv/virtiovar.h
> index 4a52907a4a5..2b43086a4e9 100644
> --- a/sys/dev/pv/virtiovar.h
> +++ b/sys/dev/pv/virtiovar.h
> @@ -151,7 +151,7 @@ struct virtio_ops {
> uint16_t (*read_queue_size)(struct virtio_softc *, uint16_t);
> void (*setup_queue)(struct virtio_softc *, struct virtqueue
> *, uint64_t);
> void (*set_status)(struct virtio_softc *, int);
> - uint64_t (*neg_features)(struct virtio_softc *, uint64_t, const
> struct virtio_feature_name *);
> + int (*neg_features)(struct virtio_softc *, const struct
> virtio_feature_name *);
> int (*poll_intr)(void *);
> };
>
> @@ -164,7 +164,8 @@ struct virtio_softc {
>
> int sc_ipl; /* set by child */
>
> - uint64_t sc_features;
> + uint64_t sc_driver_features;
> + uint64_t sc_active_features;
> int sc_indirect;
>
> int sc_nvqs; /* set by child */
> @@ -189,13 +190,21 @@ struct virtio_softc {
> #define virtio_write_device_config_8(sc, o, v)
> (sc)->sc_ops->write_dev_cfg_8(sc, o, v)
> #define virtio_read_queue_size(sc, i)
> (sc)->sc_ops->read_queue_size(sc, i)
> #define virtio_setup_queue(sc, i, v)
> (sc)->sc_ops->setup_queue(sc, i, v)
> -#define virtio_negotiate_features(sc, f, n)
> (sc)->sc_ops->neg_features(sc, f, n)
> +#define virtio_negotiate_features(sc, n)
> (sc)->sc_ops->neg_features(sc, n)
> #define virtio_poll_intr(sc)
> (sc)->sc_ops->poll_intr(sc)
>
> /* only for transport drivers */
> #define virtio_set_status(sc, i)
> (sc)->sc_ops->set_status(sc, i)
> #define virtio_device_reset(sc) virtio_set_status((sc),
> 0)
>
> +static inline int
> +virtio_has_feature(struct virtio_softc *sc, unsigned int fbit)
> +{
> + if (sc->sc_active_features & fbit)
> + return 1;
> + return 0;
> +}
> +
> int virtio_alloc_vq(struct virtio_softc*, struct virtqueue*, int, int, int,
> const char*);
> int virtio_free_vq(struct virtio_softc*, struct virtqueue*);
> diff --git a/sys/dev/pv/vmmci.c b/sys/dev/pv/vmmci.c
> index 80ba1224e3b..2313b11c71e 100644
> --- a/sys/dev/pv/vmmci.c
> +++ b/sys/dev/pv/vmmci.c
> @@ -94,7 +94,6 @@ vmmci_attach(struct device *parent, struct device *self,
> void *aux)
> {
> struct vmmci_softc *sc = (struct vmmci_softc *)self;
> struct virtio_softc *vsc = (struct virtio_softc *)parent;
> - uint64_t features;
>
> if (vsc->sc_child != NULL)
> panic("already attached to something else");
> @@ -105,10 +104,11 @@ vmmci_attach(struct device *parent, struct device
> *self, void *aux)
> vsc->sc_ipl = IPL_NET;
> sc->sc_virtio = vsc;
>
> - features = VMMCI_F_TIMESYNC | VMMCI_F_ACK | VMMCI_F_SYNCRTC;
> - features = virtio_negotiate_features(vsc, features, NULL);
> + vsc->sc_driver_features = VMMCI_F_TIMESYNC | VMMCI_F_ACK |
> + VMMCI_F_SYNCRTC;
> + virtio_negotiate_features(vsc, NULL);
>
> - if (features & VMMCI_F_TIMESYNC) {
> + if (virtio_has_feature(vsc, VMMCI_F_TIMESYNC)) {
> strlcpy(sc->sc_sensordev.xname, sc->sc_dev.dv_xname,
> sizeof(sc->sc_sensordev.xname));
> sc->sc_sensor.type = SENSOR_TIMEDELTA;
> @@ -128,7 +128,7 @@ vmmci_activate(struct device *self, int act)
> struct vmmci_softc *sc = (struct vmmci_softc *)self;
> struct virtio_softc *vsc = sc->sc_virtio;
>
> - if ((vsc->sc_features & VMMCI_F_ACK) == 0)
> + if (virtio_has_feature(vsc, VMMCI_F_ACK) == 0)
> return (0);
>
> switch (act) {
> @@ -183,8 +183,7 @@ vmmci_config_change(struct virtio_softc *vsc)
> break;
> }
>
> - if ((cmd != VMMCI_NONE) &&
> - (vsc->sc_features & VMMCI_F_ACK))
> + if ((cmd != VMMCI_NONE) && virtio_has_feature(vsc, VMMCI_F_ACK))
> virtio_write_device_config_4(vsc, VMMCI_CONFIG_COMMAND, cmd);
>
> return (1);
> --
> 2.19.0
>
