Hi Rahul,
> On 6 Sep 2022, at 10:55, Rahul Singh <rahul.si...@arm.com> wrote:
>
> From: Zhou Wang <wangzh...@hisilicon.com>
>
> Backport Linux commit a76a37777f2c. Introduce __iomb() in the smmu-v3.c
> file with other Linux compatibility definitions.
>
> Reading the 'prod' MMIO register in order to determine whether or
> not there is valid data beyond 'cons' for a given queue does not
> provide sufficient dependency ordering, as the resulting access is
> address dependent only on 'cons' and can therefore be speculated
> ahead of time, potentially allowing stale data to be read by the
> CPU.
>
> Use readl() instead of readl_relaxed() when updating the shadow copy
> of the 'prod' pointer, so that all speculated memory reads from the
> corresponding queue can occur only from valid slots.
>
> Signed-off-by: Zhou Wang <wangzh...@hisilicon.com>
> Link:
> https://lore.kernel.org/r/1601281922-117296-1-git-send-email-wangzh...@hisilicon.com
> [will: Use readl() instead of explicit barrier. Update 'cons' side to match.]
> Signed-off-by: Will Deacon <w...@kernel.org>
> Origin: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> a76a37777f2c
> Signed-off-by: Rahul Singh <rahul.si...@arm.com>
Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
Cheers
Bertrand
> ---
> Changes in v4:
> - rename iomb() to __iomb()
> Changes in v3:
> - rename __iomb() to iomb() and also move it from common file to
> smmu-v3.c file
> Changes in v2:
> - fix commit msg
> - add __iomb changes also from the origin patch
> ---
> xen/drivers/passthrough/arm/smmu-v3.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index 64d39bb4d3..229b9a4b0d 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -107,6 +107,8 @@ typedef paddr_t dma_addr_t;
> typedef paddr_t phys_addr_t;
> typedef unsigned int gfp_t;
>
> +#define __iomb() dmb(osh)
> +
> #define platform_device device
>
> #define GFP_KERNEL 0
> @@ -951,7 +953,7 @@ static void queue_sync_cons_out(struct arm_smmu_queue *q)
> * Ensure that all CPU accesses (reads and writes) to the queue
> * are complete before we update the cons pointer.
> */
> - mb();
> + __iomb();
> writel_relaxed(q->llq.cons, q->cons_reg);
> }
>
> @@ -963,8 +965,15 @@ static void queue_inc_cons(struct arm_smmu_ll_queue *q)
>
> static int queue_sync_prod_in(struct arm_smmu_queue *q)
> {
> + u32 prod;
> int ret = 0;
> - u32 prod = readl_relaxed(q->prod_reg);
> +
> + /*
> + * We can't use the _relaxed() variant here, as we must prevent
> + * speculative reads of the queue before we have determined that
> + * prod has indeed moved.
> + */
> + prod = readl(q->prod_reg);
>
> if (Q_OVF(prod) != Q_OVF(q->llq.prod))
> ret = -EOVERFLOW;
> --
> 2.25.1
>
