Re: [dpdk-dev] [PATCH v2 2/3] net/virtio: virtual PCI requires smp barriers

[Tiwei Bie <tiwei.bie@intel.com>]

On Fri, Dec 20, 2019 at 11:09:50AM +0800, Gavin Hu wrote:
> Other than real PCI reads and writes to the device memory requiring
> the io barriers, virtual pci memories are normal memory in the smp
> configuration, which requires the smp barriers.
> 
> Since the smp barriers and io barriers are identical on x86 and PPC,
> this change has only effect on aarch64.
> 
> As far as peripheral coherence order for ‘virtual’ devices, the arch
> intent is that the Hypervisor view of things takes precedence – since
> translations are made in holistic manner as the full stage1+stage2
> regime, there is no such thing as a transaction taking on “EL1”
> mapping as far as ordering. If the Hypervisor maps stage2 as Normal
> but the OS at EL1 maps it as Device-nGnRE, then it’s Normal memory and
> follows the ordering rules for Normal memory.
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> ---
>  drivers/net/virtio/virtio_pci.c | 108 +++++++++++++++++++++++++++++-----------
>  1 file changed, 78 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
> index 4468e89..64aa0a0 100644
> --- a/drivers/net/virtio/virtio_pci.c
> +++ b/drivers/net/virtio/virtio_pci.c
> @@ -24,6 +24,54 @@
>  #define PCI_CAP_ID_VNDR		0x09
>  #define PCI_CAP_ID_MSIX		0x11
>  
> +static __rte_always_inline uint8_t
> +virtio_pci_read8(const volatile void *addr)
> +{
> +	uint8_t val;
> +	val = rte_read8_relaxed(addr);
> +	rte_smp_rmb();
> +	return val;
> +}
> +
> +static __rte_always_inline uint16_t
> +virtio_pci_read16(const volatile void *addr)
> +{
> +	uint16_t val;
> +	val = rte_read16_relaxed(addr);
> +	rte_smp_rmb();
> +	return val;
> +}
> +
> +static __rte_always_inline uint32_t
> +virtio_pci_read32(const volatile void *addr)
> +{
> +	uint32_t val;
> +	val = rte_read32_relaxed(addr);
> +	rte_smp_rmb();
> +	return val;
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write8(uint8_t value, volatile void *addr)
> +{
> +	rte_smp_wmb();
> +	rte_write8_relaxed(value, addr);
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write16(uint16_t value, volatile void *addr)
> +{
> +	rte_smp_wmb();
> +	rte_write16_relaxed(value, addr);
> +}
> +
> +static __rte_always_inline void
> +virtio_pci_write32(uint32_t value, volatile void *addr)
> +{
> +	rte_smp_wmb();
> +	rte_write32_relaxed(value, addr);
> +}

We can't assume that virtio device is software running
in an SMP configuration unless VIRTIO_F_ORDER_PLATFORM
isn't negotiated.

https://github.com/oasis-tcs/virtio-spec/blob/94520b3af19c/content.tex#L5788

> +