Author: sephe
Date: Thu Jan  5 05:24:30 2017
New Revision: 311364
URL: https://svnweb.freebsd.org/changeset/base/311364

Log:
  MFC 309128,309129,309131-309136,309138-309140,309224,309225
  
  309128
      hyperv/vmbus: Commit the GPADL id only after the connection succeeds.
  
      Minor style change.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8563
  
  309129
      hyperv/vmbus: Minor style changes.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8564
  
  309131
      hyperv/vmbus: Fix sysctl tree leakage, if channel open fails.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8565
  
  309132
      hyperv/vmbus: Don't close unopened channels.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8566
  
  309133
      hyperv/vmbus: GPADL disconnect error on a revoked channel is benign.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8567
  
  309134
      hyperv/vmbus: No stranded bufring GPADL is allowed.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8568
  
  309135
      hyperv/vmbus: Return EISCONN if the bufring GPADL can't be disconnected.
  
      So that the callers of vmbus_chan_open_br() could handle the passed in
      bufring memory properly.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8569
  
  309136
      hyperv/vmbus: Don't free the bufring if its GPADL can't be disconnected.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8570
  
  309138
      hyperv/vmbus: Always try disconnect/free bufring memory upon channel close
  
      While I'm here, minor wording and style changes.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8598
  
  309139
      hyperv/vmbus: Propagate close error.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8599
  
  309140
      hyperv/vmbus: Add a simplified version of channel close.
  
      So that the caller can know the channel close error and react accordingly.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8600
  
  309224
      hyperv/vmbus: Zero out GPADL if error happens.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8601
  
  309225
      hyperv/vmbus: Add supportive transaction wait function.
  
      This function supports channel revocation properly.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D8611

Modified:
  stable/11/sys/dev/hyperv/include/vmbus.h
  stable/11/sys/dev/hyperv/vmbus/vmbus_chan.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/dev/hyperv/include/vmbus.h
==============================================================================
--- stable/11/sys/dev/hyperv/include/vmbus.h    Thu Jan  5 05:03:11 2017        
(r311363)
+++ stable/11/sys/dev/hyperv/include/vmbus.h    Thu Jan  5 05:24:30 2017        
(r311364)
@@ -116,6 +116,7 @@ struct vmbus_chan_br {
 };
 
 struct vmbus_channel;
+struct vmbus_xact;
 struct vmbus_xact_ctx;
 struct hyperv_guid;
 struct task;
@@ -129,6 +130,36 @@ vmbus_get_channel(device_t dev)
        return device_get_ivars(dev);
 }
 
+/*
+ * vmbus_chan_open_br()
+ *
+ * Return values:
+ * 0                   Succeeded.
+ * EISCONN             Failed, and the memory passed through 'br' is still
+ *                     connected.  Callers must _not_ free the the memory
+ *                     passed through 'br', if this error happens.
+ * other values                Failed.  The memory passed through 'br' is no 
longer
+ *                     connected.  Callers are free to do anything with the
+ *                     memory passed through 'br'.
+ *
+ *
+ *
+ * vmbus_chan_close_direct()
+ *
+ * NOTE:
+ * Callers of this function _must_ make sure to close all sub-channels before
+ * closing the primary channel.
+ *
+ * Return values:
+ * 0                   Succeeded.
+ * EISCONN             Failed, and the memory associated with the bufring
+ *                     is still connected.  Callers must _not_ free the the
+ *                     memory associated with the bufring, if this error
+ *                     happens.
+ * other values                Failed.  The memory associated with the bufring 
is
+ *                     no longer connected.  Callers are free to do anything
+ *                     with the memory associated with the bufring.
+ */
 int            vmbus_chan_open(struct vmbus_channel *chan,
                    int txbr_size, int rxbr_size, const void *udata, int udlen,
                    vmbus_chan_callback_t cb, void *cbarg);
@@ -136,12 +167,15 @@ int               vmbus_chan_open_br(struct vmbus_cha
                    const struct vmbus_chan_br *cbr, const void *udata,
                    int udlen, vmbus_chan_callback_t cb, void *cbarg);
 void           vmbus_chan_close(struct vmbus_channel *chan);
+int            vmbus_chan_close_direct(struct vmbus_channel *chan);
 void           vmbus_chan_intr_drain(struct vmbus_channel *chan);
 void           vmbus_chan_run_task(struct vmbus_channel *chan,
                    struct task *task);
 void           vmbus_chan_set_orphan(struct vmbus_channel *chan,
                    struct vmbus_xact_ctx *);
 void           vmbus_chan_unset_orphan(struct vmbus_channel *chan);
+const void     *vmbus_chan_xact_wait(const struct vmbus_channel *chan,
+                   struct vmbus_xact *xact, size_t *resp_len, bool can_sleep);
 
 int            vmbus_chan_gpadl_connect(struct vmbus_channel *chan,
                    bus_addr_t paddr, int size, uint32_t *gpadl);

