Hi Tiwei,
Thanks for review.
> -----Original Message-----
> From: Tiwei Bie <tiwei....@intel.com>
> Sent: Friday, December 20, 2019 4:18 PM
> To: Gavin Hu <gavin...@arm.com>
> Cc: dev@dpdk.org; nd <n...@arm.com>; david.march...@redhat.com;
> tho...@monjalon.net; rasl...@mellanox.com;
> maxime.coque...@redhat.com; hemant.agra...@nxp.com;
> jer...@marvell.com; pbhagavat...@marvell.com; Honnappa Nagarahalli
> <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> <ruifeng.w...@arm.com>; Phil Yang <phil.y...@arm.com>; Joyce Kong
> <joyce.k...@arm.com>; Steve Capper <steve.cap...@arm.com>
> Subject: Re: [PATCH v2 2/3] net/virtio: virtual PCI requires smp barriers
>
> 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...@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.
That's true, rte_smp is sufficient for *non*- VIRTIO_F_ORDER_PLATFORM,
As in the previous patch, rte_io is relaxed to a compiler barrier, rte_smp is
stronger than that, put another way, rte_smp is also sufficient for *
VIRTIO_F_ORDER_PLATFORM* case.
As X86 and PPC make no difference about rte_smp and rte_io, so everything goes
fine?
Anyway, I will update the commit log to reflect the above points.
>
> https://github.com/oasis-tcs/virtio-
> spec/blob/94520b3af19c/content.tex#L5788
>
> > +
