RE: [EXTERNAL] [RFC 03/32] bus/vmbus: fix ring buffer ordering on weakly ordered CPUs

> Subject: [EXTERNAL] [RFC 03/32] bus/vmbus: fix ring buffer ordering on weakly
> ordered CPUs
> 
> The read index update in vmbus_rxbr_read() was only protected by a compiler
> barrier. On a weakly ordered architecture the store of rindex can become visible
> to the host before the data copy completes, allowing the host to reuse and
> overwrite ring data still being read. Use a release store for the read index.
> 
> The barrier in rte_vmbus_chan_signal_tx() must order the earlier ring index
> update against the load of the host interrupt mask, and a write barrier does not
> order stores against loads. Use a full fence, which matches virt_mb() in the
> equivalent Linux and FreeBSD code.
> 
> Neither change affects generated code on x86.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Cc: [email protected]
> 
> Signed-off-by: Stephen Hemminger <[email protected]>

Reviewed-by: Long Li <[email protected]>


> ---
>  drivers/bus/vmbus/vmbus_bufring.c | 11 +++++++----
> drivers/bus/vmbus/vmbus_channel.c |  4 ++--
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/vmbus/vmbus_bufring.c
> b/drivers/bus/vmbus/vmbus_bufring.c
> index fcb97287dc..f622869974 100644
> --- a/drivers/bus/vmbus/vmbus_bufring.c
> +++ b/drivers/bus/vmbus/vmbus_bufring.c
> @@ -237,10 +237,13 @@ vmbus_rxbr_read(struct vmbus_br *rbr, void *data,
> size_t dlen, size_t skip)
>  	 */
>  	rindex = vmbus_br_idxinc(rindex, sizeof(uint64_t), br_dsize);
> 
> -	/* Update the read index _after_ the channel packet is fetched.	 */
> -	rte_compiler_barrier();
> -
> -	vbr->rindex = rindex;
> +	/*
> +	 * Update the read index after the channel packet is fetched.
> +	 * Release store ensures the host can not observe the new read
> +	 * index before the data copy is complete.
> +	 */
> +	rte_atomic_store_explicit((volatile uint32_t __rte_atomic *)&vbr-
> >rindex,
> +				  rindex, rte_memory_order_release);
> 
>  	return 0;
>  }
> diff --git a/drivers/bus/vmbus/vmbus_channel.c
> b/drivers/bus/vmbus/vmbus_channel.c
> index 6887fbad46..2648d8e3fd 100644
> --- a/drivers/bus/vmbus/vmbus_channel.c
> +++ b/drivers/bus/vmbus/vmbus_channel.c
> @@ -104,8 +104,8 @@ rte_vmbus_chan_signal_tx(struct rte_vmbus_device
> *dev, const struct vmbus_channe  {
>  	const struct vmbus_br *tbr = &chan->txbr;
> 
> -	/* Make sure all updates are done before signaling host */
> -	rte_smp_wmb();
> +	/* Order ring index update before reading host interrupt mask */
> +	rte_atomic_thread_fence(rte_memory_order_seq_cst);
> 
>  	/* If host is ignoring interrupts? */
>  	if (tbr->vbr->imask)
> --
> 2.53.0