Modified: stable/11/sys/dev/hyperv/vmbus/vmbus_chan.c
==============================================================================
--- stable/11/sys/dev/hyperv/vmbus/vmbus_chan.c Thu Jan  5 05:03:11 2017        
(r311363)
+++ stable/11/sys/dev/hyperv/vmbus/vmbus_chan.c Thu Jan  5 05:24:30 2017        
(r311364)
@@ -53,7 +53,7 @@ __FBSDID("$FreeBSD$");
 static void                    vmbus_chan_update_evtflagcnt(
                                    struct vmbus_softc *,
                                    const struct vmbus_channel *);
-static void                    vmbus_chan_close_internal(
+static int                     vmbus_chan_close_internal(
                                    struct vmbus_channel *);
 static int                     vmbus_chan_sysctl_mnf(SYSCTL_HANDLER_ARGS);
 static void                    vmbus_chan_sysctl_create(
@@ -66,6 +66,8 @@ static int                    vmbus_chan_release(struct v
 static void                    vmbus_chan_set_chmap(struct vmbus_channel *);
 static void                    vmbus_chan_clear_chmap(struct vmbus_channel *);
 static void                    vmbus_chan_detach(struct vmbus_channel *);
+static bool                    vmbus_chan_wait_revoke(
+                                   const struct vmbus_channel *);
 
 static void                    vmbus_chan_ins_prilist(struct vmbus_softc *,
                                    struct vmbus_channel *);
@@ -322,7 +324,21 @@ vmbus_chan_open(struct vmbus_channel *ch
 
        error = vmbus_chan_open_br(chan, &cbr, udata, udlen, cb, cbarg);
        if (error) {
-               hyperv_dmamem_free(&chan->ch_bufring_dma, chan->ch_bufring);
+               if (error == EISCONN) {
+                       /*
+                        * XXX
+                        * The bufring GPADL is still connected; abandon
+                        * this bufring, instead of having mysterious
+                        * crash or trashed data later on.
+                        */
+                       vmbus_chan_printf(chan, "chan%u bufring GPADL "
+                           "is still connected upon channel open error; "
+                           "leak %d bytes memory\n", chan->ch_id,
+                           txbr_size + rxbr_size);
+               } else {
+                       hyperv_dmamem_free(&chan->ch_bufring_dma,
+                           chan->ch_bufring);
+               }
                chan->ch_bufring = NULL;
        }
        return (error);
@@ -345,7 +361,7 @@ vmbus_chan_open_br(struct vmbus_channel 
        if (udlen > VMBUS_CHANMSG_CHOPEN_UDATA_SIZE) {
                vmbus_chan_printf(chan,
                    "invalid udata len %d for chan%u\n", udlen, chan->ch_id);
-               return EINVAL;
+               return (EINVAL);
        }
 
        br = cbr->cbr;
@@ -390,6 +406,8 @@ vmbus_chan_open_br(struct vmbus_channel 
        /*
         * Connect the bufrings, both RX and TX, to this channel.
         */
+       KASSERT(chan->ch_bufring_gpadl == 0,
+           ("bufring GPADL is still connected"));
        error = vmbus_chan_gpadl_connect(chan, cbr->cbr_paddr,
            txbr_size + rxbr_size, &chan->ch_bufring_gpadl);
        if (error) {
@@ -442,23 +460,33 @@ vmbus_chan_open_br(struct vmbus_channel 
        vmbus_msghc_put(sc, mh);
 
        if (status == 0) {
-               if (bootverbose) {
+               if (bootverbose)
                        vmbus_chan_printf(chan, "chan%u opened\n", chan->ch_id);
-               }
-               return 0;
+               return (0);
        }
 
        vmbus_chan_printf(chan, "failed to open chan%u\n", chan->ch_id);
        error = ENXIO;
 
 failed:
+       sysctl_ctx_free(&chan->ch_sysctl_ctx);
        vmbus_chan_clear_chmap(chan);
-       if (chan->ch_bufring_gpadl) {
-               vmbus_chan_gpadl_disconnect(chan, chan->ch_bufring_gpadl);
+       if (chan->ch_bufring_gpadl != 0) {
+               int error1;
+
+               error1 = vmbus_chan_gpadl_disconnect(chan,
+                   chan->ch_bufring_gpadl);
+               if (error1) {
+                       /*
+                        * Give caller a hint that the bufring GPADL is still
+                        * connected.
+                        */
+                       error = EISCONN;
+               }
                chan->ch_bufring_gpadl = 0;
        }
        atomic_clear_int(&chan->ch_stflags, VMBUS_CHAN_ST_OPENED);
-       return error;
+       return (error);
 }
 
 int
@@ -475,6 +503,12 @@ vmbus_chan_gpadl_connect(struct vmbus_ch
        uint64_t page_id;
 
        /*
+        * Reset GPADL, so that the result would consistent, if error
+        * happened later on.
+        */
+       *gpadl0 = 0;
+
+       /*
         * Preliminary checks.
         */
 
@@ -500,7 +534,6 @@ vmbus_chan_gpadl_connect(struct vmbus_ch
         * Allocate GPADL id.
         */
        gpadl = vmbus_gpadl_alloc(sc);
-       *gpadl0 = gpadl;
 
        /*
         * Connect this GPADL to the target channel.
@@ -579,15 +612,35 @@ vmbus_chan_gpadl_connect(struct vmbus_ch
                vmbus_chan_printf(chan, "gpadl_conn(chan%u) failed: %u\n",
                    chan->ch_id, status);
                return EIO;
-       } else {
-               if (bootverbose) {
-                       vmbus_chan_printf(chan,
-                           "gpadl_conn(chan%u) succeeded\n", chan->ch_id);
-               }
+       }
+
+       /* Done; commit the GPADL id. */
+       *gpadl0 = gpadl;
+       if (bootverbose) {
+               vmbus_chan_printf(chan, "gpadl_conn(chan%u) succeeded\n",
+                   chan->ch_id);
        }
        return 0;
 }
 
+static bool
+vmbus_chan_wait_revoke(const struct vmbus_channel *chan)
+{
+#define WAIT_COUNT     200     /* 200ms */
+
+       int i;
+
+       for (i = 0; i < WAIT_COUNT; ++i) {
+               if (vmbus_chan_is_revoked(chan))
+                       return (true);
+               /* Not sure about the context; use busy-wait. */
+               DELAY(1000);
+       }
+       return (false);
+
+#undef WAIT_COUNT
+}
+
 /*
  * Disconnect the GPA from the target channel
  */
@@ -604,7 +657,7 @@ vmbus_chan_gpadl_disconnect(struct vmbus
                vmbus_chan_printf(chan,
                    "can not get msg hypercall for gpadl_disconn(chan%u)\n",
                    chan->ch_id);
-               return EBUSY;
+               return (EBUSY);
        }
 
        req = vmbus_msghc_dataptr(mh);
@@ -614,18 +667,29 @@ vmbus_chan_gpadl_disconnect(struct vmbus
 
        error = vmbus_msghc_exec(sc, mh);
        if (error) {
+               vmbus_msghc_put(sc, mh);
+
+               if (vmbus_chan_wait_revoke(chan)) {
+                       /*
+                        * Error is benign; this channel is revoked,
+                        * so this GPADL will not be touched anymore.
+                        */
+                       vmbus_chan_printf(chan,
+                           "gpadl_disconn(revoked chan%u) msg hypercall "
+                           "exec failed: %d\n", chan->ch_id, error);
+                       return (0);
+               }
                vmbus_chan_printf(chan,
                    "gpadl_disconn(chan%u) msg hypercall exec failed: %d\n",
                    chan->ch_id, error);
-               vmbus_msghc_put(sc, mh);
-               return error;
+               return (error);
        }
 
        vmbus_msghc_wait_result(sc, mh);
        /* Discard result; no useful information */
        vmbus_msghc_put(sc, mh);
 
-       return 0;
+       return (0);
 }
 
 static void
@@ -681,16 +745,34 @@ vmbus_chan_set_chmap(struct vmbus_channe
        chan->ch_vmbus->vmbus_chmap[chan->ch_id] = chan;
 }
 
-static void
+static int
 vmbus_chan_close_internal(struct vmbus_channel *chan)
 {
        struct vmbus_softc *sc = chan->ch_vmbus;
        struct vmbus_msghc *mh;
        struct vmbus_chanmsg_chclose *req;
+       uint32_t old_stflags;
        int error;
 
-       /* TODO: stringent check */
-       atomic_clear_int(&chan->ch_stflags, VMBUS_CHAN_ST_OPENED);
+       /*
+        * NOTE:
+        * Sub-channels are closed upon their primary channel closing,
+        * so they can be closed even before they are opened.
+        */
+       for (;;) {
+               old_stflags = chan->ch_stflags;
+               if (atomic_cmpset_int(&chan->ch_stflags, old_stflags,
+                   old_stflags & ~VMBUS_CHAN_ST_OPENED))
+                       break;
+       }
+       if ((old_stflags & VMBUS_CHAN_ST_OPENED) == 0) {
+               /* Not opened yet; done */
+               if (bootverbose) {
+                       vmbus_chan_printf(chan, "chan%u not opened\n",
+                           chan->ch_id);
+               }
+               return (0);
+       }
 
        /*
         * Free this channel's sysctl tree attached to its device's
@@ -716,7 +798,8 @@ vmbus_chan_close_internal(struct vmbus_c
                vmbus_chan_printf(chan,
                    "can not get msg hypercall for chclose(chan%u)\n",
                    chan->ch_id);
-               return;
+               error = ENXIO;
+               goto disconnect;
        }
 
        req = vmbus_msghc_dataptr(mh);
@@ -730,16 +813,37 @@ vmbus_chan_close_internal(struct vmbus_c
                vmbus_chan_printf(chan,
                    "chclose(chan%u) msg hypercall exec failed: %d\n",
                    chan->ch_id, error);
-               return;
-       } else if (bootverbose) {
-               vmbus_chan_printf(chan, "close chan%u\n", chan->ch_id);
+               goto disconnect;
        }
 
+       if (bootverbose)
+               vmbus_chan_printf(chan, "chan%u closed\n", chan->ch_id);
+
+disconnect:
        /*
         * Disconnect the TX+RX bufrings from this channel.
         */
-       if (chan->ch_bufring_gpadl) {
-               vmbus_chan_gpadl_disconnect(chan, chan->ch_bufring_gpadl);
+       if (chan->ch_bufring_gpadl != 0) {
+               int error1;
+
+               error1 = vmbus_chan_gpadl_disconnect(chan,
+                   chan->ch_bufring_gpadl);
+               if (error1) {
+                       /*
+                        * XXX
+                        * The bufring GPADL is still connected; abandon
+                        * this bufring, instead of having mysterious
+                        * crash or trashed data later on.
+                        */
+                       vmbus_chan_printf(chan, "chan%u bufring GPADL "
+                           "is still connected after close\n", chan->ch_id);
+                       chan->ch_bufring = NULL;
+                       /*
+                        * Give caller a hint that the bufring GPADL is
+                        * still connected.
+                        */
+                       error = EISCONN;
+               }
                chan->ch_bufring_gpadl = 0;
        }
 
@@ -750,6 +854,42 @@ vmbus_chan_close_internal(struct vmbus_c
                hyperv_dmamem_free(&chan->ch_bufring_dma, chan->ch_bufring);
                chan->ch_bufring = NULL;
        }
+       return (error);
+}
+
+int
+vmbus_chan_close_direct(struct vmbus_channel *chan)
+{
+       int error;
+
+#ifdef INVARIANTS
+       if (VMBUS_CHAN_ISPRIMARY(chan)) {
+               struct vmbus_channel *subchan;
+
+               /*
+                * All sub-channels _must_ have been closed, or are _not_
+                * opened at all.
+                */
+               mtx_lock(&chan->ch_subchan_lock);
+               TAILQ_FOREACH(subchan, &chan->ch_subchans, ch_sublink) {
+                       KASSERT(
+                          (subchan->ch_stflags & VMBUS_CHAN_ST_OPENED) == 0,
+                          ("chan%u: subchan%u is still opened",
+                           chan->ch_id, subchan->ch_subidx));
+               }
+               mtx_unlock(&chan->ch_subchan_lock);
+       }
+#endif
+
+       error = vmbus_chan_close_internal(chan);
+       if (!VMBUS_CHAN_ISPRIMARY(chan)) {
+               /*
+                * This sub-channel is referenced, when it is linked to
+                * the primary channel; drop that reference now.
+                */
+               vmbus_chan_detach(chan);
+       }
+       return (error);
 }
 
 /*
@@ -1787,3 +1927,37 @@ vmbus_chan_unset_orphan(struct vmbus_cha
        chan->ch_orphan_xact = NULL;
        sx_xunlock(&chan->ch_orphan_lock);
 }
+
+const void *
+vmbus_chan_xact_wait(const struct vmbus_channel *chan,
+    struct vmbus_xact *xact, size_t *resp_len, bool can_sleep)
+{
+       const void *ret;
+
+       if (can_sleep)
+               ret = vmbus_xact_wait(xact, resp_len);
+       else
+               ret = vmbus_xact_busywait(xact, resp_len);
+       if (vmbus_chan_is_revoked(chan)) {
+               /*
+                * This xact probably is interrupted, and the
+                * interruption can race the reply reception,
+                * so we have to make sure that there are nothing
+                * left on the RX bufring, i.e. this xact will
+                * not be touched, once this function returns.
+                *
+                * Since the hypervisor will not put more data
+                * onto the RX bufring once the channel is revoked,
+                * the following loop will be terminated, once all
+                * data are drained by the driver's channel
+                * callback.
+                */
+               while (!vmbus_chan_rx_empty(chan)) {
+                       if (can_sleep)
+                               pause("chxact", 1);
+                       else
+                               DELAY(1000);
+               }
+       }
+       return (ret);
+}
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to